[PATCH] D71932: [DWARF] Better detect errors in Address Range Tables.

[Igor Kudrin via Phabricator via llvm-commits]

ikudrin marked an inline comment as done.
ikudrin added inline comments.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugArangeSet.cpp:167-171
+      return createStringError(
+          errc::invalid_argument,
+          "address range table at offset 0x%" PRIx64
+          " has an invalid tuple (length = 0) at offset 0x%" PRIx64,
+          Offset, *offset_ptr - tuple_size);
----------------
dblaikie wrote:
> jhenderson wrote:
> > @ikudrin, I know this was committed a while back, but I only just ran into this.
> > 
> > Why was this check added? There's nothing in the DWARF spec that I know of that requires a non-zero length for all non-terminating entries. In our Sony linker, prior to the new tombstoning functionality that has recently been implemented, we would patch a reference in the .debug_aranges to have a zero length, and -1 for the address. This used to be fine, but recent versions of llvm-dwarfdump now can't dump this. Even without the Sony changes, there are occasionally functions in the DWARF with zero length (see also my post earlier today on the llvm-dev list for .debug_loc and zero length entry descriptions).
> Yep, +1 we should allow empty ranges here. Even with better tombstoning - a) backwards compatibility with existing implementations (ld.bfd) b) even with proper tombstoning, it'd effectively create an empty range (-2, -2) - though arguably in that case we should detect the tombstoning first, before considering the range size/other issues
> 
> Here's a reproduction with commonly accessible tools:
> 
> ```
> $ cat range2.cpp
> void f1() {
> }
> void f2() {
> }
> int main() {
>   f1();
> }
> $ clang++ range2.cpp -g -fuse-ld=bfd -ffunction-sections -gdwarf-aranges -Wl,-gc-sections
> $ llvm-dwarfdump -verify a.out
> Verifying a.out:        file format elf64-x86-64
> Verifying .debug_abbrev...
> Verifying .debug_info Unit Header Chain...
> error: DIE address ranges are not contained in its parent's ranges:
> 0x0000000b: DW_TAG_compile_unit
>               DW_AT_producer    ("clang version 12.0.0 (git at github.com:llvm/llvm-project.git 80e6844e28f10c1c2c31b301c3ef980c7a11ee6b)")
>               DW_AT_language    (DW_LANG_C_plus_plus_14)
>               DW_AT_name        ("range2.cpp")
>               DW_AT_stmt_list   (0x00000000)
>               DW_AT_comp_dir    ("/usr/local/google/home/blaikie/dev/scratch")
>               DW_AT_low_pc      (0x0000000000000000)
>               DW_AT_ranges      (0x00000000
>                  [0x0000000000401110, 0x0000000000401116)
>                  [0x0000000000000001, 0x0000000000000001)
>                  [0x0000000000401120, 0x000000000040112d))
> 
> 0x00000043:   DW_TAG_subprogram
>                 DW_AT_low_pc    (0x0000000000000000)
>                 DW_AT_high_pc   (0x0000000000000006)
>                 DW_AT_frame_base        (DW_OP_reg6)
>                 DW_AT_linkage_name      ("_Z2f2v")
>                 DW_AT_name      ("f2")
>                 DW_AT_decl_file ("/usr/local/google/home/blaikie/dev/scratch/range2.cpp")
>                 DW_AT_decl_line (3)
>                 DW_AT_external  (true)
> 
> Verifying .debug_info references...
> Verifying .debug_types Unit Header Chain...
> Errors detected.
> ```
> Similarly, with GCC (which produces debug_aranges by default):
> ```
> $ g++ range2.cpp -g -fuse-ld=bfd -ffunction-sections -Wl,-gc-sections
> $ llvm-dwarfdump -verify a.out
> Verifying a.out:        file format elf64-x86-64
> Verifying .debug_abbrev...
> Verifying .debug_info Unit Header Chain...
> error: DIE address ranges are not contained in its parent's ranges:
> 0x0000000b: DW_TAG_compile_unit
>               DW_AT_producer    ("GNU C++14 10.0.0 20200111 (experimental) -mtune=generic -march=x86-64 -g -fuse-ld=bfd -ffunction-sections")
>               DW_AT_language    (DW_LANG_C_plus_plus)
>               DW_AT_name        ("range2.cpp")
>               DW_AT_comp_dir    ("/usr/local/google/home/blaikie/dev/scratch")
>               DW_AT_ranges      (0x00000000
>                  [0x0000000000401102, 0x0000000000401109)
>                  [0x0000000000000001, 0x0000000000000001)
>                  [0x0000000000401109, 0x0000000000401119))
>               DW_AT_low_pc      (0x0000000000000000)
>               DW_AT_stmt_list   (0x00000000)
> 
> 0x0000004e:   DW_TAG_subprogram
>                 DW_AT_external  (true)
>                 DW_AT_name      ("f2")
>                 DW_AT_decl_file ("/usr/local/google/home/blaikie/dev/scratch/range2.cpp")
>                 DW_AT_decl_line (3)
>                 DW_AT_decl_column       (0x06)
>                 DW_AT_linkage_name      ("_Z2f2v")
>                 DW_AT_low_pc    (0x0000000000000000)
>                 DW_AT_high_pc   (0x0000000000000007)
>                 DW_AT_frame_base        (DW_OP_call_frame_cfa)
>                 DW_AT_GNU_all_call_sites        (true)
> 
> Verifying .debug_info references...
> Verifying .debug_types Unit Header Chain...
> Errors detected.
> ```
> Why was this check added? There's nothing in the DWARF spec that I know of that requires a non-zero length for all non-terminating entries.

I tried to follow the Standard here, see 6.1.2 at DWARFv5, p. 148:
> Each descriptor is a triple consisting of a segment selector, the beginning address within that segment of a range of text or data covered by some entry owned by the corresponding compilation unit, followed by the **non-zero** length of that range.

It looks like the DWARF spec should be updated to reflect the practice.

@jhenderson, it would be great if you prepare the fix. I'll be AFK for a couple of weeks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71932/new/

https://reviews.llvm.org/D71932