jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith.
Herald added a subscriber: dang.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
During a highly parallel build with `-fimplicit-modules`, multiple translation
units may generate a PCM file for the same module at the same filesystem path
in a short sequence.
When the first Clang instance tries to verify signature of the module on
import, it discovers the new version of the PCM file (produced by latter Clang
instance) that may have a different signature, leading to a mismatch and failed
build.
To be able to debug such mismatch, it's invaluable to be able to compare the
latter PCM (on disk) with the first one (overwritten).
This patch adds new -cc1 option `-fbackup-module` that tells Clang to store
each PCM to the normal location **and** also to a path with an unique suffix
(the PID of the Clang instance).
This is mostly additional change, but `PCHContainerGenerator` now doesn't
unconditionally destroy the given `PCHBuffer` anymore and instead just
decreases its reference count.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D101758
Files:
clang/include/clang/Driver/Options.td
clang/include/clang/Frontend/FrontendOptions.h
clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
clang/lib/Frontend/FrontendActions.cpp
clang/lib/Serialization/PCHContainerOperations.cpp
clang/test/Modules/backup-module.c
clang/test/Modules/backup-module.h
Index: clang/test/Modules/backup-module.h
===================================================================
--- /dev/null
+++ clang/test/Modules/backup-module.h
@@ -0,0 +1 @@
+int function(void);
Index: clang/test/Modules/backup-module.c
===================================================================
--- /dev/null
+++ clang/test/Modules/backup-module.c
@@ -0,0 +1,10 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: %clang_cc1 %S/backup-module.h -emit-pch -o %t/backup-module.pch
+// RUN: find %t -name 'backup-module.pch.*' | not grep backup-module
+//
+// RUN: %clang_cc1 %S/backup-module.h -emit-pch -o %t/backup-module.pch -fbackup-module
+// RUN: find %t -name 'backup-module.pch.*' | grep backup-module
+// RUN: find %t -name 'backup-module.pch.*' | xargs diff %t/backup-module.pch
+
+// The purpose of this test is to make sure that the module file backup has
+// the same contents as the original.
Index: clang/lib/Serialization/PCHContainerOperations.cpp
===================================================================
--- clang/lib/Serialization/PCHContainerOperations.cpp
+++ clang/lib/Serialization/PCHContainerOperations.cpp
@@ -37,14 +37,14 @@
~RawPCHContainerGenerator() override = default;
void HandleTranslationUnit(ASTContext &Ctx) override {
+ // Decrease the reference count on function exit.
+ std::shared_ptr<PCHBuffer> Buffer = std::move(this->Buffer);
+
if (Buffer->IsComplete) {
// Make sure it hits disk now.
*OS << Buffer->Data;
OS->flush();
}
- // Free the space of the temporary buffer.
- llvm::SmallVector<char, 0> Empty;
- Buffer->Data = std::move(Empty);
}
};
Index: clang/lib/Frontend/FrontendActions.cpp
===================================================================
--- clang/lib/Frontend/FrontendActions.cpp
+++ clang/lib/Frontend/FrontendActions.cpp
@@ -9,8 +9,8 @@
#include "clang/Frontend/FrontendActions.h"
#include "clang/AST/ASTConsumer.h"
#include "clang/Basic/FileManager.h"
-#include "clang/Basic/TargetInfo.h"
#include "clang/Basic/LangStandard.h"
+#include "clang/Basic/TargetInfo.h"
#include "clang/Frontend/ASTConsumers.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/FrontendDiagnostic.h"
@@ -26,6 +26,7 @@
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/Process.h"
#include "llvm/Support/YAMLTraits.h"
#include "llvm/Support/raw_ostream.h"
#include <memory>
@@ -48,6 +49,26 @@
CI.createSema(Action.getTranslationUnitKind(),
GetCodeCompletionConsumer(CI));
}
+
+static void
+AddBackupPCHWriter(std::vector<std::unique_ptr<ASTConsumer>> &Consumers,
+ CompilerInstance &CI, StringRef InFile,
+ const std::string &OutFile,
+ std::shared_ptr<PCHBuffer> &Buffer) {
+ std::string BackupOutFile =
+ OutFile + "." + std::to_string(llvm::sys::Process::getProcessId());
+
+ std::error_code EC;
+ auto OS = std::make_unique<llvm::raw_fd_ostream>(BackupOutFile, EC);
+ if (EC) {
+ CI.getDiagnostics().Report(diag::err_fe_error_opening)
+ << BackupOutFile << EC.message();
+ return;
+ }
+
+ Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(
+ CI, std::string(InFile), BackupOutFile, std::move(OS), Buffer));
+}
} // namespace
//===----------------------------------------------------------------------===//
@@ -118,6 +139,8 @@
FrontendOpts.IncludeTimestamps, +CI.getLangOpts().CacheGeneratedPCH));
Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(
CI, std::string(InFile), OutputFile, std::move(OS), Buffer));
+ if (CI.getInvocation().getFrontendOpts().BackupGeneratedModuleFile)
+ AddBackupPCHWriter(Consumers, CI, InFile, OutputFile, Buffer);
return std::make_unique<MultiplexConsumer>(std::move(Consumers));
}
@@ -181,6 +204,8 @@
+CI.getFrontendOpts().BuildingImplicitModule));
Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(
CI, std::string(InFile), OutputFile, std::move(OS), Buffer));
+ if (CI.getInvocation().getFrontendOpts().BackupGeneratedModuleFile)
+ AddBackupPCHWriter(Consumers, CI, InFile, OutputFile, Buffer);
return std::make_unique<MultiplexConsumer>(std::move(Consumers));
}
Index: clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
===================================================================
--- clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
+++ clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
@@ -240,6 +240,8 @@
std::unique_ptr<llvm::LLVMContext> VMContext = std::move(this->VMContext);
std::unique_ptr<llvm::Module> M = std::move(this->M);
std::unique_ptr<CodeGen::CodeGenModule> Builder = std::move(this->Builder);
+ // Decrease the reference count on function exit.
+ std::shared_ptr<PCHBuffer> Buffer = std::move(this->Buffer);
if (Diags.hasErrorOccurred())
return;
@@ -306,10 +308,6 @@
LangOpts,
Ctx.getTargetInfo().getDataLayoutString(), M.get(),
BackendAction::Backend_EmitObj, std::move(OS));
-
- // Free the memory for the temporary buffer.
- llvm::SmallVector<char, 0> Empty;
- SerializedAST = std::move(Empty);
}
};
Index: clang/include/clang/Frontend/FrontendOptions.h
===================================================================
--- clang/include/clang/Frontend/FrontendOptions.h
+++ clang/include/clang/Frontend/FrontendOptions.h
@@ -274,6 +274,9 @@
/// Whether we can generate the global module index if needed.
unsigned GenerateGlobalModuleIndex : 1;
+ /// Whether we are backing up the generated module file to an unique location.
+ unsigned BackupGeneratedModuleFile : 1;
+
/// Whether we include declaration dumps in AST dumps.
unsigned ASTDumpDecls : 1;
@@ -459,11 +462,11 @@
FixWhatYouCan(false), FixOnlyWarnings(false), FixAndRecompile(false),
FixToTemporaries(false), ARCMTMigrateEmitARCErrors(false),
SkipFunctionBodies(false), UseGlobalModuleIndex(true),
- GenerateGlobalModuleIndex(true), ASTDumpDecls(false),
- ASTDumpLookups(false), BuildingImplicitModule(false),
- ModulesEmbedAllFiles(false), IncludeTimestamps(true),
- UseTemporary(true), AllowPCMWithCompilerErrors(false),
- TimeTraceGranularity(500) {}
+ GenerateGlobalModuleIndex(true), BackupGeneratedModuleFile(false),
+ ASTDumpDecls(false), ASTDumpLookups(false),
+ BuildingImplicitModule(false), ModulesEmbedAllFiles(false),
+ IncludeTimestamps(true), UseTemporary(true),
+ AllowPCMWithCompilerErrors(false), TimeTraceGranularity(500) {}
/// getInputKindForExtension - Return the appropriate input kind for a file
/// extension. For example, "c" would return Language::C.
Index: clang/include/clang/Driver/Options.td
===================================================================
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2068,6 +2068,10 @@
def fsystem_module : Flag<["-"], "fsystem-module">, Flags<[CC1Option]>,
HelpText<"Build this module as a system module. Only used with -emit-module">,
MarshallingInfoFlag<FrontendOpts<"IsSystemModule">>;
+def fbackup_module : Flag<["-"], "fbackup-module">,
+ Group<f_Group>, Flags<[CC1Option, NoDriverOption]>,
+ HelpText<"Backup the generated pre-compiled module/header to an unique location.">,
+ MarshallingInfoFlag<FrontendOpts<"BackupGeneratedModuleFile">>;
def fmodule_map_file : Joined<["-"], "fmodule-map-file=">,
Group<f_Group>, Flags<[NoXarchOption,CC1Option]>, MetaVarName<"<file>">,
HelpText<"Load this module map file">,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
