Skip to content

Commit 090dd64

Browse files
epilkepilk
committed
[Sema] Fold VLAs to constant arrays in a few more contexts
552c6c2 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, so this commit carves out two more cases where we promote VLAs which can't miscompile a conforming program: - When the VLA appears in an ivar -- this seems like a corollary to the field thing - When the VLA has an initializer -- VLAs can't have an initializer Differential revision: https://reviews.llvm.org/D90871
1 parent c8ec685 commit 090dd64

File tree

8 files changed

+124
-35
lines changed

8 files changed

+124
-35
lines changed

‎clang/include/clang/Sema/DeclSpec.h

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1844,6 +1844,9 @@ class Declarator {
18441844
/// Indicates whether the InlineParams / InlineBindings storage has been used.
18451845
unsigned InlineStorageUsed : 1;
18461846

1847+
/// Indicates whether this declarator has an initializer.
1848+
unsigned HasInitializer : 1;
1849+
18471850
/// Attrs - Attributes.
18481851
ParsedAttributes Attrs;
18491852

@@ -1892,8 +1895,8 @@ class Declarator {
18921895
FunctionDefinitionKind::Declaration)),
18931896
Redeclaration(false), Extension(false), ObjCIvar(false),
18941897
ObjCWeakProperty(false), InlineStorageUsed(false),
1895-
Attrs(ds.getAttributePool().getFactory()), AsmLabel(nullptr),
1896-
TrailingRequiresClause(nullptr),
1898+
HasInitializer(false), Attrs(ds.getAttributePool().getFactory()),
1899+
AsmLabel(nullptr), TrailingRequiresClause(nullptr),
18971900
InventedTemplateParameterList(nullptr) {}
18981901

18991902
~Declarator() {
@@ -1976,6 +1979,7 @@ class Declarator {
19761979
Attrs.clear();
19771980
AsmLabel = nullptr;
19781981
InlineStorageUsed = false;
1982+
HasInitializer = false;
19791983
ObjCIvar = false;
19801984
ObjCWeakProperty = false;
19811985
CommaLoc = SourceLocation();
@@ -2574,6 +2578,9 @@ class Declarator {
25742578
return (FunctionDefinitionKind)FunctionDefinition;
25752579
}
25762580

2581+
void setHasInitializer(bool Val = true) { HasInitializer = Val; }
2582+
bool hasInitializer() const { return HasInitializer; }
2583+
25772584
/// Returns true if this declares a real member and not a friend.
25782585
bool isFirstDeclarationOfMember() {
25792586
return getContext() == DeclaratorContext::Member &&

‎clang/lib/Parse/ParseDecl.cpp

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2192,7 +2192,22 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
21922192
}
21932193
};
21942194

2195-
// Inform the current actions module that we just parsed this declarator.
2195+
enum class InitKind { Uninitialized, Equal, CXXDirect, CXXBraced };
2196+
InitKind TheInitKind;
2197+
// If a '==' or '+=' is found, suggest a fixit to '='.
2198+
if (isTokenEqualOrEqualTypo())
2199+
TheInitKind = InitKind::Equal;
2200+
else if (Tok.is(tok::l_paren))
2201+
TheInitKind = InitKind::CXXDirect;
2202+
else if (getLangOpts().CPlusPlus11 && Tok.is(tok::l_brace) &&
2203+
(!CurParsedObjCImpl || !D.isFunctionDeclarator()))
2204+
TheInitKind = InitKind::CXXBraced;
2205+
else
2206+
TheInitKind = InitKind::Uninitialized;
2207+
if (TheInitKind != InitKind::Uninitialized)
2208+
D.setHasInitializer();
2209+
2210+
// Inform Sema that we just parsed this declarator.
21962211
Decl *ThisDecl = nullptr;
21972212
Decl *OuterDecl = nullptr;
21982213
switch (TemplateInfo.Kind) {
@@ -2254,9 +2269,9 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
22542269
}
22552270
}
22562271

2272+
switch (TheInitKind) {
22572273
// Parse declarator '=' initializer.
2258-
// If a '==' or '+=' is found, suggest a fixit to '='.
2259-
if (isTokenEqualOrEqualTypo()) {
2274+
case InitKind::Equal: {
22602275
SourceLocation EqualLoc = ConsumeToken();
22612276

22622277
if (Tok.is(tok::kw_delete)) {
@@ -2311,7 +2326,9 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
23112326
Actions.AddInitializerToDecl(ThisDecl, Init.get(),
23122327
/*DirectInit=*/false);
23132328
}
2314-
} else if (Tok.is(tok::l_paren)) {
2329+
break;
2330+
}
2331+
case InitKind::CXXDirect: {
23152332
// Parse C++ direct initializer: '(' expression-list ')'
23162333
BalancedDelimiterTracker T(*this, tok::l_paren);
23172334
T.consumeOpen();
@@ -2365,8 +2382,9 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
23652382
Actions.AddInitializerToDecl(ThisDecl, Initializer.get(),
23662383
/*DirectInit=*/true);
23672384
}
2368-
} else if (getLangOpts().CPlusPlus11 && Tok.is(tok::l_brace) &&
2369-
(!CurParsedObjCImpl || !D.isFunctionDeclarator())) {
2385+
break;
2386+
}
2387+
case InitKind::CXXBraced: {
23702388
// Parse C++0x braced-init-list.
23712389
Diag(Tok, diag::warn_cxx98_compat_generalized_initializer_lists);
23722390

@@ -2381,9 +2399,12 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
23812399
Actions.ActOnInitializerError(ThisDecl);
23822400
} else
23832401
Actions.AddInitializerToDecl(ThisDecl, Init.get(), /*DirectInit=*/true);
2384-
2385-
} else {
2402+
break;
2403+
}
2404+
case InitKind::Uninitialized: {
23862405
Actions.ActOnUninitializedDecl(ThisDecl);
2406+
break;
2407+
}
23872408
}
23882409

23892410
Actions.FinalizeDeclaration(ThisDecl);

‎clang/lib/Sema/SemaDecl.cpp

Lines changed: 36 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -6023,6 +6023,31 @@ TryToFixInvalidVariablyModifiedTypeSourceInfo(TypeSourceInfo *TInfo,
60236023
return FixedTInfo;
60246024
}
60256025

6026+
/// Attempt to fold a variable-sized type to a constant-sized type, returning
6027+
/// true if we were successful.
6028+
static bool tryToFixVariablyModifiedVarType(Sema &S, TypeSourceInfo *&TInfo,
6029+
QualType &T, SourceLocation Loc,
6030+
unsigned FailedFoldDiagID) {
6031+
bool SizeIsNegative;
6032+
llvm::APSInt Oversized;
6033+
TypeSourceInfo *FixedTInfo = TryToFixInvalidVariablyModifiedTypeSourceInfo(
6034+
TInfo, S.Context, SizeIsNegative, Oversized);
6035+
if (FixedTInfo) {
6036+
S.Diag(Loc, diag::ext_vla_folded_to_constant);
6037+
TInfo = FixedTInfo;
6038+
T = FixedTInfo->getType();
6039+
return true;
6040+
}
6041+
6042+
if (SizeIsNegative)
6043+
S.Diag(Loc, diag::err_typecheck_negative_array_size);
6044+
else if (Oversized.getBoolValue())
6045+
S.Diag(Loc, diag::err_array_too_large) << Oversized.toString(10);
6046+
else if (FailedFoldDiagID)
6047+
S.Diag(Loc, FailedFoldDiagID);
6048+
return false;
6049+
}
6050+
60266051
/// Register the given locally-scoped extern "C" declaration so
60276052
/// that it can be found later for redeclarations. We include any extern "C"
60286053
/// declaration that is not visible in the translation unit here, not just
@@ -6861,6 +6886,12 @@ NamedDecl *Sema::ActOnVariableDeclarator(
68616886
}
68626887
}
68636888

6889+
// If this variable has a variable-modified type and an initializer, try to
6890+
// fold to a constant-sized type. This is otherwise invalid.
6891+
if (D.hasInitializer() && R->isVariablyModifiedType())
6892+
tryToFixVariablyModifiedVarType(*this, TInfo, R, D.getIdentifierLoc(),
6893+
/*DiagID=*/0);
6894+
68646895
bool IsMemberSpecialization = false;
68656896
bool IsVariableTemplateSpecialization = false;
68666897
bool IsPartialSpecialization = false;
@@ -16658,27 +16689,9 @@ FieldDecl *Sema::CheckFieldDecl(DeclarationName Name, QualType T,
1665816689
// C99 6.7.2.1p8: A member of a structure or union may have any type other
1665916690
// than a variably modified type.
1666016691
if (!InvalidDecl && T->isVariablyModifiedType()) {
16661-
bool SizeIsNegative;
16662-
llvm::APSInt Oversized;
16663-
16664-
TypeSourceInfo *FixedTInfo =
16665-
TryToFixInvalidVariablyModifiedTypeSourceInfo(TInfo, Context,
16666-
SizeIsNegative,
16667-
Oversized);
16668-
if (FixedTInfo) {
16669-
Diag(Loc, diag::ext_vla_folded_to_constant);
16670-
TInfo = FixedTInfo;
16671-
T = FixedTInfo->getType();
16672-
} else {
16673-
if (SizeIsNegative)
16674-
Diag(Loc, diag::err_typecheck_negative_array_size);
16675-
else if (Oversized.getBoolValue())
16676-
Diag(Loc, diag::err_array_too_large)
16677-
<< Oversized.toString(10);
16678-
else
16679-
Diag(Loc, diag::err_typecheck_field_variable_size);
16692+
if (!tryToFixVariablyModifiedVarType(
16693+
*this, TInfo, T, Loc, diag::err_typecheck_field_variable_size))
1668016694
InvalidDecl = true;
16681-
}
1668216695
}
1668316696

1668416697
// Fields can not have abstract class types
@@ -16904,8 +16917,9 @@ Decl *Sema::ActOnIvar(Scope *S,
1690416917
// C99 6.7.2.1p8: A member of a structure or union may have any type other
1690516918
// than a variably modified type.
1690616919
else if (T->isVariablyModifiedType()) {
16907-
Diag(Loc, diag::err_typecheck_ivar_variable_size);
16908-
D.setInvalidType();
16920+
if (!tryToFixVariablyModifiedVarType(
16921+
*this, TInfo, T, Loc, diag::err_typecheck_ivar_variable_size))
16922+
D.setInvalidType();
1690916923
}
1691016924

1691116925
// Get the visibility (access control) for this ivar.

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,7 @@ constexpr int arb(int n) {
140140
int a[n]; // expected-error {{variable of non-literal type 'int [n]' cannot be defined in a constexpr function}}
141141
}
142142
// expected-warning@+1 {{variable length array folded to constant array as an extension}}
143-
constexpr long Overflow[ // expected-error {{constexpr variable cannot have non-literal type 'long const[(1 << 30) << 2]'}}
144-
(1 << 30) << 2]{}; // expected-warning {{requires 34 bits to represent}}
143+
constexpr long Overflow[(1 << 30) << 2]{}; // expected-warning {{requires 34 bits to represent}}
145144

146145
namespace inherited_ctor {
147146
struct A { constexpr A(int); };

‎clang/test/Sema/decl-in-prototype.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ void f(struct q *, struct __attribute__((aligned(4))) q *); // expected-warning
4949
// function.
5050
enum { BB = 0 };
5151
void enum_in_fun_in_fun(void (*fp)(enum { AA, BB } e)) { // expected-warning {{will not be visible}}
52-
SA(1, AA == 5); // expected-error {{variable-sized object may not be initialized}}
52+
SA(1, AA == 5); // expected-warning{{variable length array folded to constant array as an extension}}
5353
SA(2, BB == 0);
5454
}
5555

‎clang/test/Sema/vla.c

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,3 +100,33 @@ const int pr44406_a = 32;
100100
typedef struct {
101101
char c[pr44406_a]; // expected-warning {{folded to constant array as an extension}}
102102
} pr44406_s;
103+
104+
void test_fold_to_constant_array() {
105+
const int ksize = 4;
106+
107+
goto jump_over_a1; // expected-error{{cannot jump from this goto statement to its label}}
108+
char a1[ksize]; // expected-note{{variable length array}}
109+
jump_over_a1:;
110+
111+
goto jump_over_a2;
112+
char a2[ksize] = "foo"; // expected-warning{{variable length array folded to constant array as an extension}}
113+
jump_over_a2:;
114+
115+
goto jump_over_a3;
116+
char a3[ksize] = {}; // expected-warning {{variable length array folded to constant array as an extension}} expected-warning{{use of GNU empty initializer}}
117+
jump_over_a3:;
118+
119+
goto jump_over_a4; // expected-error{{cannot jump from this goto statement to its label}}
120+
char a4[ksize][2]; // expected-note{{variable length array}}
121+
jump_over_a4:;
122+
123+
char a5[ksize][2] = {}; // expected-warning {{variable length array folded to constant array as an extension}} expected-warning{{use of GNU empty initializer}}
124+
125+
int a6[ksize] = {1,2,3,4}; // expected-warning{{variable length array folded to constant array as an extension}}
126+
127+
// expected-warning@+1{{variable length array folded to constant array as an extension}}
128+
int a7[ksize] __attribute__((annotate("foo"))) = {1,2,3,4};
129+
130+
// expected-warning@+1{{variable length array folded to constant array as an extension}}
131+
char a8[2][ksize] = {{1,2,3,4},{4,3,2,1}};
132+
}

‎clang/test/SemaCXX/vla.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,9 @@ namespace PR18581 {
2020

2121
void pr23151(int (&)[*]) { // expected-error {{variable length array must be bound in function definition}}
2222
}
23+
24+
void test_fold() {
25+
char a1[(unsigned long)(int *)0+1]{}; // expected-warning{{variable length array folded to constant array as an extension}}
26+
char a2[(unsigned long)(int *)0+1] = {}; // expected-warning{{variable length array folded to constant array as an extension}}
27+
char a3[(unsigned long)(int *)0+1];
28+
}

‎clang/test/SemaObjC/variable-size-ivar.m

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// RUN: %clang_cc1 -fsyntax-only %s -verify
2+
3+
const int ksize = 42;
4+
int size = 42;
5+
6+
@interface X
7+
{
8+
int arr1[ksize]; // expected-warning{{variable length array folded to constant array}}
9+
int arr2[size]; // expected-error{{instance variables must have a constant size}}
10+
int arr3[ksize-43]; // expected-error{{array size is negative}}
11+
}
12+
@end

0 commit comments

Comments
 (0)