[PATCH] D75833: [RISCV] Support RISC-V ELF attribute section in llvm-readobj

[Hsiangkai Wang via Phabricator via llvm-commits]

HsiangKai marked 3 inline comments as done.
HsiangKai added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/RISCV/attribute.s:1
+## Test llvm-readobj & llvm-readelf can decode RISC-V attributes correctly.
+
----------------
MaskRay wrote:
> This should probably go to `test/MC/RISCV` or `test/MC/ELF/RISCV`. This is more about a test for assembly.
It is not used to test assembly. It is used to test readelf/readelf to decode attribute section correctly.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/RISCV/invalid-attr-section-size.test:1
+## This test case is used to ensure the error message is caught by llvm-readobj.
+
----------------
MaskRay wrote:
> We can have one single `invalid-*.text`. You can find plent of `--docnum=` examples in llvm-readobj/ELF/
> 
> As I mentioned, we don't need many tests testing invalid section contents. We just need to show llvm-readobj forwards RISCVAttributeParser diagnostics, which are already tested by llvm/unittests/
That's why I removed other invalid-attr*.test.

invalid-attr-section-size.test is used to test the error messages are handled by llvm-readobj. As I mentioned in the test case comments.
invalid-attr-version.test is used to test the format version is checked in llvm-readobj.

They are two different test cases.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/RISCV/validate-attr-section.test:1
+## We only implement attribute section printing for little-endian encoding.
+
----------------
MaskRay wrote:
> This can be merged into `section-types.test`, I think.
These two test cases are used for different purpose. I prefer to keep these two test cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75833