[PATCH] D107968: [llvm-readobj] Refactor ELFDumper::printAttributes()

[Jozef Lawrynowicz via Phabricator via llvm-commits]

jozefl updated this revision to Diff 366851.
jozefl added a comment.

Fixed in the attached.

There's lots of warnings in ELFDumper.cpp that use the W.startLine() approach
of printing warnings. As I understand from the LLVM Coding Standards, the
community does not want patches that performs large-scale refactoring, such as
changing the W.startLine to reportUniqueWarning, and changing the text to match
current LLVM style. Given this, I assume a bug report that details this
inconsistencies is not required, and that they will just get updated when the
surrounding code next gets updated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107968

Files:
  llvm/test/tools/llvm-readobj/ELF/RISCV/validate-attr-section.test
  llvm/tools/llvm-readobj/ELFDumper.cpp


Index: llvm/tools/llvm-readobj/ELFDumper.cpp
===================================================================
--- llvm/tools/llvm-readobj/ELFDumper.cpp
+++ llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -339,7 +339,8 @@
     return DynRegionInfo(ObjF, *this, Obj.base() + Offset, Size, EntSize);
   }
 
-  void printAttributes();
+  void printAttributes(unsigned, std::unique_ptr<ELFAttributeParser>,
+                       support::endianness);
   void printMipsReginfo();
   void printMipsOptions();
 
@@ -2557,8 +2558,22 @@
 template <class ELFT> void ELFDumper<ELFT>::printArchSpecificInfo() {
   switch (Obj.getHeader().e_machine) {
   case EM_ARM:
+    if (Obj.isLE())
+      printAttributes(ELF::SHT_ARM_ATTRIBUTES,
+                      std::make_unique<ARMAttributeParser>(&W),
+                      support::little);
+    else
+      reportUniqueWarning("attribute printing not implemented for big-endian "
+                          "ARM objects");
+    break;
   case EM_RISCV:
-    printAttributes();
+    if (Obj.isLE())
+      printAttributes(ELF::SHT_RISCV_ATTRIBUTES,
+                      std::make_unique<RISCVAttributeParser>(&W),
+                      support::little);
+    else
+      reportUniqueWarning("attribute printing not implemented for big-endian "
+                          "RISC-V objects");
     break;
   case EM_MIPS: {
     printMipsABIFlags();
@@ -2581,20 +2596,15 @@
   }
 }
 
-template <class ELFT> void ELFDumper<ELFT>::printAttributes() {
-  if (!Obj.isLE()) {
-    W.startLine() << "Attributes not implemented.\n";
-    return;
-  }
-
-  const unsigned Machine = Obj.getHeader().e_machine;
-  assert((Machine == EM_ARM || Machine == EM_RISCV) &&
-         "Attributes not implemented.");
-
+template <class ELFT>
+void ELFDumper<ELFT>::printAttributes(
+    unsigned AttrShType, std::unique_ptr<ELFAttributeParser> AttrParser,
+    support::endianness Endianness) {
+  assert((AttrShType != ELF::SHT_NULL) && AttrParser &&
+         "Incomplete ELF attribute implementation");
   DictScope BA(W, "BuildAttributes");
   for (const Elf_Shdr &Sec : cantFail(Obj.sections())) {
-    if (Sec.sh_type != ELF::SHT_ARM_ATTRIBUTES &&
-        Sec.sh_type != ELF::SHT_RISCV_ATTRIBUTES)
+    if (Sec.sh_type != AttrShType)
       continue;
 
     ArrayRef<uint8_t> Contents;
@@ -2613,13 +2623,7 @@
 
     W.printHex("FormatVersion", Contents[0]);
 
-    auto ParseAttrubutes = [&]() {
-      if (Machine == EM_ARM)
-        return ARMAttributeParser(&W).parse(Contents, support::little);
-      return RISCVAttributeParser(&W).parse(Contents, support::little);
-    };
-
-    if (Error E = ParseAttrubutes())
+    if (Error E = AttrParser->parse(Contents, Endianness))
       reportUniqueWarning("unable to dump attributes from the " +
                           describe(Sec) + ": " + toString(std::move(E)));
   }
Index: llvm/test/tools/llvm-readobj/ELF/RISCV/validate-attr-section.test
===================================================================
--- llvm/test/tools/llvm-readobj/ELF/RISCV/validate-attr-section.test
+++ llvm/test/tools/llvm-readobj/ELF/RISCV/validate-attr-section.test
@@ -1,9 +1,9 @@
 ## We only implement attribute section printing for little-endian encoding.
 
 # RUN: yaml2obj %s -o %t.o
-# RUN: llvm-readobj -A %t.o | FileCheck %s
+# RUN: llvm-readobj -A %t.o 2>&1 | FileCheck %s -DFILE=%t.o
 
-# CHECK: Attributes not implemented.
+# CHECK: warning: '[[FILE]]': attribute printing not implemented for big-endian RISC-V objects
 
 --- !ELF
 FileHeader:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D107968.366851.patch
Type: text/x-patch
Size: 3522 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210817/19051297/attachment.bin>