Timm =?utf-8?q?Bäder?= <[email protected]>, Timm =?utf-8?q?Bäder?= <[email protected]> Message-ID: In-Reply-To: <llvm.org/llvm/llvm-project/pull/[email protected]>
https://github.com/tbaederr updated https://github.com/llvm/llvm-project/pull/160350 >From 1cd424c074a14bdaf4c9ad1010ead5a568363781 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <[email protected]> Date: Tue, 23 Sep 2025 18:21:02 +0200 Subject: [PATCH 1/3] [clang][bytecode] Diagnose volatile writes --- clang/lib/AST/ByteCode/Compiler.cpp | 18 +++++++++----- clang/lib/AST/ByteCode/Compiler.h | 1 + clang/lib/AST/ByteCode/Interp.cpp | 2 ++ clang/lib/AST/ByteCode/Interp.h | 13 ++++++++++ clang/lib/AST/ByteCode/Opcodes.td | 1 + clang/test/AST/ByteCode/cxx23.cpp | 38 +++++++++++++++++++++++++++++ clang/test/AST/ByteCode/invalid.cpp | 2 +- 7 files changed, 68 insertions(+), 7 deletions(-) diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index b4da99957ee88..0b7b6cd64dd97 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -2934,8 +2934,9 @@ bool Compiler<Emitter>::VisitMaterializeTemporaryExpr( // For everyhing else, use local variables. if (SubExprT) { bool IsConst = SubExpr->getType().isConstQualified(); - unsigned LocalIndex = - allocateLocalPrimitive(E, *SubExprT, IsConst, E->getExtendingDecl()); + bool IsVolatile = SubExpr->getType().isVolatileQualified(); + unsigned LocalIndex = allocateLocalPrimitive( + E, *SubExprT, IsConst, IsVolatile, E->getExtendingDecl()); if (!this->visit(SubExpr)) return false; if (!this->emitSetLocal(*SubExprT, LocalIndex, E)) @@ -4452,6 +4453,9 @@ bool Compiler<Emitter>::visitAssignment(const Expr *LHS, const Expr *RHS, if (!this->visit(LHS)) return false; + if (LHS->getType().isVolatileQualified()) + return this->emitInvalidStore(LHS->getType().getTypePtr(), E); + // We don't support assignments in C. if (!Ctx.getLangOpts().CPlusPlus && !this->emitInvalid(E)) return false; @@ -4560,13 +4564,14 @@ bool Compiler<Emitter>::emitConst(const APSInt &Value, const Expr *E) { template <class Emitter> unsigned Compiler<Emitter>::allocateLocalPrimitive( - DeclTy &&Src, PrimType Ty, bool IsConst, const ValueDecl *ExtendingDecl, - ScopeKind SC, bool IsConstexprUnknown) { + DeclTy &&Src, PrimType Ty, bool IsConst, bool IsVolatile, + const ValueDecl *ExtendingDecl, ScopeKind SC, bool IsConstexprUnknown) { // FIXME: There are cases where Src.is<Expr*>() is wrong, e.g. // (int){12} in C. Consider using Expr::isTemporaryObject() instead // or isa<MaterializeTemporaryExpr>(). Descriptor *D = P.createDescriptor(Src, Ty, nullptr, Descriptor::InlineDescMD, - IsConst, isa<const Expr *>(Src)); + IsConst, isa<const Expr *>(Src), + /*IsMutable=*/false, IsVolatile); D->IsConstexprUnknown = IsConstexprUnknown; Scope::Local Local = this->createLocal(D); if (auto *VD = dyn_cast_if_present<ValueDecl>(Src.dyn_cast<const Decl *>())) @@ -4874,7 +4879,8 @@ Compiler<Emitter>::visitVarDecl(const VarDecl *VD, const Expr *Init, if (VarT) { unsigned Offset = this->allocateLocalPrimitive( - VD, *VarT, VD->getType().isConstQualified(), nullptr, ScopeKind::Block, + VD, *VarT, VD->getType().isConstQualified(), + VD->getType().isVolatileQualified(), nullptr, ScopeKind::Block, IsConstexprUnknown); if (Init) { // If this is a toplevel declaration, create a scope for the diff --git a/clang/lib/AST/ByteCode/Compiler.h b/clang/lib/AST/ByteCode/Compiler.h index 09599b3547888..5c46f75af4da3 100644 --- a/clang/lib/AST/ByteCode/Compiler.h +++ b/clang/lib/AST/ByteCode/Compiler.h @@ -327,6 +327,7 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>, /// Creates a local primitive value. unsigned allocateLocalPrimitive(DeclTy &&Decl, PrimType Ty, bool IsConst, + bool IsVolatile = false, const ValueDecl *ExtendingDecl = nullptr, ScopeKind SC = ScopeKind::Block, bool IsConstexprUnknown = false); diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp index 8aaefc70e506e..21af3d6ac7f90 100644 --- a/clang/lib/AST/ByteCode/Interp.cpp +++ b/clang/lib/AST/ByteCode/Interp.cpp @@ -889,6 +889,8 @@ bool CheckStore(InterpState &S, CodePtr OpPC, const Pointer &Ptr) { return false; if (!CheckConst(S, OpPC, Ptr)) return false; + if (!CheckVolatile(S, OpPC, Ptr, AK_Assign)) + return false; if (!S.inConstantContext() && isConstexprUnknown(Ptr)) return false; return true; diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h index 3bc1a67feeba2..83e0ecdd83331 100644 --- a/clang/lib/AST/ByteCode/Interp.h +++ b/clang/lib/AST/ByteCode/Interp.h @@ -3344,6 +3344,19 @@ inline bool InvalidCast(InterpState &S, CodePtr OpPC, CastKind Kind, return false; } +inline bool InvalidStore(InterpState &S, CodePtr OpPC, const Type *T) { + + if (S.getLangOpts().CPlusPlus) { + QualType VolatileType = QualType(T, 0).withVolatile(); + S.FFDiag(S.Current->getSource(OpPC), + diag::note_constexpr_access_volatile_type) + << AK_Assign << VolatileType; + } else { + S.FFDiag(S.Current->getSource(OpPC)); + } + return false; +} + inline bool InvalidDeclRef(InterpState &S, CodePtr OpPC, const DeclRefExpr *DR, bool InitializerFailed) { assert(DR); diff --git a/clang/lib/AST/ByteCode/Opcodes.td b/clang/lib/AST/ByteCode/Opcodes.td index 7af2df5318106..532c4448e6f40 100644 --- a/clang/lib/AST/ByteCode/Opcodes.td +++ b/clang/lib/AST/ByteCode/Opcodes.td @@ -797,6 +797,7 @@ def SideEffect : Opcode {} def InvalidCast : Opcode { let Args = [ArgCastKind, ArgBool]; } +def InvalidStore : Opcode { let Args = [ArgTypePtr]; } def CheckPseudoDtor : Opcode {} def InvalidDeclRef : Opcode { diff --git a/clang/test/AST/ByteCode/cxx23.cpp b/clang/test/AST/ByteCode/cxx23.cpp index 72c751d627a44..3f95a9140f261 100644 --- a/clang/test/AST/ByteCode/cxx23.cpp +++ b/clang/test/AST/ByteCode/cxx23.cpp @@ -393,6 +393,44 @@ namespace UnionMemberCallDiags { static_assert(g()); // all-error {{not an integral constant expression}} \ // all-note {{in call to}} } +#endif + +namespace VolatileWrites { + constexpr void test1() {// all20-error {{never produces a constant expression}} + int k; + volatile int &m = k; + m = 10; // all20-note {{assignment to volatile-qualified type 'volatile int'}} + } + + constexpr void test2() { // all20-error {{never produces a constant expression}} + volatile int k = 12; + + k = 13; // all20-note {{assignment to volatile-qualified type 'volatile int'}} + } + + constexpr void test3() { // all20-error {{never produces a constant expression}} + volatile int k = 12; // all20-note {{volatile object declared here}} + + *((int *)&k) = 13; // all20-note {{assignment to volatile object 'k' is not allowed in a constant expression}} + } + constexpr void test4() { // all20-error {{never produces a constant expression}} + int k = 12; + *((volatile int *)&k) = 13; // all20-note {{assignment to volatile-qualified type 'volatile int' is not allowed in a constant expression}} + } + +#if __cplusplus >= 202302L + struct S { + volatile int k; + }; + constexpr int test5() { + S s; + s.k = 12; // all-note {{assignment to volatile-qualified type 'volatile int' is not}} + + return 0; + } + static_assert(test5() == 0); // all-error{{not an integral constant expression}} \ + // all-note {{in call to}} #endif +} diff --git a/clang/test/AST/ByteCode/invalid.cpp b/clang/test/AST/ByteCode/invalid.cpp index affb40eada870..00db27419e36b 100644 --- a/clang/test/AST/ByteCode/invalid.cpp +++ b/clang/test/AST/ByteCode/invalid.cpp @@ -1,5 +1,5 @@ // RUN: %clang_cc1 -fcxx-exceptions -std=c++20 -fexperimental-new-constant-interpreter -verify=expected,both %s -// RUN: %clang_cc1 -fcxx-exceptions -std=c++20 -verify=ref,both %s +// RUN: %clang_cc1 -fcxx-exceptions -std=c++20 -verify=ref,both %s namespace Throw { >From 525e3f4ee2f6b0fd8ce7d412b0dc6bb6ca122493 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <[email protected]> Date: Thu, 25 Sep 2025 09:37:31 +0200 Subject: [PATCH 2/3] Also test parameters --- clang/lib/AST/ByteCode/Interp.h | 4 ---- clang/test/AST/ByteCode/cxx23.cpp | 20 ++++++++++++++++---- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h index 83e0ecdd83331..674173cf80121 100644 --- a/clang/lib/AST/ByteCode/Interp.h +++ b/clang/lib/AST/ByteCode/Interp.h @@ -1730,9 +1730,6 @@ inline bool GetPtrLocal(InterpState &S, CodePtr OpPC, uint32_t I) { } inline bool GetPtrParam(InterpState &S, CodePtr OpPC, uint32_t I) { - if (S.checkingPotentialConstantExpression()) { - return false; - } S.Stk.push<Pointer>(S.Current->getParamPointer(I)); return true; } @@ -3345,7 +3342,6 @@ inline bool InvalidCast(InterpState &S, CodePtr OpPC, CastKind Kind, } inline bool InvalidStore(InterpState &S, CodePtr OpPC, const Type *T) { - if (S.getLangOpts().CPlusPlus) { QualType VolatileType = QualType(T, 0).withVolatile(); S.FFDiag(S.Current->getSource(OpPC), diff --git a/clang/test/AST/ByteCode/cxx23.cpp b/clang/test/AST/ByteCode/cxx23.cpp index 3f95a9140f261..b1d6806c72548 100644 --- a/clang/test/AST/ByteCode/cxx23.cpp +++ b/clang/test/AST/ByteCode/cxx23.cpp @@ -1,8 +1,8 @@ // UNSUPPORTED: target={{.*}}-zos{{.*}} -// RUN: %clang_cc1 -std=c++20 -fsyntax-only -fcxx-exceptions -verify=ref,ref20,all,all20 %s -// RUN: %clang_cc1 -std=c++23 -fsyntax-only -fcxx-exceptions -verify=ref,ref23,all,all23 %s -// RUN: %clang_cc1 -std=c++20 -fsyntax-only -fcxx-exceptions -verify=expected20,all,all20 %s -fexperimental-new-constant-interpreter -// RUN: %clang_cc1 -std=c++23 -fsyntax-only -fcxx-exceptions -verify=expected23,all,all23 %s -fexperimental-new-constant-interpreter +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -fcxx-exceptions -Wno-deprecated-volatile -verify=ref,ref20,all,all20 %s +// RUN: %clang_cc1 -std=c++23 -fsyntax-only -fcxx-exceptions -Wno-deprecated-volatile -verify=ref,ref23,all,all23 %s +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -fcxx-exceptions -Wno-deprecated-volatile -verify=expected20,all,all20 %s -fexperimental-new-constant-interpreter +// RUN: %clang_cc1 -std=c++23 -fsyntax-only -fcxx-exceptions -Wno-deprecated-volatile -verify=expected23,all,all23 %s -fexperimental-new-constant-interpreter #define assert_active(F) if (!__builtin_is_within_lifetime(&F)) (1/0); @@ -433,4 +433,16 @@ namespace VolatileWrites { static_assert(test5() == 0); // all-error{{not an integral constant expression}} \ // all-note {{in call to}} #endif + + constexpr void test6(volatile int k) { // all20-error {{never produces a constant expression}} + k = 14; // all20-note {{assignment to volatile-qualified type 'volatile int' is not}} + } + + /// FIXME: Parameters currently not marked volatile in the bytecode interpreter. + constexpr bool test7(volatile int k) { // ref-note {{declared here}} + *((int *)&k) = 13; // ref-note {{assignment to volatile object 'k' is not allowed in a constant expression}} + return true; + } + static_assert(test7(12)); // ref-error {{not an integral constant expression}} \ + // ref-note {{in call to}} } >From 6b0ad1f250ed718ff388956ec07df16bc7f01c97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <[email protected]> Date: Thu, 25 Sep 2025 09:45:51 +0200 Subject: [PATCH 3/3] Mark parameters const/volatile --- clang/lib/AST/ByteCode/Context.cpp | 11 +++++++++-- clang/test/AST/ByteCode/cxx23.cpp | 12 +++++++----- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/clang/lib/AST/ByteCode/Context.cpp b/clang/lib/AST/ByteCode/Context.cpp index 71d0bcf61a5ff..1ef146fd31540 100644 --- a/clang/lib/AST/ByteCode/Context.cpp +++ b/clang/lib/AST/ByteCode/Context.cpp @@ -521,9 +521,13 @@ const Function *Context::getOrCreateFunction(const FunctionDecl *FuncDecl) { // Assign descriptors to all parameters. // Composite objects are lowered to pointers. for (const ParmVarDecl *PD : FuncDecl->parameters()) { + bool IsConst = PD->getType().isConstQualified(); + bool IsVolatile = PD->getType().isVolatileQualified(); + OptPrimType T = classify(PD->getType()); PrimType PT = T.value_or(PT_Ptr); - Descriptor *Desc = P->createDescriptor(PD, PT); + Descriptor *Desc = P->createDescriptor(PD, PT, nullptr, std::nullopt, IsConst, /*IsTemporary=*/false, /*IsMutable=*/false, IsVolatile); + ParamDescriptors.insert({ParamOffset, {PT, Desc}}); ParamOffsets.push_back(ParamOffset); ParamOffset += align(primSize(PT)); @@ -549,9 +553,12 @@ const Function *Context::getOrCreateObjCBlock(const BlockExpr *E) { // Assign descriptors to all parameters. // Composite objects are lowered to pointers. for (const ParmVarDecl *PD : BD->parameters()) { + bool IsConst = PD->getType().isConstQualified(); + bool IsVolatile = PD->getType().isVolatileQualified(); + OptPrimType T = classify(PD->getType()); PrimType PT = T.value_or(PT_Ptr); - Descriptor *Desc = P->createDescriptor(PD, PT); + Descriptor *Desc = P->createDescriptor(PD, PT, nullptr, std::nullopt, IsConst, /*IsTemporary=*/false, /*IsMutable=*/false, IsVolatile); ParamDescriptors.insert({ParamOffset, {PT, Desc}}); ParamOffsets.push_back(ParamOffset); ParamOffset += align(primSize(PT)); diff --git a/clang/test/AST/ByteCode/cxx23.cpp b/clang/test/AST/ByteCode/cxx23.cpp index b1d6806c72548..8028b433c7bc8 100644 --- a/clang/test/AST/ByteCode/cxx23.cpp +++ b/clang/test/AST/ByteCode/cxx23.cpp @@ -438,11 +438,13 @@ namespace VolatileWrites { k = 14; // all20-note {{assignment to volatile-qualified type 'volatile int' is not}} } - /// FIXME: Parameters currently not marked volatile in the bytecode interpreter. - constexpr bool test7(volatile int k) { // ref-note {{declared here}} - *((int *)&k) = 13; // ref-note {{assignment to volatile object 'k' is not allowed in a constant expression}} + constexpr bool test7(volatile int k) { // all-note {{declared here}} \ + // expected20-error {{never produces a constant expression}} \ + // expected20-note {{declared here}} + *((int *)&k) = 13; // all-note {{assignment to volatile object 'k' is not allowed in a constant expression}} \ + // expected20-note {{assignment to volatile object 'k' is not allowed in a constant expression}} return true; } - static_assert(test7(12)); // ref-error {{not an integral constant expression}} \ - // ref-note {{in call to}} + static_assert(test7(12)); // all-error {{not an integral constant expression}} \ + // all-note {{in call to}} } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
