<div dir="ltr">thanks. That is more readable.<div><br></div><div>David</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jun 3, 2016 at 5:43 PM, Vedant Kumar <span dir="ltr"><<a href="mailto:vsk@apple.com" target="_blank">vsk@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Closing the loop on this, this should be done in r271756.<br>
<span class="HOEnZb"><font color="#888888"><br>
vedant<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
> On Jun 3, 2016, at 4:54 PM, Vedant Kumar via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br>
><br>
> Good point, I'll take care of this shortly.<br>
><br>
> thanks<br>
> vedant<br>
><br>
>> On Jun 3, 2016, at 4:53 PM, Xinliang David Li <<a href="mailto:xinliangli@gmail.com">xinliangli@gmail.com</a>> wrote:<br>
>><br>
>> I see -- stringRef needs to be used with more care.<br>
>><br>
>> In this case, is it better to open the buffer in the caller of parseInputFilenamesFile? Passing unique_ptr around does not make that clear especially that the returned Buf looks like not used anywhere (but just to extends its lifetime).<br>
>><br>
>> David<br>
>><br>
>> On Fri, Jun 3, 2016 at 4:46 PM, Vedant Kumar <<a href="mailto:vsk@apple.com">vsk@apple.com</a>> wrote:<br>
>> The WeightedFile instances store StringRefs into the MemoryBuffer backing InputFilenamesFile.<br>
>><br>
>> Previously, the lifetime of the MemoryBuffer ended before we returned from parseInputFilenamesFile().<br>
>><br>
>> With this commit we extend the lifetime of the MemoryBuffer, so the WeightedFile instances are always valid.<br>
>><br>
>> vedant<br>
>><br>
>>> On Jun 3, 2016, at 4:44 PM, Xinliang David Li <<a href="mailto:xinliangli@gmail.com">xinliangli@gmail.com</a>> wrote:<br>
>>><br>
>>> This seems subtle. Can you explain a little more?<br>
>>><br>
>>> David<br>
>>><br>
>>> On Fri, Jun 3, 2016 at 4:12 PM, Vedant Kumar via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br>
>>> Author: vedantk<br>
>>> Date: Fri Jun 3 18:12:38 2016<br>
>>> New Revision: 271748<br>
>>><br>
>>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=271748&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=271748&view=rev</a><br>
>>> Log:<br>
>>> [llvm-profdata] Fix use-after-free from discarded MemoryBuffer (NFC)<br>
>>><br>
>>> Thanks to Justin Bogner for pointing this out!<br>
>>><br>
>>> Caught by ASAN.<br>
>>><br>
>>> Modified:<br>
>>> llvm/trunk/tools/llvm-profdata/llvm-profdata.cpp<br>
>>><br>
>>> Modified: llvm/trunk/tools/llvm-profdata/llvm-profdata.cpp<br>
>>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-profdata/llvm-profdata.cpp?rev=271748&r1=271747&r2=271748&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-profdata/llvm-profdata.cpp?rev=271748&r1=271747&r2=271748&view=diff</a><br>
>>> ==============================================================================<br>
>>> --- llvm/trunk/tools/llvm-profdata/llvm-profdata.cpp (original)<br>
>>> +++ llvm/trunk/tools/llvm-profdata/llvm-profdata.cpp Fri Jun 3 18:12:38 2016<br>
>>> @@ -223,16 +223,19 @@ static WeightedFile parseWeightedFile(co<br>
>>> return WeightedFile(FileName, Weight);<br>
>>> }<br>
>>><br>
>>> -static void parseInputFilenamesFile(const StringRef &InputFilenamesFile,<br>
>>> - WeightedFileVector &WFV) {<br>
>>> +static std::unique_ptr<MemoryBuffer><br>
>>> +parseInputFilenamesFile(const StringRef &InputFilenamesFile,<br>
>>> + WeightedFileVector &WFV) {<br>
>>> if (InputFilenamesFile == "")<br>
>>> - return;<br>
>>> + return {};<br>
>>><br>
>>> - auto Buf = MemoryBuffer::getFileOrSTDIN(InputFilenamesFile);<br>
>>> - if (!Buf)<br>
>>> - exitWithErrorCode(Buf.getError(), InputFilenamesFile);<br>
>>> + auto BufOrError = MemoryBuffer::getFileOrSTDIN(InputFilenamesFile);<br>
>>> + if (!BufOrError)<br>
>>> + exitWithErrorCode(BufOrError.getError(), InputFilenamesFile);<br>
>>> +<br>
>>> + std::unique_ptr<MemoryBuffer> Buffer = std::move(*BufOrError);<br>
>>> + StringRef Data = Buffer->getBuffer();<br>
>>><br>
>>> - StringRef Data = Buf.get()->getBuffer();<br>
>>> SmallVector<StringRef, 8> Entries;<br>
>>> Data.split(Entries, '\n', /*MaxSplit=*/-1, /*KeepEmpty=*/false);<br>
>>> for (const StringRef &FileWeightEntry : Entries) {<br>
>>> @@ -246,6 +249,8 @@ static void parseInputFilenamesFile(cons<br>
>>> else<br>
>>> WFV.emplace_back(parseWeightedFile(SanitizedEntry));<br>
>>> }<br>
>>> +<br>
>>> + return Buffer;<br>
>>> }<br>
>>><br>
>>> static int merge_main(int argc, const char *argv[]) {<br>
>>> @@ -288,7 +293,7 @@ static int merge_main(int argc, const ch<br>
>>> WeightedInputs.push_back(WeightedFile(Filename, 1));<br>
>>> for (StringRef WeightedFilename : WeightedInputFilenames)<br>
>>> WeightedInputs.push_back(parseWeightedFile(WeightedFilename));<br>
>>> - parseInputFilenamesFile(InputFilenamesFile, WeightedInputs);<br>
>>> + auto Buf = parseInputFilenamesFile(InputFilenamesFile, WeightedInputs);<br>
>>><br>
>>> if (WeightedInputs.empty())<br>
>>> exitWithError("No input files specified. See " +<br>
>>><br>
>>><br>
>>> _______________________________________________<br>
>>> llvm-commits mailing list<br>
>>> <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
>>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
>>><br>
>><br>
>><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
<br>
</div></div></blockquote></div><br></div>