[clang] fc2766c - [clang][Interp] Diagnose reads from non-const global variables (#71919)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 12 20:51:06 PST 2024
Author: Timm Baeder
Date: 2024-01-13T05:51:03+01:00
New Revision: fc2766c1d4776a8e56a7b931a779c57bf7ed3d8b
URL: https://github.com/llvm/llvm-project/commit/fc2766c1d4776a8e56a7b931a779c57bf7ed3d8b
DIFF: https://github.com/llvm/llvm-project/commit/fc2766c1d4776a8e56a7b931a779c57bf7ed3d8b.diff
LOG: [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.
Added:
Modified:
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/Interp.cpp
clang/lib/AST/Interp/Interp.h
clang/lib/AST/Interp/Opcodes.td
clang/test/AST/Interp/arrays.cpp
clang/test/AST/Interp/cxx23.cpp
clang/test/AST/Interp/literals.cpp
Removed:
################################################################################
diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index 7f8bbe78732481..d6be9a306aeaf6 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -2337,7 +2337,7 @@ bool ByteCodeExprGen<Emitter>::visitDecl(const VarDecl *VD) {
auto GlobalIndex = P.getGlobal(VD);
assert(GlobalIndex); // visitVarDecl() didn't return false.
if (VarT) {
- if (!this->emitGetGlobal(*VarT, *GlobalIndex, VD))
+ if (!this->emitGetGlobalUnchecked(*VarT, *GlobalIndex, VD))
return false;
} else {
if (!this->emitGetPtrGlobal(*GlobalIndex, VD))
diff --git a/clang/lib/AST/Interp/Interp.cpp b/clang/lib/AST/Interp/Interp.cpp
index 9de0926b9dba9c..b95a52199846fa 100644
--- a/clang/lib/AST/Interp/Interp.cpp
+++ b/clang/lib/AST/Interp/Interp.cpp
@@ -53,6 +53,21 @@ static bool Jf(InterpState &S, CodePtr &PC, int32_t Offset) {
return true;
}
+static void diagnoseNonConstVariable(InterpState &S, CodePtr OpPC,
+ const ValueDecl *VD) {
+ if (!S.getLangOpts().CPlusPlus)
+ return;
+
+ const SourceInfo &Loc = S.Current->getSource(OpPC);
+ S.FFDiag(Loc,
+ VD->getType()->isIntegralOrEnumerationType()
+ ? diag::note_constexpr_ltor_non_const_int
+ : diag::note_constexpr_ltor_non_constexpr,
+ 1)
+ << VD;
+ S.Note(VD->getLocation(), diag::note_declared_at);
+}
+
static bool CheckActive(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
AccessKinds AK) {
if (Ptr.isActive())
@@ -171,9 +186,7 @@ bool CheckExtern(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
if (!S.checkingPotentialConstantExpression() && S.getLangOpts().CPlusPlus) {
const auto *VD = Ptr.getDeclDesc()->asValueDecl();
- const SourceInfo &Loc = S.Current->getSource(OpPC);
- S.FFDiag(Loc, diag::note_constexpr_ltor_non_constexpr, 1) << VD;
- S.Note(VD->getLocation(), diag::note_declared_at);
+ diagnoseNonConstVariable(S, OpPC, VD);
}
return false;
}
@@ -216,6 +229,24 @@ bool CheckLive(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
return true;
}
+bool CheckConstant(InterpState &S, CodePtr OpPC, const Descriptor *Desc) {
+ assert(Desc);
+ if (const auto *D = Desc->asValueDecl()) {
+ if (const auto *VD = dyn_cast<VarDecl>(D);
+ VD && VD->hasGlobalStorage() &&
+ !(VD->isConstexpr() || VD->getType().isConstQualified())) {
+ diagnoseNonConstVariable(S, OpPC, VD);
+ return false;
+ }
+ }
+
+ return true;
+}
+
+static bool CheckConstant(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
+ return CheckConstant(S, OpPC, Ptr.getDeclDesc());
+}
+
bool CheckDummy(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
return !Ptr.isZero() && !Ptr.isDummy();
}
@@ -304,6 +335,9 @@ bool CheckInitialized(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
bool CheckLoad(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
if (!CheckLive(S, OpPC, Ptr, AK_Read))
return false;
+ if (!CheckConstant(S, OpPC, Ptr))
+ return false;
+
if (!CheckDummy(S, OpPC, Ptr))
return false;
if (!CheckExtern(S, OpPC, Ptr))
@@ -605,13 +639,7 @@ bool CheckDeclRef(InterpState &S, CodePtr OpPC, const DeclRefExpr *DR) {
}
} else if (const auto *VD = dyn_cast<VarDecl>(D)) {
if (!VD->getType().isConstQualified()) {
- S.FFDiag(E,
- VD->getType()->isIntegralOrEnumerationType()
- ? diag::note_constexpr_ltor_non_const_int
- : diag::note_constexpr_ltor_non_constexpr,
- 1)
- << VD;
- S.Note(VD->getLocation(), diag::note_declared_at) << VD->getSourceRange();
+ diagnoseNonConstVariable(S, OpPC, VD);
return false;
}
diff --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h
index c05dea0cc55d3c..dbbc4c09ce42a1 100644
--- a/clang/lib/AST/Interp/Interp.h
+++ b/clang/lib/AST/Interp/Interp.h
@@ -77,6 +77,9 @@ bool CheckSubobject(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
/// Checks if a pointer points to const storage.
bool CheckConst(InterpState &S, CodePtr OpPC, const Pointer &Ptr);
+/// Checks if the Descriptor is of a constexpr or const global variable.
+bool CheckConstant(InterpState &S, CodePtr OpPC, const Descriptor *Desc);
+
/// Checks if a pointer points to a mutable field.
bool CheckMutable(InterpState &S, CodePtr OpPC, const Pointer &Ptr);
@@ -1004,12 +1007,23 @@ bool SetThisField(InterpState &S, CodePtr OpPC, uint32_t I) {
template <PrimType Name, class T = typename PrimConv<Name>::T>
bool GetGlobal(InterpState &S, CodePtr OpPC, uint32_t I) {
const Block *B = S.P.getGlobal(I);
+
+ if (!CheckConstant(S, OpPC, B->getDescriptor()))
+ return false;
if (B->isExtern())
return false;
S.Stk.push<T>(B->deref<T>());
return true;
}
+/// Same as GetGlobal, but without the checks.
+template <PrimType Name, class T = typename PrimConv<Name>::T>
+bool GetGlobalUnchecked(InterpState &S, CodePtr OpPC, uint32_t I) {
+ auto *B = S.P.getGlobal(I);
+ S.Stk.push<T>(B->deref<T>());
+ return true;
+}
+
template <PrimType Name, class T = typename PrimConv<Name>::T>
bool SetGlobal(InterpState &S, CodePtr OpPC, uint32_t I) {
// TODO: emit warning.
diff --git a/clang/lib/AST/Interp/Opcodes.td b/clang/lib/AST/Interp/Opcodes.td
index 69068e87d5720a..e01b6b9eea7dbb 100644
--- a/clang/lib/AST/Interp/Opcodes.td
+++ b/clang/lib/AST/Interp/Opcodes.td
@@ -379,6 +379,7 @@ def CheckGlobalCtor : Opcode {}
// [] -> [Value]
def GetGlobal : AccessOpcode;
+def GetGlobalUnchecked : AccessOpcode;
// [Value] -> []
def InitGlobal : AccessOpcode;
// [Value] -> []
diff --git a/clang/test/AST/Interp/arrays.cpp b/clang/test/AST/Interp/arrays.cpp
index 4aa10da55dd3ae..e14ff34dd73716 100644
--- a/clang/test/AST/Interp/arrays.cpp
+++ b/clang/test/AST/Interp/arrays.cpp
@@ -566,3 +566,35 @@ namespace GH69115 {
static_assert(foo2() == 0, "");
#endif
}
+
+namespace NonConstReads {
+#if __cplusplus >= 202002L
+ void *p = nullptr; // ref-note {{declared here}} \
+ // expected-note {{declared here}}
+
+ int arr[!p]; // ref-error {{not allowed at file scope}} \
+ // expected-error {{not allowed at file scope}} \
+ // ref-warning {{variable length arrays}} \
+ // ref-note {{read of non-constexpr variable 'p'}} \
+ // expected-warning {{variable length arrays}} \
+ // expected-note {{read of non-constexpr variable 'p'}}
+ int z; // ref-note {{declared here}} \
+ // expected-note {{declared here}}
+ int a[z]; // ref-error {{not allowed at file scope}} \
+ // expected-error {{not allowed at file scope}} \
+ // ref-warning {{variable length arrays}} \
+ // ref-note {{read of non-const variable 'z'}} \
+ // expected-warning {{variable length arrays}} \
+ // expected-note {{read of non-const variable 'z'}}
+#else
+ void *p = nullptr;
+ int arr[!p]; // ref-error {{not allowed at file scope}} \
+ // expected-error {{not allowed at file scope}}
+ int z;
+ int a[z]; // ref-error {{not allowed at file scope}} \
+ // expected-error {{not allowed at file scope}}
+#endif
+
+ const int y = 0;
+ int yy[y];
+}
diff --git a/clang/test/AST/Interp/cxx23.cpp b/clang/test/AST/Interp/cxx23.cpp
index bd1cf186d519c5..133ab10023df0e 100644
--- a/clang/test/AST/Interp/cxx23.cpp
+++ b/clang/test/AST/Interp/cxx23.cpp
@@ -25,22 +25,32 @@ constexpr int g(int n) { // ref20-error {{constexpr function never produc
}
constexpr int c_thread_local(int n) { // ref20-error {{constexpr function never produces a constant expression}} \
- // ref23-error {{constexpr function never produces a constant expression}}
+ // ref23-error {{constexpr function never produces a constant expression}} \
+ // expected20-error {{constexpr function never produces a constant expression}} \
+ // expected23-error {{constexpr function never produces a constant expression}}
static _Thread_local int m = 0; // ref20-note {{control flows through the definition of a thread_local variable}} \
// ref20-warning {{is a C++23 extension}} \
// ref23-note {{control flows through the definition of a thread_local variable}} \
- // expected20-warning {{is a C++23 extension}}
- return m;
+ // expected20-warning {{is a C++23 extension}} \
+ // expected20-note {{declared here}} \
+ // expected23-note {{declared here}}
+ return m; // expected20-note {{read of non-const variable}} \
+ // expected23-note {{read of non-const variable}}
}
constexpr int gnu_thread_local(int n) { // ref20-error {{constexpr function never produces a constant expression}} \
- // ref23-error {{constexpr function never produces a constant expression}}
+ // ref23-error {{constexpr function never produces a constant expression}} \
+ // expected20-error {{constexpr function never produces a constant expression}} \
+ // expected23-error {{constexpr function never produces a constant expression}}
static __thread int m = 0; // ref20-note {{control flows through the definition of a thread_local variable}} \
// ref20-warning {{is a C++23 extension}} \
// ref23-note {{control flows through the definition of a thread_local variable}} \
- // expected20-warning {{is a C++23 extension}}
- return m;
+ // expected20-warning {{is a C++23 extension}} \
+ // expected20-note {{declared here}} \
+ // expected23-note {{declared here}}
+ return m; // expected20-note {{read of non-const variable}} \
+ // expected23-note {{read of non-const variable}}
}
constexpr int h(int n) { // ref20-error {{constexpr function never produces a constant expression}} \
diff --git a/clang/test/AST/Interp/literals.cpp b/clang/test/AST/Interp/literals.cpp
index 61825bc11438f6..77b75151a12559 100644
--- a/clang/test/AST/Interp/literals.cpp
+++ b/clang/test/AST/Interp/literals.cpp
@@ -1181,8 +1181,29 @@ namespace InvalidDeclRefs {
// expected-error {{not an integral constant expression}} \
// expected-note {{initializer of 'b02' is unknown}}
- /// FIXME: This should also be diagnosed in the new interpreter.
- int b03 = 3; // ref-note {{declared here}}
+ int b03 = 3; // ref-note {{declared here}} \
+ // expected-note {{declared here}}
static_assert(b03, ""); // ref-error {{not an integral constant expression}} \
- // ref-note {{read of non-const variable}}
+ // ref-note {{read of non-const variable}} \
+ // expected-error {{not an integral constant expression}} \
+ // expected-note {{read of non-const variable}}
+}
+
+namespace NonConstReads {
+ void *p = nullptr; // ref-note {{declared here}} \
+ // expected-note {{declared here}}
+ static_assert(!p, ""); // ref-error {{not an integral constant expression}} \
+ // ref-note {{read of non-constexpr variable 'p'}} \
+ // expected-error {{not an integral constant expression}} \
+ // expected-note {{read of non-constexpr variable 'p'}}
+
+ int arr[!p]; // ref-error {{variable length array}} \
+ // expected-error {{variable length array}}
+
+ int z; // ref-note {{declared here}} \
+ // expected-note {{declared here}}
+ static_assert(z == 0, ""); // ref-error {{not an integral constant expression}} \
+ // ref-note {{read of non-const variable 'z'}} \
+ // expected-error {{not an integral constant expression}} \
+ // expected-note {{read of non-const variable 'z'}}
}
More information about the cfe-commits
mailing list