<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Apr 7, 2015 at 3:08 PM, David Majnemer <span dir="ltr"><<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: majnemer<br>
Date: Tue Apr  7 17:08:51 2015<br>
New Revision: 234363<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=234363&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=234363&view=rev</a><br>
Log:<br>
[Sema] Correctly recurse when looking for [*] in function definitions<br>
<br>
A [*] is only allowed in a declaration for a function, not in its<br>
definition.  We didn't correctly recurse while looking for it, causing<br>
us to crash in CodeGen instead of rejecting it.<br>
<br>
This fixes PR23151.<br>
<br>
Modified:<br>
    cfe/trunk/lib/Sema/SemaChecking.cpp<br>
    cfe/trunk/test/Sema/vla.c<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=234363&r1=234362&r2=234363&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=234363&r1=234362&r2=234363&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue Apr  7 17:08:51 2015<br>
@@ -7705,6 +7705,31 @@ void Sema::CheckBitFieldInitialization(S<br>
   (void) AnalyzeBitFieldAssignment(*this, BitField, Init, InitLoc);<br>
 }<br>
<br>
+static void diagnoseArrayStarInParamType(Sema &S, QualType PType,<br>
+                                         SourceLocation Loc) {<br>
+  if (!PType->isVariablyModifiedType())<br>
+    return;<br>
+  if (const auto *PointerTy = dyn_cast<PointerType>(PType)) {<br>
+    diagnoseArrayStarInParamType(S, PointerTy->getPointeeType(), Loc);<br>
+    return;<br>
+  }<br>
+  if (const auto *ParenTy = dyn_cast<ParenType>(PType)) {<br>
+    diagnoseArrayStarInParamType(S, ParenTy->getInnerType(), Loc);<br>
+    return;<br>
+  }<br>
+<br>
+  const ArrayType *AT = S.Context.getAsArrayType(PType);<br>
+  if (!AT)<br>
+    return;<br>
+<br>
+  if (AT->getSizeModifier() != ArrayType::Star) {<br>
+    diagnoseArrayStarInParamType(S, AT->getElementType(), Loc);<br>
+    return;<br>
+  }<br>
+<br>
+  S.Diag(Loc, diag::err_array_star_in_function_definition);<br></blockquote><div><br></div><div>Looks like this won't handle DecayedType (for a decayed function parameter), ReferenceType, BlockPointerType, and a few others... but RecursiveASTVisitor seems like overkill. Perhaps we could track the locations of [*] bounds on the Scope for the function declaration instead of digging them out here?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+}<br>
+<br>
 /// CheckParmsForFunctionDef - Check that the parameters of the given<br>
 /// function are appropriate for the definition of a function. This<br>
 /// takes care of any checks that cannot be performed on the<br>
@@ -7743,15 +7768,9 @@ bool Sema::CheckParmsForFunctionDef(Parm<br>
     //   notation in their sequences of declarator specifiers to specify<br>
     //   variable length array types.<br>
     QualType PType = Param->getOriginalType();<br>
-    while (const ArrayType *AT = Context.getAsArrayType(PType)) {<br>
-      if (AT->getSizeModifier() == ArrayType::Star) {<br>
-        // FIXME: This diagnostic should point the '[*]' if source-location<br>
-        // information is added for it.<br>
-        Diag(Param->getLocation(), diag::err_array_star_in_function_definition);<br>
-        break;<br>
-      }<br>
-      PType= AT->getElementType();<br>
-    }<br>
+    // FIXME: This diagnostic should point the '[*]' if source-location<br>
+    // information is added for it.<br>
+    diagnoseArrayStarInParamType(*this, PType, Param->getLocation());<br>
<br>
     // MSVC destroys objects passed by value in the callee.  Therefore a<br>
     // function definition which takes such a parameter must be able to call the<br>
<br>
Modified: cfe/trunk/test/Sema/vla.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/vla.c?rev=234363&r1=234362&r2=234363&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/vla.c?rev=234363&r1=234362&r2=234363&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/Sema/vla.c (original)<br>
+++ cfe/trunk/test/Sema/vla.c Tue Apr  7 17:08:51 2015<br>
@@ -61,6 +61,9 @@ void pr5185(int a[*]) // expected-error<br>
 {<br>
 }<br>
<br>
+void pr23151(int (*p1)[*]) // expected-error {{variable length array must be bound in function definition}}<br>
+{}<br>
+<br>
 // Make sure this isn't treated as an error<br>
 int TransformBug(int a) {<br>
  return sizeof(*(int(*)[({ goto v; v: a;})]) 0); // expected-warning {{use of GNU statement expression extension}}<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div></div>