[lldb] Use CompileUnit::ResolveSymbolContext in SymbolFileDWARF

[kimanh]

Summary

SymbolFileDWARF::ResolveSymbolContext is currently unaware that in DWARF5 the primary file is specified at file index 0. As a result it misses to correctly resolve the symbol context for the primary file when DWARF5 debug data is used and the primary file is only specified at index 0.

This change makes use of CompileUnit::ResolveSymbolContext to resolve the symbol context. The ResolveSymbolContext in CompileUnit has been previously already updated to reflect changes in DWARF5
and contains a more readable version. It can resolve more, but will also do a bit more work than
SymbolFileDWARF::ResolveSymbolContext (getting the Module, and going through SymbolFileDWARF::ResolveSymbolContextForAddress), however, it's mostly directed by $resolve_scope
what will be resolved, and ensures that code is easier to maintain if there's only one path.

[labath]

I remember seeing this that this (duplicated) code was not dwarf5 ready, but I did not want to change it without understanding when it actually gets used. It looks like you've found that. Thanks for doing that, and for removing the duplication.

lldb/test/Shell/SymbolFile/DWARF/dwarf5-debug_line-file-index.s

99

I was puzzled by this line as the string 3 is nowhere to be found. I take it this was manually reduced? If so, I think you should be able to delete the entire subprogram and base_type tags (and the relevant abbreviations), as that is not necessary for the line tables to work.

[kimanh]

Removing subprogram and base_type tags (as they are not needed for the test)

[kimanh]

Thanks a lot for the review Pavel! I've updated the test. If it looks fine like this, could you help me to commit this change?

lldb/test/Shell/SymbolFile/DWARF/dwarf5-debug_line-file-index.s

99

Thanks for the guidance here! Yes, that's correct, I've manually reduced it but wasn't sure what else I could safely remove. I've now removed the subprogram parts, and the base_type tags. Hope that looks better now!

[labath]

In D98619#2634472, @kimanh wrote:

Thanks a lot for the review Pavel! I've updated the test. If it looks fine like this, could you help me to commit this change?

Done. Thanks for the patch.

lldb/test/Shell/SymbolFile/DWARF/dwarf5-debug_line-file-index.s

99

This looks better. Thanks. FTR, I don't think it's necessary to produce absolutely minimal inputs (though the easy-to-do reductions are definitely nice to have). I just don't want to have invalid inputs (like these missing cross-references) in the test file (unless of course, that is what you're testing, but that's not the case here).

[kimanh]

Unfortunately the test seems to run and fail on the ARM. I thoughty adding REQUIRES: x85 would skip no x86 ones, but maybe I did something wrong.

lldb/test/Shell/SymbolFile/DWARF/dwarf5-debug_line-file-index.s

3

I can see that my CL is failing on the ARM. From looking at the other tests I thought this line would prevent it from running on the ARM, but apparently it doesn't. How can I fix this?

https://lab.llvm.org/buildbot/#/builders/17/builds/5500

[labath]

REQUIRES: x86 disables the test in those builds, which don't have the X86 target enabled (in the build config). That is exactly what we want here -- the test is not actually running this code, so it should succeed regardless of the host architecture. It's working fine on aarch64, for instance (http://lab.llvm.org:8011/#/builders/96/builds/5891). I'm not sure what's the deal with the arm bot (cc @omjavaid). It could be some 32-bit thing. I'm going to try to get a better error message first, and then we'll see what to do next...

[kimanh]

Thanks for having a look, and for the explanation! Please let me know if I can do anything to help debugging this!

[omjavaid]

In D98619#2640734, @labath wrote:

REQUIRES: x86 disables the test in those builds, which don't have the X86 target enabled (in the build config). That is exactly what we want here -- the test is not actually running this code, so it should succeed regardless of the host architecture. It's working fine on aarch64, for instance (http://lab.llvm.org:8011/#/builders/96/builds/5891). I'm not sure what's the deal with the arm bot (cc @omjavaid). It could be some 32-bit thing. I'm going to try to get a better error message first, and then we'll see what to do next...

This is crashing LLDB on arm linux I have reported a bug and marked it xfail. I ll look for a fix later.

[labath]

In D98619#2641104, @omjavaid wrote:

In D98619#2640734, @labath wrote:

REQUIRES: x86 disables the test in those builds, which don't have the X86 target enabled (in the build config). That is exactly what we want here -- the test is not actually running this code, so it should succeed regardless of the host architecture. It's working fine on aarch64, for instance (http://lab.llvm.org:8011/#/builders/96/builds/5891). I'm not sure what's the deal with the arm bot (cc @omjavaid). It could be some 32-bit thing. I'm going to try to get a better error message first, and then we'll see what to do next...

This is crashing LLDB on arm linux I have reported a bug and marked it xfail. I ll look for a fix later.

Interesting. Crashing actually explains the test output. Thanks for disabling that test. The issue seems to be arm specific as I couldn't reproduce this on i386 lldb (and that makes it pretty weird).

Do you have a backtrace handy by any chance?

[kimanh]

It seems as if it's the same for windows (I don't have windows unfortunately to check):

https://lab.llvm.org/buildbot/#/builders/83/builds/4884

[omjavaid]

In D98619#2641136, @labath wrote:

In D98619#2641104, @omjavaid wrote:

In D98619#2640734, @labath wrote:

REQUIRES: x86 disables the test in those builds, which don't have the X86 target enabled (in the build config). That is exactly what we want here -- the test is not actually running this code, so it should succeed regardless of the host architecture. It's working fine on aarch64, for instance (http://lab.llvm.org:8011/#/builders/96/builds/5891). I'm not sure what's the deal with the arm bot (cc @omjavaid). It could be some 32-bit thing. I'm going to try to get a better error message first, and then we'll see what to do next...

This is crashing LLDB on arm linux I have reported a bug and marked it xfail. I ll look for a fix later.

Interesting. Crashing actually explains the test output. Thanks for disabling that test. The issue seems to be arm specific as I couldn't reproduce this on i386 lldb (and that makes it pretty weird).

Do you have a backtrace handy by any chance?

I have created http://llvm.org/pr49678. Stack trace attached to pr.

[labath]

The backtrace was very helpful. Thanks.

I think I've found the problem and I believe it should be fixed by 10d54e2f8d (though I couldn't actually check right now).

The problem was (sort of) in the incorrect reduction in the test input, which that patch should fix. That said, the incorrect input still should not have caused a crash...

[kimanh]

I'm very sorry for the incorrect reduction, and thanks for fixing this!

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

for (uint32_t cu_idx = 0, num_cus = GetNumCompileUnits(); cu_idx < num_cus;
          ++cu_idx) {
       CompileUnit *dc_cu = ParseCompileUnitAtIndex(cu_idx).get();
       if (!dc_cu)
         continue;
 
       bool file_spec_matches_cu_file_spec =
           FileSpec::Match(file_spec, dc_cu->GetPrimaryFile());
       if (check_inlines || file_spec_matches_cu_file_spec) {
-        SymbolContext sc(m_objfile_sp->GetModule());
-        sc.comp_unit = dc_cu;
-        uint32_t file_idx = UINT32_MAX;
-
-        // If we are looking for inline functions only and we don't find it
-        // in the support files, we are done.
-        if (check_inlines) {
-          file_idx =
-              sc.comp_unit->GetSupportFiles().FindFileIndex(1, file_spec, true);
-          if (file_idx == UINT32_MAX)
-            continue;
-        }
-
-        if (line != 0) {
-          LineTable *line_table = sc.comp_unit->GetLineTable();
-
-          if (line_table != nullptr && line != 0) {
-            // We will have already looked up the file index if we are
-            // searching for inline entries.
-            if (!check_inlines)
-              file_idx = sc.comp_unit->GetSupportFiles().FindFileIndex(
-                  1, file_spec, true);
-
-            if (file_idx != UINT32_MAX) {
-              uint32_t found_line;
-              uint32_t line_idx = line_table->FindLineEntryIndexByFileIndex(
-                  0, file_idx, line, false, &sc.line_entry);
-              found_line = sc.line_entry.line;
-
-              while (line_idx != UINT32_MAX) {
-                sc.function = nullptr;
-                sc.block = nullptr;
-                if (resolve_scope &
-                    (eSymbolContextFunction | eSymbolContextBlock)) {
-                  const lldb::addr_t file_vm_addr =
-                      sc.line_entry.range.GetBaseAddress().GetFileAddress();
-                  if (file_vm_addr != LLDB_INVALID_ADDRESS) {
-                    ResolveFunctionAndBlock(
-                        file_vm_addr, resolve_scope & eSymbolContextBlock, sc);
-                  }
-                }
-
-                sc_list.Append(sc);
-                line_idx = line_table->FindLineEntryIndexByFileIndex(
-                    line_idx + 1, file_idx, found_line, true, &sc.line_entry);
-              }
-            }
-          } else if (file_spec_matches_cu_file_spec && !check_inlines) {
-            // only append the context if we aren't looking for inline call
-            // sites by file and line and if the file spec matches that of
-            // the compile unit
-            sc_list.Append(sc);
-          }
-        } else if (file_spec_matches_cu_file_spec && !check_inlines) {
-          // only append the context if we aren't looking for inline call
-          // sites by file and line and if the file spec matches that of
-          // the compile unit
-          sc_list.Append(sc);
-        }
-
+        dc_cu->ResolveSymbolContext(file_spec, line, check_inlines, false,
+                                    resolve_scope, sc_list);
         if (!check_inlines)
           break;
       }
     }
   }
   return sc_list.GetSize() - prev_size;
 }
 

lldb/source/Symbol/CompileUnit.cpp

void CompileUnit::ResolveSymbolContext(const FileSpec &file_spec,
   bool file_spec_matches_cu_file_spec =
       FileSpec::Match(file_spec, this->GetPrimaryFile());
 
   // If we are not looking for inlined functions and our file spec doesn't
   // match then we are done...
   if (!file_spec_matches_cu_file_spec && !check_inlines)
     return;
 
+  SymbolContext sc(GetModule());
+  sc.comp_unit = this;
+
+  if (line == 0) {
+    if (file_spec_matches_cu_file_spec && !check_inlines) {
+      // only append the context if we aren't looking for inline call sites by
+      // file and line and if the file spec matches that of the compile unit
+      sc_list.Append(sc);
+    }
+    return;
+  }
+
   uint32_t file_idx =
       GetSupportFiles().FindFileIndex(0, file_spec, true);
   while (file_idx != UINT32_MAX) {
     file_indexes.push_back(file_idx);
     file_idx = GetSupportFiles().FindFileIndex(file_idx + 1, file_spec, true);
   }
 
   const size_t num_file_indexes = file_indexes.size();
   if (num_file_indexes == 0)
     return;
 
-  SymbolContext sc(GetModule());
-  sc.comp_unit = this;
+  LineTable *line_table = sc.comp_unit->GetLineTable();
 
-  if (line == 0) {
+  if (line_table == nullptr) {
     if (file_spec_matches_cu_file_spec && !check_inlines) {
-      // only append the context if we aren't looking for inline call sites by
-      // file and line and if the file spec matches that of the compile unit
       sc_list.Append(sc);
     }
     return;
   }
 
-  LineTable *line_table = sc.comp_unit->GetLineTable();
-
-  if (line_table == nullptr)
-    return;
-
   uint32_t line_idx;
   LineEntry line_entry;
 
   if (num_file_indexes == 1) {
     // We only have a single support file that matches, so use the line
     // table function that searches for a line entries that match a single
     // support file index
     line_idx = line_table->FindLineEntryIndexByFileIndex(

lldb/test/Shell/SymbolFile/DWARF/dwarf5-debug_line-file-index.s

+# Test handling of DWARF5 file index 0.
+
+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-pc-linux %s
+# RUN: %lldb %t -o "image lookup -f hello.c -l 1" \
+# RUN:   -o exit | FileCheck %s
+
+# CHECK: 2 matches found in hello.c:1
+  .text
+.Lfunc_begin0:
+  .file 0 "." "hello.c"
+  .loc  0 1 0                           # hello.c:1:0
+    nop
+  .loc  0 1 13 prologue_end             # hello.c:1:13
+    nop
+.Lfunc_end0:
+  .section  .debug_abbrev,"",@progbits
+  .byte 1                               # Abbreviation Code
+  .byte 17                              # DW_TAG_compile_unit
+  .byte 1                               # DW_CHILDREN_yes
+  .byte 37                              # DW_AT_producer
+  .byte 37                              # DW_FORM_strx1
+  .byte 19                              # DW_AT_language
+  .byte 5                               # DW_FORM_data2
+  .byte 3                               # DW_AT_name
+  .byte 37                              # DW_FORM_strx1
+  .byte 114                             # DW_AT_str_offsets_base
+  .byte 23                              # DW_FORM_sec_offset
+  .byte 16                              # DW_AT_stmt_list
+  .byte 23                              # DW_FORM_sec_offset
+  .byte 27                              # DW_AT_comp_dir
+  .byte 37                              # DW_FORM_strx1
+  .byte 17                              # DW_AT_low_pc
+  .byte 27                              # DW_FORM_addrx
+  .byte 18                              # DW_AT_high_pc
+  .byte 6                               # DW_FORM_data4
+  .byte 115                             # DW_AT_addr_base
+  .byte 23                              # DW_FORM_sec_offset
+  .byte 0                               # EOM(1)
+  .byte 0                               # EOM(2)
+  .byte 2                               # Abbreviation Code
+  .byte 46                              # DW_TAG_subprogram
+  .byte 0                               # DW_CHILDREN_no
+  .byte 17                              # DW_AT_low_pc
+  .byte 27                              # DW_FORM_addrx
+  .byte 18                              # DW_AT_high_pc
+  .byte 6                               # DW_FORM_data4
+  .byte 64                              # DW_AT_frame_base
+  .byte 24                              # DW_FORM_exprloc
+  .byte 3                               # DW_AT_name
+  .byte 37                              # DW_FORM_strx1
+  .byte 58                              # DW_AT_decl_file
+  .byte 11                              # DW_FORM_data1
+  .byte 59                              # DW_AT_decl_line
+  .byte 11                              # DW_FORM_data1
+  .byte 73                              # DW_AT_type
+  .byte 19                              # DW_FORM_ref4
+  .byte 63                              # DW_AT_external
+  .byte 25                              # DW_FORM_flag_present
+  .byte 0                               # EOM(1)
+  .byte 0                               # EOM(2)
+  .byte 3                               # Abbreviation Code
+  .byte 36                              # DW_TAG_base_type
+  .byte 0                               # DW_CHILDREN_no
+  .byte 3                               # DW_AT_name
+  .byte 37                              # DW_FORM_strx1
+  .byte 62                              # DW_AT_encoding
+  .byte 11                              # DW_FORM_data1
+  .byte 11                              # DW_AT_byte_size
+  .byte 11                              # DW_FORM_data1
+  .byte 0                               # EOM(1)
+  .byte 0                               # EOM(2)
+  .byte 0                               # EOM(3)
+  .section  .debug_info,"",@progbits
+.Lcu_begin0:
+  .long .Ldebug_info_end0-.Ldebug_info_start0 # Length of Unit
+.Ldebug_info_start0:
+  .short  5                               # DWARF version number
+  .byte 1                               # DWARF Unit Type
+  .byte 8                               # Address Size (in bytes)
+  .long .debug_abbrev                   # Offset Into Abbrev. Section
+  .byte 1                               # Abbrev [1] 0xc:0x2b DW_TAG_compile_unit
+  .byte 0                               # DW_AT_producer
+  .short  12                              # DW_AT_language
+  .byte 1                               # DW_AT_name
+  .long .Lstr_offsets_base0             # DW_AT_str_offsets_base
+  .long .Lline_table_start0             # DW_AT_stmt_list
+  .byte 2                               # DW_AT_comp_dir
+  .byte 0                               # DW_AT_low_pc
+  .long .Lfunc_end0-.Lfunc_begin0       # DW_AT_high_pc
+  .long .Laddr_table_base0              # DW_AT_addr_base
+.Ldebug_info_end0:
+  .section  .debug_str_offsets,"",@progbits
+  .long 24                              # Length of String Offsets Set
+  .short  5
+  .short  0
+.Lstr_offsets_base0:
+  .section  .debug_str,"MS",@progbits,1
+.Linfo_string0:
+  .asciz  "" # string offset=0
+.Linfo_string1:
+  .asciz  "hello.c"                       # string offset=101
+.Linfo_string2:
+  .asciz  "."                             # string offset=109
+  .section  .debug_str_offsets,"",@progbits
+  .long .Linfo_string0
+  .long .Linfo_string1
+  .long .Linfo_string2
+  .section  .debug_addr,"",@progbits
+.Ldebug_addr_start0:
+  .short  5                               # DWARF version number
+  .byte 8                               # Address size
+  .byte 0                               # Segment selector size
+.Laddr_table_base0:
+  .quad .Lfunc_begin0
+.Ldebug_addr_end0:
+  .section  .debug_line,"",@progbits
+.Lline_table_start0:
 No newline at end of file