Correctly handle DWARF subroutines during parallel parsing (#1215)

Fix to only process the DWARF for a function (subprogram) once if it is defined in multiple compilation units within a library or executable. This prevents duplication of parameters, local variables and use of thread-unsafe data structures from concurrent threads.
dyninst · Mar 9, 2022 · 8b400af · 8b400af
1 parent 9b5ffe3
commit 8b400af
Show file tree

Hide file tree

Showing 2 changed files with 69 additions and 14 deletions.
diff --git a/symtabAPI/src/dwarfWalker.C b/symtabAPI/src/dwarfWalker.C
@@ -80,8 +80,9 @@ using namespace std;
    } while (0)
 #define DWARF_CHECK_RET(x) DWARF_CHECK_RET_VAL(x, false)
 
-DwarfWalker::DwarfWalker(Symtab *symtab, ::Dwarf * dbg) :
+DwarfWalker::DwarfWalker(Symtab *symtab, ::Dwarf *dbg, std::shared_ptr<ParsedFuncs> pf) :
    DwarfParseActions(symtab, dbg),
+   parsedFuncs(pf),
    is_mangled_name_(false),
    modLow(0),
    modHigh(0),
@@ -96,6 +97,9 @@ DwarfWalker::DwarfWalker(Symtab *symtab, ::Dwarf * dbg) :
    next_cu_header(0),
    compile_offset(0)
 {
+    if (!parsedFuncs)  {
+        parsedFuncs = std::make_shared<ParsedFuncs>();
+    }
 }
 
 DwarfWalker::~DwarfWalker() {
@@ -180,7 +184,7 @@ bool DwarfWalker::parse() {
     } else {
 #pragma omp parallel
     {
-    DwarfWalker w(symtab(), dbg());
+    DwarfWalker w(symtab(), dbg(), parsedFuncs);
 #pragma omp for reduction(leftmost:fixUnknownMod) \
         schedule(dynamic) nowait
     for (unsigned int i = 0; i < module_dies.size(); i++) {
@@ -610,6 +614,28 @@ void DwarfParseActions::addPrettyFuncName(std::string name)
    curFunc()->addPrettyName(name, true, true);
 }
 
+
+// helper class to set and restore currentSubprogramFunction
+namespace {
+class SetAndRestoreFunction
+{
+    public:
+        SetAndRestoreFunction(FunctionBase* &v, FunctionBase* newFunc) : var(v)
+        {
+            savedFunc = var;
+            var = newFunc;
+        }
+        ~SetAndRestoreFunction()
+        {
+            var = savedFunc;
+        }
+    private:
+        FunctionBase* &var;
+        FunctionBase* savedFunc;
+};
+}
+
+
 bool DwarfWalker::parseSubprogram(DwarfWalker::inline_t func_type) {
    bool name_result;
 
@@ -645,15 +671,29 @@ bool DwarfWalker::parseSubprogram(DwarfWalker::inline_t func_type) {
       return true;
    }
 
-   if (parsedFuncs.find(func) != parsedFuncs.end()) {
-      dwarf_printf("(0x%lx) parseSubprogram not parsing children b/c curFunc() not in parsedFuncs\n", id());
-      if(name_result) {
-	  dwarf_printf("\tname is %s\n", curName().c_str());
-      }
-      setParseChild(false);
-      return true;
+   // this function's accessor lock to check if parsed and/or parse it
+   ParsedFuncs::accessor funcParsed;
+
+   // recursive if parsing a specification or abstract origin,
+   // accessor is already locked
+   bool isRecursiveSubprogramParse = (func == currentSubprogramFunction);
+
+   if (!isRecursiveSubprogramParse)  {
+       // check if function is parsed, hold mutex until scope is left
+       parsedFuncs->insert(funcParsed, func);
+       if (funcParsed->second) {
+          dwarf_printf("(0x%lx) parseSubprogram not parsing children b/c curFunc() marked parsed in parsedFuncs\n", id());
+          if(name_result) {
+              dwarf_printf("\tname is %s\n", curName().c_str());
+          }
+          setParseChild(false);
+          return true;
+       }
    }
 
+   // set the current function being parsed and restore old value on return
+   SetAndRestoreFunction setAndRestore(currentSubprogramFunction, func);
+
    if (name_result && !curName().empty()) {
       dwarf_printf("(0x%lx) Identified function name as %s\n", id(), curName().c_str());
       if (isMangledName()) {
@@ -711,7 +751,11 @@ bool DwarfWalker::parseSubprogram(DwarfWalker::inline_t func_type) {
           return false;
    }
 
-   parsedFuncs.insert(func);
+   if (!isRecursiveSubprogramParse)  {
+       // the function is now parsed
+       funcParsed->second = true;
+   }
+
     if (func_type == InlinedFunc) {
         dwarf_printf("End parseSubprogram for inlined func at 0x%p\n", (void*)func->getOffset());
     }

diff --git a/symtabAPI/src/dwarfWalker.h b/symtabAPI/src/dwarfWalker.h
@@ -214,6 +214,10 @@ class DwarfParseActions {
     Symtab *symtab_;
     virtual Object * obj() const ;
 
+    // Function object of current subprogram being parsed; used to detect
+    // parseSubprogram recursion
+    FunctionBase *currentSubprogramFunction = nullptr;
+
 }; // class DwarfParseActions 
 
 struct ContextGuard {
@@ -233,7 +237,9 @@ class DwarfWalker : public DwarfParseActions {
 
     } Error;
 
-    DwarfWalker(Symtab *symtab, Dwarf* dbg);
+    using ParsedFuncs = tbb::concurrent_hash_map<FunctionBase *, bool>;
+
+    DwarfWalker(Symtab *symtab, Dwarf* dbg, std::shared_ptr<ParsedFuncs> pf = nullptr);
 
     DwarfWalker(const DwarfWalker& o) :
             DwarfParseActions(o),
@@ -254,7 +260,12 @@ class DwarfWalker : public DwarfParseActions {
             compile_offset(o.compile_offset),
             info_type_ids_(o.info_type_ids_),
             types_type_ids_(o.types_type_ids_),
-            sig8_type_ids_(o.sig8_type_ids_) {}
+            sig8_type_ids_(o.sig8_type_ids_)
+            {
+                if (!parsedFuncs)  {
+                    parsedFuncs = std::make_shared<ParsedFuncs>();
+                }
+            }
 
     virtual ~DwarfWalker();
 
@@ -400,8 +411,8 @@ class DwarfWalker : public DwarfParseActions {
             Dwarf_Sword listLength);
 
 
-    // Header-only functions get multiple parsed.
-    std::set<FunctionBase *> parsedFuncs;
+    // Map of Function* to bool (indicates function parsed)
+    std::shared_ptr<ParsedFuncs> parsedFuncs;
 private:
     std::string name_;
     bool is_mangled_name_;