<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Nov 7, 2014 at 12:52 PM, Nick Kledzik <span dir="ltr"><<a href="mailto:kledzik@apple.com" target="_blank">kledzik@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: kledzik<br>
Date: Fri Nov 7 14:52:38 2014<br>
New Revision: 221544<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=221544&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=221544&view=rev</a><br>
Log:<br>
Fix FileArchive member MemoryBuffer early destruction<br>
<br>
When FileArchive loads a member, it instantiates a temporary MemoryBuffer<br>
which points to the member range of the archive file. The problem is that the<br>
object file parsers call getBufferIndentifer() on that temporary MemoryBuffer<br>
and store that StringRef as the _path data member for that lld::File. When<br>
FileArchive::instantiateMember() goes out of scope the MemoryBuffer is deleted<br>
and the File::._path becomes a dangling reference.<br>
<br>
The fix adds a vector<> to FileArchive to own the instantiated MemoryBuffers.<br>
In addition it fixes member's path to be the standard format<br>
(e.g. "/path/libfoo.a(foo.o)") instead of just the leaf name.<br>
<br>
Modified:<br>
lld/trunk/lib/ReaderWriter/FileArchive.cpp<br>
<br>
Modified: lld/trunk/lib/ReaderWriter/FileArchive.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/FileArchive.cpp?rev=221544&r1=221543&r2=221544&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/FileArchive.cpp?rev=221544&r1=221543&r2=221544&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/lib/ReaderWriter/FileArchive.cpp (original)<br>
+++ lld/trunk/lib/ReaderWriter/FileArchive.cpp Fri Nov 7 14:52:38 2014<br>
@@ -133,18 +133,27 @@ private:<br>
if (std::error_code ec = mbOrErr.getError())<br>
return ec;<br>
llvm::MemoryBufferRef mb = mbOrErr.get();<br>
+ std::string memberPath = (_archive->getFileName() + "("<br>
+ + mb.getBufferIdentifier() + ")").str();<br>
+<br>
if (_logLoading)<br>
- llvm::outs() << _archive->getFileName() << "(" << mb.getBufferIdentifier()<br>
- << ")"<br>
- << "\n";<br>
+ llvm::errs() << memberPath << "\n";<br>
<br>
- std::unique_ptr<MemoryBuffer> buf(MemoryBuffer::getMemBuffer(<br>
- mb.getBuffer(), mb.getBufferIdentifier(), false));<br>
+ std::unique_ptr<MemoryBuffer> memberMB(MemoryBuffer::getMemBuffer(<br>
+ mb.getBuffer(), memberPath, false));<br></blockquote><div><br>It's somewhat handy to use copy/move initialization rather than direct initialization where possible. ie favor:<br><br>std::unique_ptr<MemoryBuffer> memberMB = MemoryBuffer::getMemBuffer(...);<br><br>over "memberMB(MemoryBuffer::getMemBuffer..." in this case.<br><br>That way it's clear that the function returns ownership via unique_ptr, not raw pointer - and there's no risk of an explicit conversion from raw pointer to unique_ptr occurring accidentally (while in this case it's pretty clear/known that getMemBuffer returns ownership, writing it with move initialization makes that more explicit).<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
std::vector<std::unique_ptr<File>> files;<br>
- _registry.parseFile(buf, files);<br>
+ _registry.parseFile(memberMB, files);<br>
assert(files.size() == 1);<br>
result = std::move(files[0]);<br>
+<br>
+ // Note: The object file parsers use getBufferIdentifier() from memberMB<br>
+ // for the file path. And MemoryBuffer makes its own copy of the path.<br>
+ // That means when if memberMB is destroyed, the lld:File objects will<br>
+ // have a dangling reference for their path. To fix that, all the<br>
+ // MemoryBuffers for the archive members are owned by _memberBuffers.<br>
+ _memberBuffers.push_back(std::move(memberMB));<br>
+<br>
return std::error_code();<br>
}<br>
<br>
@@ -205,6 +214,7 @@ private:<br>
atom_collection_vector<AbsoluteAtom> _absoluteAtoms;<br>
bool _isWholeArchive;<br>
bool _logLoading;<br>
+ mutable std::vector<std::unique_ptr<MemoryBuffer>> _memberBuffers;<br>
};<br>
<br>
class ArchiveReader : public Reader {<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><br></div></div>