[lldb/DWARF] Simplify DWARFDebugInfoEntry::LookupAddress

[labath]

Summary

This method was doing a lot more than it's only caller needed
(DWARFDIE::LookupDeepestBlock) needed, so I inline it into the caller,
and remove any code which is not actually used. This includes code for
searching for the deepest function, and the code for working around
incomplete DW_AT_low_pc/high_pc attributes on a compile unit DIE (modern
compiler get this right, and this method is called on function DIEs
anyway).

This also improves our llvm consistency, as llvm::DWARFDebugInfoEntry is
just a very simple struct with no nontrivial logic.

[clayborg]

LGTM. Much cleaner using the newer DWARFDIE code. Not sure if we can unit test this?

[labath]

In D72920#1827480, @clayborg wrote:

LGTM. Much cleaner using the newer DWARFDIE code. Not sure if we can unit test this?

"image lookup --address" is sufficiently low level to be considered a unit test. We already have test using it (though we could always have more), and a lot of other integration tests fail if this goes wrong due to not being able to find local variables.

lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp

if (IsValid()) {
     if (m_die->GetAttributeValue(cu, attr, form_value, nullptr,
                                  check_specification_or_abstract_origin))
       return form_value.Reference();
   }
   return DWARFDIE();
 }
 
 DWARFDIE
-DWARFDIE::LookupDeepestBlock(lldb::addr_t file_addr) const {
-  if (IsValid()) {
-    SymbolFileDWARF *dwarf = GetDWARF();
-    DWARFUnit *cu = GetCU();
-    DWARFDebugInfoEntry *function_die = nullptr;
-    DWARFDebugInfoEntry *block_die = nullptr;
-    if (m_die->LookupAddress(file_addr, cu, &function_die, &block_die)) {
-      if (block_die && block_die != function_die) {
-        if (cu->ContainsDIEOffset(block_die->GetOffset()))
-          return DWARFDIE(cu, block_die);
-        else
-          return DWARFDIE(dwarf->DebugInfo()->GetUnit(DIERef(
-                              cu->GetSymbolFileDWARF().GetDwoNum(),
-                              cu->GetDebugSection(), block_die->GetOffset())),
-                          block_die);
+DWARFDIE::LookupDeepestBlock(lldb::addr_t address) const {
+  if (!IsValid())
+    return DWARFDIE();
+
+  DWARFDIE result;
+  bool check_children = false;
+  bool match_addr_range = false;
+  switch (Tag()) {
+  case DW_TAG_class_type:
+  case DW_TAG_namespace:
+  case DW_TAG_structure_type:
+  case DW_TAG_common_block:
+    check_children = true;
+    break;
+  case DW_TAG_compile_unit:
+  case DW_TAG_module:
+  case DW_TAG_catch_block:
+  case DW_TAG_subprogram:
+  case DW_TAG_try_block:
+  case DW_TAG_partial_unit:
+    match_addr_range = true;
+    break;
+  case DW_TAG_lexical_block:
+  case DW_TAG_inlined_subroutine:
+    check_children = true;
+    match_addr_range = true;
+    break;
+  default:
+    break;
+  }
+
+  if (match_addr_range) {
+    DWARFRangeList ranges;
+    if (m_die->GetAttributeAddressRanges(m_cu, ranges,
+                                         /*check_hi_lo_pc=*/true) &&
+        ranges.FindEntryThatContains(address)) {
+      check_children = true;
+      switch (Tag()) {
+      default:
+        break;
+
+      case DW_TAG_inlined_subroutine: // Inlined Function
+      case DW_TAG_lexical_block:      // Block { } in code
+        result = *this;
+        break;
       }
+    } else {
+      check_children = false;
     }
   }
-  return DWARFDIE();
+
+  if (check_children) {
+    for (DWARFDIE child = GetFirstChild(); child; child = child.GetSibling()) {
+      if (DWARFDIE child_result = child.LookupDeepestBlock(address))
+        return child_result;
+    }
+  }
+  return result;
 }
 
 const char *DWARFDIE::GetMangledName() const {
   if (IsValid())
     return m_die->GetMangledName(m_cu);
   else
     return nullptr;
 }

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h

void BuildAddressRangeTable(const DWARFUnit *cu,
                               DWARFDebugAranges *debug_aranges) const;
 
   void BuildFunctionAddressRangeTable(const DWARFUnit *cu,
                                       DWARFDebugAranges *debug_aranges) const;
 
   bool Extract(const lldb_private::DWARFDataExtractor &data,
                const DWARFUnit *cu, lldb::offset_t *offset_ptr);
 
-  bool LookupAddress(const dw_addr_t address, DWARFUnit *cu,
-                     DWARFDebugInfoEntry **function_die,
-                     DWARFDebugInfoEntry **block_die);
-
   size_t GetAttributes(const DWARFUnit *cu,
                        DWARFAttributes &attrs,
                        uint32_t curr_depth = 0)
       const; // "curr_depth" for internal use only, don't set this yourself!!!
 
   dw_offset_t
   GetAttributeValue(const DWARFUnit *cu, const dw_attr_t attr,
                     DWARFFormValue &formValue,

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp

if (name) {
 
     storage.append(name);
   }
   if (storage.empty())
     return nullptr;
   return storage.c_str();
 }
 
-bool DWARFDebugInfoEntry::LookupAddress(const dw_addr_t address, DWARFUnit *cu,
-                                        DWARFDebugInfoEntry **function_die,
-                                        DWARFDebugInfoEntry **block_die) {
-  bool found_address = false;
-  if (m_tag) {
-    bool check_children = false;
-    bool match_addr_range = false;
-    //  printf("0x%8.8x: %30s: address = 0x%8.8x - ", m_offset,
-    //  DW_TAG_value_to_name(tag), address);
-    switch (m_tag) {
-    case DW_TAG_array_type:
-      break;
-    case DW_TAG_class_type:
-      check_children = true;
-      break;
-    case DW_TAG_entry_point:
-    case DW_TAG_enumeration_type:
-    case DW_TAG_formal_parameter:
-    case DW_TAG_imported_declaration:
-    case DW_TAG_label:
-      break;
-    case DW_TAG_lexical_block:
-      check_children = true;
-      match_addr_range = true;
-      break;
-    case DW_TAG_member:
-    case DW_TAG_pointer_type:
-    case DW_TAG_reference_type:
-      break;
-    case DW_TAG_compile_unit:
-      match_addr_range = true;
-      break;
-    case DW_TAG_string_type:
-      break;
-    case DW_TAG_structure_type:
-      check_children = true;
-      break;
-    case DW_TAG_subroutine_type:
-    case DW_TAG_typedef:
-    case DW_TAG_union_type:
-    case DW_TAG_unspecified_parameters:
-    case DW_TAG_variant:
-      break;
-    case DW_TAG_common_block:
-      check_children = true;
-      break;
-    case DW_TAG_common_inclusion:
-    case DW_TAG_inheritance:
-      break;
-    case DW_TAG_inlined_subroutine:
-      check_children = true;
-      match_addr_range = true;
-      break;
-    case DW_TAG_module:
-      match_addr_range = true;
-      break;
-    case DW_TAG_ptr_to_member_type:
-    case DW_TAG_set_type:
-    case DW_TAG_subrange_type:
-    case DW_TAG_with_stmt:
-    case DW_TAG_access_declaration:
-    case DW_TAG_base_type:
-      break;
-    case DW_TAG_catch_block:
-      match_addr_range = true;
-      break;
-    case DW_TAG_const_type:
-    case DW_TAG_constant:
-    case DW_TAG_enumerator:
-    case DW_TAG_file_type:
-    case DW_TAG_friend:
-    case DW_TAG_namelist:
-    case DW_TAG_namelist_item:
-    case DW_TAG_packed_type:
-      break;
-    case DW_TAG_subprogram:
-      match_addr_range = true;
-      break;
-    case DW_TAG_template_type_parameter:
-    case DW_TAG_template_value_parameter:
-    case DW_TAG_GNU_template_parameter_pack:
-    case DW_TAG_thrown_type:
-      break;
-    case DW_TAG_try_block:
-      match_addr_range = true;
-      break;
-    case DW_TAG_variant_part:
-    case DW_TAG_variable:
-    case DW_TAG_volatile_type:
-    case DW_TAG_dwarf_procedure:
-    case DW_TAG_restrict_type:
-    case DW_TAG_interface_type:
-      break;
-    case DW_TAG_namespace:
-      check_children = true;
-      break;
-    case DW_TAG_imported_module:
-    case DW_TAG_unspecified_type:
-      break;
-    case DW_TAG_partial_unit:
-      match_addr_range = true;
-      break;
-    case DW_TAG_imported_unit:
-    case DW_TAG_shared_type:
-    default:
-      break;
-    }
-
-    if (match_addr_range) {
-      dw_addr_t lo_pc =
-          GetAttributeValueAsAddress(cu, DW_AT_low_pc, LLDB_INVALID_ADDRESS);
-      if (lo_pc != LLDB_INVALID_ADDRESS) {
-        dw_addr_t hi_pc = GetAttributeHighPC(cu, lo_pc, LLDB_INVALID_ADDRESS);
-        if (hi_pc != LLDB_INVALID_ADDRESS) {
-          //  printf("\n0x%8.8x: %30s: address = 0x%8.8x  [0x%8.8x - 0x%8.8x) ",
-          //  m_offset, DW_TAG_value_to_name(tag), address, lo_pc, hi_pc);
-          if ((lo_pc <= address) && (address < hi_pc)) {
-            found_address = true;
-            //  puts("***MATCH***");
-            switch (m_tag) {
-            case DW_TAG_compile_unit: // File
-            case DW_TAG_partial_unit: // File
-              check_children =
-                  ((function_die != nullptr) || (block_die != nullptr));
-              break;
-
-            case DW_TAG_subprogram: // Function
-              if (function_die)
-                *function_die = this;
-              check_children = (block_die != nullptr);
-              break;
-
-            case DW_TAG_inlined_subroutine: // Inlined Function
-            case DW_TAG_lexical_block:      // Block { } in code
-              if (block_die) {
-                *block_die = this;
-                check_children = true;
-              }
-              break;
-
-            default:
-              check_children = true;
-              break;
-            }
-          }
-        } else {
-          // Compile units may not have a valid high/low pc when there
-          // are address gaps in subroutines so we must always search
-          // if there is no valid high and low PC.
-          check_children =
-              (m_tag == DW_TAG_compile_unit || m_tag == DW_TAG_partial_unit) &&
-              ((function_die != nullptr) || (block_die != nullptr));
-        }
-      } else {
-        DWARFRangeList ranges;
-        if (GetAttributeAddressRanges(cu, ranges, /*check_hi_lo_pc*/ false) &&
-            ranges.FindEntryThatContains(address)) {
-          found_address = true;
-          //  puts("***MATCH***");
-          switch (m_tag) {
-          case DW_TAG_compile_unit: // File
-          case DW_TAG_partial_unit: // File
-              check_children =
-                  ((function_die != nullptr) || (block_die != nullptr));
-              break;
-
-          case DW_TAG_subprogram: // Function
-            if (function_die)
-              *function_die = this;
-            check_children = (block_die != nullptr);
-            break;
-
-          case DW_TAG_inlined_subroutine: // Inlined Function
-          case DW_TAG_lexical_block:      // Block { } in code
-            if (block_die) {
-              *block_die = this;
-              check_children = true;
-            }
-            break;
-
-          default:
-            check_children = true;
-            break;
-          }
-        } else {
-          check_children = false;
-        }
-      }
-    }
-
-    if (check_children) {
-      //  printf("checking children\n");
-      DWARFDebugInfoEntry *child = GetFirstChild();
-      while (child) {
-        if (child->LookupAddress(address, cu, function_die, block_die))
-          return true;
-        child = child->GetSibling();
-      }
-    }
-  }
-  return found_address;
-}
-
 lldb::offset_t DWARFDebugInfoEntry::GetFirstAttributeOffset() const {
   return GetOffset() + llvm::getULEB128Size(m_abbr_idx);
 }
 
 const DWARFAbbreviationDeclaration *
 DWARFDebugInfoEntry::GetAbbreviationDeclarationPtr(const DWARFUnit *cu) const {
   if (cu) {
     const DWARFAbbreviationDeclarationSet *abbrev_set = cu->GetAbbreviations();