From d7c2f5847af50181bba9b59dacf39cf19db9bfca Mon Sep 17 00:00:00 2001 From: Rui Ueyama Date: Sun, 31 May 2015 21:04:56 +0000 Subject: [PATCH] COFF: Make the Driver own all MemoryBuffers. NFC. Previously, a MemoryBuffer of a file was owned by each InputFile object. This patch makes the Driver own all of them. InputFiles now have only MemoryBufferRefs. This change simplifies ownership managment (particularly for ObjectFile -- the object owned a MemoryBuffer only when it's not created from an archive file, because in that case a parent archive file owned the entire buffer. Now it owns nothing unconditionally.) llvm-svn: 238690 --- lld/COFF/Driver.cpp | 34 +++++++++++++++++++++++++++------- lld/COFF/Driver.h | 7 +++++++ lld/COFF/InputFiles.cpp | 31 ++++++++----------------------- lld/COFF/InputFiles.h | 21 ++++++++------------- lld/COFF/Symbols.cpp | 2 +- 5 files changed, 51 insertions(+), 44 deletions(-) diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp index 8c3d664..750e86b 100644 --- a/lld/COFF/Driver.cpp +++ b/lld/COFF/Driver.cpp @@ -60,10 +60,18 @@ static std::string getOutputPath(llvm::opt::InputArgList *Args) { llvm_unreachable("internal error"); } -std::unique_ptr createFile(StringRef Path) { +// Opens a file. Path has to be resolved already. +// Newly created memory buffers are owned by this driver. +ErrorOr> LinkerDriver::createFile(StringRef Path) { + auto MBOrErr = MemoryBuffer::getFile(Path); + if (auto EC = MBOrErr.getError()) + return EC; + std::unique_ptr MB = std::move(MBOrErr.get()); + MemoryBufferRef MBRef = MB->getMemBufferRef(); + OwningMBs.push_back(std::move(MB)); // take ownership if (StringRef(Path).endswith_lower(".lib")) - return llvm::make_unique(Path); - return llvm::make_unique(Path); + return std::unique_ptr(new ArchiveFile(MBRef)); + return std::unique_ptr(new ObjectFile(MBRef)); } namespace { @@ -95,9 +103,15 @@ LinkerDriver::parseDirectives(StringRef S, return EC; std::unique_ptr Args = std::move(ArgsOrErr.get()); - for (auto *Arg : Args->filtered(OPT_defaultlib)) - if (Optional Path = findLib(Arg->getValue())) - Res->push_back(llvm::make_unique(*Path)); + for (auto *Arg : Args->filtered(OPT_defaultlib)) { + if (Optional Path = findLib(Arg->getValue())) { + auto FileOrErr = createFile(*Path); + if (auto EC = FileOrErr.getError()) + return EC; + std::unique_ptr File = std::move(FileOrErr.get()); + Res->push_back(std::move(File)); + } + } return std::error_code(); } @@ -289,7 +303,13 @@ bool LinkerDriver::link(int Argc, const char *Argv[]) { // Parse all input files and put all symbols to the symbol table. // The symbol table will take care of name resolution. for (StringRef Path : Inputs) { - if (auto EC = Symtab.addFile(createFile(Path))) { + auto FileOrErr = createFile(Path); + if (auto EC = FileOrErr.getError()) { + llvm::errs() << Path << ": " << EC.message() << "\n"; + return false; + } + std::unique_ptr File = std::move(FileOrErr.get()); + if (auto EC = Symtab.addFile(std::move(File))) { llvm::errs() << Path << ": " << EC.message() << "\n"; return false; } diff --git a/lld/COFF/Driver.h b/lld/COFF/Driver.h index f64011c..02efb65 100644 --- a/lld/COFF/Driver.h +++ b/lld/COFF/Driver.h @@ -47,6 +47,9 @@ public: private: StringAllocator Alloc; + // Opens a file. Path has to be resolved already. + ErrorOr> createFile(StringRef Path); + // Searches a file from search paths. Optional findFile(StringRef Filename); Optional findLib(StringRef Filename); @@ -59,6 +62,10 @@ private: std::vector SearchPaths; std::set VisitedFiles; + + // Driver is the owner of all opened files. + // InputFiles have MemoryBufferRefs to them. + std::vector> OwningMBs; }; ErrorOr> diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp index d1a6076..88406be 100644 --- a/lld/COFF/InputFiles.cpp +++ b/lld/COFF/InputFiles.cpp @@ -46,14 +46,8 @@ std::string InputFile::getShortName() { } std::error_code ArchiveFile::parse() { - // Get a memory buffer. - auto MBOrErr = MemoryBuffer::getFile(Filename); - if (auto EC = MBOrErr.getError()) - return EC; - MB = std::move(MBOrErr.get()); - - // Parse a memory buffer as an archive file. - auto ArchiveOrErr = Archive::create(MB->getMemBufferRef()); + // Parse a MemoryBufferRef as an archive file. + auto ArchiveOrErr = Archive::create(MB); if (auto EC = ArchiveOrErr.getError()) return EC; File = std::move(ArchiveOrErr.get()); @@ -89,17 +83,8 @@ ErrorOr ArchiveFile::getMember(const Archive::Symbol *Sym) { } std::error_code ObjectFile::parse() { - // MBRef is not initialized if this is not an archive member. - if (MBRef.getBuffer().empty()) { - auto MBOrErr = MemoryBuffer::getFile(Filename); - if (auto EC = MBOrErr.getError()) - return EC; - MB = std::move(MBOrErr.get()); - MBRef = MB->getMemBufferRef(); - } - // Parse a memory buffer as a COFF file. - auto BinOrErr = createBinary(MBRef); + auto BinOrErr = createBinary(MB); if (auto EC = BinOrErr.getError()) return EC; std::unique_ptr Bin = std::move(BinOrErr.get()); @@ -108,7 +93,7 @@ std::error_code ObjectFile::parse() { Bin.release(); COFFObj.reset(Obj); } else { - return make_dynamic_error_code(Twine(Filename) + " is not a COFF file."); + return make_dynamic_error_code(getName() + " is not a COFF file."); } // Read section and symbol tables. @@ -160,14 +145,14 @@ std::error_code ObjectFile::initializeSymbols() { // Get a COFFSymbolRef object. auto SymOrErr = COFFObj->getSymbol(I); if (auto EC = SymOrErr.getError()) - return make_dynamic_error_code(Twine("broken object file: ") + Filename + + return make_dynamic_error_code("broken object file: " + getName() + ": " + EC.message()); COFFSymbolRef Sym = SymOrErr.get(); // Get a symbol name. StringRef SymbolName; if (auto EC = COFFObj->getSymbolName(Sym, SymbolName)) - return make_dynamic_error_code(Twine("broken object file: ") + Filename + + return make_dynamic_error_code("broken object file: " + getName() + ": " + EC.message()); // Skip special symbols. if (SymbolName == "@comp.id" || SymbolName == "@feat.00") @@ -220,8 +205,8 @@ SymbolBody *ObjectFile::createSymbolBody(StringRef Name, COFFSymbolRef Sym, } std::error_code ImportFile::parse() { - const char *Buf = MBRef.getBufferStart(); - const char *End = MBRef.getBufferEnd(); + const char *Buf = MB.getBufferStart(); + const char *End = MB.getBufferEnd(); const auto *Hdr = reinterpret_cast(Buf); // Check if the total size is valid. diff --git a/lld/COFF/InputFiles.h b/lld/COFF/InputFiles.h index 72e992b..90e4204 100644 --- a/lld/COFF/InputFiles.h +++ b/lld/COFF/InputFiles.h @@ -63,7 +63,7 @@ private: // .lib or .a file. class ArchiveFile : public InputFile { public: - explicit ArchiveFile(StringRef S) : InputFile(ArchiveKind), Filename(S) {} + explicit ArchiveFile(MemoryBufferRef M) : InputFile(ArchiveKind), MB(M) {} static bool classof(const InputFile *F) { return F->kind() == ArchiveKind; } std::error_code parse() override; StringRef getName() override { return Filename; } @@ -79,7 +79,7 @@ public: private: std::unique_ptr File; std::string Filename; - std::unique_ptr MB; + MemoryBufferRef MB; std::vector SymbolBodies; std::set Seen; llvm::MallocAllocator Alloc; @@ -88,13 +88,10 @@ private: // .obj or .o file. This may be a member of an archive file. class ObjectFile : public InputFile { public: - explicit ObjectFile(StringRef S) : InputFile(ObjectKind), Filename(S) {} - ObjectFile(StringRef S, MemoryBufferRef M) - : InputFile(ObjectKind), Filename(S), MBRef(M) {} - + explicit ObjectFile(MemoryBufferRef M) : InputFile(ObjectKind), MB(M) {} static bool classof(const InputFile *F) { return F->kind() == ObjectKind; } std::error_code parse() override; - StringRef getName() override { return Filename; } + StringRef getName() override { return MB.getBufferIdentifier(); } std::vector &getChunks() { return Chunks; } std::vector &getSymbols() override { return SymbolBodies; } @@ -115,10 +112,8 @@ private: SymbolBody *createSymbolBody(StringRef Name, COFFSymbolRef Sym, const void *Aux, bool IsFirst); - std::string Filename; std::unique_ptr COFFObj; - std::unique_ptr MB; - MemoryBufferRef MBRef; + MemoryBufferRef MB; StringRef Directives; llvm::BumpPtrAllocator Alloc; @@ -148,15 +143,15 @@ private: // for details about the format. class ImportFile : public InputFile { public: - explicit ImportFile(MemoryBufferRef M) : InputFile(ImportKind), MBRef(M) {} + explicit ImportFile(MemoryBufferRef M) : InputFile(ImportKind), MB(M) {} static bool classof(const InputFile *F) { return F->kind() == ImportKind; } - StringRef getName() override { return MBRef.getBufferIdentifier(); } + StringRef getName() override { return MB.getBufferIdentifier(); } std::vector &getSymbols() override { return SymbolBodies; } private: std::error_code parse() override; - MemoryBufferRef MBRef; + MemoryBufferRef MB; std::vector SymbolBodies; llvm::BumpPtrAllocator Alloc; StringAllocator StringAlloc; diff --git a/lld/COFF/Symbols.cpp b/lld/COFF/Symbols.cpp index 4fd3451..8bd5e65 100644 --- a/lld/COFF/Symbols.cpp +++ b/lld/COFF/Symbols.cpp @@ -87,7 +87,7 @@ ErrorOr> Lazy::getMember() { if (Magic != file_magic::coff_object) return make_dynamic_error_code("unknown file type"); - std::unique_ptr Obj(new ObjectFile(MBRef.getBufferIdentifier(), MBRef)); + std::unique_ptr Obj(new ObjectFile(MBRef)); Obj->setParentName(File->getName()); return std::move(Obj); } -- 2.7.4