Skip to content

Commit

Permalink
[dsymutil] Prevent non-determinism due to threading.
Browse files Browse the repository at this point in the history 
Before this patch, analyzeContext called getCanonicalDIEOffset(), for
which the result depends on the timings of the setCanonicalDIEOffset()
calls in the cloneLambda. This can lead to slightly different output
between runs due to threading.

To prevent this from happening, we now record the output debug info size
after importing the modules (before any concurrent processing takes
place). This value, named the ModulesEndOffset is used to compare the
canonical DIE offset against. If the value is greater than this offset,
the canonical DIE offset has been updated during cloning, and should
therefore not be considered for pruning.

Differential revision: https://reviews.llvm.org/D51443

llvm-svn: 341649
  • Loading branch information
@JDevlieghere
JDevlieghere committed Sep 7, 2018
1 parent b746df0 commit 475ce5a
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 20 deletions.
49 changes: 32 additions & 17 deletions llvm/tools/dsymutil/DwarfLinker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ static bool analyzeContextInfo(const DWARFDie &DIE, unsigned ParentIdx,
CompileUnit &CU, DeclContext *CurrentDeclContext,
UniquingStringPool &StringPool,
DeclContextTree &Contexts,
uint64_t ModulesEndOffset,
bool InImportedModule = false) {
unsigned MyIdx = CU.getOrigUnit().getDIEIndex(DIE);
CompileUnit::DIEInfo &Info = CU.getInfo(MyIdx);
Expand Down Expand Up @@ -296,8 +297,9 @@ static bool analyzeContextInfo(const DWARFDie &DIE, unsigned ParentIdx,
Info.Prune = InImportedModule;
if (DIE.hasChildren())
for (auto Child : DIE.children())
Info.Prune &= analyzeContextInfo(Child, MyIdx, CU, CurrentDeclContext,
StringPool, Contexts, InImportedModule);
Info.Prune &=
analyzeContextInfo(Child, MyIdx, CU, CurrentDeclContext, StringPool,
Contexts, ModulesEndOffset, InImportedModule);

// Prune this DIE if it is either a forward declaration inside a
// DW_TAG_module or a DW_TAG_module that contains nothing but
Expand All @@ -306,11 +308,16 @@ static bool analyzeContextInfo(const DWARFDie &DIE, unsigned ParentIdx,
(isTypeTag(DIE.getTag()) &&
dwarf::toUnsigned(DIE.find(dwarf::DW_AT_declaration), 0));

// Don't prune it if there is no definition for the DIE.
Info.Prune &= Info.Ctxt && Info.Ctxt->getCanonicalDIEOffset();
// Only prune forward declarations inside a DW_TAG_module for which a
// definition exists elsewhere.
if (ModulesEndOffset == 0)
Info.Prune &= Info.Ctxt && Info.Ctxt->getCanonicalDIEOffset();
else
Info.Prune &= Info.Ctxt && Info.Ctxt->getCanonicalDIEOffset() > 0 &&
Info.Ctxt->getCanonicalDIEOffset() <= ModulesEndOffset;

return Info.Prune;
}
} // namespace dsymutil

static bool dieNeedsChildrenToBeMeaningful(uint32_t Tag) {
switch (Tag) {
Expand Down Expand Up @@ -2025,7 +2032,7 @@ bool DwarfLinker::registerModuleReference(
const DWARFDie &CUDie, const DWARFUnit &Unit, DebugMap &ModuleMap,
const DebugMapObject &DMO, RangesTy &Ranges, OffsetsStringPool &StringPool,
UniquingStringPool &UniquingStringPool, DeclContextTree &ODRContexts,
unsigned &UnitID, unsigned Indent, bool Quiet) {
uint64_t ModulesEndOffset, unsigned &UnitID, unsigned Indent, bool Quiet) {
std::string PCMfile = dwarf::toString(
CUDie.find({dwarf::DW_AT_dwo_name, dwarf::DW_AT_GNU_dwo_name}), "");
if (PCMfile.empty())
Expand Down Expand Up @@ -2067,9 +2074,10 @@ bool DwarfLinker::registerModuleReference(
// Cyclic dependencies are disallowed by Clang, but we still
// shouldn't run into an infinite loop, so mark it as processed now.
ClangModules.insert({PCMfile, DwoId});
if (Error E = loadClangModule(PCMfile, PCMpath, Name, DwoId, ModuleMap, DMO,
Ranges, StringPool, UniquingStringPool,
ODRContexts, UnitID, Indent + 2, Quiet)) {
if (Error E =
loadClangModule(PCMfile, PCMpath, Name, DwoId, ModuleMap, DMO, Ranges,
StringPool, UniquingStringPool, ODRContexts,
ModulesEndOffset, UnitID, Indent + 2, Quiet)) {
consumeError(std::move(E));
return false;
}
Expand Down Expand Up @@ -2103,7 +2111,7 @@ Error DwarfLinker::loadClangModule(
uint64_t DwoId, DebugMap &ModuleMap, const DebugMapObject &DMO,
RangesTy &Ranges, OffsetsStringPool &StringPool,
UniquingStringPool &UniquingStringPool, DeclContextTree &ODRContexts,
unsigned &UnitID, unsigned Indent, bool Quiet) {
uint64_t ModulesEndOffset, unsigned &UnitID, unsigned Indent, bool Quiet) {
SmallString<80> Path(Options.PrependPath);
if (sys::path::is_relative(Filename))
sys::path::append(Path, ModulePath, Filename);
Expand Down Expand Up @@ -2164,8 +2172,8 @@ Error DwarfLinker::loadClangModule(
if (!CUDie)
continue;
if (!registerModuleReference(CUDie, *CU, ModuleMap, DMO, Ranges, StringPool,
UniquingStringPool, ODRContexts, UnitID,
Indent, Quiet)) {
UniquingStringPool, ODRContexts,
ModulesEndOffset, UnitID, Indent, Quiet)) {
if (Unit) {
std::string Err =
(Filename +
Expand Down Expand Up @@ -2194,7 +2202,7 @@ Error DwarfLinker::loadClangModule(
ModuleName);
Unit->setHasInterestingContent();
analyzeContextInfo(CUDie, 0, *Unit, &ODRContexts.getRoot(),
UniquingStringPool, ODRContexts);
UniquingStringPool, ODRContexts, ModulesEndOffset);
// Keep everything.
Unit->markEverythingAsKept();
}
Expand Down Expand Up @@ -2469,14 +2477,21 @@ bool DwarfLinker::link(const DebugMap &Map) {
if (CUDie && !LLVM_UNLIKELY(Options.Update))
registerModuleReference(CUDie, *CU, ModuleMap, LinkContext.DMO,
LinkContext.Ranges, OffsetsStringPool,
UniquingStringPool, ODRContexts, UnitID);
UniquingStringPool, ODRContexts, 0, UnitID);
}
}

// If we haven't seen any CUs, pick an arbitrary valid Dwarf version anyway.
if (MaxDwarfVersion == 0)
MaxDwarfVersion = 3;

// At this point we know how much data we have emitted. We use this value to
// compare canonical DIE offsets in analyzeContextInfo to see if a definition
// is already emitted, without being affected by canonical die offsets set
// later. This prevents undeterminism when analyze and clone execute
// concurrently, as clone set the canonical DIE offset and analyze reads it.
const uint64_t ModulesEndOffset = OutputDebugInfoSize;

// These variables manage the list of processed object files.
// The mutex and condition variable are to ensure that this is thread safe.
std::mutex ProcessedFilesMutex;
Expand All @@ -2503,8 +2518,8 @@ bool DwarfLinker::link(const DebugMap &Map) {
if (!CUDie || LLVM_UNLIKELY(Options.Update) ||
!registerModuleReference(CUDie, *CU, ModuleMap, LinkContext.DMO,
LinkContext.Ranges, OffsetsStringPool,
UniquingStringPool, ODRContexts, UnitID,
Quiet)) {
UniquingStringPool, ODRContexts,
ModulesEndOffset, UnitID, Quiet)) {
LinkContext.CompileUnits.push_back(llvm::make_unique<CompileUnit>(
*CU, UnitID++, !Options.NoODR && !Options.Update, ""));
}
Expand All @@ -2517,7 +2532,7 @@ bool DwarfLinker::link(const DebugMap &Map) {
continue;
analyzeContextInfo(CurrentUnit->getOrigUnit().getUnitDIE(), 0,
*CurrentUnit, &ODRContexts.getRoot(),
UniquingStringPool, ODRContexts);
UniquingStringPool, ODRContexts, ModulesEndOffset);
}
};

Expand Down
8 changes: 5 additions & 3 deletions llvm/tools/dsymutil/DwarfLinker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,8 @@ class DwarfLinker {
RangesTy &Ranges,
OffsetsStringPool &OffsetsStringPool,
UniquingStringPool &UniquingStringPoolStringPool,
DeclContextTree &ODRContexts, unsigned &UnitID,
DeclContextTree &ODRContexts,
uint64_t ModulesEndOffset, unsigned &UnitID,
unsigned Indent = 0, bool Quiet = false);

/// Recursively add the debug info in this clang module .pcm
Expand All @@ -210,8 +211,9 @@ class DwarfLinker {
DebugMap &ModuleMap, const DebugMapObject &DMO,
RangesTy &Ranges, OffsetsStringPool &OffsetsStringPool,
UniquingStringPool &UniquingStringPool,
DeclContextTree &ODRContexts, unsigned &UnitID,
unsigned Indent = 0, bool Quiet = false);
DeclContextTree &ODRContexts, uint64_t ModulesEndOffset,
unsigned &UnitID, unsigned Indent = 0,
bool Quiet = false);

/// Flags passed to DwarfLinker::lookForDIEsToKeep
enum TraversalFlags {
Expand Down

0 comments on commit 475ce5a

Please sign in to comment.