[clang][bytecode] Diagnose comparisons with literals (#106734)

This requires adding a new opcode for PointerToBoolean casts, since we
otherwise emit too many diagnostics. But that fixes an older problem
when casting weak pointers to bool.

Author: tbaederr

clang/lib/AST/ByteCode/Compiler.cpp

@@ -505,14 +505,9 @@ bool Compiler<Emitter>::VisitCastExpr(const CastExpr *CE) {
   case CK_MemberPointerToBoolean: {
     PrimType PtrT = classifyPrim(SubExpr->getType());
 
-    // Just emit p != nullptr for this.
     if (!this->visit(SubExpr))
       return false;
-
-    if (!this->emitNull(PtrT, nullptr, CE))
-      return false;
-
-    return this->emitNE(PtrT, CE);
+    return this->emitIsNonNull(PtrT, CE);
   }
 
   case CK_IntegralComplexToBoolean:
@@ -2323,8 +2318,8 @@ bool Compiler<Emitter>::VisitMaterializeTemporaryExpr(
 
   // For everyhing else, use local variables.
   if (SubExprT) {
-    unsigned LocalIndex = allocateLocalPrimitive(
-        SubExpr, *SubExprT, /*IsConst=*/true, /*IsExtended=*/true);
+    unsigned LocalIndex = allocateLocalPrimitive(E, *SubExprT, /*IsConst=*/true,
+                                                 /*IsExtended=*/true);
     if (!this->visit(SubExpr))
       return false;
     if (!this->emitSetLocal(*SubExprT, LocalIndex, E))

clang/lib/AST/ByteCode/Interp.h

@@ -986,24 +986,7 @@ inline bool CmpHelperEQ<Pointer>(InterpState &S, CodePtr OpPC, CompareFn Fn) {
     }
   }
 
-  if (!Pointer::hasSameBase(LHS, RHS)) {
-    if (LHS.isOnePastEnd() && !RHS.isOnePastEnd() && !RHS.isZero() &&
-        RHS.getOffset() == 0) {
-      const SourceInfo &Loc = S.Current->getSource(OpPC);
-      S.FFDiag(Loc, diag::note_constexpr_pointer_comparison_past_end)
-          << LHS.toDiagnosticString(S.getASTContext());
-      return false;
-    } else if (RHS.isOnePastEnd() && !LHS.isOnePastEnd() && !LHS.isZero() &&
-               LHS.getOffset() == 0) {
-      const SourceInfo &Loc = S.Current->getSource(OpPC);
-      S.FFDiag(Loc, diag::note_constexpr_pointer_comparison_past_end)
-          << RHS.toDiagnosticString(S.getASTContext());
-      return false;
-    }
-
-    S.Stk.push<BoolT>(BoolT::from(Fn(ComparisonCategoryResult::Unordered)));
-    return true;
-  } else {
+  if (Pointer::hasSameBase(LHS, RHS)) {
     unsigned VL = LHS.getByteOffset();
     unsigned VR = RHS.getByteOffset();
 
@@ -1019,6 +1002,35 @@ inline bool CmpHelperEQ<Pointer>(InterpState &S, CodePtr OpPC, CompareFn Fn) {
     S.Stk.push<BoolT>(BoolT::from(Fn(Compare(VL, VR))));
     return true;
   }
+  // Otherwise we need to do a bunch of extra checks before returning Unordered.
+  if (LHS.isOnePastEnd() && !RHS.isOnePastEnd() && !RHS.isZero() &&
+      RHS.getOffset() == 0) {
+    const SourceInfo &Loc = S.Current->getSource(OpPC);
+    S.FFDiag(Loc, diag::note_constexpr_pointer_comparison_past_end)
+        << LHS.toDiagnosticString(S.getASTContext());
+    return false;
+  } else if (RHS.isOnePastEnd() && !LHS.isOnePastEnd() && !LHS.isZero() &&
+             LHS.getOffset() == 0) {
+    const SourceInfo &Loc = S.Current->getSource(OpPC);
+    S.FFDiag(Loc, diag::note_constexpr_pointer_comparison_past_end)
+        << RHS.toDiagnosticString(S.getASTContext());
+    return false;
+  }
+
+  bool BothNonNull = !LHS.isZero() && !RHS.isZero();
+  // Reject comparisons to literals.
+  for (const auto &P : {LHS, RHS}) {
+    if (P.isZero())
+      continue;
+    if (BothNonNull && P.pointsToLiteral()) {
+      const SourceInfo &Loc = S.Current->getSource(OpPC);
+      S.FFDiag(Loc, diag::note_constexpr_literal_comparison);
+      return false;
+    }
+  }
+
+  S.Stk.push<BoolT>(BoolT::from(Fn(ComparisonCategoryResult::Unordered)));
+  return true;
 }
 
 template <>
@@ -1030,9 +1042,10 @@ inline bool CmpHelperEQ<MemberPointer>(InterpState &S, CodePtr OpPC,
   // If either operand is a pointer to a weak function, the comparison is not
   // constant.
   for (const auto &MP : {LHS, RHS}) {
-    if (const CXXMethodDecl *MD = MP.getMemberFunction(); MD && MD->isWeak()) {
+    if (MP.isWeak()) {
       const SourceInfo &Loc = S.Current->getSource(OpPC);
-      S.FFDiag(Loc, diag::note_constexpr_mem_pointer_weak_comparison) << MD;
+      S.FFDiag(Loc, diag::note_constexpr_mem_pointer_weak_comparison)
+          << MP.getMemberFunction();
       return false;
     }
   }
@@ -2291,6 +2304,15 @@ inline bool Null(InterpState &S, CodePtr OpPC, const Descriptor *Desc) {
   return true;
 }
 
+template <PrimType Name, class T = typename PrimConv<Name>::T>
+inline bool IsNonNull(InterpState &S, CodePtr OpPC) {
+  const auto &P = S.Stk.pop<T>();
+  if (P.isWeak())
+    return false;
+  S.Stk.push<Boolean>(Boolean::from(!P.isZero()));
+  return true;
+}
+
 //===----------------------------------------------------------------------===//
 // This, ImplicitThis
 //===----------------------------------------------------------------------===//

clang/lib/AST/ByteCode/MemberPointer.h

@@ -84,6 +84,11 @@ class MemberPointer final {
 
   bool isZero() const { return Base.isZero() && !Dcl; }
   bool hasBase() const { return !Base.isZero(); }
+  bool isWeak() const {
+    if (const auto *MF = getMemberFunction())
+      return MF->isWeak();
+    return false;
+  }
 
   void print(llvm::raw_ostream &OS) const {
     OS << "MemberPtr(" << Base << " " << (const void *)Dcl << " + " << PtrOffset

clang/lib/AST/ByteCode/Opcodes.td

@@ -597,6 +597,11 @@ def Comp: Opcode {
   let HasGroup = 1;
 }
 
+def IsNonNull : Opcode {
+  let Types = [PtrTypeClass];
+  let HasGroup = 1;
+}
+
 //===----------------------------------------------------------------------===//
 // Cast, CastFP.
 //===----------------------------------------------------------------------===//

clang/lib/AST/ByteCode/Pointer.cpp

@@ -463,6 +463,17 @@ bool Pointer::hasSameArray(const Pointer &A, const Pointer &B) {
          A.getFieldDesc()->IsArray;
 }
 
+bool Pointer::pointsToLiteral() const {
+  if (isZero() || !isBlockPointer())
+    return false;
+
+  const Expr *E = block()->getDescriptor()->asExpr();
+  if (block()->isDynamic())
+    return false;
+
+  return E && !isa<MaterializeTemporaryExpr, StringLiteral>(E);
+}
+
 std::optional<APValue> Pointer::toRValue(const Context &Ctx,
                                          QualType ResultType) const {
   const ASTContext &ASTCtx = Ctx.getASTContext();

clang/lib/AST/ByteCode/Pointer.h

@@ -692,6 +692,10 @@ class Pointer {
   /// Checks if both given pointers point to the same block.
   static bool pointToSameBlock(const Pointer &A, const Pointer &B);
 
+  /// Whether this points to a block that's been created for a "literal lvalue",
+  /// i.e. a non-MaterializeTemporaryExpr Expr.
+  bool pointsToLiteral() const;
+
   /// Prints the pointer.
   void print(llvm::raw_ostream &OS) const;
 

clang/test/AST/ByteCode/builtin-functions.cpp

@@ -951,3 +951,9 @@ namespace shufflevector {
 }
 
 #endif
+
+namespace FunctionStart {
+  void a(void) {}
+  static_assert(__builtin_function_start(a) == a, ""); // both-error {{not an integral constant expression}} \
+                                                       // both-note {{comparison of addresses of literals has unspecified value}}
+}

clang/test/AST/ByteCode/weak.cpp

@@ -1,14 +1,8 @@
 // RUN: %clang_cc1 -std=c++20 -fexperimental-new-constant-interpreter -verify=expected,both %s
 // RUN: %clang_cc1 -std=c++20 -verify=ref,both %s
 
-
-
-
-/// FIXME: The new interpreter also emits the "address of weak declaration" note in the pointer-to-bool case.
-
 [[gnu::weak]] extern int a;
 int ha[(bool)&a]; // both-warning {{variable length arrays in C++ are a Clang extension}} \
-                  // expected-note {{comparison against address of weak declaration}} \
                   // both-error {{variable length array declaration not allowed at file scope}}
 int ha2[&a == nullptr]; // both-warning {{variable length arrays in C++ are a Clang extension}} \
                         // both-note {{comparison against address of weak declaration '&a' can only be performed at runtime}} \