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

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Just a few small suggestions, otherwise this looks good, thanks!



================
Comment at: llvm/test/tools/llvm-readobj/ELF/bb-addr-map.test:3
+
+## Check x64
+# RUN: yaml2obj %s -DBITS=64 -DADDR=0xFFFFFFFF1 -o %t1.x64.o
----------------
Maybe `:` or `.` to end this and the Check 32 bit line.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/bb-addr-map.test:8
+
+## Check x32
+# RUN: yaml2obj %s -DBITS=32 -o %t1.x32.o
----------------
I'm not sure "x32" is really a thing. I think you probably just want to call it "64-bit" and "32-bit".


================
Comment at: llvm/unittests/Object/ELFObjectFileTest.cpp:533
+  OverInt32LimitYamlStrings[0] += R"(
+          - AddressOffset: 0xFFFFFFFF3
+            Size:          0x4
----------------
It might make sense to do proper edge-case testing here, with values of `0xffffffff` and `0x100000000`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95511