[lldb/DWARF] Cache negative results of variable parsing

[labath]

Use a nullptr map entry to mean "variable parse was attempted and it failed" an the absence of an entry to mean "parsing hasn't been attempted yet".

I ran into this because parsing of function-static variables was generating a lot of errors (see #108806), but this seems like a good idea in general.

In theory this should be NFC, except for possibly reducing some amount of work.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Pavel Labath (labath)

Changes

Use a nullptr map entry to mean "variable parse was attempted and it failed" an the absence of an entry to mean "parsing hasn't been attempted yet".

I ran into this because parsing of function-static variables was generating a lot of errors (see #108806), but this seems like a good idea in general.

In theory this should be NFC, except for possibly reducing some amount of work.

---

Full diff: https://github.com/llvm/llvm-project/pull/108810.diff

1 Files Affected:

• (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (+12-14)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index f721ca00fd3559..bacea848988467 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3377,16 +3377,13 @@ VariableSP SymbolFileDWARF::ParseVariableDIECached(const SymbolContext &sc,
 
   DIEToVariableSP &die_to_variable = die.GetDWARF()->GetDIEToVariable();
 
-  VariableSP var_sp = die_to_variable[die.GetDIE()];
-  if (var_sp)
-    return var_sp;
-
-  var_sp = ParseVariableDIE(sc, die, LLDB_INVALID_ADDRESS);
-  if (var_sp) {
-    die_to_variable[die.GetDIE()] = var_sp;
-    if (DWARFDIE spec_die = die.GetReferencedDIE(DW_AT_specification))
-      die_to_variable[spec_die.GetDIE()] = var_sp;
-  }
+  if (auto it = die_to_variable.find(die.GetDIE()); it != die_to_variable.end())
+    return it->second;
+
+  VariableSP var_sp = ParseVariableDIE(sc, die, LLDB_INVALID_ADDRESS);
+  die_to_variable.insert_or_assign(die.GetDIE(), var_sp);
+  if (DWARFDIE spec_die = die.GetReferencedDIE(DW_AT_specification))
+    die_to_variable.insert_or_assign(spec_die.GetDIE(), var_sp);
   return var_sp;
 }
 
@@ -3799,9 +3796,10 @@ void SymbolFileDWARF::ParseAndAppendGlobalVariable(
     return;
 
   // Check to see if we have already parsed this variable or constant?
-  VariableSP var_sp = GetDIEToVariable()[die.GetDIE()];
-  if (var_sp) {
-    cc_variable_list.AddVariableIfUnique(var_sp);
+  DIEToVariableSP &die_to_variable = GetDIEToVariable();
+  if (auto it = die_to_variable.find(die.GetDIE());
+      it != die_to_variable.end()) {
+    cc_variable_list.AddVariableIfUnique(it->second);
     return;
   }
 
@@ -3835,7 +3833,7 @@ void SymbolFileDWARF::ParseAndAppendGlobalVariable(
     return;
   }
 
-  var_sp = ParseVariableDIECached(sc, die);
+  VariableSP var_sp = ParseVariableDIECached(sc, die);
   if (!var_sp)
     return;
 

[Michael137]

Are there cases where we fail to ParseVariableDIE the first time around but could succeed later on in the debug sessions?

This comment seems to suggest that's possible:

llvm-project/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Lines 3714 to 3719 in bbab5e4

	if (!symbol_context_scope) {
	// Not ready to parse this variable yet. It might be a global or static
	// variable that is in a function scope and the function in the symbol
	// context wasn't filled in yet
	return nullptr;
	}

[labath]

Good find. Somewhat surprising, but I guess it's possible. I'll try to look into whether this is some defensive coding or if this can legitimately happen for some reason.