2/2: [lldb] Fix DW_TAG_GNU_call_site-DW_AT_low_pc as produced by GCC

[jankratochvil]

Summary

D80519 added support for DW_TAG_GNU_call_site but Bug 45886 found one case did not work.
There is:

0x000000b1:     DW_TAG_GNU_call_site
                  DW_AT_low_pc  (0x000000000040111e)
                  DW_AT_abstract_origin (0x000000cc "a")
...
0x000000cc:   DW_TAG_subprogram
                DW_AT_name      ("a")
                DW_AT_prototyped        (true)
                DW_AT_low_pc    (0x0000000000401109)
                ^^^^^^^^^^^^ - here it did overwrite the 'low_pc' variable containing value 0x40111e we wanted
                DW_AT_high_pc   (0x0000000000401114)
                DW_AT_frame_base        (DW_OP_call_frame_cfa)
                DW_AT_GNU_all_call_sites        (true)

[jankratochvil]

It regressed other testcases, I have to check it more.

[jankratochvil]

The new -1 value for GetAttributes is a bit ugly. But otherwise either

• CollectCallEdges would need to reimplement GetAttributes.

or

• trying to guess which attribute belongs to which DIE from DIEOffsetAtIndex but that is also not nice because it is offset of the DIE's attribute, not offset of the DIE itself.

[jankratochvil]

# This tests that lldb is compatible with DWARF-4 entry values GNU extension
# with DW_TAG_GNU_call_site attributes order as produced by GCC:
# 0x000000b1:     DW_TAG_GNU_call_site
#                   DW_AT_low_pc  (0x000000000040111e)
#                   DW_AT_abstract_origin (0x000000cc "a")
# clang produces the attributes in opposite order:
# 0x00000064:     DW_TAG_GNU_call_site
#                   DW_AT_abstract_origin (0x0000002a "a")
#                   DW_AT_low_pc  (0x0000000000401146)

[labath]

Thanks for tracking this down, and for creating a nice test case. I think the implementation would be cleaner with a new argument, as per the inline comment.

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

113 ↗

(On Diff #269047)

Using -1 to prevent recursion is pretty unobvious. I think it would be better to add a bool recurse = true argument between attributes and depth. In fact, I'd consider even deleting the depth argument -- it's an internal detail that noone except DWARFDebugInfoEntry should be using and the single usage there can be easily changed to spec_die.GetDIE()->GetAttributes(spec_die.GetUnit(), attributes, recurse, depth+1)

lldb/test/Shell/SymbolFile/DWARF/DW_TAG_GNU_call_site-DW_AT_low_pc.s

15

Maybe place an int3 at the place where you want this to stop? Then you can delete all of the line table directives and the break location will be more obvious...

[jankratochvil]

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

113 ↗

(On Diff #269047)

I had this idea. :-) The problem is bool recurse is type-compatible with uint32_t depth which leads to uncaught bugs. Given that non-recursive call makes no sense with depth>0 I find the special value depth = -1 to be suitable for non-recursive calls.
Anyway implemented the recurse parameter in a safe way if it is worth it to separate it.

lldb/test/Shell/SymbolFile/DWARF/DW_TAG_GNU_call_site-DW_AT_low_pc.s

15

Done, nice.

[labath]

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

113 ↗

(On Diff #269047)

Right, I see. I don't think that a special tagged enum is really necessary here and flushing out the callers via two patches (like you did) would be enough. The only ones who that may impact are downstream users who merge in bulk, but: a) I doubt the downstream users are calling this function in their own code; b) we don't have to work extra hard to safeguard them.

However, given that you've already done it, I think it's fine to keep it -- though I'll probably change it back next time I work on this function.

lldb/test/Shell/SymbolFile/DWARF/DW_TAG_GNU_call_site-DW_AT_low_pc.s

47

This is formatted strangely. Tab issues?

[jankratochvil]

lldb/test/Shell/SymbolFile/DWARF/DW_TAG_GNU_call_site-DW_AT_low_pc.s

47

Yes, thanks. Some *.s files have tabs some spaces so I kept it with tabs (and fixed this one line).

[llunak]

The DWARF in the source file appears to be broken, the test fails for me on openSUSE 15.2 x86_64, with errors about DIE references outside of its CU. It fails even if I try to manually compile and debug the file, with Clang git or 9, GCC 7.5 or 9, LLDB git or 9, GDB 9.2.

seli@slim:~/tmp> clang -o DW_TAG_GNU_call_site-DW_AT_low_pc.s.tmp DW_TAG_GNU_call_site-DW_AT_low_pc.s
seli@slim:~/tmp> lldb DW_TAG_GNU_call_site-DW_AT_low_pc.s.tmp 
(lldb) target create "DW_TAG_GNU_call_site-DW_AT_low_pc.s.tmp"
Current executable set to '/home/seli/tmp/DW_TAG_GNU_call_site-DW_AT_low_pc.s.tmp' (x86_64).
(lldb) r
Process 20675 launched: '/home/seli/tmp/DW_TAG_GNU_call_site-DW_AT_low_pc.s.tmp' (x86_64)
error: DW_TAG_GNU_call_site-DW_AT_low_pc.s.tmp DW_FORM_ref* DIE reference 0x24f is outside of its CU
error: DW_TAG_GNU_call_site-DW_AT_low_pc.s.tmp DW_FORM_ref* DIE reference 0x256 is outside of its CU
Process 20675 stopped
* thread #1, name = 'DW_TAG_GNU_call', stop reason = signal SIGTRAP
    frame #0: 0x0000000000400520 DW_TAG_GNU_call_site-DW_AT_low_pc.s.tmp`a(p=<unavailable>) + 11
DW_TAG_GNU_call_site-DW_AT_low_pc.s.tmp`a:
->  0x400520 <+11>: retq   

DW_TAG_GNU_call_site-DW_AT_low_pc.s.tmp`main:
    0x400521 <+0>:  movl   $0x1, %edi
    0x400526 <+5>:  callq  0x400515                  ; a
    0x40052b <+10>: movl   $0x0, %eax
(lldb) p p
error: DW_TAG_GNU_call_site-DW_AT_low_pc.s.tmp DW_FORM_ref* DIE reference 0x24f is outside of its CU
error: DW_TAG_GNU_call_site-DW_AT_low_pc.s.tmp DW_FORM_ref* DIE reference 0x24f is outside of its CU
error: <lldb wrapper prefix>:43:31: no member named 'p' in namespace '$__lldb_local_vars'
    using $__lldb_local_vars::p;
          ~~~~~~~~~~~~~~~~~~~~^
error: <user expression 0>:1:1: use of undeclared identifier 'p'
p
^

seli@slim:~/tmp> gcc -o DW_TAG_GNU_call_site-DW_AT_low_pc.s.tmp DW_TAG_GNU_call_site-DW_AT_low_pc.s
seli@slim:~/tmp> gdb -q DW_TAG_GNU_call_site-DW_AT_low_pc.s.tmp
Reading symbols from DW_TAG_GNU_call_site-DW_AT_low_pc.s.tmp...
Dwarf Error: Cannot find DIE at 0x24f referenced from DIE at 0x158 [in module /home/seli/tmp/DW_TAG_GNU_call_site-DW_AT_low_pc.s.tmp]

[jankratochvil]

In D81334#2189533, @llunak wrote:

The DWARF in the source file appears to be broken, the test fails for me on openSUSE 15.2 x86_64, with errors about DIE references outside of its CU. It fails even if I try to manually compile and debug the file, with Clang git or 9, GCC 7.5 or 9, LLDB git or 9, GDB 9.2.

Thanks for the bugreport. SuSE must be linking some DWARF by default into all executables. Checked in a fix.

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

for (size_t i = 0; i < num_attributes; ++i) {
         call_origin = form_value.Reference();
         if (!call_origin->IsValid()) {
           LLDB_LOG(log, "CollectCallEdges: Invalid call origin in {0}",
                    function_die.GetPubname());
           break;
         }
       }
 
-      if (attr == DW_AT_low_pc)
+      // Second DW_AT_low_pc may come from DW_TAG_subprogram referenced by
+      // DW_TAG_GNU_call_site's DW_AT_abstract_origin.
+      if (attr == DW_AT_low_pc && low_pc == LLDB_INVALID_ADDRESS)
         low_pc = form_value.Address();
 
       // Extract DW_AT_call_return_pc (the PC the call returns to) if it's
       // available. It should only ever be unavailable for tail call edges, in
       // which case use LLDB_INVALID_ADDRESS.
       if (attr == DW_AT_call_return_pc)
         return_pc = form_value.Address();
 

lldb/test/Shell/SymbolFile/DWARF/DW_TAG_GNU_call_site-DW_AT_low_pc.s

+# This tests that lldb is compatible with DWARF-4 entry values GNU extension.
+
+# REQUIRES: target-x86_64, system-linux, lld
+
+# RUN: %clang_host -o %t %s
+# RUN: %lldb %t -o 'b 6' -o r -o 'p p' -o exit | FileCheck %s
+
+# CHECK: (int) $0 = 1
+
+# The DWARF has been produced and modified from:
+# static __attribute__((noinline, noclone)) void b(int x) {
+#   asm("");
+# }
+# static __attribute__((noinline, noclone)) void a(int p) {
+#   b(2);
+# }
+# int main() {
+#   a(1);
+#   return 0;
+# }
+
+  .file "DW_TAG_GNU_call_site-DW_AT_low_pc.c"
+  .text
+.Ltext0:
+  .type b, @function
+b:
+.LVL0:
+.LFB0:
+  .file 1 "DW_TAG_GNU_call_site-DW_AT_low_pc.c"
+  .loc 1 1 57
+  .cfi_startproc
+  .loc 1 2 3
+  .loc 1 3 1 is_stmt 0
+  ret
+  .cfi_endproc
+.LFE0:
+  .size b, .-b
+  .type a, @function
+a:
+.LVL1:
+.LFB1:
+  .loc 1 4 57 is_stmt 1
+  .cfi_startproc
+  .loc 1 5 3
+  movl  $2, %edi
+.LVL2:
+  .loc 1 5 3 is_stmt 0
+  call  b
+.LVL3:
+  .loc 1 6 1
+  ret
+  .cfi_endproc
+.LFE1:
+  .size a, .-a
+  .globl  main
+  .type main, @function
+main:
+.LFB2:
+  .loc 1 7 12 is_stmt 1
+  .cfi_startproc
+  .loc 1 8 3
+  movl  $1, %edi
+  call  a
+.LVL4:
+  .loc 1 9 3
+  .loc 1 10 1 is_stmt 0
+  movl  $0, %eax
+  ret
+  .cfi_endproc
+.LFE2:
+  .size main, .-main
+.Letext0:
+  .section  .debug_info,"",@progbits
+.Ldebug_info0:
+  .long .Ldebuginfo_end - .Ldebuginfo_start # Length of Compilation Unit Info
+.Ldebuginfo_start:
+  .value  0x4 # DWARF version number
+  .long .Ldebug_abbrev0 # Offset Into Abbrev. Section
+  .byte 0x8 # Pointer Size (in bytes)
+  .uleb128 0x1  # (DIE (0xb) DW_TAG_compile_unit)
+  .asciz "GNU C17 10.1.1 20200507 (Red Hat 10.1.1-1) -mtune=generic -march=x86-64 -g -Og"  # DW_AT_producer: "GNU C17 10.1.1 20200507 (Red Hat 10.1.1-1) -mtune=generic -march=x86-64 -g -Og"
+  .byte 0xc # DW_AT_language
+  .asciz "DW_TAG_GNU_call_site-DW_AT_low_pc.c"  # DW_AT_name
+  .asciz "" # DW_AT_comp_dir: "/home/jkratoch/t"
+  .quad .Ltext0 # DW_AT_low_pc
+  .quad .Letext0-.Ltext0  # DW_AT_high_pc
+  .long .Ldebug_line0 # DW_AT_stmt_list
+  .uleb128 0x2  # (DIE (0x2d) DW_TAG_subprogram)
+      # DW_AT_external
+  .asciz "main" # DW_AT_name: "main"
+  .long .Ltype_int  # DW_AT_type
+  .quad .LFB2 # DW_AT_low_pc
+  .quad .LFE2-.LFB2 # DW_AT_high_pc
+  .uleb128 0x1  # DW_AT_frame_base
+  .byte 0x9c  # DW_OP_call_frame_cfa
+      # DW_AT_GNU_all_call_sites
+  .uleb128 0x3  # (DIE (0x4f) DW_TAG_GNU_call_site)
+  .quad .LVL4 # DW_AT_low_pc
+  .long .Lfunc_a  # DW_AT_abstract_origin
+  .uleb128 0x4  # (DIE (0x5c) DW_TAG_GNU_call_site_parameter)
+  .uleb128 0x1  # DW_AT_location
+  .byte 0x55  # DW_OP_reg5
+  .uleb128 0x1  # DW_AT_GNU_call_site_value
+  .byte 0x31  # DW_OP_lit1
+  .byte 0 # end of children of DIE 0x4f
+  .byte 0 # end of children of DIE 0x2d
+.Ltype_int:
+  .uleb128 0x5  # (DIE (0x63) DW_TAG_base_type)
+  .byte 0x4 # DW_AT_byte_size
+  .byte 0x5 # DW_AT_encoding
+  .asciz "int"  # DW_AT_name
+.Lfunc_a:
+  .uleb128 0x6  # (DIE (0x6a) DW_TAG_subprogram)
+  .asciz "a"  # DW_AT_name
+      # DW_AT_prototyped
+  .quad .LFB1 # DW_AT_low_pc
+  .quad .LFE1-.LFB1 # DW_AT_high_pc
+  .uleb128 0x1  # DW_AT_frame_base
+  .byte 0x9c  # DW_OP_call_frame_cfa
+      # DW_AT_GNU_all_call_sites
+  .uleb128 0x7  # (DIE (0x86) DW_TAG_formal_parameter)
+  .asciz "p"  # DW_AT_name
+  .long .Ltype_int  # DW_AT_type
+  .long .LLST0  # DW_AT_location
+  .byte 0 # end of children of DIE 0x6a
+  .byte 0 # end of children of DIE 0xb
+.Ldebuginfo_end:
+  .section  .debug_abbrev,"",@progbits
+.Ldebug_abbrev0:
+  .uleb128 0x1  # (abbrev code)
+  .uleb128 0x11 # (TAG: DW_TAG_compile_unit)
+  .byte 0x1 # DW_children_yes
+  .uleb128 0x25 # (DW_AT_producer)
+  .uleb128 0x8  # (DW_FORM_string)
+  .uleb128 0x13 # (DW_AT_language)
+  .uleb128 0xb  # (DW_FORM_data1)
+  .uleb128 0x3  # (DW_AT_name)
+  .uleb128 0x8  # (DW_FORM_string)
+  .uleb128 0x1b # (DW_AT_comp_dir)
+  .uleb128 0x8  # (DW_FORM_string)
+  .uleb128 0x11 # (DW_AT_low_pc)
+  .uleb128 0x1  # (DW_FORM_addr)
+  .uleb128 0x12 # (DW_AT_high_pc)
+  .uleb128 0x7  # (DW_FORM_data8)
+  .uleb128 0x10 # (DW_AT_stmt_list)
+  .uleb128 0x17 # (DW_FORM_sec_offset)
+  .byte 0
+  .byte 0
+  .uleb128 0x2  # (abbrev code)
+  .uleb128 0x2e # (TAG: DW_TAG_subprogram)
+  .byte 0x1 # DW_children_yes
+  .uleb128 0x3f # (DW_AT_external)
+  .uleb128 0x19 # (DW_FORM_flag_present)
+  .uleb128 0x3  # (DW_AT_name)
+  .uleb128 0x8  # (DW_FORM_string)
+  .uleb128 0x49 # (DW_AT_type)
+  .uleb128 0x13 # (DW_FORM_ref4)
+  .uleb128 0x11 # (DW_AT_low_pc)
+  .uleb128 0x1  # (DW_FORM_addr)
+  .uleb128 0x12 # (DW_AT_high_pc)
+  .uleb128 0x7  # (DW_FORM_data8)
+  .uleb128 0x40 # (DW_AT_frame_base)
+  .uleb128 0x18 # (DW_FORM_exprloc)
+  .uleb128 0x2117 # (DW_AT_GNU_all_call_sites)
+  .uleb128 0x19 # (DW_FORM_flag_present)
+  .byte 0
+  .byte 0
+  .uleb128 0x3  # (abbrev code)
+  .uleb128 0x4109 # (TAG: DW_TAG_GNU_call_site)
+  .byte 0x1 # DW_children_yes
+  .uleb128 0x11 # (DW_AT_low_pc)
+  .uleb128 0x1  # (DW_FORM_addr)
+  .uleb128 0x31 # (DW_AT_abstract_origin)
+  .uleb128 0x13 # (DW_FORM_ref4)
+  .byte 0
+  .byte 0
+  .uleb128 0x4  # (abbrev code)
+  .uleb128 0x410a # (TAG: DW_TAG_GNU_call_site_parameter)
+  .byte 0 # DW_children_no
+  .uleb128 0x2  # (DW_AT_location)
+  .uleb128 0x18 # (DW_FORM_exprloc)
+  .uleb128 0x2111 # (DW_AT_GNU_call_site_value)
+  .uleb128 0x18 # (DW_FORM_exprloc)
+  .byte 0
+  .byte 0
+  .uleb128 0x5  # (abbrev code)
+  .uleb128 0x24 # (TAG: DW_TAG_base_type)
+  .byte 0 # DW_children_no
+  .uleb128 0xb  # (DW_AT_byte_size)
+  .uleb128 0xb  # (DW_FORM_data1)
+  .uleb128 0x3e # (DW_AT_encoding)
+  .uleb128 0xb  # (DW_FORM_data1)
+  .uleb128 0x3  # (DW_AT_name)
+  .uleb128 0x8  # (DW_FORM_string)
+  .byte 0
+  .byte 0
+  .uleb128 0x6  # (abbrev code)
+  .uleb128 0x2e # (TAG: DW_TAG_subprogram)
+  .byte 0x1 # DW_children_yes
+  .uleb128 0x3  # (DW_AT_name)
+  .uleb128 0x8  # (DW_FORM_string)
+  .uleb128 0x27 # (DW_AT_prototyped)
+  .uleb128 0x19 # (DW_FORM_flag_present)
+  .uleb128 0x11 # (DW_AT_low_pc)
+  .uleb128 0x1  # (DW_FORM_addr)
+  .uleb128 0x12 # (DW_AT_high_pc)
+  .uleb128 0x7  # (DW_FORM_data8)
+  .uleb128 0x40 # (DW_AT_frame_base)
+  .uleb128 0x18 # (DW_FORM_exprloc)
+  .uleb128 0x2117 # (DW_AT_GNU_all_call_sites)
+  .uleb128 0x19 # (DW_FORM_flag_present)
+  .byte 0
+  .byte 0
+  .uleb128 0x7  # (abbrev code)
+  .uleb128 0x5  # (TAG: DW_TAG_formal_parameter)
+  .byte 0 # DW_children_no
+  .uleb128 0x3  # (DW_AT_name)
+  .uleb128 0x8  # (DW_FORM_string)
+  .uleb128 0x49 # (DW_AT_type)
+  .uleb128 0x13 # (DW_FORM_ref4)
+  .uleb128 0x2  # (DW_AT_location)
+  .uleb128 0x17 # (DW_FORM_sec_offset)
+  .byte 0
+  .byte 0
+  .byte 0
+  .section  .debug_loc,"",@progbits
+.LLST0:
+  .quad .LVL1-.Ltext0 # Location list begin address (*.LLST0)
+  .quad .LVL2-.Ltext0 # Location list end address (*.LLST0)
+  .value  0x1 # Location expression size
+  .byte 0x55  # DW_OP_reg5
+  .quad .LVL2-.Ltext0 # Location list begin address (*.LLST0)
+  .quad .LFE1-.Ltext0 # Location list end address (*.LLST0)
+  .value  0x4 # Location expression size
+  .byte 0xf3  # DW_OP_GNU_entry_value
+  .uleb128 0x1
+  .byte 0x55  # DW_OP_reg5
+  .byte 0x9f  # DW_OP_stack_value
+  .quad 0 # Location list terminator begin (*.LLST0)
+  .quad 0 # Location list terminator end (*.LLST0)
+  .section  .debug_line,"",@progbits
+.Ldebug_line0:
+  .section  .note.GNU-stack,"",@progbits