Diagnose use of VLAs in C++ by default

Reapplication of 7339c0f with a fix
for a crash involving arrays without a size expression.

Clang supports VLAs in C++ as an extension, but we currently only warn
on their use when you pass -Wvla, -Wvla-extension, or -pedantic.
However, VLAs as they're expressed in C have been considered by WG21
and rejected, are easy to use accidentally to the surprise of users
(e.g., https://ddanilov.me/default-non-standard-features/), and they
have potential security implications beyond constant-size arrays
(https://wiki.sei.cmu.edu/confluence/display/c/ARR32-C.+Ensure+size+arguments+for+variable+length+arrays+are+in+a+valid+range).
C++ users should strongly consider using other functionality such as
std::vector instead.

This seems like sufficiently compelling evidence to warn users about
VLA use by default in C++ modes. This patch enables the -Wvla-extension
diagnostic group in C++ language modes by default, and adds the warning
group to -Wall in GNU++ language modes. The warning is still opt-in in
C language modes, where support for VLAs is somewhat less surprising to
users.

RFC: https://discourse.llvm.org/t/rfc-diagnosing-use-of-vlas-in-c/73109
Fixes #62836
Differential Revision: https://reviews.llvm.org/D156565

Author: AaronBallman

clang/docs/ReleaseNotes.rst

@@ -306,6 +306,7 @@ Improvements to Clang's diagnostics
 - Clang now displays an improved diagnostic and a note when a defaulted special
   member is marked ``constexpr`` in a class with a virtual base class
   (`#64843: <https://github.com/llvm/llvm-project/issues/64843>`_).
+<<<<<<< ours
 - ``-Wfixed-enum-extension`` and ``-Wmicrosoft-fixed-enum`` diagnostics are no longer
   emitted when building as C23, since C23 standardizes support for enums with a
   fixed underlying type.
@@ -346,6 +347,11 @@ Improvements to Clang's diagnostics
       |               ~~~~~~~~~^~~~~~~~
 - Clang now always diagnoses when using non-standard layout types in ``offsetof`` .
   (`#64619: <https://github.com/llvm/llvm-project/issues/64619>`_)
+- Clang now diagnoses use of variable-length arrays in C++ by default (and
+  under ``-Wall`` in GNU++ mode). This is an extension supported by Clang and
+  GCC, but is very easy to accidentally use without realizing it's a
+  nonportable construct that has different semantics from a constant-sized
+  array. (`#62836 <https://github.com/llvm/llvm-project/issues/62836>`_)
 
 Bug Fixes in This Version
 -------------------------

clang/include/clang/Basic/DiagnosticGroups.td

@@ -848,7 +848,8 @@ def OverridingMethodMismatch : DiagGroup<"overriding-method-mismatch">;
 def VariadicMacros : DiagGroup<"variadic-macros">;
 def VectorConversion : DiagGroup<"vector-conversion">;      // clang specific
 def VexingParse : DiagGroup<"vexing-parse">;
-def VLAExtension : DiagGroup<"vla-extension">;
+def VLAUseStaticAssert : DiagGroup<"vla-extension-static-assert">;
+def VLAExtension : DiagGroup<"vla-extension", [VLAUseStaticAssert]>;
 def VLA : DiagGroup<"vla", [VLAExtension]>;
 def VolatileRegisterVar : DiagGroup<"volatile-register-var">;
 def Visibility : DiagGroup<"visibility">;
@@ -1085,7 +1086,7 @@ def Consumed       : DiagGroup<"consumed">;
 // warning should be active _only_ when -Wall is passed in, mark it as
 // DefaultIgnore in addition to putting it here.
 def All : DiagGroup<"all", [Most, Parentheses, Switch, SwitchBool,
-                            MisleadingIndentation, PackedNonPod]>;
+                            MisleadingIndentation, PackedNonPod, VLAExtension]>;
 
 // Warnings that should be in clang-cl /w4.
 def : DiagGroup<"CL4", [All, Extra]>;

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -137,6 +137,18 @@ def err_half_const_requires_fp16 : Error<
 // C99 variable-length arrays
 def ext_vla : Extension<"variable length arrays are a C99 feature">,
   InGroup<VLAExtension>;
+// In C++ language modes, we warn by default as an extension, while in GNU++
+// language modes, we warn as an extension but add the warning group to -Wall.
+def ext_vla_cxx : ExtWarn<
+  "variable length arrays in C++ are a Clang extension">,
+  InGroup<VLAExtension>;
+def ext_vla_cxx_in_gnu_mode : Extension<ext_vla_cxx.Summary>,
+  InGroup<VLAExtension>;
+def ext_vla_cxx_static_assert : ExtWarn<
+  "variable length arrays in C++ are a Clang extension; did you mean to use "
+  "'static_assert'?">, InGroup<VLAUseStaticAssert>;
+def ext_vla_cxx_in_gnu_mode_static_assert : Extension<
+  ext_vla_cxx_static_assert.Summary>, InGroup<VLAUseStaticAssert>;
 def warn_vla_used : Warning<"variable length array used">,
   InGroup<VLA>, DefaultIgnore;
 def err_vla_in_sfinae : Error<

clang/lib/Sema/SemaType.cpp

@@ -2582,6 +2582,27 @@ QualType Sema::BuildArrayType(QualType T, ArrayType::ArraySizeModifier ASM,
     return QualType();
   }
 
+  auto IsStaticAssertLike = [](const Expr *ArraySize, ASTContext &Context) {
+    if (!ArraySize)
+      return false;
+
+    // If the array size expression is a conditional expression whose branches
+    // are both integer constant expressions, one negative and one positive,
+    // then it's assumed to be like an old-style static assertion. e.g.,
+    //   int old_style_assert[expr ? 1 : -1];
+    // We will accept any integer constant expressions instead of assuming the
+    // values 1 and -1 are always used.
+    if (const auto *CondExpr = dyn_cast_if_present<ConditionalOperator>(
+            ArraySize->IgnoreParenImpCasts())) {
+      std::optional<llvm::APSInt> LHS =
+          CondExpr->getLHS()->getIntegerConstantExpr(Context);
+      std::optional<llvm::APSInt> RHS =
+          CondExpr->getRHS()->getIntegerConstantExpr(Context);
+      return LHS && RHS && LHS->isNegative() != RHS->isNegative();
+    }
+    return false;
+  };
+
   // VLAs always produce at least a -Wvla diagnostic, sometimes an error.
   unsigned VLADiag;
   bool VLAIsError;
@@ -2598,6 +2619,15 @@ QualType Sema::BuildArrayType(QualType T, ArrayType::ArraySizeModifier ASM,
   } else if (getLangOpts().OpenMP && isInOpenMPTaskUntiedContext()) {
     VLADiag = diag::err_openmp_vla_in_task_untied;
     VLAIsError = true;
+  } else if (getLangOpts().CPlusPlus) {
+    if (getLangOpts().CPlusPlus11 && IsStaticAssertLike(ArraySize, Context))
+      VLADiag = getLangOpts().GNUMode
+                    ? diag::ext_vla_cxx_in_gnu_mode_static_assert
+                    : diag::ext_vla_cxx_static_assert;
+    else
+      VLADiag = getLangOpts().GNUMode ? diag::ext_vla_cxx_in_gnu_mode
+                                      : diag::ext_vla_cxx;
+    VLAIsError = false;
   } else {
     VLADiag = diag::ext_vla;
     VLAIsError = false;

clang/test/AST/Interp/literals.cpp

@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -fms-extensions -std=c++11 -verify %s
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -fms-extensions -std=c++20 -verify %s
-// RUN: %clang_cc1 -std=c++11 -fms-extensions -verify=ref %s
-// RUN: %clang_cc1 -std=c++20 -fms-extensions -verify=ref %s
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -Wno-vla -fms-extensions -std=c++11 -verify %s
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -Wno-vla -fms-extensions -std=c++20 -verify %s
+// RUN: %clang_cc1 -std=c++11 -fms-extensions -Wno-vla -verify=ref %s
+// RUN: %clang_cc1 -std=c++20 -fms-extensions -Wno-vla -verify=ref %s
 
 #define INT_MIN (~__INT_MAX__)
 #define INT_MAX __INT_MAX__

clang/test/Analysis/lambdas.cpp

@@ -194,8 +194,9 @@ void testFunctionPointerCapture() {
 // Captured variable-length array.
 
 void testVariableLengthArrayCaptured() {
-  int n = 2;
-  int array[n];
+  int n = 2;     // expected-note {{declared here}}
+  int array[n];  // expected-warning {{variable length arrays in C++ are a Clang extension}} \
+                    expected-note {{read of non-const variable 'n' is not allowed in a constant expression}}
   array[0] = 7;
 
   int i = [&]{

clang/test/CXX/basic/basic.types/p10.cpp

@@ -20,7 +20,7 @@ constexpr int f2(S &) { return 0; }
 
 struct BeingDefined;
 extern BeingDefined beingdefined;
-struct BeingDefined { 
+struct BeingDefined {
   static constexpr BeingDefined& t = beingdefined;
 };
 
@@ -136,11 +136,15 @@ struct ArrBad {
 };
 constexpr int f(ArrBad) { return 0; } // expected-error {{1st parameter type 'ArrBad' is not a literal type}}
 
-constexpr int arb(int n) {
-  int a[n]; // expected-error {{variable of non-literal type 'int[n]' cannot be defined in a constexpr function}}
+constexpr int arb(int n) { // expected-note {{declared here}}
+  int a[n]; // expected-error {{variable of non-literal type 'int[n]' cannot be defined in a constexpr function}} \
+               expected-warning {{variable length arrays in C++ are a Clang extension}} \
+               expected-note {{function parameter 'n' with unknown value cannot be used in a constant expression}}
 }
-// expected-warning@+1 {{variable length array folded to constant array as an extension}}
-constexpr long Overflow[(1 << 30) << 2]{}; // expected-warning {{requires 34 bits to represent}}
+constexpr long Overflow[(1 << 30) << 2]{}; // expected-warning {{requires 34 bits to represent}} \
+                                              expected-warning {{variable length array folded to constant array as an extension}} \
+                                              expected-warning {{variable length arrays in C++ are a Clang extension}} \
+                                              expected-note {{signed left shift discards bits}}
 
 namespace inherited_ctor {
   struct A { constexpr A(int); };

clang/test/CXX/dcl.dcl/dcl.spec/dcl.typedef/p2-0x.cpp

@@ -34,18 +34,22 @@ namespace IllegalSyntax {
 namespace VariableLengthArrays {
   using T = int[42]; // ok
 
-  int n = 32;
-  using T = int[n]; // expected-error {{variable length array declaration not allowed at file scope}}
+  int n = 32;       // expected-note {{declared here}}
+  using T = int[n]; // expected-error {{variable length array declaration not allowed at file scope}} \
+                       expected-warning {{variable length arrays in C++ are a Clang extension}} \
+                       expected-note {{read of non-const variable 'n' is not allowed in a constant expression}}
 
   const int m = 42;
   using U = int[m];
   using U = int[42]; // expected-note {{previous definition}}
   using U = int; // expected-error {{type alias redefinition with different types ('int' vs 'int[42]')}}
 
   void f() {
-    int n = 42;
+    int n = 42; // expected-note {{declared here}}
     goto foo; // expected-error {{cannot jump}}
-    using T = int[n]; // expected-note {{bypasses initialization of VLA type alias}}
+    using T = int[n]; // expected-note {{bypasses initialization of VLA type alias}} \
+                         expected-warning {{variable length arrays in C++ are a Clang extension}} \
+                         expected-note {{read of non-const variable 'n' is not allowed in a constant expression}}
   foo: ;
   }
 }

clang/test/CXX/expr/expr.prim/expr.prim.lambda/p4.cpp

@@ -44,21 +44,23 @@ X infer_X_return_type_2(X x) {
 }
 
 struct Incomplete; // expected-note 2{{forward declaration of 'Incomplete'}}
-void test_result_type(int N) {
+void test_result_type(int N) {  // expected-note {{declared here}}
   auto l1 = [] () -> Incomplete { }; // expected-error{{incomplete result type 'Incomplete' in lambda expression}}
 
-  typedef int vla[N];
+  typedef int vla[N]; // expected-warning {{variable length arrays in C++ are a Clang extension}} \
+                         expected-note {{function parameter 'N' with unknown value cannot be used in a constant expression}}
   auto l2 = [] () -> vla { }; // expected-error{{function cannot return array type 'vla' (aka 'int[N]')}}
 }
 
 template <typename T>
-void test_result_type_tpl(int N) {
+void test_result_type_tpl(int N) { // expected-note 2{{declared here}}
   auto l1 = []() -> T {}; // expected-error{{incomplete result type 'Incomplete' in lambda expression}}
                           // expected-note@-1{{while substituting into a lambda expression here}}
-  typedef int vla[N];
+  typedef int vla[N]; // expected-warning 2{{variable length arrays in C++ are a Clang extension}} \
+                         expected-note 2{{function parameter 'N' with unknown value cannot be used in a constant expression}}
   auto l2 = []() -> vla {}; // expected-error{{function cannot return array type 'vla' (aka 'int[N]')}}
 }
 
 void test_result_type_call() {
-  test_result_type_tpl<Incomplete>(10); // expected-note {{requested here}}
+  test_result_type_tpl<Incomplete>(10); // expected-note 2{{requested here}}
 }

clang/test/CXX/temp/temp.arg/temp.arg.type/p2.cpp

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wvla %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
 
@@ -19,13 +19,17 @@ enum {e};
 // expected-note@-2 {{unnamed type used in template argument was declared here}}
 #endif
 
-void test_f0(int n) {
+void test_f0(int n) { // #here
   int i = f0(0, e);
 #if __cplusplus <= 199711L
   // expected-warning@-2 {{template argument uses unnamed type}}
 #endif
 
-  int vla[n];
+  int vla[n]; // expected-warning {{variable length arrays in C++ are a Clang extension}}
+#if __cplusplus > 199711L
+  // expected-note@-2 {{function parameter 'n' with unknown value cannot be used in a constant expression}}
+  // expected-note@#here {{declared here}}
+#endif
   f0(0, vla); // expected-error{{no matching function for call to 'f0'}}
 }