<div dir="ltr"><div dir="ltr"><br><div>(below)</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Sep 8, 2017 at 9:15 PM Richard Smith via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: rsmith<br>
Date: Fri Sep 8 18:14:04 2017<br>
New Revision: 312851<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=312851&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=312851&view=rev</a><br>
Log:<br>
Fix ownership of the MemoryBuffer in a FrontendInputFile.<br>
<br>
This fixes a possible crash on certain kinds of corrupted AST file, but<br>
checking in an AST file corrupted in just the right way will be a maintenance<br>
nightmare because the format changes frequently.<br>
<br>
Modified:<br>
cfe/trunk/include/clang/Basic/SourceManager.h<br>
cfe/trunk/include/clang/Frontend/FrontendOptions.h<br>
cfe/trunk/lib/Basic/SourceManager.cpp<br>
cfe/trunk/lib/Frontend/CompilerInstance.cpp<br>
cfe/trunk/lib/Frontend/FrontendAction.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Basic/SourceManager.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/SourceManager.h?rev=312851&r1=312850&r2=312851&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/SourceManager.h?rev=312851&r1=312850&r2=312851&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Basic/SourceManager.h (original)<br>
+++ cfe/trunk/include/clang/Basic/SourceManager.h Fri Sep 8 18:14:04 2017<br>
@@ -212,12 +212,6 @@ namespace SrcMgr {<br>
/// this content cache. This is used for performance analysis.<br>
llvm::MemoryBuffer::BufferKind getMemoryBufferKind() const;<br>
<br>
- void setBuffer(std::unique_ptr<llvm::MemoryBuffer> B) {<br>
- assert(!Buffer.getPointer() && "MemoryBuffer already set.");<br>
- Buffer.setPointer(B.release());<br>
- Buffer.setInt(0);<br>
- }<br>
-<br>
/// \brief Get the underlying buffer, returning NULL if the buffer is not<br>
/// yet available.<br>
llvm::MemoryBuffer *getRawBuffer() const { return Buffer.getPointer(); }<br>
@@ -816,7 +810,22 @@ public:<br>
SrcMgr::CharacteristicKind FileCharacter = SrcMgr::C_User,<br>
int LoadedID = 0, unsigned LoadedOffset = 0,<br>
SourceLocation IncludeLoc = SourceLocation()) {<br>
- return createFileID(createMemBufferContentCache(std::move(Buffer)),<br>
+ return createFileID(<br>
+ createMemBufferContentCache(Buffer.release(), /*DoNotFree*/ false),<br>
+ IncludeLoc, FileCharacter, LoadedID, LoadedOffset);<br>
+ }<br>
+<br>
+ enum UnownedTag { Unowned };<br>
+<br>
+ /// \brief Create a new FileID that represents the specified memory buffer.<br>
+ ///<br>
+ /// This does no caching of the buffer and takes ownership of the<br></blockquote><div><br></div><div>Is the "and takes ownership" part correct for this new overload?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ /// MemoryBuffer, so only pass a MemoryBuffer to this once.<br>
+ FileID createFileID(UnownedTag, llvm::MemoryBuffer *Buffer,<br>
+ SrcMgr::CharacteristicKind FileCharacter = SrcMgr::C_User,<br>
+ int LoadedID = 0, unsigned LoadedOffset = 0,<br>
+ SourceLocation IncludeLoc = SourceLocation()) {<br>
+ return createFileID(createMemBufferContentCache(Buffer, /*DoNotFree*/true),<br>
IncludeLoc, FileCharacter, LoadedID, LoadedOffset);<br>
}<br>
<br>
@@ -1699,7 +1708,7 @@ private:<br>
<br>
/// \brief Create a new ContentCache for the specified memory buffer.<br>
const SrcMgr::ContentCache *<br>
- createMemBufferContentCache(std::unique_ptr<llvm::MemoryBuffer> Buf);<br>
+ createMemBufferContentCache(llvm::MemoryBuffer *Buf, bool DoNotFree);<br>
<br>
FileID getFileIDSlow(unsigned SLocOffset) const;<br>
FileID getFileIDLocal(unsigned SLocOffset) const;<br>
<br>
Modified: cfe/trunk/include/clang/Frontend/FrontendOptions.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/FrontendOptions.h?rev=312851&r1=312850&r2=312851&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/FrontendOptions.h?rev=312851&r1=312850&r2=312851&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Frontend/FrontendOptions.h (original)<br>
+++ cfe/trunk/include/clang/Frontend/FrontendOptions.h Fri Sep 8 18:14:04 2017<br>
@@ -128,21 +128,24 @@ class FrontendInputFile {<br>
/// \brief The file name, or "-" to read from standard input.<br>
std::string File;<br>
<br>
- llvm::MemoryBuffer *Buffer;<br>
+ /// The input, if it comes from a buffer rather than a file. This object<br>
+ /// does not own the buffer, and the caller is responsible for ensuring<br>
+ /// that it outlives any users.<br>
+ llvm::MemoryBuffer *Buffer = nullptr;<br>
<br>
/// \brief The kind of input, e.g., C source, AST file, LLVM IR.<br>
InputKind Kind;<br>
<br>
/// \brief Whether we're dealing with a 'system' input (vs. a 'user' input).<br>
- bool IsSystem;<br>
+ bool IsSystem = false;<br>
<br>
public:<br>
- FrontendInputFile() : Buffer(nullptr), Kind(), IsSystem(false) { }<br>
+ FrontendInputFile() { }<br>
FrontendInputFile(StringRef File, InputKind Kind, bool IsSystem = false)<br>
- : File(File.str()), Buffer(nullptr), Kind(Kind), IsSystem(IsSystem) { }<br>
- FrontendInputFile(llvm::MemoryBuffer *buffer, InputKind Kind,<br>
+ : File(File.str()), Kind(Kind), IsSystem(IsSystem) { }<br>
+ FrontendInputFile(llvm::MemoryBuffer *Buffer, InputKind Kind,<br>
bool IsSystem = false)<br>
- : Buffer(buffer), Kind(Kind), IsSystem(IsSystem) { }<br>
+ : Buffer(Buffer), Kind(Kind), IsSystem(IsSystem) { }<br>
<br>
InputKind getKind() const { return Kind; }<br>
bool isSystem() const { return IsSystem; }<br>
<br>
Modified: cfe/trunk/lib/Basic/SourceManager.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/SourceManager.cpp?rev=312851&r1=312850&r2=312851&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/SourceManager.cpp?rev=312851&r1=312850&r2=312851&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Basic/SourceManager.cpp (original)<br>
+++ cfe/trunk/lib/Basic/SourceManager.cpp Fri Sep 8 18:14:04 2017<br>
@@ -409,15 +409,16 @@ SourceManager::getOrCreateContentCache(c<br>
}<br>
<br>
<br>
-/// createMemBufferContentCache - Create a new ContentCache for the specified<br>
-/// memory buffer. This does no caching.<br>
-const ContentCache *SourceManager::createMemBufferContentCache(<br>
- std::unique_ptr<llvm::MemoryBuffer> Buffer) {<br>
+/// Create a new ContentCache for the specified memory buffer.<br>
+/// This does no caching.<br>
+const ContentCache *<br>
+SourceManager::createMemBufferContentCache(llvm::MemoryBuffer *Buffer,<br>
+ bool DoNotFree) {<br>
// Add a new ContentCache to the MemBufferInfos list and return it.<br>
ContentCache *Entry = ContentCacheAlloc.Allocate<ContentCache>();<br>
new (Entry) ContentCache();<br>
MemBufferInfos.push_back(Entry);<br>
- Entry->setBuffer(std::move(Buffer));<br>
+ Entry->replaceBuffer(Buffer, DoNotFree);<br>
return Entry;<br>
}<br>
<br>
<br>
Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=312851&r1=312850&r2=312851&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=312851&r1=312850&r2=312851&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)<br>
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Fri Sep 8 18:14:04 2017<br>
@@ -838,8 +838,8 @@ bool CompilerInstance::InitializeSourceM<br>
: Input.isSystem() ? SrcMgr::C_System : SrcMgr::C_User;<br>
<br>
if (Input.isBuffer()) {<br>
- SourceMgr.setMainFileID(SourceMgr.createFileID(<br>
- std::unique_ptr<llvm::MemoryBuffer>(Input.getBuffer()), Kind));<br>
+ SourceMgr.setMainFileID(SourceMgr.createFileID(SourceManager::Unowned,<br>
+ Input.getBuffer(), Kind));<br>
assert(SourceMgr.getMainFileID().isValid() &&<br>
"Couldn't establish MainFileID!");<br>
return true;<br>
<br>
Modified: cfe/trunk/lib/Frontend/FrontendAction.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendAction.cpp?rev=312851&r1=312850&r2=312851&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendAction.cpp?rev=312851&r1=312850&r2=312851&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Frontend/FrontendAction.cpp (original)<br>
+++ cfe/trunk/lib/Frontend/FrontendAction.cpp Fri Sep 8 18:14:04 2017<br>
@@ -754,10 +754,11 @@ bool FrontendAction::BeginSourceFile(Com<br>
goto failure;<br>
<br>
// Reinitialize the main file entry to refer to the new input.<br>
- if (!CI.InitializeSourceManager(FrontendInputFile(<br>
- Buffer.release(), Input.getKind().withFormat(InputKind::Source),<br>
- CurrentModule->IsSystem)))<br>
- goto failure;<br>
+ auto Kind = CurrentModule->IsSystem ? SrcMgr::C_System : SrcMgr::C_User;<br>
+ auto &SourceMgr = CI.getSourceManager();<br>
+ auto BufferID = SourceMgr.createFileID(std::move(Buffer), Kind);<br>
+ assert(BufferID.isValid() && "couldn't creaate module buffer ID");<br>
+ SourceMgr.setMainFileID(BufferID);<br>
}<br>
}<br>
<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div>