[clang] cb7b10c - [clang][bytecode] Fail on mutable reads from revisited variables (#133064)

[via cfe-commits]

Author: Timm Baeder
Date: 2025-03-26T12:29:31+01:00
New Revision: cb7b10c66ef8a62f22a77a1480bba382863fcc90

URL: https://github.com/llvm/llvm-project/commit/cb7b10c66ef8a62f22a77a1480bba382863fcc90
DIFF: https://github.com/llvm/llvm-project/commit/cb7b10c66ef8a62f22a77a1480bba382863fcc90.diff

LOG: [clang][bytecode] Fail on mutable reads from revisited variables (#133064)

When revisiting a variable, we do that by simply calling visitDecl() for
it, which means it will end up with the same EvalID as the rest of the
evaluation - but this way we end up allowing reads from mutable
variables. Disallow that.

Added: 
    clang/test/AST/ByteCode/codegen-mutable-read.cpp

Modified: 
    clang/lib/AST/ByteCode/Interp.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp
index 512a3f45e3a83..187477713bef8 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -602,8 +602,18 @@ bool CheckMutable(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
   // In C++14 onwards, it is permitted to read a mutable member whose
   // lifetime began within the evaluation.
   if (S.getLangOpts().CPlusPlus14 &&
-      Ptr.block()->getEvalID() == S.Ctx.getEvalID())
+      Ptr.block()->getEvalID() == S.Ctx.getEvalID()) {
+    // FIXME: This check is necessary because (of the way) we revisit
+    // 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;
+    }
     return true;
+  }
 
   const SourceInfo &Loc = S.Current->getSource(OpPC);
   const FieldDecl *Field = Ptr.getField();

diff  --git a/clang/test/AST/ByteCode/codegen-mutable-read.cpp b/clang/test/AST/ByteCode/codegen-mutable-read.cpp
new file mode 100644
index 0000000000000..afa46d34b0673
--- /dev/null
+++ b/clang/test/AST/ByteCode/codegen-mutable-read.cpp
@@ -0,0 +1,36 @@
+// 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