[clang][bytecode] Reject constexpr-unknown values from comparisons (#133701)

Author: tbaederr

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;
   }
 

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();

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