Skip to content

Commit

Permalink
[clang][DependencyScanner] Include the working directory in the conte…
Browse files Browse the repository at this point in the history
…xt hash (#73719)

The working directory is included in the PCM, but is not currently part
of the context hash. This causes problems because different builds of a
PCM with exactly the same command line can end up with different binary
content for a PCM. If a build system tracks tasks by both working
directory and command line, it may build a given PCM multiple times,
causing a "module file out of date" error when loading the PCM due to
different sizes.
  • Loading branch information
Bigcheese committed Nov 30, 2023
1 parent 7fbdee0 commit 13386c6
Show file tree
Hide file tree
Showing 2 changed files with 114 additions and 2 deletions.
9 changes: 7 additions & 2 deletions clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,8 @@ void ModuleDepCollector::applyDiscoveredDependencies(CompilerInvocation &CI) {

static std::string getModuleContextHash(const ModuleDeps &MD,
const CowCompilerInvocation &CI,
bool EagerLoadModules) {
bool EagerLoadModules,
llvm::vfs::FileSystem &VFS) {
llvm::HashBuilder<llvm::TruncatedBLAKE3<16>, llvm::endianness::native>
HashBuilder;
SmallString<32> Scratch;
Expand All @@ -325,6 +326,9 @@ static std::string getModuleContextHash(const ModuleDeps &MD,
// will be readable.
HashBuilder.add(getClangFullRepositoryVersion());
HashBuilder.add(serialization::VERSION_MAJOR, serialization::VERSION_MINOR);
llvm::ErrorOr<std::string> CWD = VFS.getCurrentWorkingDirectory();
if (CWD)
HashBuilder.add(*CWD);

// Hash the BuildInvocation without any input files.
SmallString<0> ArgVec;
Expand Down Expand Up @@ -356,7 +360,8 @@ static std::string getModuleContextHash(const ModuleDeps &MD,

void ModuleDepCollector::associateWithContextHash(
const CowCompilerInvocation &CI, ModuleDeps &Deps) {
Deps.ID.ContextHash = getModuleContextHash(Deps, CI, EagerLoadModules);
Deps.ID.ContextHash = getModuleContextHash(
Deps, CI, EagerLoadModules, ScanInstance.getVirtualFileSystem());
bool Inserted = ModuleDepsByID.insert({Deps.ID, &Deps}).second;
(void)Inserted;
assert(Inserted && "duplicate module mapping");
Expand Down
107 changes: 107 additions & 0 deletions clang/test/ClangScanDeps/working-dir.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
// RUN: rm -rf %t
// RUN: split-file %s %t
// RUN: sed -e "s|DIR|%/t|g" %t/build/compile-commands.json.in > %t/build/compile-commands.json
// RUN: clang-scan-deps -compilation-database %t/build/compile-commands.json \
// RUN: -j 1 -format experimental-full --optimize-args=all > %t/deps.db
// RUN: cat %t/deps.db | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t

// Check that there are two separate modules hashes. One for each working dir.

// CHECK: {
// CHECK-NEXT: "modules": [
// CHECK-NEXT: {
// CHECK-NEXT: "clang-module-deps":
// CHECK-NEXT: "clang-modulemap-file":
// CHECK-NEXT: "command-line": [
// CHECK: ],
// CHECK-NEXT: "context-hash": "{{.*}}",
// CHECK-NEXT: "file-deps": [
// CHECK: ],
// CHECK-NEXT: "name": "A"
// CHECK-NEXT: },
// CHECK-NEXT: {
// CHECK-NEXT: "clang-module-deps":
// CHECK-NEXT: "clang-modulemap-file":
// CHECK-NEXT: "command-line": [
// CHECK: ],
// CHECK-NEXT: "context-hash": "{{.*}}",
// CHECK-NEXT: "file-deps": [
// CHECK: ],
// CHECK-NEXT: "name": "A"
// CHECK-NEXT: },
// CHECK-NEXT: {
// CHECK-NEXT: "clang-module-deps":
// CHECK: ],
// CHECK-NEXT: "clang-modulemap-file":
// CHECK-NEXT: "command-line": [
// CHECK: ],
// CHECK-NEXT: "context-hash": "{{.*}}",
// CHECK-NEXT: "file-deps": [
// CHECK: ],
// CHECK-NEXT: "name": "B"
// CHECK-NEXT: },
// CHECK-NEXT: {
// CHECK-NEXT: "clang-module-deps":
// CHECK: ],
// CHECK-NEXT: "clang-modulemap-file":
// CHECK-NEXT: "command-line": [
// CHECK: ],
// CHECK-NEXT: "context-hash": "{{.*}}",
// CHECK-NEXT: "file-deps": [
// CHECK: ],
// CHECK-NEXT: "name": "B"
// CHECK-NEXT: }
// CHECK-NEXT: ],
// CHECK-NEXT: "translation-units": [
// CHECK: ]
// CHECK: }

//--- build/compile-commands.json.in

[
{
"directory": "DIR/build",
"command": "clang -c DIR/A.m -IDIR/modules/A -IDIR/modules/B -fmodules -fmodules-cache-path=DIR/module-cache -fimplicit-module-maps",
"file": "DIR/A.m"
},
{
"directory": "DIR",
"command": "clang -c DIR/B.m -IDIR/modules/A -IDIR/modules/B -fmodules -fmodules-cache-path=DIR/module-cache -fimplicit-module-maps",
"file": "DIR/B.m"
}
]


//--- modules/A/module.modulemap

module A {
umbrella header "A.h"
}

//--- modules/A/A.h

typedef int A_t;

//--- modules/B/module.modulemap

module B {
umbrella header "B.h"
}

//--- modules/B/B.h

#include <A.h>

typedef int B_t;

//--- A.m

#include <B.h>

A_t a = 0;

//--- B.m

#include <B.h>

B_t b = 0;

0 comments on commit 13386c6

Please sign in to comment.