jarin created this revision.
jarin added a reviewer: labath.
jarin added a project: LLDB.
Herald added a subscriber: JDevlieghere.
jarin requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: lldb-commits, sstefan1.

This is in preparation for a fix of parsing omitted abstract
parameters of inlined functions (see the bug
https://bugs.llvm.org/show_bug.cgi?id=50076#c6 for more details). The
main idea is to move the recursive parsing of variables in a function
context into a separate method (ParseVariablesInFunctionContext).

We also move the setup of block variable list to the DIE node that
starts the block rather than setting up the list when processing
the first variable DIE in that block.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110570

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -380,11 +380,14 @@
                                     const DWARFDIE &die,
                                     const lldb::addr_t func_low_pc);
 
-  size_t ParseVariables(const lldb_private::SymbolContext &sc,
-                        const DWARFDIE &orig_die,
-                        const lldb::addr_t func_low_pc, bool parse_siblings,
-                        bool parse_children,
-                        lldb_private::VariableList *cc_variable_list = nullptr);
+  void
+  ParseAndAppendGlobalVariable(const lldb_private::SymbolContext &sc,
+                               const DWARFDIE &die,
+                               lldb_private::VariableList &cc_variable_list);
+
+  size_t ParseVariablesInFunctionContext(const lldb_private::SymbolContext &sc,
+                                         const DWARFDIE &die,
+                                         const lldb::addr_t func_low_pc);
 
   bool ClassOrStructIsVirtual(const DWARFDIE &die);
 
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2135,7 +2135,7 @@
       }
     }
 
-    ParseVariables(sc, die, LLDB_INVALID_ADDRESS, false, false, &variables);
+    ParseAndAppendGlobalVariable(sc, die, variables);
     while (pruned_idx < variables.GetSize()) {
       VariableSP var_sp = variables.GetVariableAtIndex(pruned_idx);
       if (name_is_mangled ||
@@ -2188,7 +2188,7 @@
       return true;
     sc.comp_unit = GetCompUnitForDWARFCompUnit(*dwarf_cu);
 
-    ParseVariables(sc, die, LLDB_INVALID_ADDRESS, false, false, &variables);
+    ParseAndAppendGlobalVariable(sc, die, variables);
 
     return variables.GetSize() - original_size < max_matches;
   });
@@ -3049,8 +3049,8 @@
               /*check_hi_lo_pc=*/true))
         func_lo_pc = ranges.GetMinRangeBase(0);
       if (func_lo_pc != LLDB_INVALID_ADDRESS) {
-        const size_t num_variables = ParseVariables(
-            sc, function_die.GetFirstChild(), func_lo_pc, true, true);
+        const size_t num_variables =
+            ParseVariablesInFunctionContext(sc, function_die, func_lo_pc);
 
         // Let all blocks know they have parse all their variables
         sc.function->GetBlock(false).SetDidParseVariables(true, true);
@@ -3481,118 +3481,134 @@
   return DWARFDIE();
 }
 
-size_t SymbolFileDWARF::ParseVariables(const SymbolContext &sc,
-                                       const DWARFDIE &orig_die,
-                                       const lldb::addr_t func_low_pc,
-                                       bool parse_siblings, bool parse_children,
-                                       VariableList *cc_variable_list) {
-  if (!orig_die)
-    return 0;
+void SymbolFileDWARF::ParseAndAppendGlobalVariable(
+    const SymbolContext &sc, const DWARFDIE &die,
+    VariableList &cc_variable_list) {
+  if (!die)
+    return;
 
-  VariableListSP variable_list_sp;
+  dw_tag_t tag = die.Tag();
+  if (tag != DW_TAG_variable && tag != DW_TAG_constant)
+    return;
 
-  size_t vars_added = 0;
-  DWARFDIE die = orig_die;
-  while (die) {
-    dw_tag_t tag = die.Tag();
+  // 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);
+    return;
+  }
 
-    // Check to see if we have already parsed this variable or constant?
-    VariableSP var_sp = GetDIEToVariable()[die.GetDIE()];
-    if (var_sp) {
-      if (cc_variable_list)
-        cc_variable_list->AddVariableIfUnique(var_sp);
+  // We haven't already parsed it, lets do that now.
+  VariableListSP variable_list_sp;
+  DWARFDIE sc_parent_die = GetParentSymbolContextDIE(die);
+  dw_tag_t parent_tag = sc_parent_die.Tag();
+  switch (parent_tag) {
+  case DW_TAG_compile_unit:
+  case DW_TAG_partial_unit:
+    if (sc.comp_unit != nullptr) {
+      variable_list_sp = sc.comp_unit->GetVariableList(false);
     } else {
-      // We haven't already parsed it, lets do that now.
-      if ((tag == DW_TAG_variable) || (tag == DW_TAG_constant) ||
-          (tag == DW_TAG_formal_parameter && sc.function)) {
-        if (variable_list_sp.get() == nullptr) {
-          DWARFDIE sc_parent_die = GetParentSymbolContextDIE(orig_die);
-          dw_tag_t parent_tag = sc_parent_die.Tag();
-          switch (parent_tag) {
-          case DW_TAG_compile_unit:
-          case DW_TAG_partial_unit:
-            if (sc.comp_unit != nullptr) {
-              variable_list_sp = sc.comp_unit->GetVariableList(false);
-              if (variable_list_sp.get() == nullptr) {
-                variable_list_sp = std::make_shared<VariableList>();
-              }
-            } else {
-              GetObjectFile()->GetModule()->ReportError(
-                  "parent 0x%8.8" PRIx64 " %s with no valid compile unit in "
-                  "symbol context for 0x%8.8" PRIx64 " %s.\n",
-                  sc_parent_die.GetID(), sc_parent_die.GetTagAsCString(),
-                  orig_die.GetID(), orig_die.GetTagAsCString());
-            }
-            break;
+      GetObjectFile()->GetModule()->ReportError(
+          "parent 0x%8.8" PRIx64 " %s with no valid compile unit in "
+          "symbol context for 0x%8.8" PRIx64 " %s.\n",
+          sc_parent_die.GetID(), sc_parent_die.GetTagAsCString(), die.GetID(),
+          die.GetTagAsCString());
+      return;
+    }
+    break;
 
-          case DW_TAG_subprogram:
-          case DW_TAG_inlined_subroutine:
-          case DW_TAG_lexical_block:
-            if (sc.function != nullptr) {
-              // Check to see if we already have parsed the variables for the
-              // given scope
-
-              Block *block = sc.function->GetBlock(true).FindBlockByID(
-                  sc_parent_die.GetID());
-              if (block == nullptr) {
-                // This must be a specification or abstract origin with a
-                // concrete block counterpart in the current function. We need
-                // to find the concrete block so we can correctly add the
-                // variable to it
-                const DWARFDIE concrete_block_die =
-                    FindBlockContainingSpecification(
-                        GetDIE(sc.function->GetID()),
-                        sc_parent_die.GetOffset());
-                if (concrete_block_die)
-                  block = sc.function->GetBlock(true).FindBlockByID(
-                      concrete_block_die.GetID());
-              }
+  default:
+    GetObjectFile()->GetModule()->ReportError(
+        "didn't find appropriate parent DIE for variable list for "
+        "0x%8.8" PRIx64 " %s.\n",
+        die.GetID(), die.GetTagAsCString());
+    return;
+  }
 
-              if (block != nullptr) {
-                const bool can_create = false;
-                variable_list_sp = block->GetBlockVariableList(can_create);
-                if (variable_list_sp.get() == nullptr) {
-                  variable_list_sp = std::make_shared<VariableList>();
-                  block->SetVariableList(variable_list_sp);
-                }
-              }
-            }
-            break;
+  var_sp = ParseVariableDIE(sc, die, LLDB_INVALID_ADDRESS);
+  if (!var_sp)
+    return;
 
-          default:
-            GetObjectFile()->GetModule()->ReportError(
-                "didn't find appropriate parent DIE for variable list for "
-                "0x%8.8" PRIx64 " %s.\n",
-                orig_die.GetID(), orig_die.GetTagAsCString());
-            break;
-          }
-        }
+  cc_variable_list.AddVariableIfUnique(var_sp);
+  if (variable_list_sp)
+    variable_list_sp->AddVariableIfUnique(var_sp);
+}
 
-        if (variable_list_sp) {
-          VariableSP var_sp(ParseVariableDIE(sc, die, func_low_pc));
-          if (var_sp) {
-            variable_list_sp->AddVariableIfUnique(var_sp);
-            if (cc_variable_list)
-              cc_variable_list->AddVariableIfUnique(var_sp);
-            ++vars_added;
-          }
-        }
+size_t SymbolFileDWARF::ParseVariablesInFunctionContext(
+    const SymbolContext &sc, const DWARFDIE &die,
+    const lldb::addr_t func_low_pc) {
+  if (!die || !sc.function)
+    return 0;
+
+  const std::function<size_t(const DWARFDIE &, VariableListSP)>
+      parse_recursive = [this, func_low_pc, &parse_recursive,
+                         &sc](const DWARFDIE &die,
+                              VariableListSP variable_list_sp) -> size_t {
+    size_t vars_added = 0;
+    dw_tag_t tag = die.Tag();
+
+    if ((tag == DW_TAG_variable) || (tag == DW_TAG_constant) ||
+        (tag == DW_TAG_formal_parameter)) {
+      VariableSP var_sp(ParseVariableDIE(sc, die, func_low_pc));
+      if (var_sp) {
+        variable_list_sp->AddVariableIfUnique(var_sp);
+        ++vars_added;
       }
     }
 
-    bool skip_children = (sc.function == nullptr && tag == DW_TAG_subprogram);
+    switch (tag) {
+    case DW_TAG_subprogram:
+    case DW_TAG_inlined_subroutine:
+    case DW_TAG_lexical_block: {
+      // If we start a new block, compute a new block context and recurse.
+      Block *block = sc.function->GetBlock(true).FindBlockByID(die.GetID());
+      if (block == nullptr) {
+        // This must be a specification or abstract origin with a
+        // concrete block counterpart in the current function. We need
+        // to find the concrete block so we can correctly add the
+        // variable to it
+        const DWARFDIE concrete_block_die = FindBlockContainingSpecification(
+            GetDIE(sc.function->GetID()), die.GetOffset());
+        if (concrete_block_die)
+          block = sc.function->GetBlock(true).FindBlockByID(
+              concrete_block_die.GetID());
+      }
+
+      if (block == nullptr)
+        return 0;
 
-    if (!skip_children && parse_children && die.HasChildren()) {
-      vars_added += ParseVariables(sc, die.GetFirstChild(), func_low_pc, true,
-                                   true, cc_variable_list);
+      const bool can_create = false;
+      VariableListSP block_variable_list_sp =
+          block->GetBlockVariableList(can_create);
+      if (block_variable_list_sp.get() == nullptr) {
+        block_variable_list_sp = std::make_shared<VariableList>();
+        block->SetVariableList(block_variable_list_sp);
+      }
+      for (DWARFDIE child = die.GetFirstChild(); child;
+           child = child.GetSibling()) {
+        vars_added += parse_recursive(child, block_variable_list_sp);
+      }
+
+      break;
     }
 
-    if (parse_siblings)
-      die = die.GetSibling();
-    else
-      die.Clear();
-  }
-  return vars_added;
+    default:
+      // Recurse to children with the same block context.
+      for (DWARFDIE child = die.GetFirstChild(); child;
+           child = child.GetSibling()) {
+        vars_added += parse_recursive(child, variable_list_sp);
+      }
+
+      break;
+    }
+
+    return vars_added;
+  };
+
+  Block *block = sc.function->GetBlock(true).FindBlockByID(die.GetID());
+  const bool can_create = false;
+  VariableListSP variable_list_sp = block->GetBlockVariableList(can_create);
+  return parse_recursive(die, variable_list_sp);
 }
 
 /// Collect call site parameters in a DW_TAG_call_site DIE.
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to