llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) <details> <summary>Changes</summary> --- Full diff: https://github.com/llvm/llvm-project/pull/133701.diff 4 Files Affected: - (modified) clang/lib/AST/ByteCode/Interp.cpp (+13-5) - (modified) clang/lib/AST/ByteCode/Interp.h (+7) - (added) clang/test/AST/ByteCode/codegen-constexpr-unknown.cpp (+72) - (removed) clang/test/AST/ByteCode/codegen-mutable-read.cpp (-36) ``````````diff diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp index 187477713bef8..0acfe01a42410 100644 --- a/clang/lib/AST/ByteCode/Interp.cpp +++ b/clang/lib/AST/ByteCode/Interp.cpp @@ -302,6 +302,17 @@ void cleanupAfterFunctionCall(InterpState &S, CodePtr OpPC, TYPE_SWITCH(Ty, S.Stk.discard<T>()); } +// FIXME: Instead of using this fairly expensive test, we should +// just mark constexpr-unknown values when creating them. +bool isConstexprUnknown(const Pointer &P) { + if (!P.isBlockPointer()) + return false; + if (P.isDummy()) + return false; + const VarDecl *VD = P.block()->getDescriptor()->asVarDecl(); + return VD && VD->hasLocalStorage(); +} + bool CheckBCPResult(InterpState &S, const Pointer &Ptr) { if (Ptr.isDummy()) return false; @@ -607,11 +618,8 @@ bool CheckMutable(InterpState &S, CodePtr OpPC, const Pointer &Ptr) { // variables in Compiler.cpp:visitDeclRef. Revisiting a so far // unknown variable will get the same EvalID and we end up allowing // reads from mutable members of it. - if (!S.inConstantContext()) { - if (const VarDecl *VD = Ptr.block()->getDescriptor()->asVarDecl(); - VD && VD->hasLocalStorage()) - 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 ee4139fbc9530..938077a9f10ae 100644 --- a/clang/lib/AST/ByteCode/Interp.h +++ b/clang/lib/AST/ByteCode/Interp.h @@ -173,6 +173,8 @@ static bool handleOverflow(InterpState &S, CodePtr OpPC, const T &SrcValue) { bool handleFixedPointOverflow(InterpState &S, CodePtr OpPC, const FixedPoint &FP); +bool isConstexprUnknown(const Pointer &P); + enum class ShiftDir { Left, Right }; /// Checks if the shift operation is legal. @@ -1062,6 +1064,11 @@ inline bool CmpHelperEQ<Pointer>(InterpState &S, CodePtr OpPC, CompareFn Fn) { } } + if (!S.inConstantContext()) { + if (isConstexprUnknown(LHS) || isConstexprUnknown(RHS)) + return false; + } + if (Pointer::hasSameBase(LHS, RHS)) { unsigned VL = LHS.getByteOffset(); unsigned VR = RHS.getByteOffset(); diff --git a/clang/test/AST/ByteCode/codegen-constexpr-unknown.cpp b/clang/test/AST/ByteCode/codegen-constexpr-unknown.cpp new file mode 100644 index 0000000000000..f62117d5f7bec --- /dev/null +++ b/clang/test/AST/ByteCode/codegen-constexpr-unknown.cpp @@ -0,0 +1,72 @@ +// RUN: %clang_cc1 -triple x86_64-linux -emit-llvm -fcxx-exceptions -o - %s | FileCheck %s +// RUN: %clang_cc1 -triple x86_64-linux -emit-llvm -fcxx-exceptions -o - %s -fexperimental-new-constant-interpreter | FileCheck %s + + +/// In the if expression below, the read from s.i should fail. +/// If it doesn't, and we actually read the value 0, the call to +/// func() will always occur, resuliting in a runtime failure. + +struct S { + mutable int i = 0; +}; + +void func() { + __builtin_abort(); +}; + +void setI(const S &s) { + s.i = 12; +} + +int main() { + const S s; + + setI(s); + + if (s.i == 0) + func(); + + return 0; +} + +// CHECK: define dso_local noundef i32 @main() +// CHECK: br +// CHECK: if.then +// CHECK: if.end +// CHECK: ret i32 0 + + +/// Similarly, here we revisit the BindingDecl. +struct F { int x; }; +int main2() { + const F const s{99}; + const auto& [r1] = s; + if (&r1 != &s.x) + __builtin_abort(); + return 0; +} +// CHECK: define dso_local noundef i32 @_Z5main2v() +// CHECK: br +// CHECK: if.then +// CHECK: if.end +// CHECK: ret i32 0 + +/// The comparison here should work and return 0. +class X { +public: + X(); + X(const X&); + X(const volatile X &); + ~X(); +}; +extern X OuterX; +X test24() { + X x; + if (&x == &OuterX) + throw 0; + return x; +} + +// CHECK: define dso_local void @_Z6test24v +// CHECK-NOT: lpad +// CHECK-NOT: eh.resume diff --git a/clang/test/AST/ByteCode/codegen-mutable-read.cpp b/clang/test/AST/ByteCode/codegen-mutable-read.cpp deleted file mode 100644 index afa46d34b0673..0000000000000 --- a/clang/test/AST/ByteCode/codegen-mutable-read.cpp +++ /dev/null @@ -1,36 +0,0 @@ -// RUN: %clang_cc1 -triple x86_64-linux -emit-llvm -o - %s | FileCheck %s -// RUN: %clang_cc1 -triple x86_64-linux -emit-llvm -o - %s -fexperimental-new-constant-interpreter | FileCheck %s - - -/// In the if expression below, the read from s.i should fail. -/// If it doesn't, and we actually read the value 0, the call to -/// func() will always occur, resuliting in a runtime failure. - -struct S { - mutable int i = 0; -}; - -void func() { - __builtin_abort(); -}; - -void setI(const S &s) { - s.i = 12; -} - -int main() { - const S s; - - setI(s); - - if (s.i == 0) - func(); - - return 0; -} - -// CHECK: define dso_local noundef i32 @main() -// CHECK: br -// CHECK: if.then -// CHECK: if.end -// CHECK: ret i32 0 `````````` </details> https://github.com/llvm/llvm-project/pull/133701 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
