erik.pilkington created this revision.
erik.pilkington added reviewers: rsmith, rjmccall, arphaman.
Herald added subscribers: ributzka, jkorous.
erik.pilkington requested review of this revision.

D89523 <https://reviews.llvm.org/D89523> removed support for promoting VLAs to 
constant arrays when the bounds isn't an ICE, since this can result in 
miscompiling a conforming program that assumes that the array is a VLA. 
Promoting VLAs for fields is still supported, since clang doesn't support VLAs 
in fields, so no conforming program could have a field VLA.

This change is really disruptive for us (hundreds of projects are failing to 
compile with this change), so I think we should carve out two more cases where 
we promote VLAs which can't miscompile a conforming program:

1. When the VLA appears in an ivar -- this seems like a corollary to the field 
thing
2. When the VLA has an initializer -- VLAs can't have an initializer

Thanks for taking a look!


https://reviews.llvm.org/D90871

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/decl-in-prototype.c
  clang/test/Sema/vla.c
  clang/test/SemaObjC/variable-size-ivar.m

Index: clang/test/SemaObjC/variable-size-ivar.m
===================================================================
--- /dev/null
+++ clang/test/SemaObjC/variable-size-ivar.m
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify
+
+const int ksize = 42;
+int size = 42;
+
+@interface X
+{
+  int arr1[ksize]; // expected-warning{{variable length array folded to constant array}}
+  int arr2[size]; // expected-error{{instance variables must have a constant size}}
+  int arr3[ksize-43]; // expected-error{{array size is negative}}
+}
+@end
Index: clang/test/Sema/vla.c
===================================================================
--- clang/test/Sema/vla.c
+++ clang/test/Sema/vla.c
@@ -100,3 +100,29 @@
 typedef struct {
   char c[pr44406_a]; // expected-warning {{folded to constant array as an extension}}
 } pr44406_s;
+
+void test_fold_to_constant_array() {
+  const int ksize = 4;
+
+  goto jump_over_a1; // expected-error{{cannot jump from this goto statement to its label}}
+  char a1[ksize]; // expected-note{{variable length array}}
+ jump_over_a1:;
+
+  char a2[ksize] = "foo"; // expected-warning{{variable length array folded to constant array as an extension}}
+
+  char a3[ksize] = {}; // expected-warning {{variable length array folded to constant array as an extension}} expected-warning{{use of GNU empty initializer}}
+
+  goto jump_over_a4; // expected-error{{cannot jump from this goto statement to its label}}
+  char a4[ksize][2]; // expected-note{{variable length array}}
+ jump_over_a4:;
+
+  char a5[ksize][2] = {}; // expected-warning {{variable length array folded to constant array as an extension}} expected-warning{{use of GNU empty initializer}}
+
+  int a6[ksize] = {1,2,3,4}; // expected-warning{{variable length array folded to constant array as an extension}}
+
+  // expected-warning@+1{{variable length array folded to constant array as an extension}}
+  int a7[ksize] __attribute__((annotate("foo"))) = {1,2,3,4};
+
+  // expected-warning@+1{{variable length array folded to constant array as an extension}}
+  char a8[2][ksize] = {{1,2,3,4},{4,3,2,1}};
+}
Index: clang/test/Sema/decl-in-prototype.c
===================================================================
--- clang/test/Sema/decl-in-prototype.c
+++ clang/test/Sema/decl-in-prototype.c
@@ -49,7 +49,7 @@
 // function.
 enum { BB = 0 };
 void enum_in_fun_in_fun(void (*fp)(enum { AA, BB } e)) { // expected-warning {{will not be visible}}
-  SA(1, AA == 5); // expected-error {{variable-sized object may not be initialized}}
+  SA(1, AA == 5); // expected-warning{{variable length array folded to constant array as an extension}}
   SA(2, BB == 0);
 }
 
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -5542,9 +5542,9 @@
   return false;
 }
 
-Decl *Sema::ActOnDeclarator(Scope *S, Declarator &D) {
+Decl *Sema::ActOnDeclarator(Scope *S, Declarator &D, bool NextTokIsEqual) {
   D.setFunctionDefinitionKind(FDK_Declaration);
-  Decl *Dcl = HandleDeclarator(S, D, MultiTemplateParamsArg());
+  Decl *Dcl = HandleDeclarator(S, D, MultiTemplateParamsArg(), NextTokIsEqual);
 
   if (OriginalLexicalContext && OriginalLexicalContext->isObjCContainer() &&
       Dcl && Dcl->getDeclContext()->isFileContext())
@@ -5678,7 +5678,8 @@
 }
 
 NamedDecl *Sema::HandleDeclarator(Scope *S, Declarator &D,
-                                  MultiTemplateParamsArg TemplateParamLists) {
+                                  MultiTemplateParamsArg TemplateParamLists,
+                                  bool NextTokIsEqual) {
   // TODO: consider using NameInfo for diagnostic.
   DeclarationNameInfo NameInfo = GetNameForDeclarator(D);
   DeclarationName Name = NameInfo.getName();
@@ -5873,7 +5874,7 @@
                                   AddToScope);
   } else {
     New = ActOnVariableDeclarator(S, D, DC, TInfo, Previous, TemplateParamLists,
-                                  AddToScope);
+                                  AddToScope, /*Bindings=*/None, NextTokIsEqual);
   }
 
   if (!New)
@@ -6023,6 +6024,31 @@
   return FixedTInfo;
 }
 
+/// Attempt to fold a variable-sized type to a constant-sized type, returning
+/// true if it we were successful.
+static bool tryToFixVariablyModifiedVarType(Sema &S, TypeSourceInfo *&TInfo,
+                                            QualType &T, SourceLocation Loc,
+                                            unsigned FailedFoldDiagID) {
+  bool SizeIsNegative;
+  llvm::APSInt Oversized;
+  TypeSourceInfo *FixedTInfo = TryToFixInvalidVariablyModifiedTypeSourceInfo(
+      TInfo, S.Context, SizeIsNegative, Oversized);
+  if (FixedTInfo) {
+    S.Diag(Loc, diag::ext_vla_folded_to_constant);
+    TInfo = FixedTInfo;
+    T = FixedTInfo->getType();
+    return true;
+  }
+
+  if (SizeIsNegative)
+    S.Diag(Loc, diag::err_typecheck_negative_array_size);
+  else if (Oversized.getBoolValue())
+    S.Diag(Loc, diag::err_array_too_large) << Oversized.toString(10);
+  else if (FailedFoldDiagID)
+    S.Diag(Loc, FailedFoldDiagID);
+  return false;
+}
+
 /// Register the given locally-scoped extern "C" declaration so
 /// that it can be found later for redeclarations. We include any extern "C"
 /// declaration that is not visible in the translation unit here, not just
@@ -6797,7 +6823,8 @@
 NamedDecl *Sema::ActOnVariableDeclarator(
     Scope *S, Declarator &D, DeclContext *DC, TypeSourceInfo *TInfo,
     LookupResult &Previous, MultiTemplateParamsArg TemplateParamLists,
-    bool &AddToScope, ArrayRef<BindingDecl *> Bindings) {
+    bool &AddToScope, ArrayRef<BindingDecl *> Bindings,
+    bool NextTokIsEqual) {
   QualType R = TInfo->getType();
   DeclarationName Name = GetNameForDeclarator(D).getName();
 
@@ -6871,6 +6898,10 @@
   VarTemplateDecl *NewTemplate = nullptr;
   TemplateParameterList *TemplateParams = nullptr;
   if (!getLangOpts().CPlusPlus) {
+    if (NextTokIsEqual && R->isVariablyModifiedType())
+      tryToFixVariablyModifiedVarType(*this, TInfo, R, D.getIdentifierLoc(),
+                                      /*DiagID=*/0);
+
     NewVD = VarDecl::Create(Context, DC, D.getBeginLoc(), D.getIdentifierLoc(),
                             II, R, TInfo, SC);
 
@@ -16628,27 +16659,9 @@
   // C99 6.7.2.1p8: A member of a structure or union may have any type other
   // than a variably modified type.
   if (!InvalidDecl && T->isVariablyModifiedType()) {
-    bool SizeIsNegative;
-    llvm::APSInt Oversized;
-
-    TypeSourceInfo *FixedTInfo =
-      TryToFixInvalidVariablyModifiedTypeSourceInfo(TInfo, Context,
-                                                    SizeIsNegative,
-                                                    Oversized);
-    if (FixedTInfo) {
-      Diag(Loc, diag::ext_vla_folded_to_constant);
-      TInfo = FixedTInfo;
-      T = FixedTInfo->getType();
-    } else {
-      if (SizeIsNegative)
-        Diag(Loc, diag::err_typecheck_negative_array_size);
-      else if (Oversized.getBoolValue())
-        Diag(Loc, diag::err_array_too_large)
-          << Oversized.toString(10);
-      else
-        Diag(Loc, diag::err_typecheck_field_variable_size);
+    if (!tryToFixVariablyModifiedVarType(
+            *this, TInfo, T, Loc, diag::err_typecheck_field_variable_size))
       InvalidDecl = true;
-    }
   }
 
   // Fields can not have abstract class types
@@ -16869,8 +16882,9 @@
   // C99 6.7.2.1p8: A member of a structure or union may have any type other
   // than a variably modified type.
   else if (T->isVariablyModifiedType()) {
-    Diag(Loc, diag::err_typecheck_ivar_variable_size);
-    D.setInvalidType();
+    if (!tryToFixVariablyModifiedVarType(
+            *this, TInfo, T, Loc, diag::err_typecheck_ivar_variable_size))
+      D.setInvalidType();
   }
 
   // Get the visibility (access control) for this ivar.
Index: clang/lib/Parse/ParseDecl.cpp
===================================================================
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -2193,12 +2193,13 @@
     }
   };
 
-  // Inform the current actions module that we just parsed this declarator.
+  // Inform Sema that we just parsed this declarator.
   Decl *ThisDecl = nullptr;
   Decl *OuterDecl = nullptr;
   switch (TemplateInfo.Kind) {
   case ParsedTemplateInfo::NonTemplate:
-    ThisDecl = Actions.ActOnDeclarator(getCurScope(), D);
+    ThisDecl = Actions.ActOnDeclarator(getCurScope(), D,
+                                       /*HasInit=*/Tok.is(tok::equal));
     break;
 
   case ParsedTemplateInfo::Template:
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -2475,10 +2475,11 @@
                                           SourceLocation Less,
                                           SourceLocation Greater);
 
-  Decl *ActOnDeclarator(Scope *S, Declarator &D);
+  Decl *ActOnDeclarator(Scope *S, Declarator &D, bool NextTokIsEqual = false);
 
   NamedDecl *HandleDeclarator(Scope *S, Declarator &D,
-                              MultiTemplateParamsArg TemplateParameterLists);
+                              MultiTemplateParamsArg TemplateParameterLists,
+                              bool NextTokIsEqual = false);
   void RegisterLocallyScopedExternCDecl(NamedDecl *ND, Scope *S);
   bool DiagnoseClassNameShadow(DeclContext *DC, DeclarationNameInfo Info);
   bool diagnoseQualifiedDeclaration(CXXScopeSpec &SS, DeclContext *DC,
@@ -2529,7 +2530,8 @@
                                      LookupResult &Previous,
                                      MultiTemplateParamsArg TemplateParamLists,
                                      bool &AddToScope,
-                                     ArrayRef<BindingDecl *> Bindings = None);
+                                     ArrayRef<BindingDecl *> Bindings = None,
+                                     bool NextTokIsEqual = false);
   NamedDecl *
   ActOnDecompositionDeclarator(Scope *S, Declarator &D,
                                MultiTemplateParamsArg TemplateParamLists);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D90871: [... Erik Pilkington via Phabricator via cfe-commits

Reply via email to