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)
The new -1 value for GetAttributes is a bit ugly. But otherwise either
- CollectCallEdges would need to reimplement GetAttributes.
- 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.
# 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)
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.
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)
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...
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.
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.
|46 ↗||(On Diff #269328)|
This is formatted strangely. Tab issues?
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]