[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/tools/llvm-readobj/ELFDumper.cpp:2670
+  if (!Obj->isLE()) {
+    W.startLine() << "Attributes not implemented.\n";
+    return;
----------------
jhenderson wrote:
> Is there a test case for this line?
No, it is not related to this patch. Originally, printAttributes() is only implemented for `ELFDumper<ELF32LE>`. So, I check `Obj->isLE()` to keep original logic. In this way, it does not change the functionality of original version. So, I did not add a test case for it.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:2676
+      Obj->getHeader()->e_machine != EM_RISCV) {
     W.startLine() << "Attributes not implemented.\n";
     return;
----------------
jhenderson wrote:
> Is there a test case for this line?
No, it is not related to this patch.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:2703
+      if (Error E = RISCVAttributeParser(&W).parse(Contents, support::little))
+        consumeError(std::move(E));
+    }
----------------
jhenderson wrote:
> This is an existing mistake in the ARM version, but we shouldn't replicate it (and perhaps you could fix it separately). Unless `E` is going to be a duplicate of an already-reported error, this should report the error, probably as a warning, as there's no particular reason to terminate the program.
We could fix it separately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75833