Skip to content

Commit

Permalink
[clang][ScanDeps] Allow PCHs to have different VFS overlays (#82294)
Browse files Browse the repository at this point in the history 
It turns out it's not that uncommon for real code to pass a different
set of VFSs while building a PCH than while using the PCH. This can
cause problems as seen in `test/ClangScanDeps/optimize-vfs-pch.m`. If
you scan `compile-commands-tu-no-vfs-error.json` without -Werror and run
the resulting commands, Clang will emit a fatal error while trying to
emit a note saying that it can't find a remapped header.

This also adds textual tracking of VFSs for prebuilt modules that are
part of an included PCH, as the same issue can occur in a module we are
building if we drop VFSs. This has to be textual because we have no
guarantee the PCH had the same list of VFSs as the current TU.

This uses the `PrebuiltModuleListener` to collect `VFSOverlayFiles`
instead of trying to extract it out of a `serialization::ModuleFile`
each time it's needed. There's not a great way to just store a pointer
to the list of strings in the serialized AST.
  • Loading branch information
@Bigcheese
Bigcheese committed Feb 24, 2024
1 parent d42de86 commit de3b2c2
Show file tree
Hide file tree
Showing 6 changed files with 190 additions and 29 deletions.
4 changes: 3 additions & 1 deletion clang/include/clang/Basic/DiagnosticSerializationKinds.td
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ def err_pch_diagopt_mismatch : Error<"%0 is currently enabled, but was not in "
"the PCH file">;
def err_pch_modulecache_mismatch : Error<"PCH was compiled with module cache "
"path '%0', but the path is currently '%1'">;
def err_pch_vfsoverlay_mismatch : Error<"PCH was compiled with different VFS overlay files than are currently in use">;
def warn_pch_vfsoverlay_mismatch : Warning<
"PCH was compiled with different VFS overlay files than are currently in use">,
InGroup<DiagGroup<"pch-vfs-diff">>;
def note_pch_vfsoverlay_files : Note<"%select{PCH|current translation unit}0 has the following VFS overlays:\n%1">;
def note_pch_vfsoverlay_empty : Note<"%select{PCH|current translation unit}0 has no VFS overlays">;

Expand Down
5 changes: 5 additions & 0 deletions clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ struct ModuleDeps {
BuildInfo;
};

using PrebuiltModuleVFSMapT = llvm::StringMap<llvm::StringSet<>>;

class ModuleDepCollector;

/// Callback that records textual includes and direct modular includes/imports
Expand Down Expand Up @@ -214,6 +216,7 @@ class ModuleDepCollector final : public DependencyCollector {
CompilerInstance &ScanInstance, DependencyConsumer &C,
DependencyActionController &Controller,
CompilerInvocation OriginalCI,
PrebuiltModuleVFSMapT PrebuiltModuleVFSMap,
ScanningOptimizations OptimizeArgs, bool EagerLoadModules,
bool IsStdModuleP1689Format);

Expand All @@ -233,6 +236,8 @@ class ModuleDepCollector final : public DependencyCollector {
DependencyConsumer &Consumer;
/// Callbacks for computing dependency information.
DependencyActionController &Controller;
/// Mapping from prebuilt AST files to their sorted list of VFS overlay files.
PrebuiltModuleVFSMapT PrebuiltModuleVFSMap;
/// Path to the main source file.
std::string MainFile;
/// Hash identifying the compilation conditions of the current TU.
Expand Down
58 changes: 47 additions & 11 deletions clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
#include "clang/Tooling/DependencyScanning/ModuleDepCollector.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/Support/Allocator.h"
#include "llvm/Support/Error.h"
#include "llvm/TargetParser/Host.h"
Expand Down Expand Up @@ -67,7 +68,7 @@ static bool checkHeaderSearchPaths(const HeaderSearchOptions &HSOpts,
if (LangOpts.Modules) {
if (HSOpts.VFSOverlayFiles != ExistingHSOpts.VFSOverlayFiles) {
if (Diags) {
Diags->Report(diag::err_pch_vfsoverlay_mismatch);
Diags->Report(diag::warn_pch_vfsoverlay_mismatch);
auto VFSNote = [&](int Type, ArrayRef<std::string> VFSOverlays) {
if (VFSOverlays.empty()) {
Diags->Report(diag::note_pch_vfsoverlay_empty) << Type;
Expand All @@ -79,7 +80,6 @@ static bool checkHeaderSearchPaths(const HeaderSearchOptions &HSOpts,
VFSNote(0, HSOpts.VFSOverlayFiles);
VFSNote(1, ExistingHSOpts.VFSOverlayFiles);
}
return true;
}
}
return false;
Expand All @@ -93,10 +93,12 @@ class PrebuiltModuleListener : public ASTReaderListener {
public:
PrebuiltModuleListener(PrebuiltModuleFilesT &PrebuiltModuleFiles,
llvm::SmallVector<std::string> &NewModuleFiles,
PrebuiltModuleVFSMapT &PrebuiltModuleVFSMap,
const HeaderSearchOptions &HSOpts,
const LangOptions &LangOpts, DiagnosticsEngine &Diags)
: PrebuiltModuleFiles(PrebuiltModuleFiles),
NewModuleFiles(NewModuleFiles), ExistingHSOpts(HSOpts),
NewModuleFiles(NewModuleFiles),
PrebuiltModuleVFSMap(PrebuiltModuleVFSMap), ExistingHSOpts(HSOpts),
ExistingLangOpts(LangOpts), Diags(Diags) {}

bool needsImportVisitation() const override { return true; }
Expand All @@ -106,31 +108,45 @@ class PrebuiltModuleListener : public ASTReaderListener {
NewModuleFiles.push_back(Filename.str());
}

void visitModuleFile(StringRef Filename,
serialization::ModuleKind Kind) override {
CurrentFile = Filename;
}

bool ReadHeaderSearchPaths(const HeaderSearchOptions &HSOpts,
bool Complain) override {
std::vector<std::string> VFSOverlayFiles = HSOpts.VFSOverlayFiles;
PrebuiltModuleVFSMap.insert(
{CurrentFile, llvm::StringSet<>(VFSOverlayFiles)});
return checkHeaderSearchPaths(
HSOpts, ExistingHSOpts, Complain ? &Diags : nullptr, ExistingLangOpts);
}

private:
PrebuiltModuleFilesT &PrebuiltModuleFiles;
llvm::SmallVector<std::string> &NewModuleFiles;
PrebuiltModuleVFSMapT &PrebuiltModuleVFSMap;
const HeaderSearchOptions &ExistingHSOpts;
const LangOptions &ExistingLangOpts;
DiagnosticsEngine &Diags;
std::string CurrentFile;
};

/// Visit the given prebuilt module and collect all of the modules it
/// transitively imports and contributing input files.
static bool visitPrebuiltModule(StringRef PrebuiltModuleFilename,
CompilerInstance &CI,
PrebuiltModuleFilesT &ModuleFiles,
PrebuiltModuleVFSMapT &PrebuiltModuleVFSMap,
DiagnosticsEngine &Diags) {
// List of module files to be processed.
llvm::SmallVector<std::string> Worklist;
PrebuiltModuleListener Listener(
ModuleFiles, Worklist, CI.getHeaderSearchOpts(), CI.getLangOpts(), Diags);
PrebuiltModuleListener Listener(ModuleFiles, Worklist, PrebuiltModuleVFSMap,
CI.getHeaderSearchOpts(), CI.getLangOpts(),
Diags);

Listener.visitModuleFile(PrebuiltModuleFilename,
serialization::MK_ExplicitModule);
if (ASTReader::readASTFileControlBlock(
PrebuiltModuleFilename, CI.getFileManager(), CI.getModuleCache(),
CI.getPCHContainerReader(),
Expand All @@ -139,6 +155,7 @@ static bool visitPrebuiltModule(StringRef PrebuiltModuleFilename,
return true;

while (!Worklist.empty()) {
Listener.visitModuleFile(Worklist.back(), serialization::MK_ExplicitModule);
if (ASTReader::readASTFileControlBlock(
Worklist.pop_back_val(), CI.getFileManager(), CI.getModuleCache(),
CI.getPCHContainerReader(),
Expand Down Expand Up @@ -175,8 +192,19 @@ static void sanitizeDiagOpts(DiagnosticOptions &DiagOpts) {
DiagOpts.ShowCarets = false;
// Don't write out diagnostic file.
DiagOpts.DiagnosticSerializationFile.clear();
// Don't emit warnings as errors (and all other warnings too).
DiagOpts.IgnoreWarnings = true;
// Don't emit warnings except for scanning specific warnings.
// TODO: It would be useful to add a more principled way to ignore all
// warnings that come from source code. The issue is that we need to
// ignore warnings that could be surpressed by
// `#pragma clang diagnostic`, while still allowing some scanning
// warnings for things we're not ready to turn into errors yet.
// See `test/ClangScanDeps/diagnostic-pragmas.c` for an example.
llvm::erase_if(DiagOpts.Warnings, [](StringRef Warning) {
return llvm::StringSwitch<bool>(Warning)
.Cases("pch-vfs-diff", "error=pch-vfs-diff", false)
.StartsWith("no-error=", false)
.Default(true);
});
}

// Clang implements -D and -U by splatting text into a predefines buffer. This
Expand Down Expand Up @@ -300,14 +328,19 @@ class DependencyScanningAction : public tooling::ToolAction {
if (!ScanInstance.hasDiagnostics())
return false;

// Some DiagnosticConsumers require that finish() is called.
auto DiagConsumerFinisher =
llvm::make_scope_exit([DiagConsumer]() { DiagConsumer->finish(); });

ScanInstance.getPreprocessorOpts().AllowPCHWithDifferentModulesCachePath =
true;

ScanInstance.getFrontendOpts().GenerateGlobalModuleIndex = false;
ScanInstance.getFrontendOpts().UseGlobalModuleIndex = false;
ScanInstance.getFrontendOpts().ModulesShareFileManager = false;
ScanInstance.getHeaderSearchOpts().ModuleFormat = "raw";
ScanInstance.getHeaderSearchOpts().ModulesIncludeVFSUsage = true;
ScanInstance.getHeaderSearchOpts().ModulesIncludeVFSUsage =
any(OptimizeArgs & ScanningOptimizations::VFS);

ScanInstance.setFileManager(FileMgr);
// Support for virtual file system overlays.
Expand All @@ -320,12 +353,13 @@ class DependencyScanningAction : public tooling::ToolAction {
// Store the list of prebuilt module files into header search options. This
// will prevent the implicit build to create duplicate modules and will
// force reuse of the existing prebuilt module files instead.
PrebuiltModuleVFSMapT PrebuiltModuleVFSMap;
if (!ScanInstance.getPreprocessorOpts().ImplicitPCHInclude.empty())
if (visitPrebuiltModule(
ScanInstance.getPreprocessorOpts().ImplicitPCHInclude,
ScanInstance,
ScanInstance.getHeaderSearchOpts().PrebuiltModuleFiles,
ScanInstance.getDiagnostics()))
PrebuiltModuleVFSMap, ScanInstance.getDiagnostics()))
return false;

// Use the dependency scanning optimized file system if requested to do so.
Expand Down Expand Up @@ -369,8 +403,8 @@ class DependencyScanningAction : public tooling::ToolAction {
case ScanningOutputFormat::Full:
MDC = std::make_shared<ModuleDepCollector>(
std::move(Opts), ScanInstance, Consumer, Controller,
OriginalInvocation, OptimizeArgs, EagerLoadModules,
Format == ScanningOutputFormat::P1689);
OriginalInvocation, std::move(PrebuiltModuleVFSMap), OptimizeArgs,
EagerLoadModules, Format == ScanningOutputFormat::P1689);
ScanInstance.addDependencyCollector(MDC);
break;
}
Expand Down Expand Up @@ -399,6 +433,8 @@ class DependencyScanningAction : public tooling::ToolAction {
if (ScanInstance.getDiagnostics().hasErrorOccurred())
return false;

// Each action is responsible for calling finish.
DiagConsumerFinisher.release();
const bool Result = ScanInstance.ExecuteAction(*Action);

if (Result)
Expand Down
34 changes: 26 additions & 8 deletions clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,11 @@ const std::vector<std::string> &ModuleDeps::getBuildArguments() {
return std::get<std::vector<std::string>>(BuildInfo);
}

static void optimizeHeaderSearchOpts(HeaderSearchOptions &Opts,
ASTReader &Reader,
const serialization::ModuleFile &MF,
ScanningOptimizations OptimizeArgs) {
static void
optimizeHeaderSearchOpts(HeaderSearchOptions &Opts, ASTReader &Reader,
const serialization::ModuleFile &MF,
const PrebuiltModuleVFSMapT &PrebuiltModuleVFSMap,
ScanningOptimizations OptimizeArgs) {
if (any(OptimizeArgs & ScanningOptimizations::HeaderSearch)) {
// Only preserve search paths that were used during the dependency scan.
std::vector<HeaderSearchOptions::Entry> Entries;
Expand Down Expand Up @@ -65,11 +66,25 @@ static void optimizeHeaderSearchOpts(HeaderSearchOptions &Opts,
llvm::DenseSet<const serialization::ModuleFile *> Visited;
std::function<void(const serialization::ModuleFile *)> VisitMF =
[&](const serialization::ModuleFile *MF) {
VFSUsage |= MF->VFSUsage;
Visited.insert(MF);
for (const serialization::ModuleFile *Import : MF->Imports)
if (!Visited.contains(Import))
VisitMF(Import);
if (MF->Kind == serialization::MK_ImplicitModule) {
VFSUsage |= MF->VFSUsage;
// We only need to recurse into implicit modules. Other module types
// will have the correct set of VFSs for anything they depend on.
for (const serialization::ModuleFile *Import : MF->Imports)
if (!Visited.contains(Import))
VisitMF(Import);
} else {
// This is not an implicitly built module, so it may have different
// VFS options. Fall back to a string comparison instead.
auto VFSMap = PrebuiltModuleVFSMap.find(MF->FileName);
if (VFSMap == PrebuiltModuleVFSMap.end())
return;
for (std::size_t I = 0, E = VFSOverlayFiles.size(); I != E; ++I) {
if (VFSMap->second.contains(VFSOverlayFiles[I]))
VFSUsage[I] = true;
}
}
};
VisitMF(&MF);

Expand Down Expand Up @@ -596,6 +611,7 @@ ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
ScanningOptimizations::VFS)))
optimizeHeaderSearchOpts(BuildInvocation.getMutHeaderSearchOpts(),
*MDC.ScanInstance.getASTReader(), *MF,
MDC.PrebuiltModuleVFSMap,
MDC.OptimizeArgs);
if (any(MDC.OptimizeArgs & ScanningOptimizations::SystemWarnings))
optimizeDiagnosticOpts(
Expand Down Expand Up @@ -697,9 +713,11 @@ ModuleDepCollector::ModuleDepCollector(
std::unique_ptr<DependencyOutputOptions> Opts,
CompilerInstance &ScanInstance, DependencyConsumer &C,
DependencyActionController &Controller, CompilerInvocation OriginalCI,
PrebuiltModuleVFSMapT PrebuiltModuleVFSMap,
ScanningOptimizations OptimizeArgs, bool EagerLoadModules,
bool IsStdModuleP1689Format)
: ScanInstance(ScanInstance), Consumer(C), Controller(Controller),
PrebuiltModuleVFSMap(std::move(PrebuiltModuleVFSMap)),
Opts(std::move(Opts)),
CommonInvocation(
makeCommonInvocationForModuleBuild(std::move(OriginalCI))),
Expand Down

0 comments on commit de3b2c2

Please sign in to comment.