labath created this revision.
labath added reviewers: aprantl, JDevlieghere, clayborg.
Herald added a subscriber: arphaman.

SymbolFileDWARF used to load debug sections in a thread-safe manner.
When we moved to DWARFContext, we dropped the thread-safe part, because
we thought it was not necessary.

It turns out this was only mostly correct.

The "mostly" part is there because this is a problem only if we use the
manual index, as that is the only source of intra-module paralelism.
Also, this only seems to occur for extremely simple files (like the ones
I've been creating for tests lately), where we've managed to start
indexing before loading the debug_str section. Then, two threads start
to load the section simultaneously and produce wrong results.

On more complex files, something seems to be loading the debug_str section
before we start indexing, as I haven't been able to reproduce this
there, but I have not investigated what it is.

I've tried to come up with a test for this, but I haven't been able to
reproduce the problem reliably. Still, while doing so, I created a way
to generate many compile units on demand. Given that most of our tests
work with only one or two compile units, it seems like this could be
useful anyway.


https://reviews.llvm.org/D62316

Files:
  lit/SymbolFile/DWARF/parallel-indexing-stress.s
  source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
  source/Plugins/SymbolFile/DWARF/DWARFContext.h

Index: source/Plugins/SymbolFile/DWARF/DWARFContext.h
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFContext.h
+++ source/Plugins/SymbolFile/DWARF/DWARFContext.h
@@ -12,6 +12,7 @@
 #include "DWARFDataExtractor.h"
 #include "lldb/Core/Section.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/Support/Threading.h"
 #include <memory>
 
 namespace lldb_private {
@@ -20,19 +21,29 @@
   SectionList *m_main_section_list;
   SectionList *m_dwo_section_list;
 
-  llvm::Optional<DWARFDataExtractor> m_data_debug_abbrev;
-  llvm::Optional<DWARFDataExtractor> m_data_debug_addr;
-  llvm::Optional<DWARFDataExtractor> m_data_debug_aranges;
-  llvm::Optional<DWARFDataExtractor> m_data_debug_info;
-  llvm::Optional<DWARFDataExtractor> m_data_debug_line;
-  llvm::Optional<DWARFDataExtractor> m_data_debug_line_str;
-  llvm::Optional<DWARFDataExtractor> m_data_debug_macro;
-  llvm::Optional<DWARFDataExtractor> m_data_debug_str;
-  llvm::Optional<DWARFDataExtractor> m_data_debug_str_offsets;
-  llvm::Optional<DWARFDataExtractor> m_data_debug_types;
+  struct SectionData {
+    llvm::once_flag flag;
+    DWARFDataExtractor data;
+  };
+
+  SectionData m_data_debug_abbrev;
+  SectionData m_data_debug_addr;
+  SectionData m_data_debug_aranges;
+  SectionData m_data_debug_info;
+  SectionData m_data_debug_line;
+  SectionData m_data_debug_line_str;
+  SectionData m_data_debug_macro;
+  SectionData m_data_debug_str;
+  SectionData m_data_debug_str_offsets;
+  SectionData m_data_debug_types;
 
   bool isDwo() { return m_dwo_section_list != nullptr; }
 
+  const DWARFDataExtractor &
+  LoadOrGetSection(lldb::SectionType main_section_type,
+                   llvm::Optional<lldb::SectionType> dwo_section_type,
+                   SectionData &data);
+
 public:
   explicit DWARFContext(SectionList *main_section_list,
                         SectionList *dwo_section_list)
Index: source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
@@ -27,72 +27,66 @@
   return data;
 }
 
-static const DWARFDataExtractor &
-LoadOrGetSection(SectionList *section_list, SectionType section_type,
-                 llvm::Optional<DWARFDataExtractor> &extractor) {
-  if (!extractor)
-    extractor = LoadSection(section_list, section_type);
-  return *extractor;
+const DWARFDataExtractor &
+DWARFContext::LoadOrGetSection(SectionType main_section_type,
+                               llvm::Optional<SectionType> dwo_section_type,
+                               SectionData &data) {
+  llvm::call_once(data.flag, [&] {
+    if (dwo_section_type && isDwo())
+      data.data = LoadSection(m_dwo_section_list, *dwo_section_type);
+    else
+      data.data = LoadSection(m_main_section_list, main_section_type);
+  });
+  return data.data;
 }
 
 const DWARFDataExtractor &DWARFContext::getOrLoadAbbrevData() {
-  if (isDwo())
-    return LoadOrGetSection(m_dwo_section_list, eSectionTypeDWARFDebugAbbrevDwo,
-                            m_data_debug_abbrev);
-  return LoadOrGetSection(m_main_section_list, eSectionTypeDWARFDebugAbbrev,
-                          m_data_debug_abbrev);
+  return LoadOrGetSection(eSectionTypeDWARFDebugAbbrev,
+                          eSectionTypeDWARFDebugAbbrevDwo, m_data_debug_abbrev);
 }
 
 const DWARFDataExtractor &DWARFContext::getOrLoadArangesData() {
-  return LoadOrGetSection(m_main_section_list, eSectionTypeDWARFDebugAranges,
+  return LoadOrGetSection(eSectionTypeDWARFDebugAranges, llvm::None,
                           m_data_debug_aranges);
 }
 
 const DWARFDataExtractor &DWARFContext::getOrLoadAddrData() {
-  return LoadOrGetSection(m_main_section_list, eSectionTypeDWARFDebugAddr,
+  return LoadOrGetSection(eSectionTypeDWARFDebugAddr, llvm::None,
                           m_data_debug_addr);
 }
 
 const DWARFDataExtractor &DWARFContext::getOrLoadDebugInfoData() {
-  if (isDwo())
-    return LoadOrGetSection(m_dwo_section_list, eSectionTypeDWARFDebugInfoDwo,
-                            m_data_debug_info);
-  return LoadOrGetSection(m_main_section_list, eSectionTypeDWARFDebugInfo,
-                          m_data_debug_info);
+  return LoadOrGetSection(eSectionTypeDWARFDebugInfo,
+                          eSectionTypeDWARFDebugInfoDwo, m_data_debug_info);
 }
 
 const DWARFDataExtractor &DWARFContext::getOrLoadLineData() {
-  return LoadOrGetSection(m_main_section_list, eSectionTypeDWARFDebugLine,
+  return LoadOrGetSection(eSectionTypeDWARFDebugLine, llvm::None,
                           m_data_debug_line);
 }
 
 const DWARFDataExtractor &DWARFContext::getOrLoadLineStrData() {
-  return LoadOrGetSection(m_main_section_list, eSectionTypeDWARFDebugLineStr,
+  return LoadOrGetSection(eSectionTypeDWARFDebugLineStr, llvm::None,
                           m_data_debug_line_str);
 }
 
 const DWARFDataExtractor &DWARFContext::getOrLoadMacroData() {
-  return LoadOrGetSection(m_main_section_list, eSectionTypeDWARFDebugMacro,
+  return LoadOrGetSection(eSectionTypeDWARFDebugMacro, llvm::None,
                           m_data_debug_macro);
 }
 
 const DWARFDataExtractor &DWARFContext::getOrLoadStrData() {
-  if (isDwo())
-    return LoadOrGetSection(m_dwo_section_list, eSectionTypeDWARFDebugStrDwo,
-                            m_data_debug_str);
-  return LoadOrGetSection(m_main_section_list, eSectionTypeDWARFDebugStr,
-                          m_data_debug_str);
+  return LoadOrGetSection(eSectionTypeDWARFDebugStr,
+                          eSectionTypeDWARFDebugStrDwo, m_data_debug_str);
 }
 
 const DWARFDataExtractor &DWARFContext::getOrLoadStrOffsetsData() {
-  if (isDwo())
-    return LoadOrGetSection(m_dwo_section_list, eSectionTypeDWARFDebugStrOffsetsDwo,
-                            m_data_debug_str_offsets);
-  return LoadOrGetSection(m_main_section_list, eSectionTypeDWARFDebugStrOffsets,
+  return LoadOrGetSection(eSectionTypeDWARFDebugStrOffsets,
+                          eSectionTypeDWARFDebugStrOffsetsDwo,
                           m_data_debug_str_offsets);
 }
 
 const DWARFDataExtractor &DWARFContext::getOrLoadDebugTypesData() {
-  return LoadOrGetSection(m_main_section_list, eSectionTypeDWARFDebugTypes,
+  return LoadOrGetSection(eSectionTypeDWARFDebugTypes, llvm::None,
                           m_data_debug_types);
 }
Index: lit/SymbolFile/DWARF/parallel-indexing-stress.s
===================================================================
--- /dev/null
+++ lit/SymbolFile/DWARF/parallel-indexing-stress.s
@@ -0,0 +1,82 @@
+# Stress-test the parallel indexing of compile units.
+
+# RUN: llvm-mc -triple x86_64-pc-linux %s -o %t -filetype=obj
+# RUN: %lldb %t -o "target variable A" -b | FileCheck %s
+
+# CHECK-COUNT-256: A = 47
+
+	.section	.debug_str,"MS",@progbits,1
+.Linfo_string0:
+	.asciz	"Hand-written DWARF"
+.Lname:
+	.asciz	"A"
+.Linfo_string4:
+	.asciz	"int"                   # string offset=95
+
+	.section	.debug_abbrev,"",@progbits
+	.byte	1                       # Abbreviation Code
+	.byte	17                      # DW_TAG_compile_unit
+	.byte	1                       # DW_CHILDREN_yes
+	.byte	37                      # DW_AT_producer
+	.byte	14                      # DW_FORM_strp
+	.byte	0                       # EOM(1)
+	.byte	0                       # EOM(2)
+	.byte	2                       # Abbreviation Code
+	.byte	52                      # DW_TAG_variable
+	.byte	0                       # DW_CHILDREN_no
+	.byte	3                       # DW_AT_name
+	.byte	14                      # DW_FORM_strp
+	.byte	73                      # DW_AT_type
+	.byte	19                      # DW_FORM_ref4
+	.byte	2                       # DW_AT_location
+	.byte	24                      # DW_FORM_exprloc
+	.byte	0                       # EOM(1)
+	.byte	0                       # EOM(2)
+	.byte	3                       # Abbreviation Code
+	.byte	36                      # DW_TAG_base_type
+	.byte	0                       # DW_CHILDREN_no
+	.byte	3                       # DW_AT_name
+	.byte	14                      # DW_FORM_strp
+	.byte	62                      # DW_AT_encoding
+	.byte	11                      # DW_FORM_data1
+	.byte	11                      # DW_AT_byte_size
+	.byte	11                      # DW_FORM_data1
+	.byte	0                       # EOM(1)
+	.byte	0                       # EOM(2)
+	.byte	0                       # EOM(3)
+
+.macro generate_unit
+	.data
+A\@:
+	.long	47
+
+	.section	.debug_str,"MS",@progbits,1
+
+	.section	.debug_info,"",@progbits
+.Lcu_begin\@:
+	.long	.Ldebug_info_end\@-.Ldebug_info_start\@ # Length of Unit
+.Ldebug_info_start\@:
+	.short	4                       # DWARF version number
+	.long	.debug_abbrev           # Offset Into Abbrev. Section
+	.byte	8                       # Address Size (in bytes)
+	.byte	1                       # Abbrev [1] 0xb:0x30 DW_TAG_compile_unit
+	.long	.Linfo_string0          # DW_AT_producer
+	.byte	2                       # Abbrev [2] 0x1e:0x15 DW_TAG_variable
+	.long	.Lname                  # DW_AT_name
+	.long	.Ltype\@-.Lcu_begin\@   # DW_AT_type
+	.byte	9                       # DW_AT_location
+	.byte	3
+	.quad	A\@
+.Ltype\@:
+	.byte	3                       # Abbrev [3] 0x33:0x7 DW_TAG_base_type
+	.long	.Linfo_string4          # DW_AT_name
+	.byte	5                       # DW_AT_encoding
+	.byte	4                       # DW_AT_byte_size
+	.byte	0                       # End Of Children Mark
+.Ldebug_info_end\@:
+
+.endm
+
+.rept 256
+generate_unit
+.endr
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to