[PATCH] D95511: [llvm-readelf] Support dumping the BB address map section with --bb-addr-map.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/bb-addr-map.test:37
+# GNU: GNUStyle::printBBAddrMap not implemented
+--- !ELF
+FileHeader:
----------------
I'd separate the YAML from the check lines with an empty line.
The same below.

Also I'd separate LLVM checks from GNU.



================
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
----------------
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:
.....
```



================
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
----------------
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
```




================
Comment at: llvm/test/tools/llvm-readobj/ELF/bb-addr-map.test:65
+FileHeader:
+  Class:  ELFCLASS64
+  Data:  ELFDATA2LSB
----------------
It is slightly misaligned vertically.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/bb-addr-map.test:71
+    Type:    SHT_LLVM_BB_ADDR_MAP
+    Size:    8
----------------
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).


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:6456
+        "unable to dump the SHT_LLVM_BB_ADDR_MAP section: " +
+        toString(BBAddrMapOrErr.takeError()));
+    return;
----------------
It would me more consistent to use the `describe` helper to build the message. E.g. see:

```
    return createError("invalid section linked to " + describe(Obj, Sec) +
                       ": " + toString(SymtabOrErr.takeError()));
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95511