[llvm] 137a25f - [llvm-readobj, libSupport] - Refine the implementation of the code that dumps build attributes.
Georgii Rymar via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 2 02:51:47 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-commits
mailing list