Hi,

This fixes a crash in the RecursiveASTVisitor on such code:
 __typeof__(struct F*) var[invalid];

The main problem is that when the type is invalid, we don't even
try to generate TypeSourceInfo for it, which lead to a crash when we try
to visit them in the tools.
This is solved in SemaType.cpp by actually generating the TypeSourceInfo
even for invalid types.

The second problem is that if there is an error parsing the size of the
array, we bail out without actually registering that it should have been an
array. Fix that Parser::ParseBracketDeclarator.
Move the check for invalid type a bit up in Sema::ActOnUninitializedDecl
in order to avoid unnecessary diagnostic

Regards
-- 
Olivier


>From a260fa9fc35cdcd2a26a1b43b38cf6bc511972ff Mon Sep 17 00:00:00 2001
From: Olivier Goffart <[email protected]>
Date: Tue, 5 Aug 2014 23:51:32 +0200
Subject: [PATCH] Fix generating TypeSourceInfo for invalid array type

This fixes a crash in the RecursiveASTVisitor on such code:
 __typeof__(struct F*) var[invalid];

The main problem is that when the type is invalid, we don't even
try to generate TypeSourceInfo for it, which lead to a crash when we try
to visit them in the tools.
This is sloved in SemaType.cpp by actually generating the TypeSourceInfo
even for invalid types.

The second problem is that if there is an error parsing the size of the
array, we bail out without actually registering that it should have been an
array. Fix that Parser::ParseBracketDeclarator.
Move the check for invalid type a bit up in Sema::ActOnUninitializedDecl
in order to avoid unnecessary diagnostic
---
 lib/Parse/ParseDecl.cpp                       |  4 ++--
 lib/Sema/SemaDecl.cpp                         |  6 +++---
 lib/Sema/SemaType.cpp                         |  8 +++++---
 unittests/Tooling/RecursiveASTVisitorTest.cpp | 10 ++++++++++
 4 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp
index 727915c..f7a3065 100644
--- a/lib/Parse/ParseDecl.cpp
+++ b/lib/Parse/ParseDecl.cpp
@@ -5602,8 +5602,8 @@ void Parser::ParseBracketDeclarator(Declarator &D) {
   if (NumElements.isInvalid()) {
     D.setInvalidType(true);
     // If the expression was invalid, skip it.
-    SkipUntil(tok::r_square, StopAtSemi);
-    return;
+    if (!SkipUntil(tok::r_square, StopAtSemi | StopBeforeMatch))
+      return;
   }
 
   T.consumeClose();
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 5f65593..d58b932 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -8885,6 +8885,9 @@ void Sema::ActOnUninitializedDecl(Decl *RealDecl,
       return;
     }
 
+    if (Var->isInvalidDecl())
+        return;
+
     // Provide a specific diagnostic for uninitialized variable
     // definitions with incomplete array type.
     if (Type->isIncompleteArrayType()) {
@@ -8909,9 +8912,6 @@ void Sema::ActOnUninitializedDecl(Decl *RealDecl,
     if (Type->isDependentType())
       return;
 
-    if (Var->isInvalidDecl())
-      return;
-
     if (!Var->hasAttr<AliasAttr>()) {
       if (RequireCompleteType(Var->getLocation(),
                               Context.getBaseElementType(Type),
diff --git a/lib/Sema/SemaType.cpp b/lib/Sema/SemaType.cpp
index b6b024c..1d3f8c2 100644
--- a/lib/Sema/SemaType.cpp
+++ b/lib/Sema/SemaType.cpp
@@ -3242,8 +3242,6 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
 
   if (T.isNull())
     return Context.getNullTypeSourceInfo();
-  else if (D.isInvalidType())
-    return Context.getTrivialTypeSourceInfo(T);
 
   return S.GetTypeSourceInfoForDeclarator(D, T, TInfo);
 }
@@ -3810,7 +3808,8 @@ Sema::GetTypeSourceInfoForDeclarator(Declarator &D, QualType T,
     CurrTL = CurrTL.getNextTypeLoc().getUnqualifiedLoc();
   }
 
-  for (unsigned i = 0, e = D.getNumTypeObjects(); i != e; ++i) {
+  for (unsigned i = 0, e = D.getNumTypeObjects();
+       i != e && !CurrTL.isNull(); ++i) {
     // An AtomicTypeLoc might be produced by an atomic qualifier in this
     // declarator chunk.
     if (AtomicTypeLoc ATL = CurrTL.getAs<AtomicTypeLoc>()) {
@@ -3831,6 +3830,9 @@ Sema::GetTypeSourceInfoForDeclarator(Declarator &D, QualType T,
     CurrTL = CurrTL.getNextTypeLoc().getUnqualifiedLoc();
   }
 
+  if (CurrTL.isNull())
+    return TInfo;
+
   // If we have different source information for the return type, use
   // that.  This really only applies to C++ conversion functions.
   if (ReturnTypeInfo) {
diff --git a/unittests/Tooling/RecursiveASTVisitorTest.cpp b/unittests/Tooling/RecursiveASTVisitorTest.cpp
index a1a93a5..96a41a7 100644
--- a/unittests/Tooling/RecursiveASTVisitorTest.cpp
+++ b/unittests/Tooling/RecursiveASTVisitorTest.cpp
@@ -540,6 +540,16 @@ TEST(RecursiveASTVisitor, VisitsObjCPropertyType) {
       TypeLocVisitor::Lang_OBJC));
 }
 
+TEST(RecursiveASTVisitor, VisitInvalidType) {
+  TypeLocVisitor Visitor;
+  Visitor.ExpectMatch("typeof(struct F *) []", 1, 1);
+  // Even if the full type is invalid, it should still find sub types
+  Visitor.ExpectMatch("struct F", 1, 19);
+  EXPECT_FALSE(Visitor.runOver(
+      "__typeof__(struct F*) var[invalid];\n",
+      TypeLocVisitor::Lang_C));
+}
+
 TEST(RecursiveASTVisitor, VisitsLambdaExpr) {
   LambdaExprVisitor Visitor;
   Visitor.ExpectMatch("", 1, 12);
-- 
2.0.4

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to