[clang][bytecode] Fail on mutable reads from revisited variables

[tbaederr]

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.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/133064.diff

2 Files Affected:

• (modified) clang/lib/AST/ByteCode/Interp.cpp (+11-1)
• (added) clang/test/AST/ByteCode/codegen-mutable-read.cpp (+36)

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