[Lldb-commits] [PATCH] D40470: DWZ 07/12: Protect DWARFCompileUnit::m_die_array by a new mutex

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp:123
 size_t DWARFCompileUnit::ExtractDIEsIfNeeded(bool cu_die_only) {
-  const size_t initial_die_array_size = m_data->m_die_array.size();
-  if ((cu_die_only && initial_die_array_size > 0) || initial_die_array_size > 1)
-    return 0; // Already parsed
+  size_t initial_die_array_size;
+  auto already_parsed = [cu_die_only, &initial_die_array_size]() -> bool {
----------------
clayborg wrote:
> I really can't tell what any of the above changed code is doing besides adding the mutex. Seems like you are trying to emulate a condition variable to halt other threads trying to get access to the DIEs. But m_die_array_finished isn't initialized in the constructor or inlined in the header file and initial_die_array_size isn't initialized so I can't really tell what this is supposed to actually do.
This isn't initialized and it is captured and used in already_parsed below?


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp:123-137
+  size_t initial_die_array_size;
+  auto already_parsed = [cu_die_only, &initial_die_array_size]() -> bool {
+    return (cu_die_only && initial_die_array_size > 0)
+        || initial_die_array_size > 1;
+  };
+  if (already_parsed() && m_data->m_die_array_finished)
+    return 0;
----------------
I really can't tell what any of the above changed code is doing besides adding the mutex. Seems like you are trying to emulate a condition variable to halt other threads trying to get access to the DIEs. But m_die_array_finished isn't initialized in the constructor or inlined in the header file and initial_die_array_size isn't initialized so I can't really tell what this is supposed to actually do.


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp:128
+  };
+  if (already_parsed() && m_data->m_die_array_finished)
+    return 0;
----------------
initial_die_array_size is used uninitialized when calling already_parsed()


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp:131
+  std::lock_guard<std::mutex> guard(m_data->m_extractdies_mutex);
+  if (already_parsed()) {
+    lldbassert(m_data->m_die_array_finished);
----------------
initial_die_array_size is used uninitialized when calling already_parsed()


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h:74
+  std::mutex m_extractdies_mutex;
+  bool m_die_array_finished;
+
----------------
This isn't initialized anywhere.


https://reviews.llvm.org/D40470