<p dir="ltr">Thanks! </p>
<div class="gmail_quote">On May 31, 2015 5:10 PM, "Rui Ueyama" <<a href="mailto:ruiu@google.com">ruiu@google.com</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: ruiu<br>
Date: Sun May 31 16:04:56 2015<br>
New Revision: 238690<br>
<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D238690-26view-3Drev&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=2Agh7PWBLAGzReqXlEyvtX8i1OtAgSv89AIZGtwKbM4&s=3cABOw5VmtMTUf-BSSu3DUkt9U0j8bUll22dQ1iM8zQ&e=" target="_blank">http://llvm.org/viewvc/llvm-project?rev=238690&view=rev</a><br>
Log:<br>
COFF: Make the Driver own all MemoryBuffers. NFC.<br>
<br>
Previously, a MemoryBuffer of a file was owned by each InputFile object.<br>
This patch makes the Driver own all of them. InputFiles now have only<br>
MemoryBufferRefs. This change simplifies ownership managment<br>
(particularly for ObjectFile -- the object owned a MemoryBuffer only when<br>
it's not created from an archive file, because in that case a parent<br>
archive file owned the entire buffer. Now it owns nothing unconditionally.)<br>
<br>
Modified:<br>
lld/trunk/COFF/Driver.cpp<br>
lld/trunk/COFF/Driver.h<br>
lld/trunk/COFF/InputFiles.cpp<br>
lld/trunk/COFF/InputFiles.h<br>
lld/trunk/COFF/Symbols.cpp<br>
<br>
Modified: lld/trunk/COFF/Driver.cpp<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_lld_trunk_COFF_Driver.cpp-3Frev-3D238690-26r1-3D238689-26r2-3D238690-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=2Agh7PWBLAGzReqXlEyvtX8i1OtAgSv89AIZGtwKbM4&s=-4g5p-h7IHb7fv-tDJIha87Cn-AueIdc8Cdkbq4ccVk&e=" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Driver.cpp?rev=238690&r1=238689&r2=238690&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/COFF/Driver.cpp (original)<br>
+++ lld/trunk/COFF/Driver.cpp Sun May 31 16:04:56 2015<br>
@@ -60,10 +60,18 @@ static std::string getOutputPath(llvm::o<br>
llvm_unreachable("internal error");<br>
}<br>
<br>
-std::unique_ptr<InputFile> createFile(StringRef Path) {<br>
+// Opens a file. Path has to be resolved already.<br>
+// Newly created memory buffers are owned by this driver.<br>
+ErrorOr<std::unique_ptr<InputFile>> LinkerDriver::createFile(StringRef Path) {<br>
+ auto MBOrErr = MemoryBuffer::getFile(Path);<br>
+ if (auto EC = MBOrErr.getError())<br>
+ return EC;<br>
+ std::unique_ptr<MemoryBuffer> MB = std::move(MBOrErr.get());<br>
+ MemoryBufferRef MBRef = MB->getMemBufferRef();<br>
+ OwningMBs.push_back(std::move(MB)); // take ownership<br>
if (StringRef(Path).endswith_lower(".lib"))<br>
- return llvm::make_unique<ArchiveFile>(Path);<br>
- return llvm::make_unique<ObjectFile>(Path);<br>
+ return std::unique_ptr<InputFile>(new ArchiveFile(MBRef));<br>
+ return std::unique_ptr<InputFile>(new ObjectFile(MBRef));<br>
}<br>
<br>
namespace {<br>
@@ -95,9 +103,15 @@ LinkerDriver::parseDirectives(StringRef<br>
return EC;<br>
std::unique_ptr<llvm::opt::InputArgList> Args = std::move(ArgsOrErr.get());<br>
<br>
- for (auto *Arg : Args->filtered(OPT_defaultlib))<br>
- if (Optional<StringRef> Path = findLib(Arg->getValue()))<br>
- Res->push_back(llvm::make_unique<ArchiveFile>(*Path));<br>
+ for (auto *Arg : Args->filtered(OPT_defaultlib)) {<br>
+ if (Optional<StringRef> Path = findLib(Arg->getValue())) {<br>
+ auto FileOrErr = createFile(*Path);<br>
+ if (auto EC = FileOrErr.getError())<br>
+ return EC;<br>
+ std::unique_ptr<InputFile> File = std::move(FileOrErr.get());<br>
+ Res->push_back(std::move(File));<br>
+ }<br>
+ }<br>
return std::error_code();<br>
}<br>
<br>
@@ -289,7 +303,13 @@ bool LinkerDriver::link(int Argc, const<br>
// Parse all input files and put all symbols to the symbol table.<br>
// The symbol table will take care of name resolution.<br>
for (StringRef Path : Inputs) {<br>
- if (auto EC = Symtab.addFile(createFile(Path))) {<br>
+ auto FileOrErr = createFile(Path);<br>
+ if (auto EC = FileOrErr.getError()) {<br>
+ llvm::errs() << Path << ": " << EC.message() << "\n";<br>
+ return false;<br>
+ }<br>
+ std::unique_ptr<InputFile> File = std::move(FileOrErr.get());<br>
+ if (auto EC = Symtab.addFile(std::move(File))) {<br>
llvm::errs() << Path << ": " << EC.message() << "\n";<br>
return false;<br>
}<br>
<br>
Modified: lld/trunk/COFF/Driver.h<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_lld_trunk_COFF_Driver.h-3Frev-3D238690-26r1-3D238689-26r2-3D238690-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=2Agh7PWBLAGzReqXlEyvtX8i1OtAgSv89AIZGtwKbM4&s=KA0XPprxtW_gd_o2VyTHklCZbM3N9jMIeSzmhvS5DZg&e=" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Driver.h?rev=238690&r1=238689&r2=238690&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/COFF/Driver.h (original)<br>
+++ lld/trunk/COFF/Driver.h Sun May 31 16:04:56 2015<br>
@@ -47,6 +47,9 @@ public:<br>
private:<br>
StringAllocator Alloc;<br>
<br>
+ // Opens a file. Path has to be resolved already.<br>
+ ErrorOr<std::unique_ptr<InputFile>> createFile(StringRef Path);<br>
+<br>
// Searches a file from search paths.<br>
Optional<StringRef> findFile(StringRef Filename);<br>
Optional<StringRef> findLib(StringRef Filename);<br>
@@ -59,6 +62,10 @@ private:<br>
<br>
std::vector<StringRef> SearchPaths;<br>
std::set<std::string> VisitedFiles;<br>
+<br>
+ // Driver is the owner of all opened files.<br>
+ // InputFiles have MemoryBufferRefs to them.<br>
+ std::vector<std::unique_ptr<MemoryBuffer>> OwningMBs;<br>
};<br>
<br>
ErrorOr<std::unique_ptr<llvm::opt::InputArgList>><br>
<br>
Modified: lld/trunk/COFF/InputFiles.cpp<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_lld_trunk_COFF_InputFiles.cpp-3Frev-3D238690-26r1-3D238689-26r2-3D238690-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=2Agh7PWBLAGzReqXlEyvtX8i1OtAgSv89AIZGtwKbM4&s=nqwXHdToFDgCGtR6DIOSxOpLBHV72HYg8t9Jftg_Vug&e=" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/InputFiles.cpp?rev=238690&r1=238689&r2=238690&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/COFF/InputFiles.cpp (original)<br>
+++ lld/trunk/COFF/InputFiles.cpp Sun May 31 16:04:56 2015<br>
@@ -46,14 +46,8 @@ std::string InputFile::getShortName() {<br>
}<br>
<br>
std::error_code ArchiveFile::parse() {<br>
- // Get a memory buffer.<br>
- auto MBOrErr = MemoryBuffer::getFile(Filename);<br>
- if (auto EC = MBOrErr.getError())<br>
- return EC;<br>
- MB = std::move(MBOrErr.get());<br>
-<br>
- // Parse a memory buffer as an archive file.<br>
- auto ArchiveOrErr = Archive::create(MB->getMemBufferRef());<br>
+ // Parse a MemoryBufferRef as an archive file.<br>
+ auto ArchiveOrErr = Archive::create(MB);<br>
if (auto EC = ArchiveOrErr.getError())<br>
return EC;<br>
File = std::move(ArchiveOrErr.get());<br>
@@ -89,17 +83,8 @@ ErrorOr<MemoryBufferRef> ArchiveFile::ge<br>
}<br>
<br>
std::error_code ObjectFile::parse() {<br>
- // MBRef is not initialized if this is not an archive member.<br>
- if (MBRef.getBuffer().empty()) {<br>
- auto MBOrErr = MemoryBuffer::getFile(Filename);<br>
- if (auto EC = MBOrErr.getError())<br>
- return EC;<br>
- MB = std::move(MBOrErr.get());<br>
- MBRef = MB->getMemBufferRef();<br>
- }<br>
-<br>
// Parse a memory buffer as a COFF file.<br>
- auto BinOrErr = createBinary(MBRef);<br>
+ auto BinOrErr = createBinary(MB);<br>
if (auto EC = BinOrErr.getError())<br>
return EC;<br>
std::unique_ptr<Binary> Bin = std::move(BinOrErr.get());<br>
@@ -108,7 +93,7 @@ std::error_code ObjectFile::parse() {<br>
Bin.release();<br>
COFFObj.reset(Obj);<br>
} else {<br>
- return make_dynamic_error_code(Twine(Filename) + " is not a COFF file.");<br>
+ return make_dynamic_error_code(getName() + " is not a COFF file.");<br>
}<br>
<br>
// Read section and symbol tables.<br>
@@ -160,14 +145,14 @@ std::error_code ObjectFile::initializeSy<br>
// Get a COFFSymbolRef object.<br>
auto SymOrErr = COFFObj->getSymbol(I);<br>
if (auto EC = SymOrErr.getError())<br>
- return make_dynamic_error_code(Twine("broken object file: ") + Filename +<br>
+ return make_dynamic_error_code("broken object file: " + getName() +<br>
": " + EC.message());<br>
COFFSymbolRef Sym = SymOrErr.get();<br>
<br>
// Get a symbol name.<br>
StringRef SymbolName;<br>
if (auto EC = COFFObj->getSymbolName(Sym, SymbolName))<br>
- return make_dynamic_error_code(Twine("broken object file: ") + Filename +<br>
+ return make_dynamic_error_code("broken object file: " + getName() +<br>
": " + EC.message());<br>
// Skip special symbols.<br>
if (SymbolName == "@<a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__comp.id&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=2Agh7PWBLAGzReqXlEyvtX8i1OtAgSv89AIZGtwKbM4&s=19ewPVkvlW1ZHAGVm4yPhhRUPMtI0odS7RNAh85Bs-k&e=" target="_blank">comp.id</a>" || SymbolName == "@feat.00")<br>
@@ -220,8 +205,8 @@ SymbolBody *ObjectFile::createSymbolBody<br>
}<br>
<br>
std::error_code ImportFile::parse() {<br>
- const char *Buf = MBRef.getBufferStart();<br>
- const char *End = MBRef.getBufferEnd();<br>
+ const char *Buf = MB.getBufferStart();<br>
+ const char *End = MB.getBufferEnd();<br>
const auto *Hdr = reinterpret_cast<const coff_import_header *>(Buf);<br>
<br>
// Check if the total size is valid.<br>
<br>
Modified: lld/trunk/COFF/InputFiles.h<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_lld_trunk_COFF_InputFiles.h-3Frev-3D238690-26r1-3D238689-26r2-3D238690-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=2Agh7PWBLAGzReqXlEyvtX8i1OtAgSv89AIZGtwKbM4&s=bwIMgnrW4rGnFywZ36Mc0J_ogK_VBtDcG5VH-oBL008&e=" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/InputFiles.h?rev=238690&r1=238689&r2=238690&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/COFF/InputFiles.h (original)<br>
+++ lld/trunk/COFF/InputFiles.h Sun May 31 16:04:56 2015<br>
@@ -63,7 +63,7 @@ private:<br>
// .lib or .a file.<br>
class ArchiveFile : public InputFile {<br>
public:<br>
- explicit ArchiveFile(StringRef S) : InputFile(ArchiveKind), Filename(S) {}<br>
+ explicit ArchiveFile(MemoryBufferRef M) : InputFile(ArchiveKind), MB(M) {}<br>
static bool classof(const InputFile *F) { return F->kind() == ArchiveKind; }<br>
std::error_code parse() override;<br>
StringRef getName() override { return Filename; }<br>
@@ -79,7 +79,7 @@ public:<br>
private:<br>
std::unique_ptr<Archive> File;<br>
std::string Filename;<br>
- std::unique_ptr<MemoryBuffer> MB;<br>
+ MemoryBufferRef MB;<br>
std::vector<SymbolBody *> SymbolBodies;<br>
std::set<const char *> Seen;<br>
llvm::MallocAllocator Alloc;<br>
@@ -88,13 +88,10 @@ private:<br>
// .obj or .o file. This may be a member of an archive file.<br>
class ObjectFile : public InputFile {<br>
public:<br>
- explicit ObjectFile(StringRef S) : InputFile(ObjectKind), Filename(S) {}<br>
- ObjectFile(StringRef S, MemoryBufferRef M)<br>
- : InputFile(ObjectKind), Filename(S), MBRef(M) {}<br>
-<br>
+ explicit ObjectFile(MemoryBufferRef M) : InputFile(ObjectKind), MB(M) {}<br>
static bool classof(const InputFile *F) { return F->kind() == ObjectKind; }<br>
std::error_code parse() override;<br>
- StringRef getName() override { return Filename; }<br>
+ StringRef getName() override { return MB.getBufferIdentifier(); }<br>
std::vector<Chunk *> &getChunks() { return Chunks; }<br>
std::vector<SymbolBody *> &getSymbols() override { return SymbolBodies; }<br>
<br>
@@ -115,10 +112,8 @@ private:<br>
SymbolBody *createSymbolBody(StringRef Name, COFFSymbolRef Sym,<br>
const void *Aux, bool IsFirst);<br>
<br>
- std::string Filename;<br>
std::unique_ptr<COFFObjectFile> COFFObj;<br>
- std::unique_ptr<MemoryBuffer> MB;<br>
- MemoryBufferRef MBRef;<br>
+ MemoryBufferRef MB;<br>
StringRef Directives;<br>
llvm::BumpPtrAllocator Alloc;<br>
<br>
@@ -148,15 +143,15 @@ private:<br>
// for details about the format.<br>
class ImportFile : public InputFile {<br>
public:<br>
- explicit ImportFile(MemoryBufferRef M) : InputFile(ImportKind), MBRef(M) {}<br>
+ explicit ImportFile(MemoryBufferRef M) : InputFile(ImportKind), MB(M) {}<br>
static bool classof(const InputFile *F) { return F->kind() == ImportKind; }<br>
- StringRef getName() override { return MBRef.getBufferIdentifier(); }<br>
+ StringRef getName() override { return MB.getBufferIdentifier(); }<br>
std::vector<SymbolBody *> &getSymbols() override { return SymbolBodies; }<br>
<br>
private:<br>
std::error_code parse() override;<br>
<br>
- MemoryBufferRef MBRef;<br>
+ MemoryBufferRef MB;<br>
std::vector<SymbolBody *> SymbolBodies;<br>
llvm::BumpPtrAllocator Alloc;<br>
StringAllocator StringAlloc;<br>
<br>
Modified: lld/trunk/COFF/Symbols.cpp<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_lld_trunk_COFF_Symbols.cpp-3Frev-3D238690-26r1-3D238689-26r2-3D238690-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=2Agh7PWBLAGzReqXlEyvtX8i1OtAgSv89AIZGtwKbM4&s=ZWe_b7laOD32WjnBy1zd8mIszT2s65Zb_dSEwLX1f1Y&e=" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Symbols.cpp?rev=238690&r1=238689&r2=238690&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/COFF/Symbols.cpp (original)<br>
+++ lld/trunk/COFF/Symbols.cpp Sun May 31 16:04:56 2015<br>
@@ -87,7 +87,7 @@ ErrorOr<std::unique_ptr<InputFile>> Lazy<br>
if (Magic != file_magic::coff_object)<br>
return make_dynamic_error_code("unknown file type");<br>
<br>
- std::unique_ptr<InputFile> Obj(new ObjectFile(MBRef.getBufferIdentifier(), MBRef));<br>
+ std::unique_ptr<InputFile> Obj(new ObjectFile(MBRef));<br>
Obj->setParentName(File->getName());<br>
return std::move(Obj);<br>
}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>