Skip to content

Commit fc2766c

Browse files
tbaederrtbaederr
authored
[clang][Interp] Diagnose reads from non-const global variables (#71919)
This fixes a long-standing FIXME item. Unfortunately it changes the diagnostic output of the tests added in `cxx23.cpp`, but they were wrong before and are wrong after, so no big deal.
1 parent 33aaad9 commit fc2766c

File tree

7 files changed

+126
-20
lines changed

7 files changed

+126
-20
lines changed

‎clang/lib/AST/Interp/ByteCodeExprGen.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2337,7 +2337,7 @@ bool ByteCodeExprGen<Emitter>::visitDecl(const VarDecl *VD) {
23372337
auto GlobalIndex = P.getGlobal(VD);
23382338
assert(GlobalIndex); // visitVarDecl() didn't return false.
23392339
if (VarT) {
2340-
if (!this->emitGetGlobal(*VarT, *GlobalIndex, VD))
2340+
if (!this->emitGetGlobalUnchecked(*VarT, *GlobalIndex, VD))
23412341
return false;
23422342
} else {
23432343
if (!this->emitGetPtrGlobal(*GlobalIndex, VD))

‎clang/lib/AST/Interp/Interp.cpp‎

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,21 @@ static bool Jf(InterpState &S, CodePtr &PC, int32_t Offset) {
5353
return true;
5454
}
5555

56+
static void diagnoseNonConstVariable(InterpState &S, CodePtr OpPC,
57+
const ValueDecl *VD) {
58+
if (!S.getLangOpts().CPlusPlus)
59+
return;
60+
61+
const SourceInfo &Loc = S.Current->getSource(OpPC);
62+
S.FFDiag(Loc,
63+
VD->getType()->isIntegralOrEnumerationType()
64+
? diag::note_constexpr_ltor_non_const_int
65+
: diag::note_constexpr_ltor_non_constexpr,
66+
1)
67+
<< VD;
68+
S.Note(VD->getLocation(), diag::note_declared_at);
69+
}
70+
5671
static bool CheckActive(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
5772
AccessKinds AK) {
5873
if (Ptr.isActive())
@@ -171,9 +186,7 @@ bool CheckExtern(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
171186

172187
if (!S.checkingPotentialConstantExpression() && S.getLangOpts().CPlusPlus) {
173188
const auto *VD = Ptr.getDeclDesc()->asValueDecl();
174-
const SourceInfo &Loc = S.Current->getSource(OpPC);
175-
S.FFDiag(Loc, diag::note_constexpr_ltor_non_constexpr, 1) << VD;
176-
S.Note(VD->getLocation(), diag::note_declared_at);
189+
diagnoseNonConstVariable(S, OpPC, VD);
177190
}
178191
return false;
179192
}
@@ -216,6 +229,24 @@ bool CheckLive(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
216229
return true;
217230
}
218231

232+
bool CheckConstant(InterpState &S, CodePtr OpPC, const Descriptor *Desc) {
233+
assert(Desc);
234+
if (const auto *D = Desc->asValueDecl()) {
235+
if (const auto *VD = dyn_cast<VarDecl>(D);
236+
VD && VD->hasGlobalStorage() &&
237+
!(VD->isConstexpr() || VD->getType().isConstQualified())) {
238+
diagnoseNonConstVariable(S, OpPC, VD);
239+
return false;
240+
}
241+
}
242+
243+
return true;
244+
}
245+
246+
static bool CheckConstant(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
247+
return CheckConstant(S, OpPC, Ptr.getDeclDesc());
248+
}
249+
219250
bool CheckDummy(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
220251
return !Ptr.isZero() && !Ptr.isDummy();
221252
}
@@ -304,6 +335,9 @@ bool CheckInitialized(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
304335
bool CheckLoad(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
305336
if (!CheckLive(S, OpPC, Ptr, AK_Read))
306337
return false;
338+
if (!CheckConstant(S, OpPC, Ptr))
339+
return false;
340+
307341
if (!CheckDummy(S, OpPC, Ptr))
308342
return false;
309343
if (!CheckExtern(S, OpPC, Ptr))
@@ -605,13 +639,7 @@ bool CheckDeclRef(InterpState &S, CodePtr OpPC, const DeclRefExpr *DR) {
605639
}
606640
} else if (const auto *VD = dyn_cast<VarDecl>(D)) {
607641
if (!VD->getType().isConstQualified()) {
608-
S.FFDiag(E,
609-
VD->getType()->isIntegralOrEnumerationType()
610-
? diag::note_constexpr_ltor_non_const_int
611-
: diag::note_constexpr_ltor_non_constexpr,
612-
1)
613-
<< VD;
614-
S.Note(VD->getLocation(), diag::note_declared_at) << VD->getSourceRange();
642+
diagnoseNonConstVariable(S, OpPC, VD);
615643
return false;
616644
}
617645

‎clang/lib/AST/Interp/Interp.h‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,9 @@ bool CheckSubobject(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
7777
/// Checks if a pointer points to const storage.
7878
bool CheckConst(InterpState &S, CodePtr OpPC, const Pointer &Ptr);
7979

80+
/// Checks if the Descriptor is of a constexpr or const global variable.
81+
bool CheckConstant(InterpState &S, CodePtr OpPC, const Descriptor *Desc);
82+
8083
/// Checks if a pointer points to a mutable field.
8184
bool CheckMutable(InterpState &S, CodePtr OpPC, const Pointer &Ptr);
8285

@@ -1004,12 +1007,23 @@ bool SetThisField(InterpState &S, CodePtr OpPC, uint32_t I) {
10041007
template <PrimType Name, class T = typename PrimConv<Name>::T>
10051008
bool GetGlobal(InterpState &S, CodePtr OpPC, uint32_t I) {
10061009
const Block *B = S.P.getGlobal(I);
1010+
1011+
if (!CheckConstant(S, OpPC, B->getDescriptor()))
1012+
return false;
10071013
if (B->isExtern())
10081014
return false;
10091015
S.Stk.push<T>(B->deref<T>());
10101016
return true;
10111017
}
10121018

1019+
/// Same as GetGlobal, but without the checks.
1020+
template <PrimType Name, class T = typename PrimConv<Name>::T>
1021+
bool GetGlobalUnchecked(InterpState &S, CodePtr OpPC, uint32_t I) {
1022+
auto *B = S.P.getGlobal(I);
1023+
S.Stk.push<T>(B->deref<T>());
1024+
return true;
1025+
}
1026+
10131027
template <PrimType Name, class T = typename PrimConv<Name>::T>
10141028
bool SetGlobal(InterpState &S, CodePtr OpPC, uint32_t I) {
10151029
// TODO: emit warning.

‎clang/lib/AST/Interp/Opcodes.td‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,7 @@ def CheckGlobalCtor : Opcode {}
379379

380380
// [] -> [Value]
381381
def GetGlobal : AccessOpcode;
382+
def GetGlobalUnchecked : AccessOpcode;
382383
// [Value] -> []
383384
def InitGlobal : AccessOpcode;
384385
// [Value] -> []

‎clang/test/AST/Interp/arrays.cpp‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -566,3 +566,35 @@ namespace GH69115 {
566566
static_assert(foo2() == 0, "");
567567
#endif
568568
}
569+
570+
namespace NonConstReads {
571+
#if __cplusplus >= 202002L
572+
void *p = nullptr; // ref-note {{declared here}} \
573+
// expected-note {{declared here}}
574+
575+
int arr[!p]; // ref-error {{not allowed at file scope}} \
576+
// expected-error {{not allowed at file scope}} \
577+
// ref-warning {{variable length arrays}} \
578+
// ref-note {{read of non-constexpr variable 'p'}} \
579+
// expected-warning {{variable length arrays}} \
580+
// expected-note {{read of non-constexpr variable 'p'}}
581+
int z; // ref-note {{declared here}} \
582+
// expected-note {{declared here}}
583+
int a[z]; // ref-error {{not allowed at file scope}} \
584+
// expected-error {{not allowed at file scope}} \
585+
// ref-warning {{variable length arrays}} \
586+
// ref-note {{read of non-const variable 'z'}} \
587+
// expected-warning {{variable length arrays}} \
588+
// expected-note {{read of non-const variable 'z'}}
589+
#else
590+
void *p = nullptr;
591+
int arr[!p]; // ref-error {{not allowed at file scope}} \
592+
// expected-error {{not allowed at file scope}}
593+
int z;
594+
int a[z]; // ref-error {{not allowed at file scope}} \
595+
// expected-error {{not allowed at file scope}}
596+
#endif
597+
598+
const int y = 0;
599+
int yy[y];
600+
}

‎clang/test/AST/Interp/cxx23.cpp‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,22 +25,32 @@ constexpr int g(int n) { // ref20-error {{constexpr function never produc
2525
}
2626

2727
constexpr int c_thread_local(int n) { // ref20-error {{constexpr function never produces a constant expression}} \
28-
// ref23-error {{constexpr function never produces a constant expression}}
28+
// ref23-error {{constexpr function never produces a constant expression}} \
29+
// expected20-error {{constexpr function never produces a constant expression}} \
30+
// expected23-error {{constexpr function never produces a constant expression}}
2931
static _Thread_local int m = 0; // ref20-note {{control flows through the definition of a thread_local variable}} \
3032
// ref20-warning {{is a C++23 extension}} \
3133
// ref23-note {{control flows through the definition of a thread_local variable}} \
32-
// expected20-warning {{is a C++23 extension}}
33-
return m;
34+
// expected20-warning {{is a C++23 extension}} \
35+
// expected20-note {{declared here}} \
36+
// expected23-note {{declared here}}
37+
return m; // expected20-note {{read of non-const variable}} \
38+
// expected23-note {{read of non-const variable}}
3439
}
3540

3641

3742
constexpr int gnu_thread_local(int n) { // ref20-error {{constexpr function never produces a constant expression}} \
38-
// ref23-error {{constexpr function never produces a constant expression}}
43+
// ref23-error {{constexpr function never produces a constant expression}} \
44+
// expected20-error {{constexpr function never produces a constant expression}} \
45+
// expected23-error {{constexpr function never produces a constant expression}}
3946
static __thread int m = 0; // ref20-note {{control flows through the definition of a thread_local variable}} \
4047
// ref20-warning {{is a C++23 extension}} \
4148
// ref23-note {{control flows through the definition of a thread_local variable}} \
42-
// expected20-warning {{is a C++23 extension}}
43-
return m;
49+
// expected20-warning {{is a C++23 extension}} \
50+
// expected20-note {{declared here}} \
51+
// expected23-note {{declared here}}
52+
return m; // expected20-note {{read of non-const variable}} \
53+
// expected23-note {{read of non-const variable}}
4454
}
4555

4656
constexpr int h(int n) { // ref20-error {{constexpr function never produces a constant expression}} \

‎clang/test/AST/Interp/literals.cpp‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1181,8 +1181,29 @@ namespace InvalidDeclRefs {
11811181
// expected-error {{not an integral constant expression}} \
11821182
// expected-note {{initializer of 'b02' is unknown}}
11831183

1184-
/// FIXME: This should also be diagnosed in the new interpreter.
1185-
int b03 = 3; // ref-note {{declared here}}
1184+
int b03 = 3; // ref-note {{declared here}} \
1185+
// expected-note {{declared here}}
11861186
static_assert(b03, ""); // ref-error {{not an integral constant expression}} \
1187-
// ref-note {{read of non-const variable}}
1187+
// ref-note {{read of non-const variable}} \
1188+
// expected-error {{not an integral constant expression}} \
1189+
// expected-note {{read of non-const variable}}
1190+
}
1191+
1192+
namespace NonConstReads {
1193+
void *p = nullptr; // ref-note {{declared here}} \
1194+
// expected-note {{declared here}}
1195+
static_assert(!p, ""); // ref-error {{not an integral constant expression}} \
1196+
// ref-note {{read of non-constexpr variable 'p'}} \
1197+
// expected-error {{not an integral constant expression}} \
1198+
// expected-note {{read of non-constexpr variable 'p'}}
1199+
1200+
int arr[!p]; // ref-error {{variable length array}} \
1201+
// expected-error {{variable length array}}
1202+
1203+
int z; // ref-note {{declared here}} \
1204+
// expected-note {{declared here}}
1205+
static_assert(z == 0, ""); // ref-error {{not an integral constant expression}} \
1206+
// ref-note {{read of non-const variable 'z'}} \
1207+
// expected-error {{not an integral constant expression}} \
1208+
// expected-note {{read of non-const variable 'z'}}
11881209
}

0 commit comments

Comments
 (0)