Skip to content

Commit 358edc7

Browse files
authored
Revert "Indexer: Register every encountered file with the help of special "locations"" (#14214)
This reverts pull request #14196. **Reason for revert:** This change broke the `trial-build` CI pipeline, blocking all other project builds. Reverting to restore CI stability. The root cause will be investigated before re-landing. cc @oliverchang @DavidKorczynski
1 parent 703d268 commit 358edc7

File tree

9 files changed

+66
-101
lines changed

9 files changed

+66
-101
lines changed

infra/indexer/frontend/common.cc

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,13 @@
1515
#include "indexer/frontend/common.h"
1616

1717
#include <cstdint>
18+
#include <cstdlib>
1819
#include <filesystem> // NOLINT
20+
#include <iostream>
1921
#include <string>
2022

2123
#include "indexer/index/in_memory_index.h"
2224
#include "indexer/index/types.h"
23-
#include "absl/log/check.h"
2425
#include "absl/strings/string_view.h"
2526
#include "clang/Basic/SourceLocation.h"
2627
#include "clang/Basic/SourceManager.h"
@@ -29,15 +30,10 @@
2930
namespace oss_fuzz {
3031
namespace indexer {
3132

32-
std::string ToNormalizedAbsolutePath(
33-
absl::string_view path, const clang::SourceManager& source_manager) {
33+
std::string CleanPath(absl::string_view path, absl::string_view cwd) {
3434
std::filesystem::path native_path = std::filesystem::path(path);
3535
if (!native_path.is_absolute()) {
36-
llvm::ErrorOr<std::string> cwd = source_manager.getFileManager()
37-
.getVirtualFileSystem()
38-
.getCurrentWorkingDirectory();
39-
QCHECK(cwd) << "unable to get cwd";
40-
native_path = std::filesystem::path(*cwd);
36+
native_path = std::filesystem::path(cwd);
4137
native_path.append(path);
4238
}
4339
return native_path.lexically_normal();
@@ -87,9 +83,17 @@ LocationId GetLocationId(InMemoryIndex& index,
8783
end_line = start_line;
8884
}
8985

86+
llvm::ErrorOr<std::string> cwd = source_manager.getFileManager()
87+
.getVirtualFileSystem()
88+
.getCurrentWorkingDirectory();
89+
if (!cwd) {
90+
std::cerr << "unable to get cwd\n";
91+
exit(1);
92+
}
93+
9094
if (IsRealPath(path)) {
9195
// This is a real file path, so normalize it.
92-
path = ToNormalizedAbsolutePath(path, source_manager);
96+
path = CleanPath(path, *cwd);
9397
}
9498
return index.GetLocationId({path, start_line, end_line});
9599
}

infra/indexer/frontend/common.h

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,12 @@
1515
#ifndef OSS_FUZZ_INFRA_INDEXER_FRONTEND_COMMON_H_
1616
#define OSS_FUZZ_INFRA_INDEXER_FRONTEND_COMMON_H_
1717

18-
#include <string>
19-
2018
#include "indexer/index/in_memory_index.h"
2119
#include "indexer/index/types.h"
22-
#include "absl/strings/string_view.h"
2320
#include "clang/Basic/SourceLocation.h"
24-
#include "clang/Basic/SourceManager.h"
2521

2622
namespace oss_fuzz {
2723
namespace indexer {
28-
// Converts a source-level `path` into a normalized absolute form suitable for
29-
// passing to the indexer as a location path.
30-
std::string ToNormalizedAbsolutePath(
31-
absl::string_view path, const clang::SourceManager& source_manager);
32-
3324
// Converts a pair of `SourceLocation` to a `LocationId` for a location in the
3425
// index.
3526
LocationId GetLocationId(InMemoryIndex& index,

infra/indexer/frontend/frontend_test.cc

Lines changed: 3 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,9 @@ TEST(ParseCommandLineTest, HashInsideDoubleQuotes) {
100100
} // namespace frontend_internal
101101

102102
namespace {
103-
typedef void (*TExtraSourceTreeAction)(const std::filesystem::path&);
104-
105103
std::unique_ptr<InMemoryIndex> GetSnippetIndex(
106104
std::string code, const std::vector<std::string>& extra_args = {},
107-
bool fail_on_error = false, TExtraSourceTreeAction extra_action = nullptr) {
105+
bool fail_on_error = false) {
108106
auto source_dir = std::filesystem::path(::testing::TempDir()) / "src";
109107
std::filesystem::remove_all(source_dir);
110108
CHECK(std::filesystem::create_directory(source_dir));
@@ -116,16 +114,14 @@ std::unique_ptr<InMemoryIndex> GetSnippetIndex(
116114
std::string source_file_path = (source_dir / "snippet.cc").string();
117115
std::string source_dir_path = source_dir.string();
118116

119-
if (extra_action != nullptr) {
120-
extra_action(source_dir);
121-
}
122-
123117
auto index_dir = std::filesystem::path(::testing::TempDir()) / "idx";
124118
std::filesystem::remove_all(index_dir);
125119
CHECK(std::filesystem::create_directory(index_dir));
126120
std::string index_dir_path = index_dir.string();
127121
std::string sysroot_path = "/";
122+
128123
FileCopier file_copier(source_dir_path, index_dir_path, {sysroot_path});
124+
129125
std::unique_ptr<MergeQueue> merge_queue = MergeQueue::Create(1);
130126
auto index_action = std::make_unique<IndexAction>(file_copier, *merge_queue);
131127
const bool result = clang::tooling::runToolOnCodeWithArgs(
@@ -3867,40 +3863,5 @@ TEST(FrontendTest, AliasedSymbol) {
38673863
EXPECT_HAS_ENTITY(index, Entity::Kind::kFunction, "", "bar", "()",
38683864
"snippet.cc", 2, 2);
38693865
}
3870-
3871-
TEST(FrontendTest, GhostFileLocations) {
3872-
FlatIndex index =
3873-
std::move(
3874-
*GetSnippetIndex(
3875-
/*code=*/"#include \"ghostfile.h\"\n",
3876-
/*extra_args=*/{},
3877-
/*fail_on_error=*/true,
3878-
/*extra_action=*/
3879-
[](const std::filesystem::path& source_dir) {
3880-
std::ofstream ghost_file(source_dir / "ghostfile.h");
3881-
ghost_file
3882-
<< "// Copyright 2025 Google Inc. All rights reserved.";
3883-
CHECK(ghost_file.good());
3884-
}))
3885-
.Export();
3886-
3887-
bool found_self = false;
3888-
bool found_include = false;
3889-
bool found_other = false;
3890-
for (const Location& location : index.locations) {
3891-
if (location.is_whole_file()) {
3892-
if (location.path().ends_with("snippet.cc")) {
3893-
found_self = true;
3894-
} else if (location.path().ends_with("ghostfile.h")) {
3895-
found_include = true;
3896-
}
3897-
} else if (location.is_real()) {
3898-
found_other = true;
3899-
}
3900-
}
3901-
EXPECT_TRUE(found_self);
3902-
EXPECT_TRUE(found_include);
3903-
EXPECT_FALSE(found_other);
3904-
}
39053866
} // namespace indexer
39063867
} // namespace oss_fuzz

infra/indexer/frontend/index_action.cc

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -20,19 +20,16 @@
2020
#include <vector>
2121

2222
#include "indexer/frontend/ast_visitor.h"
23-
#include "indexer/frontend/common.h"
2423
#include "indexer/frontend/pp_callbacks.h"
2524
#include "indexer/index/file_copier.h"
2625
#include "indexer/index/in_memory_index.h"
27-
#include "indexer/index/types.h"
2826
#include "indexer/merge_queue.h"
2927
#include "absl/flags/flag.h"
3028
#include "absl/log/check.h"
3129
#include "absl/strings/match.h"
3230
#include "absl/strings/string_view.h"
3331
#include "clang/AST/ASTConsumer.h"
3432
#include "clang/Frontend/CompilerInstance.h"
35-
#include "clang/Frontend/Utils.h"
3633
#include "clang/Lex/Pragma.h"
3734
#include "clang/Lex/Preprocessor.h"
3835
#include "llvm/ADT/StringRef.h"
@@ -65,10 +62,7 @@ IndexAction::IndexAction(FileCopier& file_copier, MergeQueue& merge_queue)
6562
bool IndexAction::BeginSourceFileAction(clang::CompilerInstance& compiler) {
6663
CHECK(index_);
6764

68-
dependencies_collector_ = std::make_unique<AllDependenciesCollector>();
69-
7065
clang::Preprocessor& preprocessor = compiler.getPreprocessor();
71-
dependencies_collector_->attachToPreprocessor(preprocessor);
7266
preprocessor.addPPCallbacks(
7367
std::make_unique<PpCallbacks>(*index_, compiler.getSourceManager()));
7468
for (const std::string& ignored_pragma :
@@ -81,24 +75,7 @@ bool IndexAction::BeginSourceFileAction(clang::CompilerInstance& compiler) {
8175
return !absl::EndsWith(compiler.getFrontendOpts().Inputs[0].getFile(), ".S");
8276
}
8377

84-
void IndexAction::EndSourceFileAction() {
85-
const clang::SourceManager& source_manager =
86-
getCompilerInstance().getSourceManager();
87-
for (const std::string& filename :
88-
dependencies_collector_->getDependencies()) {
89-
if (!IsRealPath(filename)) {
90-
continue;
91-
}
92-
const auto absolute_path =
93-
ToNormalizedAbsolutePath(filename, source_manager);
94-
// Create a "whole file" location per filename to make sure files without
95-
// indexed symbols are still copied and e.g. accounted for in deltas.
96-
index_->GetLocationId(Location::WholeFile(absolute_path));
97-
}
98-
dependencies_collector_.reset();
99-
100-
merge_queue_.Add(std::move(index_));
101-
}
78+
void IndexAction::EndSourceFileAction() { merge_queue_.Add(std::move(index_)); }
10279

10380
std::unique_ptr<clang::ASTConsumer> IndexAction::CreateASTConsumer(
10481
clang::CompilerInstance& compiler, llvm::StringRef path) {

infra/indexer/frontend/index_action.h

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,18 +22,11 @@
2222
#include "indexer/merge_queue.h"
2323
#include "clang/AST/ASTConsumer.h"
2424
#include "clang/Frontend/FrontendAction.h"
25-
#include "clang/Frontend/Utils.h"
2625
#include "clang/Tooling/Tooling.h"
2726
#include "llvm/ADT/StringRef.h"
2827

2928
namespace oss_fuzz {
3029
namespace indexer {
31-
class AllDependenciesCollector : public clang::DependencyCollector {
32-
public:
33-
// Also include files from the "system" locations.
34-
bool needSystemDependencies() override { return true; }
35-
};
36-
3730
// IndexAction provides the entry-point for the indexing tooling. This should
3831
// typically not be used directly, and the functions exposed in
3932
// indexer/frontend.h should be used instead.
@@ -50,7 +43,6 @@ class IndexAction : public clang::ASTFrontendAction {
5043
private:
5144
std::unique_ptr<InMemoryIndex> index_;
5245
MergeQueue& merge_queue_;
53-
std::unique_ptr<AllDependenciesCollector> dependencies_collector_;
5446
};
5547

5648
class IndexActionFactory : public clang::tooling::FrontendActionFactory {

infra/indexer/index/in_memory_index.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,7 @@ class InMemoryIndex {
4949
// The `GetXxxId` functions return the id of an existing, matching object if
5050
// there is already one in the index, or allocate a new id if there is not an
5151
// identical object in the index.
52-
// `GetLocationId` expects a location with an absolute path if not built-in;
53-
// use `ToNormalizedAbsolutePath` to obtain one.
52+
// `GetLocationId` expects a location with an absolute path if not built-in.
5453
LocationId GetLocationId(Location location);
5554
EntityId GetEntityId(const Entity& entity);
5655
const Entity& GetEntityById(EntityId entity_id) const;

infra/indexer/index/types.h

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,18 +53,11 @@ class Location {
5353
public:
5454
Location(absl::string_view path, uint32_t start_line, uint32_t end_line);
5555

56-
static Location WholeFile(absl::string_view path) {
57-
return Location(path, /*start_line=*/0, /*end_line=*/0);
58-
}
59-
6056
inline const std::string& path() const { return path_; }
6157
inline uint32_t start_line() const { return start_line_; }
6258
inline uint32_t end_line() const { return end_line_; }
6359

6460
inline bool is_real() const { return IsRealPath(path()); }
65-
inline bool is_whole_file() const {
66-
return start_line_ == 0 && end_line_ == 0;
67-
}
6861

6962
private:
7063
friend class InMemoryIndex;
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
# Copyright 2025 Google LLC
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
#
15+
################################################################################
16+
17+
FROM gcr.io/oss-fuzz-base/base-clang-full:ubuntu-20-04
18+
19+
RUN mkdir /indexer
20+
WORKDIR /indexer
21+
COPY . /indexer
22+
23+
RUN apt-get update && apt-get install -y libsqlite3-dev make zlib1g-dev
24+
RUN mkdir build && cd build && cmake .. && cmake --build . -j -v
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
# Copyright 2025 Google LLC
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
#
15+
################################################################################
16+
17+
FROM gcr.io/oss-fuzz-base/base-clang-full:ubuntu-24-04
18+
19+
RUN mkdir /indexer
20+
WORKDIR /indexer
21+
COPY . /indexer
22+
23+
RUN apt-get update && apt-get install -y libsqlite3-dev make zlib1g-dev
24+
RUN mkdir build && cd build && cmake .. && cmake --build . -j -v

0 commit comments

Comments
 (0)