Skip to content

Commit 43ae87c

Browse files
Steven6798Steven6798
committed
[ELD Improve error handling across readers and plugin APIs
Replace FIXME paths that silently returned false, null, or zero with structured diagnostics and Expected error propagation. Validate compressed sections, unsupported ELF sections, DWARF creation, memory allocation, checksum access, and reproduction-file creation failures Fix: #1499 Signed-off-by: Steven Ramirez Rosa <ramirezr@qti.qualcomm.com>
1 parent 5416118 commit 43ae87c

11 files changed

Lines changed: 84 additions & 63 deletions

File tree

‎include/eld/Diagnostics/DiagCommonKinds.inc‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,9 @@ DIAG(linker_crash_use_reproduce, DiagnosticEngine::Fatal,
124124
DIAG(plugin_crash, DiagnosticEngine::Fatal,
125125
"!!!PLUGIN CRASHED!!!\nUser specified plugin %0 caused segmentation fault")
126126
DIAG(unable_to_create_temporary_file, DiagnosticEngine::Fatal,
127-
"Unable to create temporary file %0")
127+
"Unable to create temporary file %0: %1")
128+
DIAG(fatal_unable_to_open_file, DiagnosticEngine::Fatal,
129+
"Unable to open file %0: %1")
128130
DIAG(unable_to_write_json_file, DiagnosticEngine::Error,
129131
"Unable to write JSON file %0 due to error %1")
130132
DIAG(unsupported_linker_flavor, DiagnosticEngine::Error,
@@ -177,6 +179,8 @@ DIAG(verbose_using_basepath_for_mapfiles, DiagnosticEngine::Verbose,
177179
"Using '%0' for computing relative paths in map files")
178180
DIAG(fatal_invalid_diag_id, DiagnosticEngine::Fatal,
179181
"Invalid diagnostic ID: %0")
182+
DIAG(fatal_invalid_diag_format, DiagnosticEngine::Fatal,
183+
"Invalid format for diagnostic %0: %1")
180184
DIAG(fatal_missing_diag_args, DiagnosticEngine::Fatal,
181185
"Missing argument %0 when reporting diagnostic '%1'")
182186
DIAG(error_invalid_input_format, DiagnosticEngine::Error,

‎include/eld/Diagnostics/DiagReaders.inc‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,11 @@ DIAG(section_does_not_have_proper_permissions, DiagnosticEngine::Warning,
3333
DIAG(corrupt_input_file, DiagnosticEngine::Fatal, "%0 : %1")
3434
DIAG(error_group_flags, DiagnosticEngine::Error,
3535
"Unsupported SHT_GROUP flags in `%0' (%1) in file %2: 0x%3")
36+
DIAG(fatal_group_signature_symbol_not_found, DiagnosticEngine::Fatal,
37+
"cannot find signature symbol for group section `%0' in input file `%1'")
38+
DIAG(fatal_zlib_unavailable, DiagnosticEngine::Fatal,
39+
"cannot decompress section `%0' in input file `%1': zlib support is "
40+
"unavailable")
3641
DIAG(discarding_section, DiagnosticEngine::Verbose,
3742
"Discard section %0 in file %1")
3843
DIAG(eh_frame_read_error, DiagnosticEngine::Error,

‎lib/Diagnostics/Diagnostic.cpp‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -116,11 +116,9 @@ eld::Expected<void> Diagnostic::format(const char *PBegin, const char *PEnd,
116116
}
117117
}
118118
if (!isdigit(*CurChar)) {
119-
// FIXME: This should be returned using eld::Expected as well!
120-
llvm::report_fatal_error(llvm::Twine("In diagnostic: ") +
121-
llvm::Twine(getID()) + llvm::Twine(": ") +
122-
llvm::Twine(PBegin) +
123-
llvm::Twine("\nNo given arugment number:\n"));
119+
return std::make_unique<plugin::DiagnosticEntry>(plugin::DiagnosticEntry(
120+
Diag::fatal_invalid_diag_format,
121+
{std::to_string(getID()), std::string(PBegin, PEnd)}));
124122
}
125123

126124
unsigned int ArgNo = *CurChar - '0';

‎lib/LinkerWrapper/GnuLdDriver.cpp‎

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1213,7 +1213,8 @@ bool GnuLdDriver::processOptions(llvm::opt::InputArgList &Args) {
12131213
std::error_code EC =
12141214
llvm::sys::fs::createTemporaryFile("reproduce", "tar", outputPath);
12151215
if (EC) {
1216-
Config.raise(Diag::unable_to_create_temporary_file) << "reproduce.tar";
1216+
Config.raise(Diag::unable_to_create_temporary_file)
1217+
<< "reproduce.tar" << EC.message();
12171218
return false;
12181219
}
12191220
Config.options().setTarFile(outputPath.str().str());
@@ -2030,10 +2031,30 @@ void GnuLdDriver::defaultSignalHandler(void *cookie) {
20302031
std::error_code EC =
20312032
llvm::sys::fs::createTemporaryFile("reproduce", "sh", outputPath);
20322033

2034+
if (EC) {
2035+
if (DiagEngineUsable)
2036+
DiagEngine->raise(Diag::unable_to_create_temporary_file)
2037+
<< "reproduce.sh" << EC.message();
2038+
else
2039+
llvm::errs() << "Fatal: Unable to create temporary file reproduce.sh: "
2040+
<< EC.message() << "\n";
2041+
return;
2042+
}
2043+
20332044
std::error_code error;
20342045
auto file = std::make_unique<llvm::raw_fd_ostream>(outputPath.str(), error,
20352046
llvm::sys::fs::OF_None);
20362047

2048+
if (error) {
2049+
if (DiagEngineUsable)
2050+
DiagEngine->raise(Diag::fatal_unable_to_open_file)
2051+
<< outputPath.str() << error.message();
2052+
else
2053+
llvm::errs() << "Fatal: Unable to open temporary file "
2054+
<< outputPath.str() << ": " << error.message() << "\n";
2055+
return;
2056+
}
2057+
20372058
bool pluginCrash = false;
20382059
for (eld::Plugin *P : ThisModule->getScript().getPlugins()) {
20392060
if (P->isRunning()) {
@@ -2053,15 +2074,6 @@ void GnuLdDriver::defaultSignalHandler(void *cookie) {
20532074
llvm::errs() << "Fatal: !!!UNEXPECTED LINKER BEHAVIOR!!!\n";
20542075
}
20552076

2056-
// FIXME: EC should be checked before using outputPath variable.
2057-
if (EC || error) {
2058-
if (DiagEngineUsable)
2059-
DiagEngine->raise(Diag::linker_crash_use_reproduce) << "--reproduce";
2060-
else
2061-
llvm::errs() << "Fatal: Please rerun link with --reproduce and contact "
2062-
"support\n";
2063-
return;
2064-
}
20652077
*file << commandLine;
20662078
if (DiagEngineUsable)
20672079
DiagEngine->raise(Diag::linker_crash_use_reproduce) << outputPath.str();

‎lib/LinkerWrapper/LinkerWrapper.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -800,8 +800,10 @@ eld::Expected<void>
800800
LinkerWrapper::deleteDWARFInfoForInputFile(plugin::InputFile &F) const {
801801
eld::InputFile *IF = F.getInputFile();
802802
eld::ELFObjectFile *EObj = llvm::dyn_cast<eld::ELFObjectFile>(IF);
803-
// FIXME: Maybe return an error here?
804-
if (!EObj || !EObj->hasDWARFContext())
803+
if (!EObj)
804+
return std::make_unique<DiagnosticEntry>(
805+
DiagnosticEntry(Diag::error_invalid_input_file, {F.getFileName()}));
806+
if (!EObj->hasDWARFContext())
805807
return {};
806808
EObj->deleteDWARFContext();
807809
return {};
@@ -832,7 +834,6 @@ bool LinkerWrapper::is64Bits() const {
832834
}
833835

834836
char *LinkerWrapper::getUninitBuffer(size_t S) const {
835-
// FIXME: Raise plugin_request_memory diagnostic!
836837
return m_Module.getUninitBuffer(S);
837838
}
838839

‎lib/Readers/DynamicELFReader.cpp‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,11 @@
88
#include "eld/Core/Module.h"
99
#include "eld/Diagnostics/MsgHandler.h"
1010
#include "eld/Input/ELFDynObjectFile.h"
11+
#include "eld/PluginAPI/DiagnosticEntry.h"
1112
#include "eld/Readers/ELFSection.h"
1213
#include "eld/SymbolResolver/LDSymbol.h"
1314
#include "eld/SymbolResolver/ResolveInfo.h"
15+
#include <string>
1416
#ifdef ELD_ENABLE_SYMBOL_VERSIONING
1517
#include "llvm/BinaryFormat/ELF.h"
1618
#include "eld/Diagnostics/DiagnosticEngine.h"
@@ -166,9 +168,10 @@ eld::Expected<ELFSection *> DynamicELFReader<ELFT>::createSection(
166168
// Setup all section properties.
167169
LinkerSectionKind kind = this->classifySectionKind(rawSectHdr, sectionName);
168170

169-
// FIXME: Emit some diagnostic here.
170171
if (kind == LinkerSectionKind::Error)
171-
return static_cast<ELFSection *>(nullptr);
172+
return std::make_unique<plugin::DiagnosticEntry>(plugin::DiagnosticEntry(
173+
Diag::err_unsupported_section,
174+
{sectionName, std::to_string(rawSectHdr.sh_type)}));
172175

173176
ELFSection *section =
174177
this->m_Module.getScript().sectionMap().createELFSection(

‎lib/Readers/ELFDynObjParser.cpp‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,17 @@ eld::Expected<bool> ELFDynObjParser::parseFile(InputFile &inputFile) {
3939

4040
eld::Expected<bool> expCheckFlags = ELFReader->checkFlags();
4141

42-
// FIXME: Linker should give some error if checkFlags return false.
43-
if (expCheckFlags.has_value() && expCheckFlags.value()) {
44-
LayoutInfo *layoutInfo = m_Module.getLayoutInfo();
45-
if (layoutInfo) {
46-
std::string flagStr = ELFReader->getFlagString();
47-
if (!flagStr.empty()) {
48-
std::string flag = "[" + std::string(flagStr) + "]";
49-
layoutInfo->recordInputActions(LayoutInfo::Load, inputFile.getInput(),
50-
flag);
51-
}
42+
ELDEXP_RETURN_DIAGENTRY_IF_ERROR(expCheckFlags);
43+
if (!expCheckFlags.value())
44+
return false;
45+
46+
LayoutInfo *layoutInfo = m_Module.getLayoutInfo();
47+
if (layoutInfo) {
48+
std::string flagStr = ELFReader->getFlagString();
49+
if (!flagStr.empty()) {
50+
std::string flag = "[" + std::string(flagStr) + "]";
51+
layoutInfo->recordInputActions(LayoutInfo::Load, inputFile.getInput(),
52+
flag);
5253
}
5354
}
5455

‎lib/Readers/ELFReader.cpp‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -188,8 +188,6 @@ ELFReader<ELFT>::classifySectionKind(Elf_Shdr rawSectHdr,
188188
(rawSectHdr.sh_type >= llvm::ELF::SHT_LOUSER &&
189189
rawSectHdr.sh_type <= llvm::ELF::SHT_HIUSER))
190190
return LinkerSectionKind::Target;
191-
config.raise(Diag::err_unsupported_section)
192-
<< sectionName << rawSectHdr.sh_type;
193191
return LinkerSectionKind::Error;
194192
}
195193
}
@@ -597,16 +595,16 @@ eld::Expected<bool> ELFReader<ELFT>::readCompressedSection(ELFSection *S) {
597595
return true;
598596
llvm::StringRef rawData = S->getContents();
599597

600-
// FIXME: Return error here.
601598
if (rawData.size() < sizeof(typename ELFReader<ELFT>::Elf_Chdr))
602-
return false;
599+
return std::make_unique<plugin::DiagnosticEntry>(plugin::DiagnosticEntry(
600+
Diag::err_cannot_read_section, {S->name().str()}));
603601

604602
const typename ELFReader<ELFT>::Elf_Chdr *hdr =
605603
reinterpret_cast<const typename ELFReader<ELFT>::Elf_Chdr *>(
606604
rawData.data());
607-
// FIXME: Return error here.
608605
if (hdr->ch_type != llvm::ELF::ELFCOMPRESS_ZLIB)
609-
return false;
606+
return std::make_unique<plugin::DiagnosticEntry>(plugin::DiagnosticEntry(
607+
Diag::err_cannot_read_section, {S->name().str()}));
610608

611609
size_t uncompressedSize = hdr->ch_size;
612610
typename ELFReader<ELFT>::uintX_t alignment =

‎lib/Readers/ELFRelocObjParser.cpp‎

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -148,22 +148,18 @@ eld::Expected<bool> ELFRelocObjParser::readSections(ELFReaderBase &ELFReader) {
148148
eld::Expected<bool> expReadCompressedSection =
149149
ELFReader.readCompressedSection(S);
150150
ELDEXP_RETURN_DIAGENTRY_IF_ERROR(expReadCompressedSection);
151-
if (!expReadCompressedSection.value())
152-
return std::make_unique<plugin::DiagnosticEntry>(
153-
plugin::DiagnosticEntry(Diag::err_cannot_read_section,
154-
{S->name().str()}));
155151
} else if (!backend.readSection(*inputFile, S)) {
156152
return std::make_unique<plugin::DiagnosticEntry>(
157153
plugin::DiagnosticEntry(Diag::err_cannot_read_section,
158154
{S->name().str()}));
159155
}
160156
} break;
161157
case LinkerSectionKind::MergeStr: {
162-
if (S->isCompressed())
163-
if (!ELFReader.readCompressedSection(S))
164-
return std::make_unique<plugin::DiagnosticEntry>(
165-
plugin::DiagnosticEntry(Diag::err_cannot_read_section,
166-
{S->name().str()}));
158+
if (S->isCompressed()) {
159+
eld::Expected<bool> expReadCompressedSection =
160+
ELFReader.readCompressedSection(S);
161+
ELDEXP_RETURN_DIAGENTRY_IF_ERROR(expReadCompressedSection);
162+
}
167163
addMergeStringSection(S, mergeStrFragments);
168164
} break;
169165
case LinkerSectionKind::Debug: {
@@ -384,10 +380,6 @@ ELFRelocObjParser::readDebugSection(ELFReaderBase &ELFReader, ELFSection *S) {
384380
eld::Expected<bool> expReadCompressed =
385381
ELFReader.readCompressedSection(S);
386382
ELDEXP_RETURN_DIAGENTRY_IF_ERROR(expReadCompressed);
387-
if (!expReadCompressed.value())
388-
return std::make_unique<plugin::DiagnosticEntry>(
389-
plugin::DiagnosticEntry(Diag::err_cannot_read_section,
390-
{S->name().str()}));
391383
} else if (!backend.readSection(*inputFile, S)) {
392384
return std::make_unique<plugin::DiagnosticEntry>(plugin::DiagnosticEntry(
393385
Diag::err_cannot_read_section, {S->name().str()}));
@@ -458,9 +450,9 @@ eld::Expected<bool> ELFRelocObjParser::readGroups(ELFReaderBase &ELFReader) {
458450
ELFReader.readOneGroup(llvm::cast<ELFSection>(S));
459451
ELDEXP_RETURN_DIAGENTRY_IF_ERROR(expReadOneGroup);
460452

461-
// FIXME: Return an error instead!
462453
if (!expReadOneGroup.value())
463-
return false;
454+
return std::make_unique<plugin::DiagnosticEntry>(plugin::DiagnosticEntry(
455+
Diag::err_cannot_read_section, {S->name().str()}));
464456
}
465457
return true;
466458
}

‎lib/Readers/ExecELFReader.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "eld/Core/Module.h"
1010
#include "eld/Input/ELFObjectFile.h"
1111
#include "eld/PluginAPI/Expected.h"
12+
#include <string>
1213

1314
using namespace eld;
1415

@@ -54,9 +55,10 @@ eld::Expected<ELFSection *> ExecELFReader<ELFT>::createSection(
5455
// FIXME: sectName can be extracted from rawSectHdr.
5556
LinkerSectionKind kind = this->classifySectionKind(rawSectHdr, sectName);
5657

57-
// FIXME: Emit some diagnostic here.
5858
if (kind == LinkerSectionKind::Error)
59-
return static_cast<ELFSection *>(nullptr);
59+
return std::make_unique<plugin::DiagnosticEntry>(plugin::DiagnosticEntry(
60+
Diag::err_unsupported_section,
61+
{sectName, std::to_string(rawSectHdr.sh_type)}));
6062

6163
bool SectionIsIgnore = false;
6264
// Embedded bitcode sections must be not regarded in linking. However they

0 commit comments

Comments
 (0)