labath marked 8 inline comments as done.
labath added inline comments.
================
Comment at: include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h:91
+ class EntryIterator {
+ public:
----------------
I went for an iterator-like approach (instead of a callback or direct
materialization) because it's easier to use. In particular it makes it possible
to reuse this stuff in the dumping code, which would have been pretty hard with
callbacks.
================
Comment at: include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h:138
+ llvm::Optional<object::SectionedAddress> BaseAddr;
+ std::function<Optional<object::SectionedAddress>(uint32_t)>
+ AddrOffsetResolver;
----------------
Using a callback is consistent with what rnglists code does. This uses a
std::function instead of a function_ref because it's easier for iterators to
escape out of scopes. However, I've wondered if we shouldn't define an
AddrOffsetResolver interface that llvm, lldb DWARFUnits and anyone else who
wishes so (unit tests ?) can implement.
================
Comment at: lib/DebugInfo/DWARF/DWARFDebugLoc.cpp:164
+ if (!BaseAddr)
+ return createStringError(errc::invalid_argument,
+ "Cannot interpret DW_LLE_offset_pair entry due "
----------------
A significant deviation from the rnglists code: Here I return an error if it is
not possible to compute the address range correctly. The rnglists parser cannot
compute the value, it will just return something, which can be very far from
the correct range -- for instance, it's happy to use the base_addressx index as
the offset, if the index cannot be resolved correctly. And it doesn't provide
any indication that it has done so, which doesn't seem like a very useful
behavior.
If this is the behavior we want, I can also try to make the rnglists parser do
something similar.
================
Comment at: lib/DebugInfo/DWARF/DWARFDebugLoc.cpp:291-295
+ EntryIterator Absolute =
+ getAbsoluteLocations(
+ SectionedAddress{BaseAddr, SectionedAddress::UndefSection},
+ LookupPooledAddress)
+ .begin();
----------------
This parallel iteration is not completely nice, but I think it's worth being
able to reuse the absolute range computation code. I'm open to ideas for
improvement though.
================
Comment at: test/CodeGen/X86/debug-loclists.ll:16
; CHECK-NEXT: 0x00000000:
-; CHECK-NEXT: [0x0000000000000000, 0x0000000000000004): DW_OP_breg5 RDI+0
-; CHECK-NEXT: [0x0000000000000004, 0x0000000000000012): DW_OP_breg3 RBX+0
-
-; There is no way to use llvm-dwarfdump atm (2018, october) to verify the
DW_LLE_* codes emited,
-; because dumper is not yet implements that. Use asm code to do this check
instead.
-;
-; RUN: llc -mtriple=x86_64-pc-linux -filetype=asm < %s -o - | FileCheck %s
--check-prefix=ASM
-; ASM: .section .debug_loclists,"",@progbits
-; ASM-NEXT: .long .Ldebug_loclist_table_end0-.Ldebug_loclist_table_start0 #
Length
-; ASM-NEXT: .Ldebug_loclist_table_start0:
-; ASM-NEXT: .short 5 # Version
-; ASM-NEXT: .byte 8 # Address size
-; ASM-NEXT: .byte 0 # Segment selector size
-; ASM-NEXT: .long 0 # Offset entry count
-; ASM-NEXT: .Lloclists_table_base0:
-; ASM-NEXT: .Ldebug_loc0:
-; ASM-NEXT: .byte 4 # DW_LLE_offset_pair
-; ASM-NEXT: .uleb128 .Lfunc_begin0-.Lfunc_begin0 # starting offset
-; ASM-NEXT: .uleb128 .Ltmp0-.Lfunc_begin0 # ending offset
-; ASM-NEXT: .byte 2 # Loc expr size
-; ASM-NEXT: .byte 117 # DW_OP_breg5
-; ASM-NEXT: .byte 0 # 0
-; ASM-NEXT: .byte 4 # DW_LLE_offset_pair
-; ASM-NEXT: .uleb128 .Ltmp0-.Lfunc_begin0 # starting offset
-; ASM-NEXT: .uleb128 .Ltmp1-.Lfunc_begin0 # ending offset
-; ASM-NEXT: .byte 2 # Loc expr size
-; ASM-NEXT: .byte 115 # DW_OP_breg3
-; ASM-NEXT: .byte 0 # 0
-; ASM-NEXT: .byte 0 # DW_LLE_end_of_list
-; ASM-NEXT: .Ldebug_loclist_table_end0:
+; CHECK-NEXT: [DW_LLE_offset_pair ]: 0x0000000000000000, 0x0000000000000004
=> [0x0000000000000000, 0x0000000000000004) DW_OP_breg5 RDI+0
+; CHECK-NEXT: [DW_LLE_offset_pair ]: 0x0000000000000004, 0x0000000000000012
=> [0x0000000000000004, 0x0000000000000012) DW_OP_breg3 RBX+0
----------------
This tries to follow the RLE format as closely as possible, but I think
something like
```
[DW_LLE_offset_pair, 0x0000000000000000, 0x0000000000000004] =>
[0x0000000000000000, 0x0000000000000004): DW_OP_breg5 RDI+0
```
would make more sense (both here and for RLE).
================
Comment at: test/DebugInfo/X86/fission-ranges.ll:48
; CHECK: [[A]]:
-; CHECK-NEXT: Addr idx 2 (w/ length 15): DW_OP_consts +0, DW_OP_stack_value
-; CHECK-NEXT: Addr idx 3 (w/ length 15): DW_OP_reg0 RAX
-; CHECK-NEXT: Addr idx 4 (w/ length 18): DW_OP_breg7 RSP-8
+; CHECK-NEXT: [DW_LLE_startx_length]: 0x00000002, 0x0000000f => <Failed to
read address offset 2> DW_OP_consts +0, DW_OP_stack_value
+; CHECK-NEXT: [DW_LLE_startx_length]: 0x00000003, 0x0000000f => <Failed to
read address offset 3> DW_OP_reg0 RAX
----------------
This is somewhat annoying, because the entries printed through the loclists
section will always have this error (as we don't have the DWARFUnit). I'll have
to figure out a way to suppress those, while still keeping them around when
printing from DWARFDie (as there a failure means a real error).
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68270/new/
https://reviews.llvm.org/D68270
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
