diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp --- a/lld/MachO/Driver.cpp +++ b/lld/MachO/Driver.cpp @@ -261,9 +261,9 @@ return v; } -static InputFile *addFile(StringRef path, bool forceLoadArchive, - bool isBundleLoader = false) { - Optional buffer = readFile(path); +static InputFile *addLinkableFile(StringRef path, bool forceLoadArchive, + bool isBundleLoader = false) { + Optional buffer = readLinkableFile(path); if (!buffer) return nullptr; MemoryBufferRef mbref = *buffer; @@ -279,7 +279,7 @@ error(path + ": archive has no index; run ranlib to add one"); if (config->allLoad || forceLoadArchive) { - if (Optional buffer = readFile(path)) { + if (Optional buffer = readLinkableFile(path)) { for (const ArchiveMember &member : getArchiveMembers(*buffer)) { if (Optional file = loadArchiveMember( member.mbref, member.modTime, path, /*objCOnly=*/false)) { @@ -300,7 +300,7 @@ // we already found that it contains an ObjC symbol. We should also // consider creating a LazyObjFile class in order to avoid double-loading // these files here and below (as part of the ArchiveFile). - if (Optional buffer = readFile(path)) { + if (Optional buffer = readLinkableFile(path)) { for (const ArchiveMember &member : getArchiveMembers(*buffer)) { if (Optional file = loadArchiveMember( member.mbref, member.modTime, path, /*objCOnly=*/true)) { @@ -351,7 +351,8 @@ static void addLibrary(StringRef name, bool isWeak) { if (Optional path = findLibrary(name)) { - auto *dylibFile = dyn_cast_or_null(addFile(*path, false)); + auto *dylibFile = + dyn_cast_or_null(addLinkableFile(*path, false)); if (isWeak && dylibFile) dylibFile->forceWeakImport = true; return; @@ -361,7 +362,8 @@ static void addFramework(StringRef name, bool isWeak) { if (Optional path = findFramework(name)) { - auto *dylibFile = dyn_cast_or_null(addFile(*path, false)); + auto *dylibFile = + dyn_cast_or_null(addLinkableFile(*path, false)); if (isWeak && dylibFile) dylibFile->forceWeakImport = true; return; @@ -402,13 +404,13 @@ } } -static void addFileList(StringRef path) { - Optional buffer = readFile(path); +static void addFileListFile(StringRef path) { + Optional buffer = readRawFile(path); if (!buffer) return; MemoryBufferRef mbref = *buffer; for (StringRef path : args::getLines(mbref)) - addFile(path, false); + addLinkableFile(path, false); } // An order file has one entry per line, in the following format: @@ -426,7 +428,7 @@ // // The file can also have line comments that start with '#'. static void parseOrderFile(StringRef path) { - Optional buffer = readFile(path); + Optional buffer = readRawFile(path); if (!buffer) { error("Could not read order file at " + path); return; @@ -774,7 +776,7 @@ if (const opt::Arg *arg = args.getLastArg(OPT_bundle_loader)) { if (config->outputType != MH_BUNDLE) error("-bundle_loader can only be used with MachO bundle output"); - addFile(arg->getValue(), false, true); + addLinkableFile(arg->getValue(), false, true); } config->ltoObjPath = args.getLastArgValue(OPT_object_path_lto); config->ltoNewPassManager = @@ -849,7 +851,7 @@ return !errorCount(); } - initLLVM(); // must be run before any call to addFile() + initLLVM(); // must be run before any call to addLinkableFile() for (const auto &arg : args) { const auto &opt = arg->getOption(); @@ -859,18 +861,18 @@ // TODO: are any of these better handled via filtered() or getLastArg()? switch (opt.getID()) { case OPT_INPUT: - addFile(arg->getValue(), false); + addLinkableFile(arg->getValue(), false); break; case OPT_weak_library: - if (auto *dylibFile = - dyn_cast_or_null(addFile(arg->getValue(), false))) + if (auto *dylibFile = dyn_cast_or_null( + addLinkableFile(arg->getValue(), false))) dylibFile->forceWeakImport = true; break; case OPT_filelist: - addFileList(arg->getValue()); + addFileListFile(arg->getValue()); break; case OPT_force_load: - addFile(arg->getValue(), true); + addLinkableFile(arg->getValue(), true); break; case OPT_l: case OPT_weak_l: @@ -945,7 +947,7 @@ StringRef segName = arg->getValue(0); StringRef sectName = arg->getValue(1); StringRef fileName = arg->getValue(2); - Optional buffer = readFile(fileName); + Optional buffer = readRawFile(fileName); if (buffer) inputFiles.insert(make(*buffer, segName, sectName)); } diff --git a/lld/MachO/DriverUtils.cpp b/lld/MachO/DriverUtils.cpp --- a/lld/MachO/DriverUtils.cpp +++ b/lld/MachO/DriverUtils.cpp @@ -132,7 +132,7 @@ os << "-o " << quote(path::filename(arg->getValue())) << "\n"; break; case OPT_filelist: - if (Optional buffer = readFile(arg->getValue())) + if (Optional buffer = readRawFile(arg->getValue())) for (StringRef path : args::getLines(*buffer)) os << quote(rewritePath(path)) << "\n"; break; diff --git a/lld/MachO/InputFiles.h b/lld/MachO/InputFiles.h --- a/lld/MachO/InputFiles.h +++ b/lld/MachO/InputFiles.h @@ -173,7 +173,8 @@ extern llvm::SetVector inputFiles; -llvm::Optional readFile(StringRef path); +llvm::Optional readRawFile(StringRef path); +llvm::Optional readLinkableFile(StringRef path); const llvm::MachO::load_command * findCommand(const llvm::MachO::mach_header_64 *, uint32_t type); diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp --- a/lld/MachO/InputFiles.cpp +++ b/lld/MachO/InputFiles.cpp @@ -91,7 +91,8 @@ int InputFile::idCount = 0; // Open a given file path and return it as a memory-mapped file. -Optional macho::readFile(StringRef path) { +// Perform no sanity checks--just open, map & return. +Optional macho::readRawFile(StringRef path) { // Open a file. auto mbOrErr = MemoryBuffer::getFile(path); if (auto ec = mbOrErr.getError()) { @@ -102,6 +103,26 @@ std::unique_ptr &mb = *mbOrErr; MemoryBufferRef mbref = mb->getMemBufferRef(); make>(std::move(mb)); // take mb ownership + return mbref; +} + +// Open a given file path and return it as a memory-mapped file. +// Assume the file has one of a variety of linkable formats and +// perform some basic sanity checks, notably minimum length. +Optional macho::readLinkableFile(StringRef path) { + Optional maybeMbref = readRawFile(path); + if (!maybeMbref) { + return None; + } + MemoryBufferRef mbref = *maybeMbref; + + // ld64 hard-codes 20 as minimum header size, which is presumably + // the smallest header among the the various linkable input formats + const size_t minimumLinkableFileHeaderSize = 20; + if (mbref.getBufferSize() < minimumLinkableFileHeaderSize) { + error("file is too small to contain a linkable-file header: " + path); + return None; + } // If this is a regular non-fat file, return it. const char *buf = mbref.getBufferStart(); @@ -544,7 +565,7 @@ // The path can point to either a dylib or a .tbd file. static Optional loadDylib(StringRef path, DylibFile *umbrella) { - Optional mbref = readFile(path); + Optional mbref = readLinkableFile(path); if (!mbref) { error("could not read dylib file at " + path); return {}; diff --git a/lld/test/MachO/invalid/bad-archive.s b/lld/test/MachO/invalid/bad-archive.s --- a/lld/test/MachO/invalid/bad-archive.s +++ b/lld/test/MachO/invalid/bad-archive.s @@ -6,7 +6,7 @@ # RUN: not %lld %t.o %t.a -o /dev/null 2>&1 | FileCheck -DFILE=%t.a %s # RUN: not %lld %t.o -force_load %t.a -o /dev/null 2>&1 | FileCheck -DFILE=%t.a %s # RUN: not %lld %t.o -ObjC %t.a -o /dev/null 2>&1 | FileCheck -DFILE=%t.a %s -# CHECK: error: [[FILE]]: failed to parse archive: truncated or malformed archive (remaining size of archive too small for next archive member header at offset 8) +# CHECK: error: file is too small to contain a linkable-file header: [[FILE]] .global _main _main: diff --git a/lld/test/MachO/invalid/invalid-fat-narch.s b/lld/test/MachO/invalid/invalid-fat-narch.s --- a/lld/test/MachO/invalid/invalid-fat-narch.s +++ b/lld/test/MachO/invalid/invalid-fat-narch.s @@ -2,7 +2,7 @@ # RUN: yaml2obj %s -o %t.o # RUN: not %lld -o /dev/null %t.o 2>&1 | \ # RUN: FileCheck %s -DFILE=%t.o -# CHECK: error: [[FILE]]: fat_arch struct extends beyond end of file +# CHECK: error: file is too small to contain a linkable-file header: [[FILE]] !fat-mach-o FatHeader: diff --git a/lld/test/MachO/rename.s b/lld/test/MachO/rename.s --- a/lld/test/MachO/rename.s +++ b/lld/test/MachO/rename.s @@ -7,11 +7,24 @@ # RUN: not %lld \ # RUN: -rename_section B@GUS_SEG b@gus_sect S/ASHY_SEG st*rry_sect \ # RUN: -rename_section __FROM_SECT __from_sect __TO_SECT \ -# RUN: -o /dev/null %t.o +# RUN: -o /dev/null %t.o 2>&1 | FileCheck %s --check-prefix=BAD1 + +# BAD1-DAG: error: invalid name for segment or section: B@GUS_SEG +# BAD1-DAG: error: invalid name for segment or section: b@gus_sect +# BAD1-DAG: error: invalid name for segment or section: S/ASHY_SEG +# BAD1-DAG: error: invalid name for segment or section: st*rry_sect +# BAD1-DAG: error: invalid name for segment or section: -o +# BAD1-DAG: error: file is too small to contain a linkable-file header: + # RUN: not %lld \ # RUN: -rename_segment H#SHY_SEG pl+ssy_sect \ # RUN: -rename_segment __FROM_SEG \ -# RUN: -o /dev/null %t.o +# RUN: -o /dev/null %t.o 2>&1 | FileCheck %s --check-prefix=BAD2 + +# BAD2-DAG: error: invalid name for segment or section: pl+ssy_sect +# BAD2-DAG: error: invalid name for segment or section: H#SHY_SEG +# BAD2-DAG: error: invalid name for segment or section: -o +# BAD2-DAG: error: file is too small to contain a linkable-file header: ## Check that section and segment renames happen # RUN: %lld \