From a20168d0307860047ad7c8a2074f98fc25b057c2 Mon Sep 17 00:00:00 2001 From: James Henderson Date: Fri, 25 Sep 2020 10:21:39 +0100 Subject: [PATCH] [Archive] Don't throw away errors for malformed archive members When adding an archive member with a problem, e.g. a new bitcode with an old archiver, containing an unsupported attribute, or an ELF file with a malformed symbol table, the archiver would throw away the error and simply add the member to the archive without any symbol entries. This meant that the resultant archive could be silently unusable when not using --whole-archive, and result in unexpected undefined symbols. This change fixes this issue by addressing two FIXMEs and only throwing away not-an-object errors. However, this meant that some LLD tests which didn't need symbol tables and were using invalid members deliberately to test the linker's malformed input handling no longer worked, so this patch also stops the archiver from looking for symbols in an object if it doesn't require a symbol table, and updates the tests accordingly. Differential Revision: https://reviews.llvm.org/D88288 Reviewed by: grimar, rupprecht, MaskRay --- lld/test/ELF/invalid/data-encoding.test | 2 +- lld/test/ELF/invalid/invalid-file-class.test | 2 +- llvm/include/llvm/Object/SymbolicFile.h | 2 + llvm/lib/Object/ArchiveWriter.cpp | 42 +++++++++++--------- llvm/lib/Object/SymbolicFile.cpp | 53 +++++++++++++++++++------- llvm/test/Object/archive-malformed-object.test | 38 ++++++++++++++++++ llvm/test/Object/archive-unknown-filetype.test | 11 ++++++ 7 files changed, 116 insertions(+), 34 deletions(-) create mode 100644 llvm/test/Object/archive-malformed-object.test create mode 100644 llvm/test/Object/archive-unknown-filetype.test diff --git a/lld/test/ELF/invalid/data-encoding.test b/lld/test/ELF/invalid/data-encoding.test index 5f0550f..94862af 100644 --- a/lld/test/ELF/invalid/data-encoding.test +++ b/lld/test/ELF/invalid/data-encoding.test @@ -4,7 +4,7 @@ # Check we report this. # RUN: yaml2obj %s -o %t.o -# RUN: llvm-ar rcs %t.a %t.o +# RUN: llvm-ar rcS %t.a %t.o # RUN: not ld.lld --whole-archive %t.a -o /dev/null 2>&1 | FileCheck %s # CHECK: {{.*}}.a({{.*}}.o): corrupted ELF file: invalid data encoding diff --git a/lld/test/ELF/invalid/invalid-file-class.test b/lld/test/ELF/invalid/invalid-file-class.test index 1b40d69..3f54786 100644 --- a/lld/test/ELF/invalid/invalid-file-class.test +++ b/lld/test/ELF/invalid/invalid-file-class.test @@ -11,7 +11,7 @@ ## EV_CURRENT(1), ELFOSABI_LINUX(3), , ET_REL(1), EM_NONE(0) # RUN: echo -e -n "\x7f\x45\x4c\x46\x00\x01\x01\x03\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00" > %t/invalid.o -# RUN: llvm-ar --format=gnu cr %t/invalid-class.a %t/invalid.o +# RUN: llvm-ar --format=gnu crS %t/invalid-class.a %t/invalid.o # RUN: not ld.lld -whole-archive %t/invalid-class.a -o /dev/null 2>&1 | FileCheck %s # CHECK: invalid-class.a(invalid.o): corrupted ELF file: invalid file class diff --git a/llvm/include/llvm/Object/SymbolicFile.h b/llvm/include/llvm/Object/SymbolicFile.h index a0d8b72..5c96461 100644 --- a/llvm/include/llvm/Object/SymbolicFile.h +++ b/llvm/include/llvm/Object/SymbolicFile.h @@ -173,6 +173,8 @@ public: static bool classof(const Binary *v) { return v->isSymbolic(); } + + static bool isSymbolicFile(file_magic Type, const LLVMContext *Context); }; inline BasicSymbolRef::BasicSymbolRef(DataRefImpl SymbolP, diff --git a/llvm/lib/Object/ArchiveWriter.cpp b/llvm/lib/Object/ArchiveWriter.cpp index ca8ffa7..08f1b85 100644 --- a/llvm/lib/Object/ArchiveWriter.cpp +++ b/llvm/lib/Object/ArchiveWriter.cpp @@ -359,22 +359,21 @@ getSymbols(MemoryBufferRef Buf, raw_ostream &SymNames, bool &HasObject) { // reference to it, thus SymbolicFile should be destroyed first. LLVMContext Context; std::unique_ptr Obj; - if (identify_magic(Buf.getBuffer()) == file_magic::bitcode) { + + const file_magic Type = identify_magic(Buf.getBuffer()); + // Treat unsupported file types as having no symbols. + if (!object::SymbolicFile::isSymbolicFile(Type, &Context)) + return Ret; + if (Type == file_magic::bitcode) { auto ObjOrErr = object::SymbolicFile::createSymbolicFile( Buf, file_magic::bitcode, &Context); - if (!ObjOrErr) { - // FIXME: check only for "not an object file" errors. - consumeError(ObjOrErr.takeError()); - return Ret; - } + if (!ObjOrErr) + return ObjOrErr.takeError(); Obj = std::move(*ObjOrErr); } else { auto ObjOrErr = object::SymbolicFile::createSymbolicFile(Buf); - if (!ObjOrErr) { - // FIXME: check only for "not an object file" errors. - consumeError(ObjOrErr.takeError()); - return Ret; - } + if (!ObjOrErr) + return ObjOrErr.takeError(); Obj = std::move(*ObjOrErr); } @@ -393,7 +392,7 @@ getSymbols(MemoryBufferRef Buf, raw_ostream &SymNames, bool &HasObject) { static Expected> computeMemberData(raw_ostream &StringTable, raw_ostream &SymNames, object::Archive::Kind Kind, bool Thin, bool Deterministic, - ArrayRef NewMembers) { + bool NeedSymbols, ArrayRef NewMembers) { static char PaddingData[8] = {'\n', '\n', '\n', '\n', '\n', '\n', '\n', '\n'}; // This ignores the symbol table, but we only need the value mod 8 and the @@ -494,13 +493,17 @@ computeMemberData(raw_ostream &StringTable, raw_ostream &SymNames, ModTime, Size); Out.flush(); - Expected> Symbols = - getSymbols(Buf, SymNames, HasObject); - if (auto E = Symbols.takeError()) - return std::move(E); + std::vector Symbols; + if (NeedSymbols) { + Expected> SymbolsOrErr = + getSymbols(Buf, SymNames, HasObject); + if (auto E = SymbolsOrErr.takeError()) + return std::move(E); + Symbols = std::move(*SymbolsOrErr); + } Pos += Header.size() + Data.size() + Padding.size(); - Ret.push_back({std::move(*Symbols), std::move(Header), Data, Padding}); + Ret.push_back({std::move(Symbols), std::move(Header), Data, Padding}); } // If there are no symbols, emit an empty symbol table, to satisfy Solaris // tools, older versions of which expect a symbol table in a non-empty @@ -564,8 +567,9 @@ static Error writeArchiveToStream(raw_ostream &Out, SmallString<0> StringTableBuf; raw_svector_ostream StringTable(StringTableBuf); - Expected> DataOrErr = computeMemberData( - StringTable, SymNames, Kind, Thin, Deterministic, NewMembers); + Expected> DataOrErr = + computeMemberData(StringTable, SymNames, Kind, Thin, Deterministic, + WriteSymtab, NewMembers); if (Error E = DataOrErr.takeError()) return E; std::vector &Data = *DataOrErr; diff --git a/llvm/lib/Object/SymbolicFile.cpp b/llvm/lib/Object/SymbolicFile.cpp index 3db4ad9..72014fd 100644 --- a/llvm/lib/Object/SymbolicFile.cpp +++ b/llvm/lib/Object/SymbolicFile.cpp @@ -41,20 +41,14 @@ SymbolicFile::createSymbolicFile(MemoryBufferRef Object, file_magic Type, if (Type == file_magic::unknown) Type = identify_magic(Data); + if (!isSymbolicFile(Type, Context)) + return errorCodeToError(object_error::invalid_file_type); + switch (Type) { case file_magic::bitcode: - if (Context) - return IRObjectFile::create(Object, *Context); - LLVM_FALLTHROUGH; - case file_magic::unknown: - case file_magic::archive: - case file_magic::coff_cl_gl_object: - case file_magic::macho_universal_binary: - case file_magic::windows_resource: - case file_magic::pdb: - case file_magic::minidump: - case file_magic::tapi_file: - return errorCodeToError(object_error::invalid_file_type); + // Context is guaranteed to be non-null here, because bitcode magic only + // indicates a symbolic file when Context is non-null. + return IRObjectFile::create(Object, *Context); case file_magic::elf: case file_magic::elf_executable: case file_magic::elf_shared_object: @@ -95,6 +89,39 @@ SymbolicFile::createSymbolicFile(MemoryBufferRef Object, file_magic Type, MemoryBufferRef(BCData->getBuffer(), Object.getBufferIdentifier()), *Context); } + default: + llvm_unreachable("Unexpected Binary File Type"); + } +} + +bool SymbolicFile::isSymbolicFile(file_magic Type, const LLVMContext *Context) { + switch (Type) { + case file_magic::bitcode: + return Context != nullptr; + case file_magic::elf: + case file_magic::elf_executable: + case file_magic::elf_shared_object: + case file_magic::elf_core: + case file_magic::macho_executable: + case file_magic::macho_fixed_virtual_memory_shared_lib: + case file_magic::macho_core: + case file_magic::macho_preload_executable: + case file_magic::macho_dynamically_linked_shared_lib: + case file_magic::macho_dynamic_linker: + case file_magic::macho_bundle: + case file_magic::macho_dynamically_linked_shared_lib_stub: + case file_magic::macho_dsym_companion: + case file_magic::macho_kext_bundle: + case file_magic::pecoff_executable: + case file_magic::xcoff_object_32: + case file_magic::xcoff_object_64: + case file_magic::wasm_object: + case file_magic::coff_import_library: + case file_magic::elf_relocatable: + case file_magic::macho_object: + case file_magic::coff_object: + return true; + default: + return false; } - llvm_unreachable("Unexpected Binary File Type"); } diff --git a/llvm/test/Object/archive-malformed-object.test b/llvm/test/Object/archive-malformed-object.test new file mode 100644 index 0000000..7d502c3 --- /dev/null +++ b/llvm/test/Object/archive-malformed-object.test @@ -0,0 +1,38 @@ +## Show that the archive library emits error messages when adding malformed +## objects. + +# RUN: rm -rf %t.dir +# RUN: split-file %s %t.dir +# RUN: cd %t.dir + +## Malformed bitcode object. +# RUN: llvm-as input.ll -o input.bc +# RUN: %python -c "with open('input.bc', 'a') as f: f.truncate(10)" +# RUN: not llvm-ar rc bad.a input.bc 2>&1 | FileCheck %s --check-prefix=ERR1 + +# ERR1: error: bad.a: Invalid bitcode signature + +## Non-bitcode malformed file. +# RUN: yaml2obj input.yaml -o input.o +# RUN: not llvm-ar rc bad.a input.o 2>&1 | FileCheck %s --check-prefix=ERR2 + +# ERR2: error: bad.a: section header table goes past the end of the file: e_shoff = 0x9999 + +## Don't emit an error if the symbol table is not required. +# RUN: llvm-ar rcS good.a input.o input.bc +# RUN: llvm-ar t good.a | FileCheck %s --check-prefix=CONTENTS + +# CONTENTS: input.o +# CONTENTS-NEXT: input.bc + +#--- input.ll +target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-pc-linux" + +#--- input.yaml +--- !ELF +FileHeader: + Class: ELFCLASS64 + Data: ELFDATA2LSB + Type: ET_REL + EShOff: 0x9999 diff --git a/llvm/test/Object/archive-unknown-filetype.test b/llvm/test/Object/archive-unknown-filetype.test new file mode 100644 index 0000000..5647501 --- /dev/null +++ b/llvm/test/Object/archive-unknown-filetype.test @@ -0,0 +1,11 @@ +## Show that the archive library does not emit an error or add any symbols to +## the archive symbol table, when it encounters an unknown file type, but still +## adds the file to the archive. + +# RUN: echo something > %t +# RUN: rm -f %t.a +# RUN: llvm-ar rc %t.a %t +# RUN: llvm-ar t %t.a | FileCheck %s --check-prefix=CONTENTS -DFILE=%basename_t +# RUN: llvm-nm --print-armap %t.a | FileCheck %s --allow-empty --implicit-check-not={{.}} + +# CONTENTS: [[FILE]] -- 2.7.4