Skip to content

Commit bbbefa8

Browse files
committed
[clang][deps] Fix module context hash for constant strings
We were not hashing constant strings in the command-line, only ones that required allocations. This was causing us to get the same hash across different flag options. rdar://101053855 Differential Revision: https://reviews.llvm.org/D143027 (cherry picked from commit 223e99f)
1 parent 054c580 commit bbbefa8

File tree

3 files changed

+34
-11
lines changed

3 files changed

+34
-11
lines changed

clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -297,13 +297,12 @@ static std::string getModuleContextHash(const ModuleDeps &MD,
297297
MutableCI.getFrontendOpts().PathPrefixMappings, {});
298298

299299
// Hash the BuildInvocation without any input files.
300-
SmallVector<const char *, 32> DummyArgs;
301-
CI.generateCC1CommandLine(DummyArgs, [&](const Twine &Arg) {
302-
Scratch.clear();
303-
StringRef Str = Arg.toStringRef(Scratch);
304-
HashBuilder.add(Str);
305-
return "<unused>";
306-
});
300+
SmallVector<const char *, 32> Args;
301+
llvm::BumpPtrAllocator Alloc;
302+
llvm::StringSaver Saver(Alloc);
303+
CI.generateCC1CommandLine(
304+
Args, [&](const Twine &Arg) { return Saver.save(Arg).data(); });
305+
HashBuilder.addRange(Args);
307306

308307
// Hash the module dependencies. These paths may differ even if the invocation
309308
// is identical if they depend on the contents of the files in the TU -- for
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
[
2+
{
3+
"directory": "DIR",
4+
"command": "clang -c DIR/tu.c -fmodules -fmodules-cache-path=DIR/cache -IDIR/b -o DIR/tu_b.o -fapplication-extension",
5+
"file": "DIR/tu.c"
6+
}
7+
]

clang/test/ClangScanDeps/modules-context-hash.c

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,18 @@
77

88
// RUN: sed "s|DIR|%/t|g" %S/Inputs/modules-context-hash/cdb_a.json.template > %t/cdb_a.json
99
// RUN: sed "s|DIR|%/t|g" %S/Inputs/modules-context-hash/cdb_b.json.template > %t/cdb_b.json
10+
// RUN: sed "s|DIR|%/t|g" %S/Inputs/modules-context-hash/cdb_b2.json.template > %t/cdb_b2.json
1011

11-
// We run two separate scans. The context hash for "a" and "b" can differ between
12+
// We run separate scans. The context hash for "a" and "b" can differ between
1213
// systems. If we'd scan both Clang invocations in a single run, the order of JSON
1314
// entities would be non-deterministic. To prevent this, run the scans separately
1415
// and verify that the context hashes differ with a single FileCheck invocation.
1516
//
16-
// RUN: clang-scan-deps -compilation-database %t/cdb_a.json -format experimental-full -j 1 > %t/result.json
17-
// RUN: clang-scan-deps -compilation-database %t/cdb_b.json -format experimental-full -j 1 >> %t/result.json
18-
// RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t -check-prefix=CHECK
17+
// RUN: clang-scan-deps -compilation-database %t/cdb_a.json -format experimental-full -j 1 > %t/result_a.json
18+
// RUN: clang-scan-deps -compilation-database %t/cdb_b.json -format experimental-full -j 1 > %t/result_b.json
19+
// RUN: clang-scan-deps -compilation-database %t/cdb_b2.json -format experimental-full -j 1 > %t/result_b2.json
20+
// RUN: cat %t/result_a.json %t/result_b.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t -check-prefix=CHECK
21+
// RUN: cat %t/result_b.json %t/result_b2.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t -check-prefix=FLAG_ONLY
1922

2023
// CHECK: {
2124
// CHECK-NEXT: "modules": [
@@ -91,3 +94,17 @@
9194
// CHECK-NEXT: ],
9295
// CHECK-NEXT: "input-file": "[[PREFIX]]/tu.c"
9396
// CHECK-NEXT: }
97+
98+
// B and B2 only differ by -fapplication-extension
99+
100+
// FLAG_ONLY: "modules": [
101+
// FLAG_ONLY-NEXT: {
102+
// FLAG_ONLY: "context-hash": "[[HASH_MOD_B1:.*]]"
103+
// FLAG_ONLY-NOT: "-fapplication-extension"
104+
105+
// FLAG_ONLY: "modules": [
106+
// FLAG_ONLY-NEXT: {
107+
// FLAG_ONLY-NOT: "context-hash": "[[HASH_MOD_B1]]"
108+
// FLAG_ONLY: "-fapplication-extension"
109+
// FLAG_ONLY: "translation-units": [
110+
// FLAG_ONLY-NOT: "context-hash": "[[HASH_MOD_B1]]"

0 commit comments

Comments
 (0)