[PATCH] D68383: [llvm-readelf/llvm-objdump] - Improve/refactor the implementation of SHT_LLVM_ADDRSIG section dumping.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: test/tools/llvm-readobj/elf-addrsig.test:1
+# Show that llvm-readobj can dump SHT_LLVM_ADDRSIG sections.
+
----------------
'##'

FWIW, it looks like there's some coverage in the MC tests, but I think it makes sense to have a dedicated test here.


================
Comment at: test/tools/llvm-readobj/elf-addrsig.test:30
+
+## Check we report a warning when SHT_LLVM_ADDRSIG is broken (e.g. contains a mailformed uleb128).
+
----------------
mailformed -> malformed


================
Comment at: test/tools/llvm-readobj/elf-addrsig.test:33
+# RUN: yaml2obj --docnum=2 %s -o %t2.o
+# RUN: llvm-readobj --addrsig %t2.o 2>&1 | FileCheck %s -DFILE=%t2.o --check-prefix=MAILFORMED
+
----------------
MAILFORMED -> MALFORMED


================
Comment at: test/tools/llvm-readobj/elf-addrsig.test:80
+## Check that llvm-readobj dumps SHT_LLVM_ADDRSIG section when --all is specified.
+## TODO: llvm-readelf does not dump it. Add a better test when GNU-style dumping will be implemented.
+
----------------
llvm-readelf does not dump it, and should not dump it either with --all (see also the discussion about .stack_sizes dumping), so this comment should be updated to say so.


================
Comment at: test/tools/llvm-readobj/elf-addrsig.test:82
+
+# RUN: yaml2obj --docnum=1 %s -o %t4.o
+# RUN: llvm-readobj --all %t4.o | FileCheck %s --check-prefix LLVM
----------------
Why rebuild?


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

https://reviews.llvm.org/D68383