[lldb/DWARF] Ignore debug info pointing to the low addresses

[labath]

Summary

specifically, ignore addresses that point before the first code section.

This resurrects D87172 with several notable changes:

• it fixes a bug where the early exits in InitializeObject left m_first_code_address "initialized" to LLDB_INVALID_ADDRESS (0xfff..f), which caused _everything_ to be ignored.
• it extends the line table fix to function parsing as well, where it replaces a similar check which was checking the executable permissions of the section. This was insufficient because some position-independent elf executables can have an executable segment mapped at file address zero. (What makes this fix different is that it checks for the executable-ness of the sections contained within that segment, and those will not be at address zero.)
• It uses a different test case, with an elf file with near-zero addresses, and checks for both line table and function parsing.

[shafik]

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

472

It would be nice to wrap this up into InitializeFirstCodeAddress(...) it could be done by making iterating over the section list a lambda and then checking after that or splitting it into a helper etc

[labath]

Move the recursion into a helper function

[labath]

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

472

Yeah I was wondering about that myself. I've created a new function for that now.

[clayborg]

Looks good. Do we need a follow up patch to avoid creating functions that should have been stripped?

[labath]

In D112058#3073451, @clayborg wrote:

Looks good. Do we need a follow up patch to avoid creating functions that should have been stripped?

I'm not entirely sure what you mean by that. Are you referring to the fact that SymbolFileDWARF::ResolveFunction creates an lldb_private::Function, which it does not add the to result SymbolContextList (instead of not creating the object in the first place?

If yes, then that sounds like a good idea. I don't know if this has more user-facing implications, but I was definitely puzzled by the fact that the stripped function still showed up in "image dump symfile". I'll see what can be done about that.

[clayborg]

In D112058#3074618, @labath wrote:

In D112058#3073451, @clayborg wrote:

Looks good. Do we need a follow up patch to avoid creating functions that should have been stripped?

I'm not entirely sure what you mean by that. Are you referring to the fact that SymbolFileDWARF::ResolveFunction creates an lldb_private::Function, which it does not add the to result SymbolContextList (instead of not creating the object in the first place?

If yes, then that sounds like a good idea. I don't know if this has more user-facing implications, but I was definitely puzzled by the fact that the stripped function still showed up in "image dump symfile". I'll see what can be done about that.

Yeah, that is what I was talking about. I think if we modify SymbolFileDWARF::ParseFunction() that would be the easiest to quickly get the ranges for the DW_TAG_subprogram and avoid calling DWARFASTParser::ParseFunctionFromDWARF() if the low PC is less than m_first_code_address we can avoid bloat in the ASTs and keep our memory footprint down in the LLDB process.

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

struct DecodedUID {
     DIERef ref;
   };
   llvm::Optional<DecodedUID> DecodeUID(lldb::user_id_t uid);
 
   void FindDwpSymbolFile();
 
   const lldb_private::FileSpecList &GetTypeUnitSupportFiles(DWARFTypeUnit &tu);
 
+  void InitializeFirstCodeAddressRecursive(
+      const lldb_private::SectionList &section_list);
+
+  void InitializeFirstCodeAddress();
+
   lldb::ModuleWP m_debug_map_module_wp;
   SymbolFileDWARFDebugMap *m_debug_map_symfile;
 
   llvm::once_flag m_dwp_symfile_once_flag;
   std::shared_ptr<SymbolFileDWARFDwo> m_dwp_symfile;
 
   lldb_private::DWARFContext m_context;
 
protected:
   UniqueDWARFASTTypeMap m_unique_ast_type_map;
   DIEToTypePtr m_die_to_type;
   DIEToVariableSP m_die_to_variable_sp;
   DIEToClangType m_forward_decl_die_to_clang_type;
   ClangTypeToDIE m_forward_decl_clang_type_to_die;
   llvm::DenseMap<dw_offset_t, lldb_private::FileSpecList>
       m_type_unit_support_files;
   std::vector<uint32_t> m_lldb_cu_to_dwarf_unit;
+  /// DWARF does not provide a good way for traditional (concatenating) linkers
+  /// to invalidate debug info describing dead-stripped code. These linkers will
+  /// keep the debug info but resolve any addresses referring to such code as
+  /// zero (BFD) or a small positive integer (zero + relocation addend -- GOLD).
+  /// Try to filter out this debug info by comparing it to the lowest code
+  /// address in the module.
+  lldb::addr_t m_first_code_address = LLDB_INVALID_ADDRESS;
 };
 
 #endif // LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_SYMBOLFILEDWARF_H

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

if (type_system_or_err) {
     type_system_or_err->SetSymbolFile(this);
   }
   return type_system_or_err;
 }
 
 void SymbolFileDWARF::InitializeObject() {
   Log *log = LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_INFO);
 
+  InitializeFirstCodeAddress();
+
   if (!GetGlobalPluginProperties().IgnoreFileIndexes()) {
     StreamString module_desc;
     GetObjectFile()->GetModule()->GetDescription(module_desc.AsRawOstream(),
                                                  lldb::eDescriptionLevelBrief);
     DWARFDataExtractor apple_names, apple_namespaces, apple_types, apple_objc;
     LoadSectionData(eSectionTypeDWARFAppleNames, apple_names);
     LoadSectionData(eSectionTypeDWARFAppleNamespaces, apple_namespaces);
     LoadSectionData(eSectionTypeDWARFAppleTypes, apple_types);
if (debug_names.GetByteSize() > 0) {
                      "Unable to read .debug_names data: {0}");
     }
   }
 
   m_index =
       std::make_unique<ManualDWARFIndex>(*GetObjectFile()->GetModule(), *this);
 }
 
+void SymbolFileDWARF::InitializeFirstCodeAddress() {
+  InitializeFirstCodeAddressRecursive(
+      *m_objfile_sp->GetModule()->GetSectionList());
+  if (m_first_code_address == LLDB_INVALID_ADDRESS)
+    m_first_code_address = 0;
+}
+
+void SymbolFileDWARF::InitializeFirstCodeAddressRecursive(
+    const lldb_private::SectionList &section_list) {
+  for (SectionSP section_sp : section_list) {
+    if (section_sp->GetChildren().GetSize() > 0) {
+      InitializeFirstCodeAddressRecursive(section_sp->GetChildren());
+    } else if (section_sp->GetType() == eSectionTypeCode) {
+      m_first_code_address =
+          std::min(m_first_code_address, section_sp->GetFileAddress());
+    }
+  }
+}
+
 bool SymbolFileDWARF::SupportedVersion(uint16_t version) {
   return version >= 2 && version <= 5;
 }
 
 uint32_t SymbolFileDWARF::CalculateAbilities() {
   uint32_t abilities = 0;
   if (m_objfile_sp != nullptr) {
     const Section *section = nullptr;
bool SymbolFileDWARF::ParseLineTable(CompileUnit &comp_unit) {
 
   // FIXME: Rather than parsing the whole line table and then copying it over
   // into LLDB, we should explore using a callback to populate the line table
   // while we parse to reduce memory usage.
   std::vector<std::unique_ptr<LineSequence>> sequences;
   // The Sequences view contains only valid line sequences. Don't iterate over
   // the Rows directly.
   for (const llvm::DWARFDebugLine::Sequence &seq : line_table->Sequences) {
+    // Ignore line sequences that do not start after the first code address.
+    // All addresses generated in a sequence are incremental so we only need
+    // to check the first one of the sequence. Check the comment at the
+    // m_first_code_address declaration for more details on this.
+    if (seq.LowPC < m_first_code_address)
+      continue;
     std::unique_ptr<LineSequence> sequence =
         LineTable::CreateLineSequenceContainer();
     for (unsigned idx = seq.FirstRowIndex; idx < seq.LastRowIndex; ++idx) {
       const llvm::DWARFDebugLine::Row &row = line_table->Rows[idx];
       LineTable::AppendLineEntryToSequence(
           sequence.get(), row.Address.Address, row.Line, row.Column, row.File,
           row.IsStmt, row.BasicBlock, row.PrologueEnd, row.EpilogueBegin,
           row.EndSequence);
if (inlined_die) {
         sc.block = function_block.FindBlockByID(inlined_die.GetOffset());
       if (sc.block == nullptr || !sc.block->GetStartAddress(addr))
         addr.Clear();
     } else {
       sc.block = nullptr;
       addr = sc.function->GetAddressRange().GetBaseAddress();
     }
 
-
-    if (auto section_sp = addr.GetSection()) {
-      if (section_sp->GetPermissions() & ePermissionsExecutable) {
-        sc_list.Append(sc);
-        return true;
-      }
-    }
-  }
+    if (addr.IsValid() && addr.GetFileAddress() >= m_first_code_address) {
 
   return false;
 }
 
 bool SymbolFileDWARF::DIEInDeclContext(const CompilerDeclContext &decl_ctx,
                                        const DWARFDIE &die) {
   // If we have no parent decl context to match this DIE matches, and if the
   // parent decl context isn't valid, we aren't trying to look for any

lldb/test/Shell/SymbolFile/DWARF/lit.local.cfg

-config.suffixes = ['.cpp', '.m', '.mm', '.s', '.test', '.ll', '.c']
+config.suffixes = ['.cpp', '.m', '.mm', '.s', '.test', '.ll', '.c', '.yaml']

lldb/test/Shell/SymbolFile/DWARF/x86/dead-code-filtering.yaml

+# RUN: yaml2obj %s > %t
+# RUN: %lldb %t -o "image dump line-table a.c" -o "image lookup -n _start" -o "image lookup -n f" -o exit | FileCheck %s
+
+# CHECK-LABEL: image dump line-table a.c
+# CHECK-NEXT: Line table for a.c
+# CHECK-NEXT: 0x0000000000000080: a.c:1
+# CHECK-NEXT: 0x0000000000000084: a.c:1
+# CHECK-NEXT: 0x0000000000000086: a.c:1
+# CHECK-EMPTY:
+# CHECK-NEXT: image lookup -n _start
+# CHECK-NEXT: 1 match found
+# CHECK-LABEL: image lookup -n f
+# CHECK-EMPTY:
+
+--- !ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_X86_64
+ProgramHeaders:
+  - Type:            PT_LOAD
+    Flags:           [ PF_X, PF_R ]
+    Offset:          0x0000
+    FirstSec:        .text
+    LastSec:         .text
+    Align:           0x1000
+Sections:
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:         0x80
+    AddressAlign:    0x10
+    Content:         554889E55DC3
+  - Name:            .debug_abbrev
+    Type:            SHT_PROGBITS
+    AddressAlign:    0x1
+  - Name:            .debug_info
+    Type:            SHT_PROGBITS
+    AddressAlign:    0x1
+  - Name:            .debug_line
+    Type:            SHT_PROGBITS
+    AddressAlign:    0x1
+DWARF:
+  debug_ranges:
+    - Offset:          0x0
+      AddrSize:        0x8
+      Entries:
+        - LowOffset:       0x0
+          HighOffset:      0x6
+        - LowOffset:       0x80
+          HighOffset:      0x86
+  debug_abbrev:
+    - ID:              0
+      Table:
+        - Code:            0x0000000000000001
+          Tag:             DW_TAG_compile_unit
+          Children:        DW_CHILDREN_yes
+          Attributes:
+            - Attribute:       DW_AT_producer
+              Form:            DW_FORM_string
+            - Attribute:       DW_AT_name
+              Form:            DW_FORM_string
+            - Attribute:       DW_AT_stmt_list
+              Form:            DW_FORM_sec_offset
+            - Attribute:       DW_AT_low_pc
+              Form:            DW_FORM_addr
+            - Attribute:       DW_AT_ranges
+              Form:            DW_FORM_sec_offset
+        - Code:            0x0000000000000002
+          Tag:             DW_TAG_subprogram
+          Children:        DW_CHILDREN_no
+          Attributes:
+            - Attribute:       DW_AT_low_pc
+              Form:            DW_FORM_addr
+            - Attribute:       DW_AT_high_pc
+              Form:            DW_FORM_data4
+            - Attribute:       DW_AT_name
+              Form:            DW_FORM_string
+  debug_info:
+    - Version:         4
+      AbbrevTableID:   0
+      AbbrOffset:      0x0000000000000000
+      AddrSize:        8
+      Entries:
+        - AbbrCode:        0x00000001
+          Values:
+            - CStr:            Hand-written DWARF
+            - CStr:            a.c
+            - Value:           0x0000000000000000
+            - Value:           0x0000000000000000
+            - Value:           0x0000000000000000
+        - AbbrCode:        0x00000002
+          Values:
+            - Value:           0x0000000000000000
+            - Value:           0x0000000000000006
+            - CStr:            f
+        - AbbrCode:        0x00000002
+          Values:
+            - Value:           0x0000000000000080
+            - Value:           0x0000000000000006
+            - CStr:            _start
+        - AbbrCode:        0x00000000
+  debug_line:
+    - Version:         4
+      MinInstLength:   1
+      MaxOpsPerInst:   1
+      DefaultIsStmt:   1
+      LineBase:        251
+      LineRange:       14
+      OpcodeBase:      13
+      StandardOpcodeLengths: [ 0, 1, 1, 1, 1, 0, 0, 0, 1, 0, 0, 1 ]
+      IncludeDirs:     []
+      Files:
+        - Name:            a.c
+          DirIdx:          0
+          ModTime:         0
+          Length:          0
+      Opcodes:
+        - Opcode:          DW_LNS_extended_op
+          ExtLen:          9
+          SubOpcode:       DW_LNE_set_address
+          Data:            0
+        - Opcode:          DW_LNS_copy
+          Data:            0
+        - Opcode:          DW_LNS_set_prologue_end
+          Data:            0
+        - Opcode:          0x4a
+          Data:            0
+        - Opcode:          DW_LNS_advance_pc
+          Data:            2
+        - Opcode:          DW_LNS_extended_op
+          ExtLen:          1
+          SubOpcode:       DW_LNE_end_sequence
+          Data:            0
+        - Opcode:          DW_LNS_extended_op
+          ExtLen:          9
+          SubOpcode:       DW_LNE_set_address
+          Data:            0x000080
+        - Opcode:          DW_LNS_copy
+          Data:            0
+        - Opcode:          DW_LNS_set_prologue_end
+          Data:            0
+        - Opcode:          0x4a
+          Data:            0
+        - Opcode:          DW_LNS_advance_pc
+          Data:            2
+        - Opcode:          DW_LNS_extended_op
+          ExtLen:          1
+          SubOpcode:       DW_LNE_end_sequence
+          Data:            0
+...