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))); } } _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
