[llvm] 3edef60 - [llvm-objdump] Change errors to warnings for symbol section name dumping
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 11 08:38:06 PDT 2023
Author: Fangrui Song
Date: 2023-07-11T08:38:02-07:00
New Revision: 3edef604cfbcdf9a658213de1340e4b6b63eb9d3
URL: https://github.com/llvm/llvm-project/commit/3edef604cfbcdf9a658213de1340e4b6b63eb9d3
DIFF: https://github.com/llvm/llvm-project/commit/3edef604cfbcdf9a658213de1340e4b6b63eb9d3.diff
LOG: [llvm-objdump] Change errors to warnings for symbol section name dumping
Port D69671 (llvm-readobj) to llvm-objdump. Add a class llvm::objdump::Dumper
and move some free functions into Dumper so that they can call
reportUniqueWarning.
Warnings seems preferable in these cases as the issue is localized and we can
continue dumping other information.
Differential Revision: https://reviews.llvm.org/D154754
Added:
llvm/test/tools/llvm-objdump/ELF/section-symbols.test
Modified:
llvm/test/tools/llvm-objdump/invalid-symbol-table-size.test
llvm/tools/llvm-objdump/MachODump.cpp
llvm/tools/llvm-objdump/llvm-objdump.cpp
llvm/tools/llvm-objdump/llvm-objdump.h
Removed:
################################################################################
diff --git a/llvm/test/tools/llvm-objdump/ELF/section-symbols.test b/llvm/test/tools/llvm-objdump/ELF/section-symbols.test
new file mode 100644
index 00000000000000..1dc416b38e8b79
--- /dev/null
+++ b/llvm/test/tools/llvm-objdump/ELF/section-symbols.test
@@ -0,0 +1,64 @@
+## ELF section symbols use the corresponding section names when printing
+## unnamed symbols. This test verifies this and also that appropriate things
+## are printed if the section is somehow invalid.
+
+# RUN: yaml2obj %s -o %t1
+# RUN: llvm-objdump -r --syms %t1 2>&1 | \
+# RUN: FileCheck %s -DFILE=%t1 --implicit-check-not=warning:
+
+# CHECK: SYMBOL TABLE:
+# CHECK-NEXT: 00000000 l d .foo 00000000 .foo
+# CHECK-NEXT: 00000000 l d .foo 00000000 .foo
+# CHECK-NEXT: warning: '[[FILE]]': invalid section index: 67
+# CHECK-NEXT: warning: '[[FILE]]': invalid section index: 68
+
+# CHECK: RELOCATION RECORDS FOR [.foo]:
+# CHECK-NEXT: OFFSET TYPE VALUE
+# CHECK-NEXT: 00000001 R_X86_64_NONE .foo
+# CHECK-NEXT: 00000002 R_X86_64_NONE .foo
+# CHECK-NEXT: 00000003 R_X86_64_NONE {{$}}
+# CHECK-NEXT: 00000004 R_X86_64_NONE {{$}}
+
+--- !ELF
+FileHeader:
+ Class: ELFCLASS32
+ Data: ELFDATA2LSB
+ Type: ET_REL
+ Machine: EM_X86_64
+Sections:
+ - Name: .foo
+ Type: SHT_PROGBITS
+ - Name: .rela.foo
+ Type: SHT_RELA
+ Link: .symtab
+ Info: .foo
+ Relocations:
+ - Offset: 0x1
+ Symbol: 1
+ Type: R_X86_64_NONE
+ - Offset: 0x2
+ Symbol: 2
+ Type: R_X86_64_NONE
+ - Offset: 0x3
+ Symbol: 3
+ Type: R_X86_64_NONE
+ - Offset: 0x4
+ Symbol: 4
+ Type: R_X86_64_NONE
+Symbols:
+## Case 1: a valid unnamed section symbol.
+ - Name: ""
+ Section: .foo
+ Type: STT_SECTION
+## Case 2: a valid named section symbol.
+ - Name: "symbol1"
+ Section: .foo
+ Type: STT_SECTION
+## Case 3: an unnamed section symbol with invalid index.
+ - Name: ""
+ Index: 0x43
+ Type: STT_SECTION
+## Case 4: a named section symbol with invalid index.
+ - Name: "symbol2"
+ Index: 0x44
+ Type: STT_SECTION
diff --git a/llvm/test/tools/llvm-objdump/invalid-symbol-table-size.test b/llvm/test/tools/llvm-objdump/invalid-symbol-table-size.test
index ed03121472e904..d3afda50c9f026 100644
--- a/llvm/test/tools/llvm-objdump/invalid-symbol-table-size.test
+++ b/llvm/test/tools/llvm-objdump/invalid-symbol-table-size.test
@@ -3,17 +3,17 @@
## a) Broken .symtab symbol table. Valid .dynsym symbol table.
# RUN: yaml2obj -DBITS=32 -DSIZE=33 -DDYNSIZE=32 %s -o %t.32-bit.o
-# RUN: not llvm-objdump --syms %t.32-bit.o 2>&1 | FileCheck -DSIZE=33 -DSYMSIZE=16 -DINDEX=2 %s
+# RUN: llvm-objdump --syms %t.32-bit.o 2>&1 | FileCheck -DSIZE=33 -DSYMSIZE=16 -DINDEX=2 %s
# RUN: yaml2obj -DBITS=64 -DSIZE=49 -DDYNSIZE=48 %s -o %t.64-bit.o
-# RUN: not llvm-objdump --syms %t.64-bit.o 2>&1 | FileCheck -DSIZE=49 -DSYMSIZE=24 -DINDEX=2 %s
+# RUN: llvm-objdump --syms %t.64-bit.o 2>&1 | FileCheck -DSIZE=49 -DSYMSIZE=24 -DINDEX=2 %s
## b) Broken .dynsym symbol table. Valid .symtab symbol table.
# RUN: yaml2obj -DBITS=32 -DSIZE=32 -DDYNSIZE=33 %s -o %t.32-bit.o
-# RUN: not llvm-objdump --dynamic-syms %t.32-bit.o 2>&1 | FileCheck -DSIZE=33 -DSYMSIZE=16 -DINDEX=3 %s
+# RUN: llvm-objdump --dynamic-syms %t.32-bit.o 2>&1 | FileCheck -DSIZE=33 -DSYMSIZE=16 -DINDEX=3 %s
# RUN: yaml2obj -DBITS=64 -DSIZE=48 -DDYNSIZE=49 %s -o %t.64-bit.o
-# RUN: not llvm-objdump --dynamic-syms %t.64-bit.o 2>&1 | FileCheck -DSIZE=49 -DSYMSIZE=24 -DINDEX=3 %s
+# RUN: llvm-objdump --dynamic-syms %t.64-bit.o 2>&1 | FileCheck -DSIZE=49 -DSYMSIZE=24 -DINDEX=3 %s
-# CHECK: error: {{.*}} section [index [[INDEX]]] has an invalid sh_size ([[SIZE]]) which is not a multiple of its sh_entsize ([[SYMSIZE]])
+# CHECK: warning: {{.*}} section [index [[INDEX]]] has an invalid sh_size ([[SIZE]]) which is not a multiple of its sh_entsize ([[SYMSIZE]])
--- !ELF
FileHeader:
diff --git a/llvm/tools/llvm-objdump/MachODump.cpp b/llvm/tools/llvm-objdump/MachODump.cpp
index d1bfcb709ac939..0e10403ef2d70a 100644
--- a/llvm/tools/llvm-objdump/MachODump.cpp
+++ b/llvm/tools/llvm-objdump/MachODump.cpp
@@ -2142,6 +2142,7 @@ static void printObjcMetaData(MachOObjectFile *O, bool verbose);
static void ProcessMachO(StringRef Name, MachOObjectFile *MachOOF,
StringRef ArchiveMemberName = StringRef(),
StringRef ArchitectureName = StringRef()) {
+ Dumper D(*MachOOF);
// If we are doing some processing here on the Mach-O file print the header
// info. And don't print it otherwise like in the case of printing the
// UniversalHeaders or ArchiveHeaders.
@@ -2227,7 +2228,7 @@ static void ProcessMachO(StringRef Name, MachOObjectFile *MachOOF,
if (DylibId)
PrintDylibs(MachOOF, true);
if (SymbolTable)
- printSymbolTable(*MachOOF, ArchiveName, ArchitectureName);
+ D.printSymbolTable(ArchiveName, ArchitectureName);
if (UnwindInfo)
printMachOUnwindInfo(MachOOF);
if (PrivateHeaders) {
diff --git a/llvm/tools/llvm-objdump/llvm-objdump.cpp b/llvm/tools/llvm-objdump/llvm-objdump.cpp
index bce76eea48f837..052b7f1472e61e 100644
--- a/llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ b/llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -247,6 +247,15 @@ static StringRef ToolName;
std::unique_ptr<BuildIDFetcher> BIDFetcher;
ExitOnError ExitOnErr;
+void Dumper::reportUniqueWarning(Error Err) {
+ reportUniqueWarning(toString(std::move(Err)));
+}
+
+void Dumper::reportUniqueWarning(const Twine &Msg) {
+ if (Warnings.insert(StringRef(Msg.str())).second)
+ reportWarning(Msg, O.getFileName());
+}
+
namespace {
struct FilterResult {
// True if the section should not be skipped.
@@ -2168,23 +2177,22 @@ static void disassembleObject(ObjectFile *Obj, bool InlineRelocs) {
SecondarySTI.get(), PIP, SP, InlineRelocs);
}
-void objdump::printRelocations(const ObjectFile *Obj) {
- StringRef Fmt = Obj->getBytesInAddress() > 4 ? "%016" PRIx64 :
- "%08" PRIx64;
+void Dumper::printRelocations() {
+ StringRef Fmt = O.getBytesInAddress() > 4 ? "%016" PRIx64 : "%08" PRIx64;
// Build a mapping from relocation target to a vector of relocation
// sections. Usually, there is an only one relocation section for
// each relocated section.
MapVector<SectionRef, std::vector<SectionRef>> SecToRelSec;
uint64_t Ndx;
- for (const SectionRef &Section : ToolSectionFilter(*Obj, &Ndx)) {
- if (Obj->isELF() && (ELFSectionRef(Section).getFlags() & ELF::SHF_ALLOC))
+ for (const SectionRef &Section : ToolSectionFilter(O, &Ndx)) {
+ if (O.isELF() && (ELFSectionRef(Section).getFlags() & ELF::SHF_ALLOC))
continue;
if (Section.relocation_begin() == Section.relocation_end())
continue;
Expected<section_iterator> SecOrErr = Section.getRelocatedSection();
if (!SecOrErr)
- reportError(Obj->getFileName(),
+ reportError(O.getFileName(),
"section (" + Twine(Ndx) +
"): unable to get a relocation target: " +
toString(SecOrErr.takeError()));
@@ -2192,9 +2200,9 @@ void objdump::printRelocations(const ObjectFile *Obj) {
}
for (std::pair<SectionRef, std::vector<SectionRef>> &P : SecToRelSec) {
- StringRef SecName = unwrapOrError(P.first.getName(), Obj->getFileName());
+ StringRef SecName = unwrapOrError(P.first.getName(), O.getFileName());
outs() << "\nRELOCATION RECORDS FOR [" << SecName << "]:\n";
- uint32_t OffsetPadding = (Obj->getBytesInAddress() > 4 ? 16 : 8);
+ uint32_t OffsetPadding = (O.getBytesInAddress() > 4 ? 16 : 8);
uint32_t TypePadding = 24;
outs() << left_justify("OFFSET", OffsetPadding) << " "
<< left_justify("TYPE", TypePadding) << " "
@@ -2209,7 +2217,7 @@ void objdump::printRelocations(const ObjectFile *Obj) {
continue;
Reloc.getTypeName(RelocName);
if (Error E = getRelocationValueString(Reloc, ValueStr))
- reportError(std::move(E), Obj->getFileName());
+ reportUniqueWarning(std::move(E));
outs() << format(Fmt.data(), Address) << " "
<< left_justify(RelocName, TypePadding) << " " << ValueStr
@@ -2374,8 +2382,8 @@ void objdump::printSectionContents(const ObjectFile *Obj) {
}
}
-void objdump::printSymbolTable(const ObjectFile &O, StringRef ArchiveName,
- StringRef ArchitectureName, bool DumpDynamic) {
+void Dumper::printSymbolTable(StringRef ArchiveName, StringRef ArchitectureName,
+ bool DumpDynamic) {
if (O.isCOFF() && !DumpDynamic) {
outs() << "\nSYMBOL TABLE:\n";
printCOFFSymbolTable(cast<const COFFObjectFile>(O));
@@ -2387,8 +2395,7 @@ void objdump::printSymbolTable(const ObjectFile &O, StringRef ArchiveName,
if (!DumpDynamic) {
outs() << "\nSYMBOL TABLE:\n";
for (auto I = O.symbol_begin(); I != O.symbol_end(); ++I)
- printSymbol(O, *I, {}, FileName, ArchiveName, ArchitectureName,
- DumpDynamic);
+ printSymbol(*I, {}, FileName, ArchiveName, ArchitectureName, DumpDynamic);
return;
}
@@ -2410,17 +2417,21 @@ void objdump::printSymbolTable(const ObjectFile &O, StringRef ArchiveName,
(void)!SymbolVersionsOrErr;
}
for (auto &Sym : Symbols)
- printSymbol(O, Sym, *SymbolVersionsOrErr, FileName, ArchiveName,
+ printSymbol(Sym, *SymbolVersionsOrErr, FileName, ArchiveName,
ArchitectureName, DumpDynamic);
}
-void objdump::printSymbol(const ObjectFile &O, const SymbolRef &Symbol,
- ArrayRef<VersionEntry> SymbolVersions,
- StringRef FileName, StringRef ArchiveName,
- StringRef ArchitectureName, bool DumpDynamic) {
+void Dumper::printSymbol(const SymbolRef &Symbol,
+ ArrayRef<VersionEntry> SymbolVersions,
+ StringRef FileName, StringRef ArchiveName,
+ StringRef ArchitectureName, bool DumpDynamic) {
const MachOObjectFile *MachO = dyn_cast<const MachOObjectFile>(&O);
- uint64_t Address = unwrapOrError(Symbol.getAddress(), FileName, ArchiveName,
- ArchitectureName);
+ Expected<uint64_t> AddrOrErr = Symbol.getAddress();
+ if (!AddrOrErr) {
+ reportUniqueWarning(AddrOrErr.takeError());
+ return;
+ }
+ uint64_t Address = *AddrOrErr;
if ((Address < StartAddress) || (Address > StopAddress))
return;
SymbolRef::Type Type =
@@ -2806,6 +2817,8 @@ static void checkForInvalidStartStopAddress(ObjectFile *Obj,
static void dumpObject(ObjectFile *O, const Archive *A = nullptr,
const Archive::Child *C = nullptr) {
+ Dumper D(*O);
+
// Avoid other output when using a raw option.
if (!RawClangAST) {
outs() << '\n';
@@ -2819,6 +2832,9 @@ static void dumpObject(ObjectFile *O, const Archive *A = nullptr,
if (HasStartAddressFlag || HasStopAddressFlag)
checkForInvalidStartStopAddress(O, StartAddress, StopAddress);
+ // TODO: Change print* free functions to Dumper member functions to utilitize
+ // stateful functions like reportUniqueWarning.
+
// Note: the order here matches GNU objdump for compatability.
StringRef ArchiveName = A ? A->getFileName() : "";
if (ArchiveHeaders && !MachOOpt && C)
@@ -2830,10 +2846,10 @@ static void dumpObject(ObjectFile *O, const Archive *A = nullptr,
if (SectionHeaders)
printSectionHeaders(*O);
if (SymbolTable)
- printSymbolTable(*O, ArchiveName);
+ D.printSymbolTable(ArchiveName);
if (DynamicSymbolTable)
- printSymbolTable(*O, ArchiveName, /*ArchitectureName=*/"",
- /*DumpDynamic=*/true);
+ D.printSymbolTable(ArchiveName, /*ArchitectureName=*/"",
+ /*DumpDynamic=*/true);
if (DwarfDumpType != DIDT_Null) {
std::unique_ptr<DIContext> DICtx = DWARFContext::create(*O);
// Dump the complete DWARF structure.
@@ -2842,7 +2858,7 @@ static void dumpObject(ObjectFile *O, const Archive *A = nullptr,
DICtx->dump(outs(), DumpOpts);
}
if (Relocations && !Disassemble)
- printRelocations(O);
+ D.printRelocations();
if (DynamicRelocations)
printDynamicRelocations(O);
if (SectionContents)
diff --git a/llvm/tools/llvm-objdump/llvm-objdump.h b/llvm/tools/llvm-objdump/llvm-objdump.h
index 7acd892ce8d877..ffc13611daf9a2 100644
--- a/llvm/tools/llvm-objdump/llvm-objdump.h
+++ b/llvm/tools/llvm-objdump/llvm-objdump.h
@@ -65,6 +65,26 @@ extern bool UnwindInfo;
extern StringSet<> FoundSectionSet;
+class Dumper {
+ const object::ObjectFile &O;
+ StringSet<> Warnings;
+
+public:
+ Dumper(const object::ObjectFile &O) : O(O) {}
+
+ void reportUniqueWarning(Error Err);
+ void reportUniqueWarning(const Twine &Msg);
+
+ void printSymbolTable(StringRef ArchiveName,
+ StringRef ArchitectureName = StringRef(),
+ bool DumpDynamic = false);
+ void printSymbol(const object::SymbolRef &Symbol,
+ ArrayRef<object::VersionEntry> SymbolVersions,
+ StringRef FileName, StringRef ArchiveName,
+ StringRef ArchitectureName, bool DumpDynamic);
+ void printRelocations();
+};
+
// Various helper functions.
/// Creates a SectionFilter with a standard predicate that conditionally skips
@@ -77,17 +97,9 @@ object::SectionFilter ToolSectionFilter(const llvm::object::ObjectFile &O,
uint64_t *Idx = nullptr);
bool isRelocAddressLess(object::RelocationRef A, object::RelocationRef B);
-void printRelocations(const object::ObjectFile *O);
void printDynamicRelocations(const object::ObjectFile *O);
void printSectionHeaders(object::ObjectFile &O);
void printSectionContents(const object::ObjectFile *O);
-void printSymbolTable(const object::ObjectFile &O, StringRef ArchiveName,
- StringRef ArchitectureName = StringRef(),
- bool DumpDynamic = false);
-void printSymbol(const object::ObjectFile &O, const object::SymbolRef &Symbol,
- ArrayRef<object::VersionEntry> SymbolVersions,
- StringRef FileName, StringRef ArchiveName,
- StringRef ArchitectureName, bool DumpDynamic);
[[noreturn]] void reportError(StringRef File, const Twine &Message);
[[noreturn]] void reportError(Error E, StringRef FileName,
StringRef ArchiveName = "",
More information about the llvm-commits
mailing list