<div dir="ltr">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?)<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jan 26, 2016 at 2:45 AM, Dmitry Polukhin via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">DmitryPolukhin created this revision.<br>
DmitryPolukhin added a reviewer: rjmccall.<br>
DmitryPolukhin added a subscriber: cfe-commits.<br>
<br>
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.<br>
<br>
<a href="http://reviews.llvm.org/D16572" rel="noreferrer" target="_blank">http://reviews.llvm.org/D16572</a><br>
<br>
Files:<br>
  lib/Parse/ParseExprCXX.cpp<br>
  test/Parser/cxx-ambig-paren-expr-asan.cpp<br>
<br>
Index: test/Parser/cxx-ambig-paren-expr-asan.cpp<br>
===================================================================<br>
--- /dev/null<br>
+++ test/Parser/cxx-ambig-paren-expr-asan.cpp<br>
@@ -0,0 +1,8 @@<br>
+// RUN: %clang_cc1 -fsyntax-only -pedantic -verify %s<br>
+<br>
+// This syntax error used to cause use-after free due to token local buffer<br>
+// in ParseCXXAmbiguousParenExpression.<br>
+int H((int()[)]);<br>
+// expected-error@-1 {{expected expression}}<br>
+// expected-error@-2 {{expected ']'}}<br>
+// expected-note@-3 {{to match this '['}}<br>
Index: lib/Parse/ParseExprCXX.cpp<br>
===================================================================<br>
--- lib/Parse/ParseExprCXX.cpp<br>
+++ lib/Parse/ParseExprCXX.cpp<br>
@@ -3083,10 +3083,17 @@<br>
<br>
   // The current token should go after the cached tokens.<br>
   Toks.push_back(Tok);<br>
+<br>
+  // Make a copy of stored token to pass ownership to EnterTokenStream function.<br>
+  // This copy is required because we may exit from this function when some<br>
+  // tokens are not consumed yet.<br>
+  Token *Buffer = new Token[Toks.size()];<br>
+  std::copy(Toks.begin(), Toks.end(), Buffer);<br>
+<br>
   // Re-enter the stored parenthesized tokens into the token stream, so we may<br>
   // parse them now.<br>
-  PP.EnterTokenStream(Toks.data(), Toks.size(),<br>
-                      true/*DisableMacroExpansion*/, false/*OwnsTokens*/);<br>
+  PP.EnterTokenStream(Buffer, Toks.size(),<br>
+                      true/*DisableMacroExpansion*/, true/*OwnsTokens*/);<br>
   // Drop the current token and bring the first cached one. It's the same token<br>
   // as when we entered this function.<br>
   ConsumeAnyToken();<br>
<br>
<br>
<br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br></div>