Skip to content

Commit 5afbc1c

Browse files
committed
Fix a crash in running llvm-objdump -t with an invalid Mach-O file already
in the test suite. While this is not really an interesting tool and option to run on a Mach-O file to show the symbol table in a generic libObject format it shouldn’t crash. The reason for the crash was in MachOObjectFile::getSymbolType() when it was calling MachOObjectFile::getSymbolSection() without checking its return value for the error case. What makes this fix require a fair bit of diffs is that the method getSymbolType() is in the class ObjectFile defined without an ErrorOr<> so I needed to add that all the sub classes.  And all of the uses needed to be updated and the return value needed to be checked for the error case. The MachOObjectFile version of getSymbolType() “can” get an error in trying to come up with the libObject’s internal SymbolRef::Type when the Mach-O symbol symbol type is an N_SECT type because the code is trying to select from the SymbolRef::ST_Data or SymbolRef::ST_Function values for the SymbolRef::Type. And it needs the Mach-O section to use isData() and isBSS to determine if it will return SymbolRef::ST_Data. One other possible fix I considered is to simply return SymbolRef::ST_Other when MachOObjectFile::getSymbolSection() returned an error. But since in the past when I did such changes that “ate an error in the libObject code” I was asked instead to push the error out of the libObject code I chose not to implement the fix this way. As currently written both the COFF and ELF versions of getSymbolType() can’t get an error. But if isReservedSectionNumber() wanted to check for the two known negative values rather than allowing all negative values or the code wanted to add the same check as in getSymbolAddress() to use getSection() and check for the error then these versions of getSymbolType() could return errors. At the end of the day the error printed now is the generic “Invalid data was encountered while parsing the file” for object_error::parse_failed. In the future when we thread Lang’s new TypedError for recoverable error handling though libObject this will improve. And where the added // Diagnostic(… comment is, it would be changed to produce and error message like “bad section index (42) for symbol at index 8” for this case. llvm-svn: 264187
1 parent 7876f18 commit 5afbc1c

File tree

15 files changed

+75
-24
lines changed

15 files changed

+75
-24
lines changed

llvm/include/llvm/Object/COFF.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -684,7 +684,7 @@ class COFFObjectFile : public ObjectFile {
684684
uint64_t getSymbolValueImpl(DataRefImpl Symb) const override;
685685
uint64_t getCommonSymbolSizeImpl(DataRefImpl Symb) const override;
686686
uint32_t getSymbolFlags(DataRefImpl Symb) const override;
687-
SymbolRef::Type getSymbolType(DataRefImpl Symb) const override;
687+
ErrorOr<SymbolRef::Type> getSymbolType(DataRefImpl Symb) const override;
688688
ErrorOr<section_iterator> getSymbolSection(DataRefImpl Symb) const override;
689689
void moveSectionNext(DataRefImpl &Sec) const override;
690690
std::error_code getSectionName(DataRefImpl Sec,

llvm/include/llvm/Object/ELFObjectFile.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ template <class ELFT> class ELFObjectFile : public ELFObjectFileBase {
205205
uint32_t getSymbolFlags(DataRefImpl Symb) const override;
206206
uint8_t getSymbolOther(DataRefImpl Symb) const override;
207207
uint8_t getSymbolELFType(DataRefImpl Symb) const override;
208-
SymbolRef::Type getSymbolType(DataRefImpl Symb) const override;
208+
ErrorOr<SymbolRef::Type> getSymbolType(DataRefImpl Symb) const override;
209209
ErrorOr<section_iterator> getSymbolSection(const Elf_Sym *Symb,
210210
const Elf_Shdr *SymTab) const;
211211
ErrorOr<section_iterator> getSymbolSection(DataRefImpl Symb) const override;
@@ -445,7 +445,8 @@ uint8_t ELFObjectFile<ELFT>::getSymbolELFType(DataRefImpl Symb) const {
445445
}
446446

447447
template <class ELFT>
448-
SymbolRef::Type ELFObjectFile<ELFT>::getSymbolType(DataRefImpl Symb) const {
448+
ErrorOr<SymbolRef::Type>
449+
ELFObjectFile<ELFT>::getSymbolType(DataRefImpl Symb) const {
449450
const Elf_Sym *ESym = getSymbol(Symb);
450451

451452
switch (ESym->getType()) {

llvm/include/llvm/Object/MachO.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ class MachOObjectFile : public ObjectFile {
208208
ErrorOr<uint64_t> getSymbolAddress(DataRefImpl Symb) const override;
209209
uint32_t getSymbolAlignment(DataRefImpl Symb) const override;
210210
uint64_t getCommonSymbolSizeImpl(DataRefImpl Symb) const override;
211-
SymbolRef::Type getSymbolType(DataRefImpl Symb) const override;
211+
ErrorOr<SymbolRef::Type> getSymbolType(DataRefImpl Symb) const override;
212212
uint32_t getSymbolFlags(DataRefImpl Symb) const override;
213213
ErrorOr<section_iterator> getSymbolSection(DataRefImpl Symb) const override;
214214
unsigned getSymbolSectionID(SymbolRef Symb) const;

llvm/include/llvm/Object/ObjectFile.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ class SymbolRef : public BasicSymbolRef {
143143
/// @brief Get the alignment of this symbol as the actual value (not log 2).
144144
uint32_t getAlignment() const;
145145
uint64_t getCommonSize() const;
146-
SymbolRef::Type getType() const;
146+
ErrorOr<SymbolRef::Type> getType() const;
147147

148148
/// @brief Get section this symbol is defined in reference to. Result is
149149
/// end_sections() if it is undefined or is an absolute symbol.
@@ -201,7 +201,7 @@ class ObjectFile : public SymbolicFile {
201201
virtual uint64_t getSymbolValueImpl(DataRefImpl Symb) const = 0;
202202
virtual uint32_t getSymbolAlignment(DataRefImpl Symb) const;
203203
virtual uint64_t getCommonSymbolSizeImpl(DataRefImpl Symb) const = 0;
204-
virtual SymbolRef::Type getSymbolType(DataRefImpl Symb) const = 0;
204+
virtual ErrorOr<SymbolRef::Type> getSymbolType(DataRefImpl Symb) const = 0;
205205
virtual ErrorOr<section_iterator>
206206
getSymbolSection(DataRefImpl Symb) const = 0;
207207

@@ -329,7 +329,7 @@ inline ErrorOr<section_iterator> SymbolRef::getSection() const {
329329
return getObject()->getSymbolSection(getRawDataRefImpl());
330330
}
331331

332-
inline SymbolRef::Type SymbolRef::getType() const {
332+
inline ErrorOr<SymbolRef::Type> SymbolRef::getType() const {
333333
return getObject()->getSymbolType(getRawDataRefImpl());
334334
}
335335

llvm/lib/DebugInfo/Symbolize/SymbolizableObjectFile.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,10 @@ std::error_code SymbolizableObjectFile::addSymbol(const SymbolRef &Symbol,
119119
uint64_t SymbolSize,
120120
DataExtractor *OpdExtractor,
121121
uint64_t OpdAddress) {
122-
SymbolRef::Type SymbolType = Symbol.getType();
122+
ErrorOr<SymbolRef::Type> SymbolTypeOrErr = Symbol.getType();
123+
if (auto EC = SymbolTypeOrErr.getError())
124+
return EC;
125+
SymbolRef::Type SymbolType = *SymbolTypeOrErr;
123126
if (SymbolType != SymbolRef::ST_Function && SymbolType != SymbolRef::ST_Data)
124127
return std::error_code();
125128
ErrorOr<uint64_t> SymbolAddressOrErr = Symbol.getAddress();

llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,9 @@ RuntimeDyldImpl::loadObjectImpl(const object::ObjectFile &Obj) {
169169
if (Flags & SymbolRef::SF_Common)
170170
CommonSymbols.push_back(*I);
171171
else {
172-
object::SymbolRef::Type SymType = I->getType();
172+
ErrorOr<object::SymbolRef::Type> SymTypeOrErr = I->getType();
173+
Check(SymTypeOrErr.getError());
174+
object::SymbolRef::Type SymType = *SymTypeOrErr;
173175

174176
// Get symbol name.
175177
ErrorOr<StringRef> NameOrErr = I->getName();

llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1190,7 +1190,10 @@ relocation_iterator RuntimeDyldELF::processRelocationRef(
11901190
RTDyldSymbolTable::const_iterator gsi = GlobalSymbolTable.end();
11911191
if (Symbol != Obj.symbol_end()) {
11921192
gsi = GlobalSymbolTable.find(TargetName.data());
1193-
SymType = Symbol->getType();
1193+
ErrorOr<SymbolRef::Type> SymTypeOrErr = Symbol->getType();
1194+
if (std::error_code EC = SymTypeOrErr.getError())
1195+
report_fatal_error(EC.message());
1196+
SymType = *SymTypeOrErr;
11941197
}
11951198
if (gsi != GlobalSymbolTable.end()) {
11961199
const auto &SymInfo = gsi->second;

llvm/lib/Object/COFFObjectFile.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ ErrorOr<uint64_t> COFFObjectFile::getSymbolAddress(DataRefImpl Ref) const {
179179
return Result;
180180
}
181181

182-
SymbolRef::Type COFFObjectFile::getSymbolType(DataRefImpl Ref) const {
182+
ErrorOr<SymbolRef::Type> COFFObjectFile::getSymbolType(DataRefImpl Ref) const {
183183
COFFSymbolRef Symb = getCOFFSymbol(Ref);
184184
int32_t SectionNumber = Symb.getSectionNumber();
185185

llvm/lib/Object/MachOObjectFile.cpp

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -443,7 +443,8 @@ uint64_t MachOObjectFile::getCommonSymbolSizeImpl(DataRefImpl DRI) const {
443443
return getNValue(DRI);
444444
}
445445

446-
SymbolRef::Type MachOObjectFile::getSymbolType(DataRefImpl Symb) const {
446+
ErrorOr<SymbolRef::Type>
447+
MachOObjectFile::getSymbolType(DataRefImpl Symb) const {
447448
MachO::nlist_base Entry = getSymbolTableEntryBase(this, Symb);
448449
uint8_t n_type = Entry.n_type;
449450

@@ -455,7 +456,10 @@ SymbolRef::Type MachOObjectFile::getSymbolType(DataRefImpl Symb) const {
455456
case MachO::N_UNDF :
456457
return SymbolRef::ST_Unknown;
457458
case MachO::N_SECT :
458-
section_iterator Sec = *getSymbolSection(Symb);
459+
ErrorOr<section_iterator> SecOrError = getSymbolSection(Symb);
460+
if (!SecOrError)
461+
return SecOrError.getError();
462+
section_iterator Sec = *SecOrError;
459463
if (Sec->isData() || Sec->isBSS())
460464
return SymbolRef::ST_Data;
461465
return SymbolRef::ST_Function;
@@ -511,8 +515,11 @@ MachOObjectFile::getSymbolSection(DataRefImpl Symb) const {
511515
return section_end();
512516
DataRefImpl DRI;
513517
DRI.d.a = index - 1;
514-
if (DRI.d.a >= Sections.size())
518+
if (DRI.d.a >= Sections.size()){
519+
// Diagnostic("bad section index (" + index + ") for symbol at index " +
520+
// SymbolIndex);
515521
return object_error::parse_failed;
522+
}
516523
return section_iterator(SectionRef(DRI, this));
517524
}
518525

llvm/test/Object/macho-invalid.test

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,9 @@ INVALID-SECTION-IDX-SYMBOL-SEC-m: 0000000100000000 (?,?) [referenced dynamically
5454
RUN: llvm-nm -pax %p/Inputs/macho-invalid-section-index-getSectionRawName 2>&1 \
5555
RUN: | FileCheck -check-prefix INVALID-SECTION-IDX-SYMBOL-SEC-pax %s
5656
INVALID-SECTION-IDX-SYMBOL-SEC-pax: 0000000100000000 0f 42 0010 00000065 __mh_execute_header
57+
RUN: not llvm-objdump -t %p/Inputs/macho-invalid-section-index-getSectionRawName 2>&1 \
58+
RUN: | FileCheck -check-prefix INVALID-SECTION-IDX-SYMBOL-SEC-objdump %s
59+
INVALID-SECTION-IDX-SYMBOL-SEC-objdump: Invalid data was encountered while parsing the file.
5760

5861
RUN: not llvm-objdump -private-headers %p/Inputs/macho-invalid-header 2>&1 | FileCheck -check-prefix INVALID-HEADER %s
5962
INVALID-HEADER: The file was not recognized as a valid object file.

0 commit comments

Comments
 (0)