Make DIE_IS_BEING_PARSED local to the current thread.

[JDevlieghere]

Summary

This is an attempt to realize Pavel's suggestion from D48393

In D48393#1139327, @labath wrote:

It's even more complicated than that, in case you really have reference cycles, you can have multiple threads starting parsing from different points in that cycle, and getting deadlocked waiting for the DIE_IS_BEING_PARSED results from each other.

The only sane algorithm I can come up right now is to make the list of parsed dies local to each thread/parsing entity (e.g. via a "visited" list), and only update the global map once the parsing has completed (successfully or not). This can potentially duplicate some effort where one thread parses a type only to find out that it has already been parsed, but hopefully that is not going to be the common case. The alternative is some complicated resource cycle detection scheme.

[clayborg]

A little background might help here: The lldb_private::Module lock is used to prevent multiple queries into the DWARF from stomping on each other.

Multi-threaded DWARF parsing was primarily added to speed up indexing and the only place it is used. Is that not true? Indexing is just creating name tables and the accelerator tables we need so that we can partially parse the DWARF later. No type parsing should be happening when indexing. All other accesses to the DWARF must be done via a SymbolFile API that takes the module lock to stop multiple threads from stomping on each other.

This fix will not work for a few reasons:

• multiple threads should not be allowed to parse types at the same time, the module lock must be taken, if that isn't the case, then this needs to be fixed
• even if we were to allow this fix and if multiple threads are able to parse DWARF at the same time, this fix wouldn't work. We have one clang AST per symbol and we convert DWARF into clang ASTs. We can't have multiple threads creating a "class Foo" at the same time and populating them at anytime.

So my main point is we need to use the module lock to avoid having multiple threads doing important work that will cause crashes.

source/Plugins/SymbolFile/DWARF/CMakeLists.txt

add_lldb_library(lldbPluginSymbolFileDWARF PLUGIN
   LogChannelDWARF.cpp
   ManualDWARFIndex.cpp
   NameToDIE.cpp
   SymbolFileDWARF.cpp
   SymbolFileDWARFDwo.cpp
   SymbolFileDWARFDwoDwp.cpp
   SymbolFileDWARFDwp.cpp
   SymbolFileDWARFDebugMap.cpp
+  ThreadSafeDIEMap.cpp
   UniqueDWARFASTType.cpp
 
   LINK_LIBS
     clangAST
     clangBasic
     lldbCore
     lldbExpression
     lldbHost

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

 #include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/lldb-private.h"
 
 // Project includes
 #include "DWARFDataExtractor.h"
 #include "DWARFDefines.h"
 #include "DWARFIndex.h"
+#include "ThreadSafeDIEMap.h"
 #include "UniqueDWARFASTType.h"
 
 //----------------------------------------------------------------------
 // Forward Declarations for this DWARF plugin
 //----------------------------------------------------------------------
 class DebugMapModule;
 class DWARFAbbreviationDeclaration;
 class DWARFAbbreviationDeclarationSet;
 class DWARFileUnit;
 class DWARFDebugAbbrev;
 class DWARFDebugAranges;
 class DWARFDebugInfo;
 class DWARFDebugInfoEntry;
 class DWARFDebugLine;
 class DWARFDebugRanges;
 class DWARFDeclContext;
 class DWARFDIECollection;
 class DWARFFormValue;
 class SymbolFileDWARFDebugMap;
 class SymbolFileDWARFDwo;
 class SymbolFileDWARFDwp;
 
-#define DIE_IS_BEING_PARSED ((lldb_private::Type *)1)
-
 class SymbolFileDWARF : public lldb_private::SymbolFile,
                         public lldb_private::UserID {
 public:
   friend class SymbolFileDWARFDebugMap;
   friend class SymbolFileDWARFDwo;
   friend class DebugMapModule;
   friend struct DIERef;
   friend class DWARFUnit;
public:
 
   static bool
   DIEInDeclContext(const lldb_private::CompilerDeclContext *parent_decl_ctx,
                    const DWARFDIE &die);
 
   void Dump(lldb_private::Stream &s) override;
 
 protected:
-  typedef lldb_private::ThreadSafeDenseMap<const DWARFDebugInfoEntry *,
-                                           lldb_private::Type *>
-      DIEToTypePtr;
+  typedef lldb_private::ThreadSafeDIEMap DIEToTypePtr;
   typedef lldb_private::ThreadSafeDenseMap<const DWARFDebugInfoEntry *,
                                            lldb::VariableSP>
       DIEToVariableSP;
   typedef lldb_private::ThreadSafeDenseMap<const DWARFDebugInfoEntry *,
                                            lldb::opaque_compiler_type_t>
       DIEToClangType;
   typedef lldb_private::ThreadSafeDenseMap<lldb::opaque_compiler_type_t, DIERef>
       ClangTypeToDIE;

source/Plugins/SymbolFile/DWARF/ThreadSafeDIEMap.h

+//===-- ThreadSafeDIEMap.h --------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef SymbolFileDWARF_ThreadSafeDIEMap_h_
+#define SymbolFileDWARF_ThreadSafeDIEMap_h_
+
+#include "lldb/Core/ThreadSafeDenseMap.h"
+#include "lldb/Symbol/Type.h"
+
+class DWARFDebugInfoEntry;
+
+#define DIE_IS_BEING_PARSED ((lldb_private::Type *)1)
+
+namespace lldb_private {
+
+/// Special kind of ThreadSafeMap for mapping DWARF DIEs to types. What makes
+/// this class special is that the value DIE_IS_BEING_PARSED is not stored in
+/// the global map, but in thread local storage. The map is only updated when
+/// parsing is complete.
+class ThreadSafeDIEMap : public ThreadSafeDenseMap<const DWARFDebugInfoEntry *,
+                                                   lldb_private::Type *> {
+
+  typedef ThreadSafeDenseMap<const DWARFDebugInfoEntry *, lldb_private::Type *>
+      Parent;
+
+public:
+  void Set(const DWARFDebugInfoEntry *die, lldb_private::Type *type);
+  lldb_private::Type *Lookup(const DWARFDebugInfoEntry *die);
+};
+
+} // namespace lldb_private
+
+#endif // SymbolFileDWARF_ThreadSafeDIEMap_h_

source/Plugins/SymbolFile/DWARF/ThreadSafeDIEMap.cpp

+//===-- ThreadSafeDIEMap.cpp ------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ThreadSafeDIEMap.h"
+#include "DWARFDebugInfo.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+thread_local llvm::DenseSet<const DWARFDebugInfoEntry *> g_dies_being_parsed;
+
+void ThreadSafeDIEMap::Set(const DWARFDebugInfoEntry *die,
+                           lldb_private::Type *type) {
+  if (type == DIE_IS_BEING_PARSED) {
+    g_dies_being_parsed.insert(die);
+  } else {
+    Parent::Set(die, type);
+    g_dies_being_parsed.erase(die);
+  }
+}
+
+lldb_private::Type *ThreadSafeDIEMap::Lookup(const DWARFDebugInfoEntry *die) {
+  // Check the map in case the value was updated from another thread.
+  auto *t = Parent::Lookup(die);
+  if (g_dies_being_parsed.count(die)) {
+    if (t == nullptr)
+      return DIE_IS_BEING_PARSED;
+    g_dies_being_parsed.erase(die);
+  }
+  return t;
+}