Re: [PATCH] D16572: PR23057: fix use-after-free due to local token buffer in ParseCXXAmbiguousParenExpression

[Dmitry Polukhin via cfe-commits]

DmitryPolukhin updated this revision to Diff 46644.
DmitryPolukhin added a comment.

Use EOF token instead of copy buffer. This approach looks a bit more fragile 
but definitely more efficient, PTAL.


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,9 @@
+// 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 '['}}
+// expected-error@-4 {{expected ';' after top level declarator}}
Index: lib/Parse/ParseExprCXX.cpp
===================================================================
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -3081,6 +3081,14 @@
     ParseAs = NotCastExpr ? SimpleExpr : CastExpr;
   }
 
+  // Create a fake EOF to mark end of Toks buffer.
+  Token AttrEnd;
+  AttrEnd.startToken();
+  AttrEnd.setKind(tok::eof);
+  AttrEnd.setLocation(Tok.getLocation());
+  AttrEnd.setEofData(Toks.data());
+  Toks.push_back(AttrEnd);
+
   // The current token should go after the cached tokens.
   Toks.push_back(Tok);
   // Re-enter the stored parenthesized tokens into the token stream, so we may
@@ -3105,6 +3113,10 @@
     Tracker.consumeClose();
     ColonProt.restore();
 
+    // Consume EOF marker for Toks buffer.
+    assert(Tok.is(tok::eof) && Tok.getEofData() == AttrEnd.getEofData());
+    ConsumeAnyToken();
+
     if (ParseAs == CompoundLiteral) {
       ExprType = CompoundLiteral;
       if (DeclaratorInfo.isInvalidType())
@@ -3141,10 +3153,16 @@
 
   // Match the ')'.
   if (Result.isInvalid()) {
-    SkipUntil(tok::r_paren, StopAtSemi);
+    while (Tok.isNot(tok::eof))
+      ConsumeAnyToken();
+    assert(Tok.getEofData() == AttrEnd.getEofData());
+    ConsumeAnyToken();
     return ExprError();
   }
 
   Tracker.consumeClose();
+  // Consume EOF marker for Toks buffer.
+  assert(Tok.is(tok::eof) && Tok.getEofData() == AttrEnd.getEofData());
+  ConsumeAnyToken();
   return Result;
 }



Index: test/Parser/cxx-ambig-paren-expr-asan.cpp
===================================================================
--- /dev/null
+++ test/Parser/cxx-ambig-paren-expr-asan.cpp
@@ -0,0 +1,9 @@
+// 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 '['}}
+// expected-error@-4 {{expected ';' after top level declarator}}
Index: lib/Parse/ParseExprCXX.cpp
===================================================================
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -3081,6 +3081,14 @@
     ParseAs = NotCastExpr ? SimpleExpr : CastExpr;
   }
 
+  // Create a fake EOF to mark end of Toks buffer.
+  Token AttrEnd;
+  AttrEnd.startToken();
+  AttrEnd.setKind(tok::eof);
+  AttrEnd.setLocation(Tok.getLocation());
+  AttrEnd.setEofData(Toks.data());
+  Toks.push_back(AttrEnd);
+
   // The current token should go after the cached tokens.
   Toks.push_back(Tok);
   // Re-enter the stored parenthesized tokens into the token stream, so we may
@@ -3105,6 +3113,10 @@
     Tracker.consumeClose();
     ColonProt.restore();
 
+    // Consume EOF marker for Toks buffer.
+    assert(Tok.is(tok::eof) && Tok.getEofData() == AttrEnd.getEofData());
+    ConsumeAnyToken();
+
     if (ParseAs == CompoundLiteral) {
       ExprType = CompoundLiteral;
       if (DeclaratorInfo.isInvalidType())
@@ -3141,10 +3153,16 @@
 
   // Match the ')'.
   if (Result.isInvalid()) {
-    SkipUntil(tok::r_paren, StopAtSemi);
+    while (Tok.isNot(tok::eof))
+      ConsumeAnyToken();
+    assert(Tok.getEofData() == AttrEnd.getEofData());
+    ConsumeAnyToken();
     return ExprError();
   }
 
   Tracker.consumeClose();
+  // Consume EOF marker for Toks buffer.
+  assert(Tok.is(tok::eof) && Tok.getEofData() == AttrEnd.getEofData());
+  ConsumeAnyToken();
   return Result;
 }

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits