Skip to content

Commit

Permalink
ELF: Rename error -> fatal and redefine error as a non-noreturn funct…
Browse files Browse the repository at this point in the history
…ion.

In many situations, we don't want to exit at the first error even in the
process model. For example, it is better to report all undefined symbols
rather than reporting the first one that the linker picked up randomly.

In order to handle such errors, we don't need to wrap everything with
ErrorOr (thanks for David Blaikie for pointing this out!) Instead, we
can set a flag to record the fact that we found an error and keep it
going until it reaches a reasonable checkpoint.

This idea should be applicable to other places. For example, we can
ignore broken relocations and check for errors after visiting all relocs.

In this patch, I rename error to fatal, and introduce another version of
error which doesn't call exit. That function instead sets HasError to true.
Once HasError becomes true, it stays true, so that we know that there
was an error if it is true.

I think introducing a non-noreturn error reporting function is by itself
a good idea, and it looks to me that this also provides a gradual path
towards lld-as-a-library (or at least embed-lld-to-your-program) without
sacrificing code readability with lots of ErrorOr's.

http://reviews.llvm.org/D16641

llvm-svn: 259069
  • Loading branch information
rui314 committed Jan 28, 2016
1 parent ec8d086 commit 64cfffd
Show file tree
Hide file tree
Showing 14 changed files with 142 additions and 115 deletions.
30 changes: 16 additions & 14 deletions lld/ELF/Driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,14 @@ using namespace lld::elf2;
Configuration *elf2::Config;
LinkerDriver *elf2::Driver;

void elf2::link(ArrayRef<const char *> Args) {
bool elf2::link(ArrayRef<const char *> Args) {
HasError = false;
Configuration C;
LinkerDriver D;
Config = &C;
Driver = &D;
Driver->main(Args.slice(1));
return !HasError;
}

static std::pair<ELFKind, uint16_t> parseEmulation(StringRef S) {
Expand All @@ -53,22 +55,22 @@ static std::pair<ELFKind, uint16_t> parseEmulation(StringRef S) {
if (S == "aarch64linux")
return {ELF64LEKind, EM_AARCH64};
if (S == "i386pe" || S == "i386pep" || S == "thumb2pe")
error("Windows targets are not supported on the ELF frontend: " + S);
error("Unknown emulation: " + S);
fatal("Windows targets are not supported on the ELF frontend: " + S);
fatal("Unknown emulation: " + S);
}

// Returns slices of MB by parsing MB as an archive file.
// Each slice consists of a member file in the archive.
static std::vector<MemoryBufferRef> getArchiveMembers(MemoryBufferRef MB) {
ErrorOr<std::unique_ptr<Archive>> FileOrErr = Archive::create(MB);
error(FileOrErr, "Failed to parse archive");
fatal(FileOrErr, "Failed to parse archive");
std::unique_ptr<Archive> File = std::move(*FileOrErr);

std::vector<MemoryBufferRef> V;
for (const ErrorOr<Archive::Child> &C : File->children()) {
error(C, "Could not get the child of the archive " + File->getFileName());
fatal(C, "Could not get the child of the archive " + File->getFileName());
ErrorOr<MemoryBufferRef> MbOrErr = C->getMemoryBufferRef();
error(MbOrErr, "Could not get the buffer for a child of the archive " +
fatal(MbOrErr, "Could not get the buffer for a child of the archive " +
File->getFileName());
V.push_back(*MbOrErr);
}
Expand All @@ -82,7 +84,7 @@ void LinkerDriver::addFile(StringRef Path) {
if (Config->Verbose)
llvm::outs() << Path << "\n";
auto MBOrErr = MemoryBuffer::getFile(Path);
error(MBOrErr, "cannot open " + Path);
fatal(MBOrErr, "cannot open " + Path);
std::unique_ptr<MemoryBuffer> &MB = *MBOrErr;
MemoryBufferRef MBRef = MB->getMemBufferRef();
OwningMBs.push_back(std::move(MB)); // take MB ownership
Expand Down Expand Up @@ -114,15 +116,15 @@ static void checkOptions(opt::InputArgList &Args) {
// of executables or DSOs. We don't support that since the feature
// does not seem to provide more value than the static archiver.
if (Args.hasArg(OPT_relocatable))
error("-r option is not supported. Use 'ar' command instead.");
fatal("-r option is not supported. Use 'ar' command instead.");

// The MIPS ABI as of 2016 does not support the GNU-style symbol lookup
// table which is a relatively new feature.
if (Config->EMachine == EM_MIPS && Config->GnuHash)
error("The .gnu.hash section is not compatible with the MIPS target.");
fatal("The .gnu.hash section is not compatible with the MIPS target.");

if (Config->EMachine == EM_AMDGPU && !Config->Entry.empty())
error("-e option is not valid for AMDGPU.");
fatal("-e option is not valid for AMDGPU.");
}

static StringRef
Expand Down Expand Up @@ -161,7 +163,7 @@ void LinkerDriver::main(ArrayRef<const char *> ArgsArr) {
link<ELF64BE>(Args);
return;
default:
error("-m or at least a .o file required");
fatal("-m or at least a .o file required");
}
}

Expand Down Expand Up @@ -217,7 +219,7 @@ void LinkerDriver::readConfigs(opt::InputArgList &Args) {
if (auto *Arg = Args.getLastArg(OPT_O)) {
StringRef Val = Arg->getValue();
if (Val.getAsInteger(10, Config->Optimize))
error("Invalid optimization level");
fatal("Invalid optimization level");
}

if (auto *Arg = Args.getLastArg(OPT_hash_style)) {
Expand All @@ -228,7 +230,7 @@ void LinkerDriver::readConfigs(opt::InputArgList &Args) {
} else if (S == "both") {
Config->GnuHash = true;
} else if (S != "sysv")
error("Unknown hash style: " + S);
fatal("Unknown hash style: " + S);
}

for (auto *Arg : Args.filtered(OPT_undefined))
Expand Down Expand Up @@ -267,7 +269,7 @@ void LinkerDriver::createFiles(opt::InputArgList &Args) {
}

if (Files.empty())
error("no input files.");
fatal("no input files.");
}

template <class ELFT> void LinkerDriver::link(opt::InputArgList &Args) {
Expand Down
4 changes: 2 additions & 2 deletions lld/ELF/Driver.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ namespace elf2 {

extern class LinkerDriver *Driver;

// Entry point of the ELF linker.
void link(ArrayRef<const char *> Args);
// Entry point of the ELF linker. Returns true on success.
bool link(ArrayRef<const char *> Args);

class LinkerDriver {
public:
Expand Down
6 changes: 3 additions & 3 deletions lld/ELF/DriverUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,15 @@ opt::InputArgList elf2::parseArgs(llvm::BumpPtrAllocator *A,
// Parse options and then do error checking.
opt::InputArgList Args = Table.ParseArgs(Vec, MissingIndex, MissingCount);
if (MissingCount)
error(Twine("missing arg value for \"") + Args.getArgString(MissingIndex) +
fatal(Twine("missing arg value for \"") + Args.getArgString(MissingIndex) +
"\", expected " + Twine(MissingCount) +
(MissingCount == 1 ? " argument.\n" : " arguments"));

iterator_range<opt::arg_iterator> Unknowns = Args.filtered(OPT_UNKNOWN);
for (auto *Arg : Unknowns)
warning("warning: unknown argument: " + Arg->getSpelling());
if (Unknowns.begin() != Unknowns.end())
error("unknown argument(s) found");
fatal("unknown argument(s) found");

return Args;
}
Expand Down Expand Up @@ -104,7 +104,7 @@ std::string elf2::searchLibrary(StringRef Path) {
if (!S.empty())
return S;
}
error("Unable to find library -l" + Path);
fatal("Unable to find library -l" + Path);
}

// Makes a path by concatenating Dir and File.
Expand Down
29 changes: 22 additions & 7 deletions lld/ELF/Error.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,38 @@
namespace lld {
namespace elf2 {

bool HasError;

void warning(const Twine &Msg) { llvm::errs() << Msg << "\n"; }

void error(const Twine &Msg) {
llvm::errs() << Msg << "\n";
exit(1);
HasError = true;
}

void error(std::error_code EC, const Twine &Prefix) {
if (!EC)
return;
error(Prefix + ": " + EC.message());
if (EC)
error(Prefix + ": " + EC.message());
}

void error(std::error_code EC) {
if (!EC)
return;
error(EC.message());
if (EC)
error(EC.message());
}

void fatal(const Twine &Msg) {
llvm::errs() << Msg << "\n";
exit(1);
}

void fatal(std::error_code EC, const Twine &Prefix) {
if (EC)
fatal(Prefix + ": " + EC.message());
}

void fatal(std::error_code EC) {
if (EC)
fatal(EC.message());
}

} // namespace elf2
Expand Down
13 changes: 12 additions & 1 deletion lld/ELF/Error.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@
namespace lld {
namespace elf2 {

extern bool HasError;

void warning(const Twine &Msg);

LLVM_ATTRIBUTE_NORETURN void error(const Twine &Msg);
void error(const Twine &Msg);
void error(std::error_code EC, const Twine &Prefix);
void error(std::error_code EC);

Expand All @@ -26,6 +28,15 @@ template <typename T> void error(const ErrorOr<T> &V, const Twine &Prefix) {
}
template <typename T> void error(const ErrorOr<T> &V) { error(V.getError()); }

LLVM_ATTRIBUTE_NORETURN void fatal(const Twine &Msg);
void fatal(std::error_code EC, const Twine &Prefix);
void fatal(std::error_code EC);

template <typename T> void fatal(const ErrorOr<T> &V, const Twine &Prefix) {
fatal(V.getError(), Prefix);
}
template <typename T> void fatal(const ErrorOr<T> &V) { fatal(V.getError()); }

} // namespace elf2
} // namespace lld

Expand Down
Loading

0 comments on commit 64cfffd

Please sign in to comment.