[BoundsSafety] Allow evaluating the same CLE in a bounds-check

BoundsSafety can reuse the same CompoundLiteralExpr in a bounds-check
condition. This commit changes const-eval for CompoundLiteralExpr to
reset the state and evaluate it again when the temporary for that
CompoundLiteralExpr already exists.

In addition, we skip const-eval in Sema when building a bounds-check
condition. That const-eval is used only for diagnostics and we don't
want to diagnose AST that we are generating.

This is merely a workaround. The proper solution is to not emit the AST
for bounds-check condition and defer it to CodeGen.

rdar://157033241

Author: patrykstefanski

clang/include/clang/Sema/Sema.h

@@ -7934,7 +7934,8 @@ class Sema final : public SemaBase {
   /// built-in operations; ActOnBinOp handles overloaded operators.
   ExprResult CreateBuiltinBinOp(SourceLocation OpLoc, BinaryOperatorKind Opc,
                                 Expr *LHSExpr, Expr *RHSExpr,
-                                bool ForFoldExpression = false);
+                                bool ForFoldExpression = false,
+                                bool ForBoundsCheckExpression = false);
   void LookupBinOp(Scope *S, SourceLocation OpLoc, BinaryOperatorKind Opc,
                    UnresolvedSetImpl &Functions);
 
@@ -8642,7 +8643,7 @@ class Sema final : public SemaBase {
       BinaryOperatorKind Opc);
   QualType CheckLogicalOperands( // C99 6.5.[13,14]
       ExprResult &LHS, ExprResult &RHS, SourceLocation Loc,
-      BinaryOperatorKind Opc);
+      BinaryOperatorKind Opc, bool Diagnose = true);
   // CheckAssignmentOperands is used for both simple and compound assignment.
   // For simple assignment, pass both expressions and a null converted type.
   // For compound assignment, pass both expressions and the converted type.

clang/lib/AST/ExprConstant.cpp

@@ -9448,6 +9448,20 @@ LValueExprEvaluator::VisitCompoundLiteralExpr(const CompoundLiteralExpr *E) {
     *Lit = APValue();
   } else {
     assert(!Info.getLangOpts().CPlusPlus);
+
+    // TO_UPSTREAM(BoundsSafety) ON
+    // BoundsSafety can reuse the same CLE when emitting a bounds-check
+    // condition. If the state already exist, reset it and evaluate again.
+    // FIXME: Same as above, we could re-use the previously evaluated value.
+    APValue *Val;
+    if (Info.getLangOpts().BoundsSafety &&
+        (Val = Info.CurrentCall->getCurrentTemporary(E))) {
+      Lit = Val;
+      Result.set(E);
+      *Lit = APValue();
+    } else
+    // TO_UPSTREAM(BoundsSafety) OFF
+
     Lit = &Info.CurrentCall->createTemporary(E, E->getInitializer()->getType(),
                                              ScopeKind::Block, Result);
   }

clang/lib/Sema/SemaChecking.cpp

@@ -14430,6 +14430,14 @@ class SequenceChecker : public ConstEvaluatedExprVisitor<SequenceChecker> {
     SequenceExpressionsInOrder(PLIE->getInitExprs());
   }
 
+  void VisitMaterializeSequenceExpr(const MaterializeSequenceExpr *MSE) {
+    Visit(MSE->getWrappedExpr());
+  }
+
+  void VisitBoundsCheckExpr(const BoundsCheckExpr *BCE) {
+    Visit(BCE->getGuardedExpr());
+  }
+
 private:
   void SequenceExpressionsInOrder(ArrayRef<const Expr *> ExpressionList) {
     SmallVector<SequenceTree::Seq, 32> Elts;

clang/lib/Sema/SemaExpr.cpp

@@ -16503,7 +16503,8 @@ inline QualType Sema::CheckBitwiseOperands(ExprResult &LHS, ExprResult &RHS,
 // C99 6.5.[13,14]
 inline QualType Sema::CheckLogicalOperands(ExprResult &LHS, ExprResult &RHS,
                                            SourceLocation Loc,
-                                           BinaryOperatorKind Opc) {
+                                           BinaryOperatorKind Opc,
+                                           bool Diagnose) {
   // Check vector operands differently.
   if (LHS.get()->getType()->isVectorType() ||
       RHS.get()->getType()->isVectorType())
@@ -16534,7 +16535,8 @@ inline QualType Sema::CheckLogicalOperands(ExprResult &LHS, ExprResult &RHS,
   // Diagnose cases where the user write a logical and/or but probably meant a
   // bitwise one.  We do this when the LHS is a non-bool integer and the RHS
   // is a constant.
-  if (!EnumConstantInBoolContext && LHS.get()->getType()->isIntegerType() &&
+  if (Diagnose && !EnumConstantInBoolContext &&
+      LHS.get()->getType()->isIntegerType() &&
       !LHS.get()->getType()->isBooleanType() &&
       RHS.get()->getType()->isIntegerType() && !RHS.get()->isValueDependent() &&
       // Don't warn in macros or template instantiations.
@@ -18250,7 +18252,8 @@ static bool needsConversionOfHalfVec(bool OpRequiresConversion, ASTContext &Ctx,
 
 ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
                                     BinaryOperatorKind Opc, Expr *LHSExpr,
-                                    Expr *RHSExpr, bool ForFoldExpression) {
+                                    Expr *RHSExpr, bool ForFoldExpression,
+                                    bool ForBoundsCheckExpression) {
   if (getLangOpts().CPlusPlus11 && isa<InitListExpr>(RHSExpr)) {
     // The syntax only allows initializer lists on the RHS of assignment,
     // so we don't need to worry about accepting invalid code for
@@ -18404,7 +18407,8 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
   case BO_LAnd:
   case BO_LOr:
     ConvertHalfVec = true;
-    ResultTy = CheckLogicalOperands(LHS, RHS, OpLoc, Opc);
+    ResultTy = CheckLogicalOperands(LHS, RHS, OpLoc, Opc,
+                                    /*Diagnose=*/!ForBoundsCheckExpression);
     break;
   case BO_MulAssign:
   case BO_DivAssign:
@@ -26188,7 +26192,9 @@ ExprResult BoundsCheckBuilder::BuildImplicitPointerArith(Sema &S,
     Pointer = CastResult.get();
   }
 
-  return S.CreateBuiltinBinOp(SourceLocation(), Opc, Pointer, RHS);
+  return S.CreateBuiltinBinOp(SourceLocation(), Opc, Pointer, RHS,
+                              /*ForFoldExpression=*/false,
+                              /*ForBoundsCheckExpression=*/true);
 }
 
 OpaqueValueExpr *BoundsCheckBuilder::OpaqueWrap(Sema &S, Expr *E) {
@@ -26207,7 +26213,9 @@ bool BoundsCheckBuilder::BuildIndexBounds(Sema &S, Expr *Min, Expr *Max,
     if (!(Max = Upper.get()))
       return false;
   }
-  ExprResult Count = S.CreateBuiltinBinOp(Max->getBeginLoc(), BO_Sub, Max, Min);
+  ExprResult Count = S.CreateBuiltinBinOp(Max->getBeginLoc(), BO_Sub, Max, Min,
+                                          /*ForFoldExpression=*/false,
+                                          /*ForBoundsCheckExpression=*/true);
   if (!Count.get())
     return false;
 
@@ -26230,7 +26238,8 @@ bool BoundsCheckBuilder::BuildIndexBounds(Sema &S, Expr *Min, Expr *Max,
     R.push_back(OpaqueStart);
     IsSigned |= OpaqueStart->getType()->isSignedIntegerOrEnumerationType();
     ExprResult CountTotal = S.CreateBuiltinBinOp(
-        OpaqueStart->getBeginLoc(), BO_Add, OpaqueStart, AccessCount);
+        OpaqueStart->getBeginLoc(), BO_Add, OpaqueStart, AccessCount,
+        /*ForFoldExpression=*/false, /*ForBoundsCheckExpression=*/true);
     if (!(AccessCount = CountTotal.get()))
       return false;
   }
@@ -26415,7 +26424,9 @@ ExprResult BoundsCheckBuilder::Build(Sema &S, Expr *GuardedValue) {
     if (NullCheck.isInvalid())
       return ExprError();
     // !Ptr || 0 <= Count <= (Upper - Ptr)
-    Cond = S.CreateBuiltinBinOp(Loc, BO_LOr, NullCheck.get(), Cond.get());
+    Cond = S.CreateBuiltinBinOp(Loc, BO_LOr, NullCheck.get(), Cond.get(),
+                                /*ForFoldExpression=*/false,
+                                /*ForBoundsCheckExpression=*/true);
     if (Cond.isInvalid())
       return ExprError();
   }
@@ -26430,8 +26441,10 @@ ExprResult BoundsCheckBuilder::Build(Sema &S, Expr *GuardedValue) {
     ExprResult BasePtrBoundsCheck = BuildLEChecks(S, Loc, Bounds, OVEs);
     if (!BasePtrBoundsCheck.get())
       return ExprError();
-    Cond = S.CreateBuiltinBinOp(
-        Loc, BO_LAnd, BasePtrBoundsCheck.get(), Cond.get());
+    Cond =
+        S.CreateBuiltinBinOp(Loc, BO_LAnd, BasePtrBoundsCheck.get(), Cond.get(),
+                             /*ForFoldExpression=*/false,
+                             /*ForBoundsCheckExpression=*/true);
     if (!Cond.get())
       return ExprError();
   }
@@ -26471,15 +26484,18 @@ ExprResult BoundsCheckBuilder::BuildLEChecks(
       Bound = OVEs.back();
     }
 
-    ExprResult LEBounds = S.CreateBuiltinBinOp(Loc, BO_LE, Bound, Next);
+    ExprResult LEBounds = S.CreateBuiltinBinOp(
+        Loc, BO_LE, Bound, Next, /*ForFoldExpression=*/false,
+        /*ForBoundsCheckExpression=*/true);
     if (LEBounds.isInvalid())
       return ExprError();
 
     if (!CondAccum) {
       CondAccum = LEBounds.get();
     } else {
-      ExprResult CondAnd = S.CreateBuiltinBinOp(Loc, BO_LAnd,
-                                                CondAccum, LEBounds.get());
+      ExprResult CondAnd = S.CreateBuiltinBinOp(
+          Loc, BO_LAnd, CondAccum, LEBounds.get(), /*ForFoldExpression=*/false,
+          /*ForBoundsCheckExpression=*/true);
       if (CondAnd.isInvalid())
         return ExprError();
       CondAccum = CondAnd.get();

clang/test/BoundsSafety/CodeGen/compound-literal-func-arg-O2.c

@@ -0,0 +1,26 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 5
+
+// RUN: %clang_cc1 -O2 -triple arm64-apple-ios -emit-llvm -fbounds-safety -o - %s | FileCheck %s
+
+void foo(char tag[3]);
+
+// CHECK-LABEL: define void @bar(
+// CHECK-SAME: ) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  [[ENTRY:.*:]]
+// CHECK-NEXT:    [[DOTCOMPOUNDLITERAL:%.*]] = alloca [3 x i8], align 1
+// CHECK-NEXT:    store i8 65, ptr [[DOTCOMPOUNDLITERAL]], align 1, !tbaa [[TBAA2:![0-9]+]]
+// CHECK-NEXT:    [[ARRAYINIT_ELEMENT:%.*]] = getelementptr inbounds nuw i8, ptr [[DOTCOMPOUNDLITERAL]], i64 1
+// CHECK-NEXT:    store i8 66, ptr [[ARRAYINIT_ELEMENT]], align 1, !tbaa [[TBAA2]]
+// CHECK-NEXT:    [[ARRAYINIT_ELEMENT1:%.*]] = getelementptr inbounds nuw i8, ptr [[DOTCOMPOUNDLITERAL]], i64 2
+// CHECK-NEXT:    store i8 67, ptr [[ARRAYINIT_ELEMENT1]], align 1, !tbaa [[TBAA2]]
+// CHECK-NEXT:    call void @foo(ptr noundef nonnull [[DOTCOMPOUNDLITERAL]]) #[[ATTR2:[0-9]+]]
+// CHECK-NEXT:    ret void
+//
+void bar(void) {
+  foo((char[3]){'A', 'B', 'C'});
+}
+//.
+// CHECK: [[TBAA2]] = !{[[META3:![0-9]+]], [[META3]], i64 0}
+// CHECK: [[META3]] = !{!"omnipotent char", [[META4:![0-9]+]], i64 0}
+// CHECK: [[META4]] = !{!"Simple C/C++ TBAA"}
+//.