Skip to content

Commit

Permalink
[llvm/Object] - Convert SectionRef::getName() to return Expected<>
Browse files Browse the repository at this point in the history
SectionRef::getName() returns std::error_code now.
Returning Expected<> instead has multiple benefits.

For example, it forces user to check the error returned.
Also Expected<> may keep a valuable string error message,
what is more useful than having a error code.
(Object\invalid.test was updated to show the new messages printed.)

This patch makes a change for all users to switch to Expected<> version.

Note: in a few places the error returned was ignored before my changes.
In such places I left them ignored. My intention was to convert the interface
used, and not to improve and/or the existent users in this patch.
(Though I think this is good idea for a follow-ups to revisit such places
and either remove consumeError calls or comment each of them to clarify why
it is OK to have them).

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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@368812 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
George Rimar committed Aug 14, 2019
1 parent 6b59294 commit 0d89498
Show file tree
Hide file tree
Showing 40 changed files with 423 additions and 240 deletions.
12 changes: 7 additions & 5 deletions include/llvm/Object/ELFObjectFile.h
Original file line number Diff line number Diff line change
Expand Up @@ -461,13 +461,15 @@ Expected<StringRef> ELFObjectFile<ELFT>::getSymbolName(DataRefImpl Sym) const {
if (!SymStrTabOrErr)
return SymStrTabOrErr.takeError();
Expected<StringRef> Name = ESym->getName(*SymStrTabOrErr);
if (Name && !Name->empty())
return Name;

// If the symbol name is empty use the section name.
if ((!Name || Name->empty()) && ESym->getType() == ELF::STT_SECTION) {
StringRef SecName;
Expected<section_iterator> Sec = getSymbolSection(Sym);
if (Sec && !(*Sec)->getName(SecName))
return SecName;
if (ESym->getType() == ELF::STT_SECTION) {
if (Expected<section_iterator> SecOrErr = getSymbolSection(Sym)) {
consumeError(Name.takeError());
return (*SecOrErr)->getName();
}
}
return Name;
}
Expand Down
10 changes: 3 additions & 7 deletions include/llvm/Object/ObjectFile.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ class SectionRef {

void moveNext();

std::error_code getName(StringRef &Result) const;
Expected<StringRef> getName() const;
uint64_t getAddress() const;
uint64_t getIndex() const;
uint64_t getSize() const;
Expand Down Expand Up @@ -434,12 +434,8 @@ inline void SectionRef::moveNext() {
return OwningObject->moveSectionNext(SectionPimpl);
}

inline std::error_code SectionRef::getName(StringRef &Result) const {
Expected<StringRef> NameOrErr = OwningObject->getSectionName(SectionPimpl);
if (!NameOrErr)
return errorToErrorCode(NameOrErr.takeError());
Result = *NameOrErr;
return std::error_code();
inline Expected<StringRef> SectionRef::getName() const {
return OwningObject->getSectionName(SectionPimpl);
}

inline uint64_t SectionRef::getAddress() const {
Expand Down
13 changes: 10 additions & 3 deletions lib/DebugInfo/DWARF/DWARFContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1506,7 +1506,11 @@ class DWARFObjInMemory final : public DWARFObject {
StringMap<unsigned> SectionAmountMap;
for (const SectionRef &Section : Obj.sections()) {
StringRef Name;
Section.getName(Name);
if (auto NameOrErr = Section.getName())
Name = *NameOrErr;
else
consumeError(NameOrErr.takeError());

++SectionAmountMap[Name];
SectionNames.push_back({ Name, true });

Expand Down Expand Up @@ -1571,12 +1575,15 @@ class DWARFObjInMemory final : public DWARFObject {
continue;

StringRef RelSecName;
StringRef RelSecData;
RelocatedSection->getName(RelSecName);
if (auto NameOrErr = RelocatedSection->getName())
RelSecName = *NameOrErr;
else
consumeError(NameOrErr.takeError());

// If the section we're relocating was relocated already by the JIT,
// then we used the relocated version above, so we do not need to process
// relocations for it now.
StringRef RelSecData;
if (L && L->getLoadedSectionContents(*RelocatedSection, RelSecData))
continue;

Expand Down
9 changes: 5 additions & 4 deletions lib/DebugInfo/Symbolize/SymbolizableObjectFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,11 @@ SymbolizableObjectFile::create(const object::ObjectFile *Obj,
// PowerPC64 ELF.
if (Obj->getArch() == Triple::ppc64) {
for (section_iterator Section : Obj->sections()) {
StringRef Name;
if (auto EC = Section->getName(Name))
return EC;
if (Name == ".opd") {
Expected<StringRef> NameOrErr = Section->getName();
if (!NameOrErr)
return errorToErrorCode(NameOrErr.takeError());

if (*NameOrErr == ".opd") {
Expected<StringRef> E = Section->getContents();
if (!E)
return errorToErrorCode(E.takeError());
Expand Down
6 changes: 5 additions & 1 deletion lib/DebugInfo/Symbolize/Symbolize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,11 @@ bool getGNUDebuglinkContents(const ObjectFile *Obj, std::string &DebugName,
return false;
for (const SectionRef &Section : Obj->sections()) {
StringRef Name;
Section.getName(Name);
if (Expected<StringRef> NameOrErr = Section.getName())
Name = *NameOrErr;
else
consumeError(NameOrErr.takeError());

Name = Name.substr(Name.find_first_not_of("._"));
if (Name == "gnu_debuglink") {
Expected<StringRef> ContentsOrErr = Section.getContents();
Expand Down
7 changes: 4 additions & 3 deletions lib/ExecutionEngine/JITLink/MachOAtomGraphBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,10 @@ Error MachOAtomGraphBuilder::parseSections() {
assert((SecRef.getAlignment() <= std::numeric_limits<uint32_t>::max()) &&
"Section alignment does not fit in 32 bits");

StringRef Name;
if (auto EC = SecRef.getName(Name))
return errorCodeToError(EC);
Expected<StringRef> NameOrErr = SecRef.getName();
if (!NameOrErr)
return NameOrErr.takeError();
StringRef Name = *NameOrErr;

unsigned SectionIndex = SecRef.getIndex() + 1;

Expand Down
14 changes: 8 additions & 6 deletions lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -535,9 +535,10 @@ Error RuntimeDyldImpl::computeTotalAllocSize(const ObjectFile &Obj,
bool IsCode = Section.isText();
bool IsReadOnly = isReadOnlyData(Section);

StringRef Name;
if (auto EC = Section.getName(Name))
return errorCodeToError(EC);
Expected<StringRef> NameOrErr = Section.getName();
if (!NameOrErr)
return NameOrErr.takeError();
StringRef Name = *NameOrErr;

uint64_t StubBufSize = computeSectionStubBufSize(Obj, Section);

Expand Down Expand Up @@ -777,9 +778,10 @@ RuntimeDyldImpl::emitSection(const ObjectFile &Obj,
// anyway, so we should guarantee that the alignment is always at least 1.
Alignment = std::max(1u, Alignment);

StringRef Name;
if (auto EC = Section.getName(Name))
return errorCodeToError(EC);
Expected<StringRef> NameOrErr = Section.getName();
if (!NameOrErr)
return NameOrErr.takeError();
StringRef Name = *NameOrErr;

StubBufSize = computeSectionStubBufSize(Obj, Section);

Expand Down
34 changes: 23 additions & 11 deletions lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,13 @@ createRTDyldELFObject(MemoryBufferRef Buffer, const ObjectFile &SourceObject,
// Iterate over all sections in the object.
auto SI = SourceObject.section_begin();
for (const auto &Sec : Obj->sections()) {
StringRef SectionName;
Sec.getName(SectionName);
if (SectionName != "") {
Expected<StringRef> NameOrErr = Sec.getName();
if (!NameOrErr) {
consumeError(NameOrErr.takeError());
continue;
}

if (*NameOrErr != "") {
DataRefImpl ShdrRef = Sec.getRawDataRefImpl();
Elf_Shdr *shdr = const_cast<Elf_Shdr *>(
reinterpret_cast<const Elf_Shdr *>(ShdrRef.p));
Expand Down Expand Up @@ -567,10 +571,11 @@ Error RuntimeDyldELF::findPPC64TOCSection(const ELFObjectFileBase &Obj,

// The TOC consists of sections .got, .toc, .tocbss, .plt in that
// order. The TOC starts where the first of these sections starts.
for (auto &Section: Obj.sections()) {
StringRef SectionName;
if (auto EC = Section.getName(SectionName))
return errorCodeToError(EC);
for (auto &Section : Obj.sections()) {
Expected<StringRef> NameOrErr = Section.getName();
if (!NameOrErr)
return NameOrErr.takeError();
StringRef SectionName = *NameOrErr;

if (SectionName == ".got"
|| SectionName == ".toc"
Expand Down Expand Up @@ -605,9 +610,10 @@ Error RuntimeDyldELF::findOPDEntrySection(const ELFObjectFileBase &Obj,
if (RelSecI == Obj.section_end())
continue;

StringRef RelSectionName;
if (auto EC = RelSecI->getName(RelSectionName))
return errorCodeToError(EC);
Expected<StringRef> NameOrErr = RelSecI->getName();
if (!NameOrErr)
return NameOrErr.takeError();
StringRef RelSectionName = *NameOrErr;

if (RelSectionName != ".opd")
continue;
Expand Down Expand Up @@ -1879,8 +1885,14 @@ Error RuntimeDyldELF::finalizeLoad(const ObjectFile &Obj,
ObjSectionToIDMap::iterator i, e;
for (i = SectionMap.begin(), e = SectionMap.end(); i != e; ++i) {
const SectionRef &Section = i->first;

StringRef Name;
Section.getName(Name);
Expected<StringRef> NameOrErr = Section.getName();
if (NameOrErr)
Name = *NameOrErr;
else
consumeError(NameOrErr.takeError());

if (Name == ".eh_frame") {
UnregisteredEHFrameSections.push_back(i->second);
break;
Expand Down
5 changes: 4 additions & 1 deletion lib/ExecutionEngine/RuntimeDyld/RuntimeDyldMachO.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,10 @@ RuntimeDyldMachOCRTPBase<Impl>::finalizeLoad(const ObjectFile &Obj,

for (const auto &Section : Obj.sections()) {
StringRef Name;
Section.getName(Name);
if (Expected<StringRef> NameOrErr = Section.getName())
Name = *NameOrErr;
else
consumeError(NameOrErr.takeError());

// Force emission of the __text, __eh_frame, and __gcc_except_tab sections
// if they're present. Otherwise call down to the impl to handle other
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,14 +284,14 @@ class RuntimeDyldCOFFX86_64 : public RuntimeDyldCOFF {
// Look for and record the EH frame section IDs.
for (const auto &SectionPair : SectionMap) {
const object::SectionRef &Section = SectionPair.first;
StringRef Name;
if (auto EC = Section.getName(Name))
return errorCodeToError(EC);
Expected<StringRef> NameOrErr = Section.getName();
if (!NameOrErr)
return NameOrErr.takeError();

// Note unwind info is stored in .pdata but often points to .xdata
// with an IMAGE_REL_AMD64_ADDR32NB relocation. Using a memory manager
// that keeps sections ordered in relation to __ImageBase is necessary.
if (Name == ".pdata")
if ((*NameOrErr) == ".pdata")
UnregisteredEHFrameSections.push_back(SectionPair.second);
}
return Error::success();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,10 @@ class RuntimeDyldMachOARM
Error finalizeSection(const ObjectFile &Obj, unsigned SectionID,
const SectionRef &Section) {
StringRef Name;
Section.getName(Name);
if (Expected<StringRef> NameOrErr = Section.getName())
Name = *NameOrErr;
else
consumeError(NameOrErr.takeError());

if (Name == "__nl_symbol_ptr")
return populateIndirectSymbolPointersSection(cast<MachOObjectFile>(Obj),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,10 @@ class RuntimeDyldMachOI386
Error finalizeSection(const ObjectFile &Obj, unsigned SectionID,
const SectionRef &Section) {
StringRef Name;
Section.getName(Name);
if (Expected<StringRef> NameOrErr = Section.getName())
Name = *NameOrErr;
else
consumeError(NameOrErr.takeError());

if (Name == "__jump_table")
return populateJumpTable(cast<MachOObjectFile>(Obj), Section, SectionID);
Expand Down
9 changes: 5 additions & 4 deletions lib/Object/COFFObjectFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -994,11 +994,12 @@ std::error_code COFFObjectFile::getSection(int32_t Index,
std::error_code COFFObjectFile::getSection(StringRef SectionName,
const coff_section *&Result) const {
Result = nullptr;
StringRef SecName;
for (const SectionRef &Section : sections()) {
if (std::error_code E = Section.getName(SecName))
return E;
if (SecName == SectionName) {
auto NameOrErr = Section.getName();
if (!NameOrErr)
return errorToErrorCode(NameOrErr.takeError());

if (*NameOrErr == SectionName) {
Result = getCOFFSection(Section);
return std::error_code();
}
Expand Down
13 changes: 9 additions & 4 deletions lib/Object/Decompressor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,15 @@ bool Decompressor::isGnuStyle(StringRef Name) {
}

bool Decompressor::isCompressed(const object::SectionRef &Section) {
StringRef Name;
if (Section.getName(Name))
return false;
return Section.isCompressed() || isGnuStyle(Name);
if (Section.isCompressed())
return true;

Expected<StringRef> SecNameOrErr = Section.getName();
if (SecNameOrErr)
return isGnuStyle(*SecNameOrErr);

consumeError(SecNameOrErr.takeError());
return false;
}

bool Decompressor::isCompressedELFSection(uint64_t Flags, StringRef Name) {
Expand Down
8 changes: 6 additions & 2 deletions lib/Object/ELFObjectFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -392,9 +392,13 @@ ELFObjectFileBase::getPltAddresses() const {
return {};
Optional<SectionRef> Plt = None, RelaPlt = None, GotPlt = None;
for (const SectionRef &Section : sections()) {
StringRef Name;
if (Section.getName(Name))
Expected<StringRef> NameOrErr = Section.getName();
if (!NameOrErr) {
consumeError(NameOrErr.takeError());
continue;
}
StringRef Name = *NameOrErr;

if (Name == ".plt")
Plt = Section;
else if (Name == ".rela.plt" || Name == ".rel.plt")
Expand Down
15 changes: 9 additions & 6 deletions lib/Object/MachOObjectFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1986,13 +1986,12 @@ Expected<SectionRef> MachOObjectFile::getSection(unsigned SectionIndex) const {
}

Expected<SectionRef> MachOObjectFile::getSection(StringRef SectionName) const {
StringRef SecName;
for (const SectionRef &Section : sections()) {
if (std::error_code E = Section.getName(SecName))
return errorCodeToError(E);
if (SecName == SectionName) {
auto NameOrErr = Section.getName();
if (!NameOrErr)
return NameOrErr.takeError();
if (*NameOrErr == SectionName)
return Section;
}
}
return errorCodeToError(object_error::parse_failed);
}
Expand Down Expand Up @@ -3995,7 +3994,11 @@ BindRebaseSegInfo::BindRebaseSegInfo(const object::MachOObjectFile *Obj) {
uint64_t CurSegAddress;
for (const SectionRef &Section : Obj->sections()) {
SectionInfo Info;
Section.getName(Info.SectionName);
Expected<StringRef> NameOrErr = Section.getName();
if (!NameOrErr)
consumeError(NameOrErr.takeError());
else
Info.SectionName = *NameOrErr;
Info.Address = Section.getAddress();
Info.Size = Section.getSize();
Info.SegmentName =
Expand Down
8 changes: 4 additions & 4 deletions lib/Object/Object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -251,10 +251,10 @@ void LLVMMoveToNextSymbol(LLVMSymbolIteratorRef SI) {

// SectionRef accessors
const char *LLVMGetSectionName(LLVMSectionIteratorRef SI) {
StringRef ret;
if (std::error_code ec = (*unwrap(SI))->getName(ret))
report_fatal_error(ec.message());
return ret.data();
auto NameOrErr = (*unwrap(SI))->getName();
if (!NameOrErr)
report_fatal_error(NameOrErr.takeError());
return NameOrErr->data();
}

uint64_t LLVMGetSectionSize(LLVMSectionIteratorRef SI) {
Expand Down
8 changes: 4 additions & 4 deletions lib/ProfileData/Coverage/CoverageMappingReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -666,11 +666,11 @@ static Expected<SectionRef> lookupSection(ObjectFile &OF, StringRef Name) {
};
Name = stripSuffix(Name);

StringRef FoundName;
for (const auto &Section : OF.sections()) {
if (auto EC = Section.getName(FoundName))
return errorCodeToError(EC);
if (stripSuffix(FoundName) == Name)
Expected<StringRef> NameOrErr = Section.getName();
if (!NameOrErr)
return NameOrErr.takeError();
if (stripSuffix(*NameOrErr) == Name)
return Section;
}
return make_error<CoverageMapError>(coveragemap_error::no_data_found);
Expand Down
9 changes: 5 additions & 4 deletions lib/XRay/InstrumentationMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,11 @@ loadObj(StringRef Filename, object::OwningBinary<object::ObjectFile> &ObjFile,
StringRef Contents = "";
const auto &Sections = ObjFile.getBinary()->sections();
auto I = llvm::find_if(Sections, [&](object::SectionRef Section) {
StringRef Name = "";
if (Section.getName(Name))
return false;
return Name == "xray_instr_map";
Expected<StringRef> NameOrErr = Section.getName();
if (NameOrErr)
return *NameOrErr == "xray_instr_map";
consumeError(NameOrErr.takeError());
return false;
});

if (I == Sections.end())
Expand Down
Loading

0 comments on commit 0d89498

Please sign in to comment.