[llvm-branch-commits] [llvm] 137a25f - [llvm-readobj, libSupport] - Refine the implementation of the code that dumps build attributes.

Georgii Rymar via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Wed Dec 2 02:55:43 PST 2020


Author: Georgii Rymar
Date: 2020-12-02T13:51:32+03:00
New Revision: 137a25f04a515e5ea8f24c897e34b4cd236239a8

URL: https://github.com/llvm/llvm-project/commit/137a25f04a515e5ea8f24c897e34b4cd236239a8
DIFF: https://github.com/llvm/llvm-project/commit/137a25f04a515e5ea8f24c897e34b4cd236239a8.diff

LOG: [llvm-readobj, libSupport] - Refine the implementation of the code that dumps build attributes.

This implementation of `ELFDumper<ELFT>::printAttributes()` in llvm-readobj has issues:
1) It crashes when the content of the attribute section is empty.
2) It uses `unwrapOrError` and `reportWarning` calls, though
   ideally we want to use `reportUniqueWarning`.
3) It contains a TODO about redundant format version check.

`lib/Support/ELFAttributeParser.cpp` uses a hardcoded constant instead of the named constant.

This patch fixes all these issues.

Differential revision: https://reviews.llvm.org/D92318

Added: 
    

Modified: 
    llvm/lib/Support/ELFAttributeParser.cpp
    llvm/test/tools/llvm-readobj/ELF/RISCV/attributes-invalid.test
    llvm/tools/llvm-readobj/ELFDumper.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Support/ELFAttributeParser.cpp b/llvm/lib/Support/ELFAttributeParser.cpp
index df955cdf5d30..2a30794bc1e9 100644
--- a/llvm/lib/Support/ELFAttributeParser.cpp
+++ b/llvm/lib/Support/ELFAttributeParser.cpp
@@ -200,7 +200,7 @@ Error ELFAttributeParser::parse(ArrayRef<uint8_t> section,
 
   // Unrecognized format-version.
   uint8_t formatVersion = de.getU8(cursor);
-  if (formatVersion != 'A')
+  if (formatVersion != ELFAttrs::Format_Version)
     return createStringError(errc::invalid_argument,
                              "unrecognized format-version: 0x" +
                                  utohexstr(formatVersion));

diff  --git a/llvm/test/tools/llvm-readobj/ELF/RISCV/attributes-invalid.test b/llvm/test/tools/llvm-readobj/ELF/RISCV/attributes-invalid.test
index 547ed93bcd10..882bbb53b577 100644
--- a/llvm/test/tools/llvm-readobj/ELF/RISCV/attributes-invalid.test
+++ b/llvm/test/tools/llvm-readobj/ELF/RISCV/attributes-invalid.test
@@ -5,11 +5,16 @@
 ## here we use 'B' (42).
 
 # RUN: yaml2obj %s -D BITS=32 -DCONTENT=42 -o %t1.32.o
-# RUN: llvm-readobj -A %t1.32.o 2>&1 | FileCheck -DFILE=%t1 %s --check-prefix=ERR-FORMAT
+# RUN: llvm-readobj -A %t1.32.o 2>&1 | \
+# RUN:   FileCheck -DFILE=%t1 %s --implicit-check-not=warning: --check-prefix=ERR-FORMAT
 # RUN: yaml2obj %s -D BITS=64 -DCONTENT=42 -o %t1.64.o
-# RUN: llvm-readobj -A %t1.64.o 2>&1 | FileCheck -DFILE=%t1 %s --check-prefix=ERR-FORMAT
+# RUN: llvm-readobj -A %t1.64.o 2>&1 | \
+# RUN:   FileCheck -DFILE=%t1 %s --implicit-check-not=warning: --check-prefix=ERR-FORMAT
 
-# ERR-FORMAT: warning: '[[FILE]].{{32|64}}.o': unrecognised FormatVersion: 0x42
+# ERR-FORMAT:      BuildAttributes {
+# ERR-FORMAT-NEXT:   FormatVersion: 0x42
+# ERR-FORMAT-NEXT: warning: '[[FILE]].{{32|64}}.o': unable to dump attributes from the SHT_RISCV_ATTRIBUTES section with index 1: unrecognized format-version: 0x42
+# ERR-FORMAT-NEXT: }
 
 --- !ELF
 FileHeader:
@@ -18,15 +23,54 @@ FileHeader:
   Type:    ET_REL
   Machine: EM_RISCV
 Sections:
-  - Name:    .riscv.attributes
-    Type:    SHT_RISCV_ATTRIBUTES
-    Content: [[CONTENT]]
+  - Name:     .riscv.attributes
+    Type:     SHT_RISCV_ATTRIBUTES
+    Content:  [[CONTENT]]
+    ShOffset: [[SHOFFSET=<none>]]
 
 ## Check we report a warning when we are unable to parse the attribute section data.
+## FIXME: this test case documents that we don't close the "Section X" clause of
+##        the output properly when a warning is reported by the attributes parser.
 
 # RUN: yaml2obj %s -D BITS=32 -DCONTENT=4100000000 -o %t2.32.o
-# RUN: llvm-readobj -A %t2.32.o 2>&1 | FileCheck -DFILE=%t2 %s --check-prefix=ERR-LENGTH
+# RUN: llvm-readobj -A %t2.32.o 2>&1 | \
+# RUN:   FileCheck -DFILE=%t2 %s --implicit-check-not=warning: --check-prefix=ERR-LENGTH
 # RUN: yaml2obj %s -D BITS=64 -DCONTENT=4100000000 -o %t2.64.o
-# RUN: llvm-readobj -A %t2.64.o 2>&1 | FileCheck -DFILE=%t2 %s --check-prefix=ERR-LENGTH
+# RUN: llvm-readobj -A %t2.64.o 2>&1 | \
+# RUN:   FileCheck -DFILE=%t2 %s --implicit-check-not=warning: --check-prefix=ERR-LENGTH
 
-# ERR-LENGTH: warning: '[[FILE]].{{32|64}}.o': invalid section length 0 at offset 0x1
+# ERR-LENGTH:      BuildAttributes {
+# ERR-LENGTH-NEXT:   FormatVersion: 0x41
+# ERR-LENGTH-NEXT:   Section 1 {
+# ERR-LENGTH-NEXT: warning: '[[FILE]].{{32|64}}.o': unable to dump attributes from the SHT_RISCV_ATTRIBUTES section with index 1: invalid section length 0 at offset 0x1
+# ERR-LENGTH-NEXT:   }
+# ERR-LENGTH-NOT: {{.}}
+
+## Check that we don't report a warning when the SHT_RISCV_ATTRIBUTES section contains the
+## valid format version byte and no other data.
+
+# RUN: yaml2obj %s -DCONTENT=41 -o %t3.o
+# RUN: llvm-readobj -A %t3.o 2>&1 | \
+# RUN:   FileCheck %s --implicit-check-not=warning: --check-prefix=NO-CONTENT
+
+# NO-CONTENT:      BuildAttributes {
+# NO-CONTENT-NEXT:   FormatVersion: 0x41
+# NO-CONTENT-NEXT: }
+
+## Check we report a warning when we are unable to read the content of the SHT_RISCV_ATTRIBUTES section.
+
+# RUN: yaml2obj %s -DCONTENT="''" -DSHOFFSET=0xffffffff -o %t4.o
+# RUN: llvm-readobj -A %t4.o 2>&1 | \
+# RUN:   FileCheck %s -DFILE=%t4.o --implicit-check-not=warning: --check-prefix=BROKEN-CONTENT
+
+# BROKEN-CONTENT:      BuildAttributes {
+# BROKEN-CONTENT-NEXT: warning: '[[FILE]]': unable to read the content of the SHT_RISCV_ATTRIBUTES section with index 1: section [index 1] has a sh_offset (0xffffffff) + sh_size (0x0) that is greater than the file size (0x168)
+# BROKEN-CONTENT-NEXT: }
+
+# RUN: yaml2obj %s -DCONTENT="''" -o %t5.o
+# RUN: llvm-readobj -A %t5.o 2>&1 | \
+# RUN:   FileCheck %s -DFILE=%t5.o --implicit-check-not=warning: --check-prefix=EMPTY-CONTENT
+
+# EMPTY-CONTENT:      BuildAttributes {
+# EMPTY-CONTENT-NEXT: warning: '[[FILE]]': the SHT_RISCV_ATTRIBUTES section with index 1 is empty
+# EMPTY-CONTENT-NEXT: }

diff  --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index d5313d6c182a..646616f6649d 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -2896,26 +2896,31 @@ template <class ELFT> void ELFDumper<ELFT>::printAttributes() {
         Sec.sh_type != ELF::SHT_RISCV_ATTRIBUTES)
       continue;
 
-    ArrayRef<uint8_t> Contents =
-        unwrapOrError(ObjF.getFileName(), Obj.getSectionContents(Sec));
-    if (Contents[0] != ELFAttrs::Format_Version) {
-      reportWarning(createError(Twine("unrecognised FormatVersion: 0x") +
-                                Twine::utohexstr(Contents[0])),
-                    ObjF.getFileName());
+    ArrayRef<uint8_t> Contents;
+    if (Expected<ArrayRef<uint8_t>> ContentOrErr =
+            Obj.getSectionContents(Sec)) {
+      Contents = *ContentOrErr;
+      if (Contents.empty()) {
+        reportUniqueWarning("the " + describe(Sec) + " is empty");
+        continue;
+      }
+    } else {
+      reportUniqueWarning("unable to read the content of the " + describe(Sec) +
+                          ": " + toString(ContentOrErr.takeError()));
       continue;
     }
+
     W.printHex("FormatVersion", Contents[0]);
-    if (Contents.size() == 1)
-      continue;
 
-    // TODO: Delete the redundant FormatVersion check above.
-    if (Machine == EM_ARM) {
-      if (Error E = ARMAttributeParser(&W).parse(Contents, support::little))
-        reportWarning(std::move(E), ObjF.getFileName());
-    } else if (Machine == EM_RISCV) {
-      if (Error E = RISCVAttributeParser(&W).parse(Contents, support::little))
-        reportWarning(std::move(E), ObjF.getFileName());
-    }
+    auto ParseAttrubutes = [&]() {
+      if (Machine == EM_ARM)
+        return ARMAttributeParser(&W).parse(Contents, support::little);
+      return RISCVAttributeParser(&W).parse(Contents, support::little);
+    };
+
+    if (Error E = ParseAttrubutes())
+      reportUniqueWarning("unable to dump attributes from the " +
+                          describe(Sec) + ": " + toString(std::move(E)));
   }
 }
 


        


More information about the llvm-branch-commits mailing list