[llvm] r271748 - [llvm-profdata] Fix use-after-free from discarded MemoryBuffer (NFC)

[Vedant Kumar via llvm-commits]

Closing the loop on this, this should be done in r271756.

vedant

> On Jun 3, 2016, at 4:54 PM, Vedant Kumar via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Good point, I'll take care of this shortly.
> 
> thanks
> vedant
> 
>> On Jun 3, 2016, at 4:53 PM, Xinliang David Li <xinliangli at gmail.com> wrote:
>> 
>> I see -- stringRef needs to be used with more care.
>> 
>> 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).
>> 
>> David
>> 
>> On Fri, Jun 3, 2016 at 4:46 PM, Vedant Kumar <vsk at apple.com> wrote:
>> The WeightedFile instances store StringRefs into the MemoryBuffer backing InputFilenamesFile.
>> 
>> Previously, the lifetime of the MemoryBuffer ended before we returned from parseInputFilenamesFile().
>> 
>> With this commit we extend the lifetime of the MemoryBuffer, so the WeightedFile instances are always valid.
>> 
>> vedant
>> 
>>> On Jun 3, 2016, at 4:44 PM, Xinliang David Li <xinliangli at gmail.com> wrote:
>>> 
>>> This seems subtle. Can you explain a little more?
>>> 
>>> David
>>> 
>>> On Fri, Jun 3, 2016 at 4:12 PM, Vedant Kumar via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>> Author: vedantk
>>> Date: Fri Jun  3 18:12:38 2016
>>> New Revision: 271748
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=271748&view=rev
>>> Log:
>>> [llvm-profdata] Fix use-after-free from discarded MemoryBuffer (NFC)
>>> 
>>> Thanks to Justin Bogner for pointing this out!
>>> 
>>> Caught by ASAN.
>>> 
>>> Modified:
>>>    llvm/trunk/tools/llvm-profdata/llvm-profdata.cpp
>>> 
>>> Modified: llvm/trunk/tools/llvm-profdata/llvm-profdata.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-profdata/llvm-profdata.cpp?rev=271748&r1=271747&r2=271748&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/tools/llvm-profdata/llvm-profdata.cpp (original)
>>> +++ llvm/trunk/tools/llvm-profdata/llvm-profdata.cpp Fri Jun  3 18:12:38 2016
>>> @@ -223,16 +223,19 @@ static WeightedFile parseWeightedFile(co
>>>   return WeightedFile(FileName, Weight);
>>> }
>>> 
>>> -static void parseInputFilenamesFile(const StringRef &InputFilenamesFile,
>>> -                                    WeightedFileVector &WFV) {
>>> +static std::unique_ptr<MemoryBuffer>
>>> +parseInputFilenamesFile(const StringRef &InputFilenamesFile,
>>> +                        WeightedFileVector &WFV) {
>>>   if (InputFilenamesFile == "")
>>> -    return;
>>> +    return {};
>>> 
>>> -  auto Buf = MemoryBuffer::getFileOrSTDIN(InputFilenamesFile);
>>> -  if (!Buf)
>>> -    exitWithErrorCode(Buf.getError(), InputFilenamesFile);
>>> +  auto BufOrError = MemoryBuffer::getFileOrSTDIN(InputFilenamesFile);
>>> +  if (!BufOrError)
>>> +    exitWithErrorCode(BufOrError.getError(), InputFilenamesFile);
>>> +
>>> +  std::unique_ptr<MemoryBuffer> Buffer = std::move(*BufOrError);
>>> +  StringRef Data = Buffer->getBuffer();
>>> 
>>> -  StringRef Data = Buf.get()->getBuffer();
>>>   SmallVector<StringRef, 8> Entries;
>>>   Data.split(Entries, '\n', /*MaxSplit=*/-1, /*KeepEmpty=*/false);
>>>   for (const StringRef &FileWeightEntry : Entries) {
>>> @@ -246,6 +249,8 @@ static void parseInputFilenamesFile(cons
>>>     else
>>>       WFV.emplace_back(parseWeightedFile(SanitizedEntry));
>>>   }
>>> +
>>> +  return Buffer;
>>> }
>>> 
>>> static int merge_main(int argc, const char *argv[]) {
>>> @@ -288,7 +293,7 @@ static int merge_main(int argc, const ch
>>>     WeightedInputs.push_back(WeightedFile(Filename, 1));
>>>   for (StringRef WeightedFilename : WeightedInputFilenames)
>>>     WeightedInputs.push_back(parseWeightedFile(WeightedFilename));
>>> -  parseInputFilenamesFile(InputFilenamesFile, WeightedInputs);
>>> +  auto Buf = parseInputFilenamesFile(InputFilenamesFile, WeightedInputs);
>>> 
>>>   if (WeightedInputs.empty())
>>>     exitWithError("No input files specified. See " +
>>> 
>>> 
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>> 
>> 
>> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits