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

[Timm Baeder via cfe-commits]

https://github.com/tbaederr created https://github.com/llvm/llvm-project/pull/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.

>From ca5afdcea9675618d25c14464ac6b67b3bfa24b2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Wed, 26 Mar 2025 10:59:29 +0100
Subject: [PATCH] [clang][bytecode] Fail on mutable reads from revisited
 variables

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.
---
 clang/lib/AST/ByteCode/Interp.cpp             | 12 ++++++-
 .../AST/ByteCode/codegen-mutable-read.cpp     | 36 +++++++++++++++++++
 2 files changed, 47 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/AST/ByteCode/codegen-mutable-read.cpp

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