I don't fully understand your explanation. Is your change introducing a memory leak? (or was the old code causing a double free (if EnterTokenStream does take ownership of this argument)in addition to use-after-free?)
On Tue, Jan 26, 2016 at 2:45 AM, Dmitry Polukhin via cfe-commits < cfe-commits@lists.llvm.org> wrote: > DmitryPolukhin created this revision. > DmitryPolukhin added a reviewer: rjmccall. > DmitryPolukhin added a subscriber: cfe-commits. > > To completely eliminate use-after-free in this place I had to copy tokens > into new array and pass ownership. As far as I understand the code it is > not possible to guarantee that all inserted tokens are consumed before > exiting from this function. Depending on how many tokens were consumed > before SkipUntil is called, it may not happen due to unbalanced brackets, > etc. Other places where EnterTokenStream is called usually pass vector that > some class owns but here it is not possible because this place can be > called recursively. > > http://reviews.llvm.org/D16572 > > Files: > lib/Parse/ParseExprCXX.cpp > test/Parser/cxx-ambig-paren-expr-asan.cpp > > Index: test/Parser/cxx-ambig-paren-expr-asan.cpp > =================================================================== > --- /dev/null > +++ test/Parser/cxx-ambig-paren-expr-asan.cpp > @@ -0,0 +1,8 @@ > +// RUN: %clang_cc1 -fsyntax-only -pedantic -verify %s > + > +// This syntax error used to cause use-after free due to token local > buffer > +// in ParseCXXAmbiguousParenExpression. > +int H((int()[)]); > +// expected-error@-1 {{expected expression}} > +// expected-error@-2 {{expected ']'}} > +// expected-note@-3 {{to match this '['}} > Index: lib/Parse/ParseExprCXX.cpp > =================================================================== > --- lib/Parse/ParseExprCXX.cpp > +++ lib/Parse/ParseExprCXX.cpp > @@ -3083,10 +3083,17 @@ > > // The current token should go after the cached tokens. > Toks.push_back(Tok); > + > + // Make a copy of stored token to pass ownership to EnterTokenStream > function. > + // This copy is required because we may exit from this function when > some > + // tokens are not consumed yet. > + Token *Buffer = new Token[Toks.size()]; > + std::copy(Toks.begin(), Toks.end(), Buffer); > + > // Re-enter the stored parenthesized tokens into the token stream, so > we may > // parse them now. > - PP.EnterTokenStream(Toks.data(), Toks.size(), > - true/*DisableMacroExpansion*/, false/*OwnsTokens*/); > + PP.EnterTokenStream(Buffer, Toks.size(), > + true/*DisableMacroExpansion*/, true/*OwnsTokens*/); > // Drop the current token and bring the first cached one. It's the same > token > // as when we entered this function. > ConsumeAnyToken(); > > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
