Timm =?utf-8?q?Bäder?= <tbae...@redhat.com> Message-ID: <llvm.org/llvm/llvm-project/pull/76...@github.com> In-Reply-To:
https://github.com/tbaederr created https://github.com/llvm/llvm-project/pull/76718 Depends on https://github.com/llvm/llvm-project/pull/71919 In CheckConstant(), consider that in C++98 const variables may not be read at all, and diagnose that accordingly. >From f0496851aae146bd8ee1587f00a75c0a3ec93005 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbae...@redhat.com> Date: Thu, 9 Nov 2023 15:45:05 +0100 Subject: [PATCH 1/2] [clang][Interp] Diagnose reads from non-const global variables --- clang/lib/AST/Interp/ByteCodeExprGen.cpp | 2 +- clang/lib/AST/Interp/Interp.cpp | 44 ++++++++++++++++++------ clang/lib/AST/Interp/Interp.h | 14 ++++++++ clang/lib/AST/Interp/Opcodes.td | 1 + clang/test/AST/Interp/arrays.cpp | 32 +++++++++++++++++ clang/test/AST/Interp/cxx23.cpp | 22 ++++++++---- clang/test/AST/Interp/literals.cpp | 27 +++++++++++++-- 7 files changed, 122 insertions(+), 20 deletions(-) diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp index e6b3097a80d8f7..edbcf4005aedbf 100644 --- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp +++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp @@ -2330,7 +2330,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 a82d1c3c7c622a..5c4280382565dc 100644 --- a/clang/lib/AST/Interp/Interp.cpp +++ b/clang/lib/AST/Interp/Interp.cpp @@ -53,6 +53,18 @@ static bool Jf(InterpState &S, CodePtr &PC, int32_t Offset) { return true; } +static void diagnoseNonConstVariable(InterpState &S, CodePtr OpPC, + const ValueDecl *VD) { + 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()) @@ -159,9 +171,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; } @@ -204,6 +214,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(); } @@ -292,6 +320,8 @@ bool CheckInitialized(InterpState &S, CodePtr OpPC, const Pointer &Ptr, bool CheckLoad(InterpState &S, CodePtr OpPC, const Pointer &Ptr) { if (!CheckDummy(S, OpPC, Ptr)) return false; + if (!CheckConstant(S, OpPC, Ptr)) + return false; if (!CheckLive(S, OpPC, Ptr, AK_Read)) return false; if (!CheckExtern(S, OpPC, Ptr)) @@ -593,13 +623,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 828d4ea35526d6..a6a52d2ab006fc 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 c455731e76699f..ea5c16b3695089 100644 --- a/clang/test/AST/Interp/arrays.cpp +++ b/clang/test/AST/Interp/arrays.cpp @@ -558,3 +558,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 85adfe551384d2..5d87ddcebb9115 100644 --- a/clang/test/AST/Interp/literals.cpp +++ b/clang/test/AST/Interp/literals.cpp @@ -1177,8 +1177,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'}} } >From e3bb90a4db68ebf51c81fda5dcfcd20e9da5302d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbae...@redhat.com> Date: Sat, 30 Dec 2023 20:46:13 +0100 Subject: [PATCH 2/2] [clang][Interp] Fix diagnosing non-const variables pre-C++11 In CheckConstant(), consider that in C++98 const variables may not be read at all, and diagnose that accordingly. --- clang/lib/AST/Interp/Interp.cpp | 45 ++++++++++++++++++++++++++------- clang/test/AST/Interp/cxx11.cpp | 24 ++++++++++++++++++ clang/test/AST/Interp/cxx98.cpp | 36 ++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 9 deletions(-) create mode 100644 clang/test/AST/Interp/cxx11.cpp create mode 100644 clang/test/AST/Interp/cxx98.cpp diff --git a/clang/lib/AST/Interp/Interp.cpp b/clang/lib/AST/Interp/Interp.cpp index 5c4280382565dc..fb4696e21d80f2 100644 --- a/clang/lib/AST/Interp/Interp.cpp +++ b/clang/lib/AST/Interp/Interp.cpp @@ -55,13 +55,20 @@ static bool Jf(InterpState &S, CodePtr &PC, int32_t Offset) { 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; + + if (VD->getType()->isIntegralOrEnumerationType()) + S.FFDiag(Loc, diag::note_constexpr_ltor_non_const_int, 1) << VD; + else + S.FFDiag(Loc, + S.getLangOpts().CPlusPlus11 + ? diag::note_constexpr_ltor_non_constexpr + : diag::note_constexpr_ltor_non_integral, + 1) + << VD << VD->getType(); S.Note(VD->getLocation(), diag::note_declared_at); } @@ -216,12 +223,32 @@ bool CheckLive(InterpState &S, CodePtr OpPC, const Pointer &Ptr, bool CheckConstant(InterpState &S, CodePtr OpPC, const Descriptor *Desc) { assert(Desc); + + auto IsConstType = [&S](const VarDecl *VD) -> bool { + if (VD->isConstexpr()) + return true; + + if (S.getLangOpts().CPlusPlus && !S.getLangOpts().CPlusPlus11) + return false; + + QualType T = VD->getType(); + if (T.isConstQualified()) + return true; + + if (const auto *RT = T->getAs<ReferenceType>()) + return RT->getPointeeType().isConstQualified(); + + if (const auto *PT = T->getAs<PointerType>()) + return PT->getPointeeType().isConstQualified(); + + return false; + }; + if (const auto *D = Desc->asValueDecl()) { if (const auto *VD = dyn_cast<VarDecl>(D); - VD && VD->hasGlobalStorage() && - !(VD->isConstexpr() || VD->getType().isConstQualified())) { + VD && VD->hasGlobalStorage() && !IsConstType(VD)) { diagnoseNonConstVariable(S, OpPC, VD); - return false; + return S.inConstantContext(); } } diff --git a/clang/test/AST/Interp/cxx11.cpp b/clang/test/AST/Interp/cxx11.cpp new file mode 100644 index 00000000000000..81e293fec17502 --- /dev/null +++ b/clang/test/AST/Interp/cxx11.cpp @@ -0,0 +1,24 @@ +// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify=both,expected -std=c++11 %s +// RUN: %clang_cc1 -verify=both,ref -std=c++11 %s + +// expected-no-diagnostics + +namespace IntOrEnum { + const int k = 0; + const int &p = k; + template<int n> struct S {}; + S<p> s; +} + +const int cval = 2; +template <int> struct C{}; +template struct C<cval>; + + +/// FIXME: This example does not get properly diagnosed in the new interpreter. +extern const int recurse1; +const int recurse2 = recurse1; // ref-note {{here}} +const int recurse1 = 1; +int array1[recurse1]; +int array2[recurse2]; // ref-warning 2{{variable length array}} \ + // ref-note {{initializer of 'recurse2' is not a constant expression}} diff --git a/clang/test/AST/Interp/cxx98.cpp b/clang/test/AST/Interp/cxx98.cpp new file mode 100644 index 00000000000000..bc96723c2287da --- /dev/null +++ b/clang/test/AST/Interp/cxx98.cpp @@ -0,0 +1,36 @@ +// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify=both,expected -std=c++98 %s +// RUN: %clang_cc1 -verify=both,ref -std=c++98 %s + + + +namespace IntOrEnum { + const int k = 0; + const int &p = k; // both-note {{declared here}} + template<int n> struct S {}; + S<p> s; // both-error {{not an integral constant expression}} \ + // both-note {{read of variable 'p' of non-integral, non-enumeration type 'const int &'}} +} + +const int cval = 2; +template <int> struct C{}; +template struct C<cval>; + + +/// FIXME: This example does not get properly diagnosed in the new interpreter. +extern const int recurse1; +const int recurse2 = recurse1; // ref-note {{here}} +const int recurse1 = 1; +int array1[recurse1]; +int array2[recurse2]; // ref-warning 2{{variable length array}} \ + // ref-note {{initializer of 'recurse2' is not a constant expression}} \ + // expected-warning {{variable length array}} \ + // expected-error {{variable length array}} + +int NCI; // expected-note {{declared here}} \ + // ref-note {{declared here}} +int NCIA[NCI]; // expected-warning {{variable length array}} \ + // expected-error {{variable length array}} \\ + // expected-note {{read of non-const variable 'NCI'}} \ + // ref-warning {{variable length array}} \ + // ref-error {{variable length array}} \\ + // ref-note {{read of non-const variable 'NCI'}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
