[clang-tools-extra] r256562 - [clang-tidy] Fix a use-after-free bug found by asan

[Alexander Kornienko via cfe-commits]

On Tue, Dec 29, 2015 at 5:22 PM, Benjamin Kramer <benny.kra at gmail.com>
wrote:

> On Tue, Dec 29, 2015 at 5:14 PM, Alexander Kornienko via cfe-commits
> <cfe-commits at lists.llvm.org> wrote:
> > Author: alexfh
> > Date: Tue Dec 29 10:14:38 2015
> > New Revision: 256562
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=256562&view=rev
> > Log:
> > [clang-tidy] Fix a use-after-free bug found by asan
> >
> > Modified:
> >
>  clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
> >
> > Modified:
> clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp?rev=256562&r1=256561&r2=256562&view=diff
> >
> ==============================================================================
> > ---
> clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
> (original)
> > +++
> clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
> Tue Dec 29 10:14:38 2015
> > @@ -491,9 +491,10 @@ void SimplifyBooleanExprCheck::check(con
> >  bool containsDiscardedTokens(
> >      const ast_matchers::MatchFinder::MatchResult &Result,
> >      CharSourceRange CharRange) {
> > -  StringRef ReplacementText =
> > +  std::string ReplacementText =
> >        Lexer::getSourceText(CharRange, *Result.SourceManager,
> > -                           Result.Context->getLangOpts()).str();
> > +                           Result.Context->getLangOpts())
> > +          .str();
>
> Can you just drop the .str() and change the type back to StringRef?
>

That was what I asked the author of the original patch. However, Lexer
expects the buffer to end with a '\0', so the author did only a half of my
suggestion and I skipped the issue ;)

An alternative solution would be to create a lexer from the whole buffer,
but then the code would be much more complicated for no reason. Keep in
mind that we only do this when we're generating a warning anyway, so
copying a string is not going to be a bottleneck.


>
> - Ben
>
> >    Lexer Lex(CharRange.getBegin(), Result.Context->getLangOpts(),
> >              ReplacementText.data(), ReplacementText.data(),
> >              ReplacementText.data() + ReplacementText.size());
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151229/3263b679/attachment.html>