[lld] 4af1522 - [lld-macho] Rework length check when opening input files
Greg McGary via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 2 13:01:07 PST 2021
Author: Greg McGary
Date: 2021-03-02T13:00:57-08:00
New Revision: 4af1522a855e94c909630a1f1962241c1573eac8
URL: https://github.com/llvm/llvm-project/commit/4af1522a855e94c909630a1f1962241c1573eac8
DIFF: https://github.com/llvm/llvm-project/commit/4af1522a855e94c909630a1f1962241c1573eac8.diff
LOG: [lld-macho] Rework length check when opening input files
This reverts diff D97610 (commit 0223ab035c199e537a0040857ba147ced87fd533) and adds a one-line fix to verify that a `MemoryBufferRef` has sufficient length before reading a 4-byte magic number.
Differential Revision: https://reviews.llvm.org/D97757
Added:
Modified:
lld/MachO/Driver.cpp
lld/MachO/DriverUtils.cpp
lld/MachO/InputFiles.cpp
lld/MachO/InputFiles.h
lld/test/MachO/rename.s
Removed:
lld/test/MachO/invalid/tiny-input.s
################################################################################
diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index b27c2f1956a48..7426fcb0fae88 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -263,7 +263,7 @@ static std::vector<ArchiveMember> getArchiveMembers(MemoryBufferRef mb) {
static InputFile *addFile(StringRef path, bool forceLoadArchive,
bool isBundleLoader = false) {
- Optional<MemoryBufferRef> buffer = readLinkableFile(path);
+ Optional<MemoryBufferRef> buffer = readFile(path);
if (!buffer)
return nullptr;
MemoryBufferRef mbref = *buffer;
@@ -279,7 +279,7 @@ static InputFile *addFile(StringRef path, bool forceLoadArchive,
error(path + ": archive has no index; run ranlib to add one");
if (config->allLoad || forceLoadArchive) {
- if (Optional<MemoryBufferRef> buffer = readLinkableFile(path)) {
+ if (Optional<MemoryBufferRef> buffer = readFile(path)) {
for (const ArchiveMember &member : getArchiveMembers(*buffer)) {
if (Optional<InputFile *> file = loadArchiveMember(
member.mbref, member.modTime, path, /*objCOnly=*/false)) {
@@ -300,7 +300,7 @@ static InputFile *addFile(StringRef path, bool forceLoadArchive,
// 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<MemoryBufferRef> buffer = readLinkableFile(path)) {
+ if (Optional<MemoryBufferRef> buffer = readFile(path)) {
for (const ArchiveMember &member : getArchiveMembers(*buffer)) {
if (Optional<InputFile *> file = loadArchiveMember(
member.mbref, member.modTime, path, /*objCOnly=*/true)) {
@@ -403,7 +403,7 @@ void macho::parseLCLinkerOption(InputFile* f, unsigned argc, StringRef data) {
}
static void addFileList(StringRef path) {
- Optional<MemoryBufferRef> buffer = readRawFile(path);
+ Optional<MemoryBufferRef> buffer = readFile(path);
if (!buffer)
return;
MemoryBufferRef mbref = *buffer;
@@ -426,7 +426,7 @@ static void addFileList(StringRef path) {
//
// The file can also have line comments that start with '#'.
static void parseOrderFile(StringRef path) {
- Optional<MemoryBufferRef> buffer = readRawFile(path);
+ Optional<MemoryBufferRef> buffer = readFile(path);
if (!buffer) {
error("Could not read order file at " + path);
return;
@@ -940,7 +940,7 @@ bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
StringRef segName = arg->getValue(0);
StringRef sectName = arg->getValue(1);
StringRef fileName = arg->getValue(2);
- Optional<MemoryBufferRef> buffer = readRawFile(fileName);
+ Optional<MemoryBufferRef> buffer = readFile(fileName);
if (buffer)
inputFiles.insert(make<OpaqueFile>(*buffer, segName, sectName));
}
diff --git a/lld/MachO/DriverUtils.cpp b/lld/MachO/DriverUtils.cpp
index c01d0a8ed8f11..6d9413ea4e578 100644
--- a/lld/MachO/DriverUtils.cpp
+++ b/lld/MachO/DriverUtils.cpp
@@ -132,7 +132,7 @@ std::string macho::createResponseFile(const opt::InputArgList &args) {
os << "-o " << quote(path::filename(arg->getValue())) << "\n";
break;
case OPT_filelist:
- if (Optional<MemoryBufferRef> buffer = readRawFile(arg->getValue()))
+ if (Optional<MemoryBufferRef> buffer = readFile(arg->getValue()))
for (StringRef path : args::getLines(*buffer))
os << quote(rewritePath(path)) << "\n";
break;
diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index 0015cff2e4a5b..feab688799767 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -91,8 +91,7 @@ std::unique_ptr<TarWriter> macho::tar;
int InputFile::idCount = 0;
// Open a given file path and return it as a memory-mapped file.
-// Perform no sanity checks--just open, map & return.
-Optional<MemoryBufferRef> macho::readRawFile(StringRef path) {
+Optional<MemoryBufferRef> macho::readFile(StringRef path) {
// Open a file.
auto mbOrErr = MemoryBuffer::getFile(path);
if (auto ec = mbOrErr.getError()) {
@@ -103,32 +102,12 @@ Optional<MemoryBufferRef> macho::readRawFile(StringRef path) {
std::unique_ptr<MemoryBuffer> &mb = *mbOrErr;
MemoryBufferRef mbref = mb->getMemBufferRef();
make<std::unique_ptr<MemoryBuffer>>(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<MemoryBufferRef> macho::readLinkableFile(StringRef path) {
- Optional<MemoryBufferRef> 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
- // LLD are less demanding. We insist on having only enough data for
- // a magic number.
- if (mbref.getBufferSize() < sizeof(uint32_t)) {
- error("file is too small to contain a magic number: " + path);
- return None;
- }
// If this is a regular non-fat file, return it.
const char *buf = mbref.getBufferStart();
auto *hdr = reinterpret_cast<const MachO::fat_header *>(buf);
- if (read32be(&hdr->magic) != MachO::FAT_MAGIC) {
+ if (mbref.getBufferSize() < sizeof(uint32_t) ||
+ read32be(&hdr->magic) != MachO::FAT_MAGIC) {
if (tar)
tar->append(relativeToRoot(path), mbref.getBuffer());
return mbref;
@@ -565,7 +544,7 @@ void ObjFile::parseDebugInfo() {
// The path can point to either a dylib or a .tbd file.
static Optional<DylibFile *> loadDylib(StringRef path, DylibFile *umbrella) {
- Optional<MemoryBufferRef> mbref = readLinkableFile(path);
+ Optional<MemoryBufferRef> mbref = readFile(path);
if (!mbref) {
error("could not read dylib file at " + path);
return {};
diff --git a/lld/MachO/InputFiles.h b/lld/MachO/InputFiles.h
index 5ee3f78350988..10935050584d0 100644
--- a/lld/MachO/InputFiles.h
+++ b/lld/MachO/InputFiles.h
@@ -173,8 +173,7 @@ class BitcodeFile : public InputFile {
extern llvm::SetVector<InputFile *> inputFiles;
-llvm::Optional<MemoryBufferRef> readRawFile(StringRef path);
-llvm::Optional<MemoryBufferRef> readLinkableFile(StringRef path);
+llvm::Optional<MemoryBufferRef> readFile(StringRef path);
const llvm::MachO::load_command *
findCommand(const llvm::MachO::mach_header_64 *, uint32_t type);
diff --git a/lld/test/MachO/invalid/tiny-input.s b/lld/test/MachO/invalid/tiny-input.s
deleted file mode 100644
index 64409b96d9ca3..0000000000000
--- a/lld/test/MachO/invalid/tiny-input.s
+++ /dev/null
@@ -1,18 +0,0 @@
-# REQUIRES: x86
-
-## Check that files too short to have a magic number are rejected as inputs
-# RUN: echo -n 1 >%t-1.o
-# RUN: echo -n 12 >%t-2.o
-# RUN: echo -n 123 >%t-3.o
-# RUN: echo -n 1234 >%t-4.o
-# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
-# RUN: not %lld -o %t %t.o %t-1.o %t-2.o %t-3.o %t-4.o 2>&1 | FileCheck %s
-
-# CHECK: error: file is too small to contain a magic number: {{.*}}-1.o
-# CHECK: error: file is too small to contain a magic number: {{.*}}-2.o
-# CHECK: error: file is too small to contain a magic number: {{.*}}-3.o
-# CHECK: error: {{.*}}-4.o: unhandled file type
-
-.global _main
-_main:
- ret
diff --git a/lld/test/MachO/rename.s b/lld/test/MachO/rename.s
index 2a0567416c525..0b74ff1fdfd28 100644
--- a/lld/test/MachO/rename.s
+++ b/lld/test/MachO/rename.s
@@ -14,7 +14,7 @@
# 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 magic number:
+# BAD1-DAG: error: {{.*}}: unhandled file type
# RUN: not %lld \
# RUN: -rename_segment H#SHY_SEG PL+SSY_SEG \
@@ -24,7 +24,7 @@
# BAD2-DAG: error: invalid name for segment or section: H#SHY_SEG
# BAD2-DAG: error: invalid name for segment or section: PL+SSY_SEG
# BAD2-DAG: error: invalid name for segment or section: -o
-# BAD2-DAG: error: file is too small to contain a magic number:
+# BAD2-DAG: error: {{.*}}: unhandled file type
## Check that section and segment renames happen
# RUN: %lld \
More information about the llvm-commits
mailing list