Skip to content

Commit a4a33bd

Browse files
committed
[clang][deps][cas] Ensure AST file inputs are tracked in cas-fs
The scanner may not need to load all inputs of an AST file, but the compilation could trigger lazy loading. Ensure we access all input files when caching with cas-fs so that they are tracked correctly. Fixes a nasty bug where including a module from a PCH could produce fatal errors about missing files in the importing compilation. (cherry picked from commit 277e0d6)
1 parent 30325cb commit a4a33bd

File tree

2 files changed

+113
-0
lines changed

2 files changed

+113
-0
lines changed

clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -609,6 +609,23 @@ Error FullDependencyConsumer::initialize(CompilerInstance &ScanInstance,
609609
return Error::success();
610610
}
611611

612+
/// Ensure that all files reachable from imported modules/pch are tracked.
613+
/// These could be loaded lazily during compilation.
614+
static void trackASTFileInputs(CompilerInstance &CI,
615+
llvm::cas::CachingOnDiskFileSystem &CacheFS) {
616+
auto Reader = CI.getASTReader();
617+
if (!Reader)
618+
return;
619+
620+
for (serialization::ModuleFile &MF : Reader->getModuleManager()) {
621+
Reader->visitInputFiles(
622+
MF, /*IncludeSystem=*/true, /*Complain=*/false,
623+
[](const serialization::InputFile &IF, bool isSystem) {
624+
// Visiting input files triggers the file system lookup.
625+
});
626+
}
627+
}
628+
612629
Error FullDependencyConsumer::finalize(CompilerInstance &ScanInstance,
613630
CompilerInvocation &NewInvocation) {
614631
if (CacheFS) {
@@ -624,6 +641,8 @@ Error FullDependencyConsumer::finalize(CompilerInstance &ScanInstance,
624641
(void)CacheFS->status(NewInvocation.getCodeGenOpts().SampleProfileFile);
625642
(void)CacheFS->status(NewInvocation.getCodeGenOpts().ProfileRemappingFile);
626643

644+
trackASTFileInputs(ScanInstance, *CacheFS);
645+
627646
auto E = CacheFS
628647
->createTreeFromNewAccesses(
629648
[&](const llvm::vfs::CachedDirectoryEntry &Entry,
@@ -670,6 +689,8 @@ Error FullDependencyConsumer::initializeModuleBuild(
670689
Error FullDependencyConsumer::finalizeModuleBuild(
671690
CompilerInstance &ModuleScanInstance) {
672691
if (CacheFS) {
692+
trackASTFileInputs(ModuleScanInstance, *CacheFS);
693+
673694
std::optional<cas::CASID> RootID;
674695
auto E = CacheFS
675696
->createTreeFromNewAccesses(
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
// Ensure all files references by a cached module can be found.
2+
3+
// REQUIRES: ondisk_cas
4+
5+
// RUN: rm -rf %t
6+
// RUN: split-file %s %t
7+
// RUN: sed "s|DIR|%/t|g" %t/cdb_pch.json.template > %t/cdb_pch.json
8+
// RUN: sed "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
9+
10+
// == Scan PCH
11+
// RUN: clang-scan-deps -compilation-database %t/cdb_pch.json \
12+
// RUN: -cas-path %t/cas -action-cache-path %t/cache -module-files-dir %t/outputs \
13+
// RUN: -format experimental-full -mode preprocess-dependency-directives \
14+
// RUN: > %t/deps_pch.json
15+
16+
// == Build PCH
17+
// RUN: %deps-to-rsp %t/deps_pch.json --module-name A > %t/A.rsp
18+
// RUN: %deps-to-rsp %t/deps_pch.json --tu-index 0 > %t/pch.rsp
19+
20+
// RUN: %clang @%t/A.rsp
21+
// RUN: %clang @%t/pch.rsp
22+
23+
// == Scan TU, including PCH
24+
// RUN: clang-scan-deps -compilation-database %t/cdb.json \
25+
// RUN: -cas-path %t/cas -action-cache-path %t/cache -module-files-dir %t/outputs \
26+
// RUN: -format experimental-full -mode preprocess-dependency-directives \
27+
// RUN: > %t/deps.json
28+
29+
// == Build TU, including PCH
30+
// RUN: %deps-to-rsp %t/deps.json --module-name C > %t/C.rsp
31+
// RUN: %deps-to-rsp %t/deps.json --tu-index 0 > %t/tu.rsp
32+
33+
// RUN: %clang @%t/C.rsp
34+
// RUN: %clang @%t/tu.rsp
35+
36+
//--- cdb_pch.json.template
37+
[
38+
{
39+
"directory" : "DIR",
40+
"command" : "clang_tool -I DIR -x c-header DIR/prefix.h -o DIR/prefix.h.pch -fmodules -fimplicit-modules -fimplicit-module-maps -fmodules-cache-path=DIR/module-cache -Rcompile-job-cache",
41+
"file" : "DIR/prefix.h"
42+
},
43+
]
44+
45+
//--- cdb.json.template
46+
[
47+
{
48+
"directory" : "DIR",
49+
"command" : "clang_tool -I DIR -fsyntax-only DIR/tu.c -include DIR/prefix.h -fmodules -fimplicit-modules -fimplicit-module-maps -fmodules-cache-path=DIR/module-cache -Rcompile-job-cache",
50+
"file" : "DIR/tu.c"
51+
},
52+
]
53+
54+
// The test below is a specific instance that was failing in the past. The test
55+
// complexity is required to trigger looking up an input file of a module that
56+
// would not be automatically visited when importing that module during
57+
// dependency scanning, because it is only triggered incidentally. The anatomy
58+
// of the specific test case is:
59+
// * module A has a reference to B's modulemap but does not import B
60+
// because it was for an excluded header
61+
// * module C imports A and performs macro expansion of module_macro
62+
// * looking up the location for the macro definition does binary search and
63+
// incidentally deserializes the SLocEntry for B's modulemap. With small
64+
// modules such as in this test case, the binary search is *likely* to hit
65+
// that specific SLocEntry, and it did at the time this test was written.
66+
// * module A must be "prebuilt"; otherwise the scanner will visit all inputs
67+
// during implicit module validation; so we load it via PCH.
68+
69+
//--- A/module.modulemap
70+
module A { header "A.h" }
71+
72+
//--- A/A.h
73+
#include "B/B.h"
74+
#define module_macro int
75+
76+
//--- B/module.modulemap
77+
module B { exclude header "B.h" }
78+
79+
//--- B/B.h
80+
81+
//--- module.modulemap
82+
module C { header "C.h" }
83+
84+
//--- C.h
85+
#include "A/A.h"
86+
module_macro x;
87+
88+
//--- prefix.h
89+
#include "A/A.h"
90+
91+
//--- tu.c
92+
#include "C.h"

0 commit comments

Comments
 (0)