[PATCH] D95511: [llvm-readelf] Support dumping the BB address map section with --bb-addr-map.
Rahman Lavaee via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 29 01:15:04 PST 2021
rahmanl added a comment.
Thanks for the comments @grimar
================
Comment at: llvm/test/tools/llvm-readobj/ELF/bb-addr-map.test:60
+
+# RUN: yaml2obj --docnum=2 %s -o %t2.o
+# RUN: llvm-readobj %t2.o --bb-addr-map 2>&1 | FileCheck %s -DFILE=%t2.o --check-prefix=INVALID
----------------
grimar wrote:
> This needs a comment to describe what is exactly wrong with the section.
> There are multiple possible issues that might occur, but you test only one of them.
>
> I'd suggest to create a valid section and use `ShSize` key with a macro to truncate it
> by different ways. This will allow to easily test each of cases
>
> ```
> uint32_t Offset = Data.getULEB128(Cur);
> uint32_t Size = Data.getULEB128(Cur);
> uint32_t Metadata = Data.getULEB128(Cur);
> ```
>
> I.e. you can just reuse the first YAML:
>
>
> ```
> Sections:
> - Name: .llvm_bb_addr_map
> Type: SHT_LLVM_BB_ADDR_MAP
> ShSize: [[SIZE=<none>]] <--- just add this line and use the power of macros
> Entries:
> .....
> ```
>
Thanks for the suggestion. Adopted.
I am not sure if I want to exercise more invalid cases. Of course I can truncate the content by different sizes, but I don't think it will add a lot of value.
================
Comment at: llvm/test/tools/llvm-readobj/ELF/bb-addr-map.test:62
+# RUN: llvm-readobj %t2.o --bb-addr-map 2>&1 | FileCheck %s -DFILE=%t2.o --check-prefix=INVALID
+# INVALID: warning: '[[FILE]]': unable to dump the SHT_LLVM_BB_ADDR_MAP section: unable to decode LEB128 at offset 0x{{([[:xdigit:]]{8})}}: malformed uleb128, extends past end
+--- !ELF
----------------
grimar wrote:
> I am not sure that `0x{{([[:xdigit:]]{8})}}` is usefull? I'd just check the offset.
> Also I guess you can reuse this check line for other broken cases and have a `[[OFFSET]]` macro instead.
>
>
> ```
> ... | FileCheck %s -DFILE=%t2.o --check-prefix=INVALID -DOFFSET=0x....
> # INVALID: warning: '[[FILE]]': unable to dump the SHT_LLVM_BB_ADDR_MAP section: unable to decode LEB128 at offset [[OFFSET]]: malformed uleb128, extends past end
> ```
>
>
Thanks. Used `[[OFFSET]]` even though it's only used for one test case.
================
Comment at: llvm/test/tools/llvm-readobj/ELF/bb-addr-map.test:71
+ Type: SHT_LLVM_BB_ADDR_MAP
+ Size: 8
----------------
grimar wrote:
> Usually we reduce the number of spaces to minimal.
>
> ```
> - Name: .llvm_bb_addr_map
> Type: SHT_LLVM_BB_ADDR_MAP
> Size: 8
> ```
>
> (The same applies for the first YAML).
I removed this YAML since I am reusing the first YAML for the invalid case.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95511/new/
https://reviews.llvm.org/D95511
More information about the llvm-commits
mailing list