[llvm] r341649 - [dsymutil] Prevent non-determinism due to threading.

Jonas Devlieghere via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 7 03:29:22 PDT 2018


Author: jdevlieghere
Date: Fri Sep  7 03:29:22 2018
New Revision: 341649

URL: http://llvm.org/viewvc/llvm-project?rev=341649&view=rev
Log:
[dsymutil] Prevent non-determinism due to threading.

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

Modified:
    llvm/trunk/tools/dsymutil/DwarfLinker.cpp
    llvm/trunk/tools/dsymutil/DwarfLinker.h

Modified: llvm/trunk/tools/dsymutil/DwarfLinker.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/dsymutil/DwarfLinker.cpp?rev=341649&r1=341648&r2=341649&view=diff
==============================================================================
--- llvm/trunk/tools/dsymutil/DwarfLinker.cpp (original)
+++ llvm/trunk/tools/dsymutil/DwarfLinker.cpp Fri Sep  7 03:29:22 2018
@@ -256,6 +256,7 @@ static bool analyzeContextInfo(const DWA
                                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);
@@ -296,8 +297,9 @@ static bool analyzeContextInfo(const DWA
   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
@@ -306,11 +308,16 @@ static bool analyzeContextInfo(const DWA
                 (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) {
@@ -2025,7 +2032,7 @@ bool DwarfLinker::registerModuleReferenc
     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())
@@ -2067,9 +2074,10 @@ bool DwarfLinker::registerModuleReferenc
   // 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;
   }
@@ -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);
@@ -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 +
@@ -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();
     }
@@ -2469,7 +2477,7 @@ bool DwarfLinker::link(const DebugMap &M
       if (CUDie && !LLVM_UNLIKELY(Options.Update))
         registerModuleReference(CUDie, *CU, ModuleMap, LinkContext.DMO,
                                 LinkContext.Ranges, OffsetsStringPool,
-                                UniquingStringPool, ODRContexts, UnitID);
+                                UniquingStringPool, ODRContexts, 0, UnitID);
     }
   }
 
@@ -2477,6 +2485,13 @@ bool DwarfLinker::link(const DebugMap &M
   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;
@@ -2503,8 +2518,8 @@ bool DwarfLinker::link(const DebugMap &M
       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, ""));
       }
@@ -2517,7 +2532,7 @@ bool DwarfLinker::link(const DebugMap &M
         continue;
       analyzeContextInfo(CurrentUnit->getOrigUnit().getUnitDIE(), 0,
                          *CurrentUnit, &ODRContexts.getRoot(),
-                         UniquingStringPool, ODRContexts);
+                         UniquingStringPool, ODRContexts, ModulesEndOffset);
     }
   };
 

Modified: llvm/trunk/tools/dsymutil/DwarfLinker.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/dsymutil/DwarfLinker.h?rev=341649&r1=341648&r2=341649&view=diff
==============================================================================
--- llvm/trunk/tools/dsymutil/DwarfLinker.h (original)
+++ llvm/trunk/tools/dsymutil/DwarfLinker.h Fri Sep  7 03:29:22 2018
@@ -199,7 +199,8 @@ private:
                                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
@@ -210,8 +211,9 @@ private:
                         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 {




More information about the llvm-commits mailing list