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

[Vedant Kumar via llvm-commits]

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
> >
> 
>