Skip to content

Commit 80d3756

Browse files
patrykstefanskipatrykstefanski
committed
[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
1 parent c67fa5d commit 80d3756

File tree

5 files changed

+80
-15
lines changed

5 files changed

+80
-15
lines changed

‎clang/include/clang/Sema/Sema.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7934,7 +7934,8 @@ class Sema final : public SemaBase {
79347934
/// built-in operations; ActOnBinOp handles overloaded operators.
79357935
ExprResult CreateBuiltinBinOp(SourceLocation OpLoc, BinaryOperatorKind Opc,
79367936
Expr *LHSExpr, Expr *RHSExpr,
7937-
bool ForFoldExpression = false);
7937+
bool ForFoldExpression = false,
7938+
bool ForBoundsCheckExpression = false);
79387939
void LookupBinOp(Scope *S, SourceLocation OpLoc, BinaryOperatorKind Opc,
79397940
UnresolvedSetImpl &Functions);
79407941

@@ -8642,7 +8643,7 @@ class Sema final : public SemaBase {
86428643
BinaryOperatorKind Opc);
86438644
QualType CheckLogicalOperands( // C99 6.5.[13,14]
86448645
ExprResult &LHS, ExprResult &RHS, SourceLocation Loc,
8645-
BinaryOperatorKind Opc);
8646+
BinaryOperatorKind Opc, bool Diagnose = true);
86468647
// CheckAssignmentOperands is used for both simple and compound assignment.
86478648
// For simple assignment, pass both expressions and a null converted type.
86488649
// For compound assignment, pass both expressions and the converted type.

‎clang/lib/AST/ExprConstant.cpp‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9448,6 +9448,20 @@ LValueExprEvaluator::VisitCompoundLiteralExpr(const CompoundLiteralExpr *E) {
94489448
*Lit = APValue();
94499449
} else {
94509450
assert(!Info.getLangOpts().CPlusPlus);
9451+
9452+
// TO_UPSTREAM(BoundsSafety) ON
9453+
// BoundsSafety can reuse the same CLE when emitting a bounds-check
9454+
// condition. If the state already exist, reset it and evaluate again.
9455+
// FIXME: Same as above, we could re-use the previously evaluated value.
9456+
APValue *Val;
9457+
if (Info.getLangOpts().BoundsSafety &&
9458+
(Val = Info.CurrentCall->getCurrentTemporary(E))) {
9459+
Lit = Val;
9460+
Result.set(E);
9461+
*Lit = APValue();
9462+
} else
9463+
// TO_UPSTREAM(BoundsSafety) OFF
9464+
94519465
Lit = &Info.CurrentCall->createTemporary(E, E->getInitializer()->getType(),
94529466
ScopeKind::Block, Result);
94539467
}

‎clang/lib/Sema/SemaChecking.cpp‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14430,6 +14430,14 @@ class SequenceChecker : public ConstEvaluatedExprVisitor<SequenceChecker> {
1443014430
SequenceExpressionsInOrder(PLIE->getInitExprs());
1443114431
}
1443214432

14433+
void VisitMaterializeSequenceExpr(const MaterializeSequenceExpr *MSE) {
14434+
Visit(MSE->getWrappedExpr());
14435+
}
14436+
14437+
void VisitBoundsCheckExpr(const BoundsCheckExpr *BCE) {
14438+
Visit(BCE->getGuardedExpr());
14439+
}
14440+
1443314441
private:
1443414442
void SequenceExpressionsInOrder(ArrayRef<const Expr *> ExpressionList) {
1443514443
SmallVector<SequenceTree::Seq, 32> Elts;

‎clang/lib/Sema/SemaExpr.cpp‎

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16503,7 +16503,8 @@ inline QualType Sema::CheckBitwiseOperands(ExprResult &LHS, ExprResult &RHS,
1650316503
// C99 6.5.[13,14]
1650416504
inline QualType Sema::CheckLogicalOperands(ExprResult &LHS, ExprResult &RHS,
1650516505
SourceLocation Loc,
16506-
BinaryOperatorKind Opc) {
16506+
BinaryOperatorKind Opc,
16507+
bool Diagnose) {
1650716508
// Check vector operands differently.
1650816509
if (LHS.get()->getType()->isVectorType() ||
1650916510
RHS.get()->getType()->isVectorType())
@@ -16534,7 +16535,8 @@ inline QualType Sema::CheckLogicalOperands(ExprResult &LHS, ExprResult &RHS,
1653416535
// Diagnose cases where the user write a logical and/or but probably meant a
1653516536
// bitwise one. We do this when the LHS is a non-bool integer and the RHS
1653616537
// is a constant.
16537-
if (!EnumConstantInBoolContext && LHS.get()->getType()->isIntegerType() &&
16538+
if (Diagnose && !EnumConstantInBoolContext &&
16539+
LHS.get()->getType()->isIntegerType() &&
1653816540
!LHS.get()->getType()->isBooleanType() &&
1653916541
RHS.get()->getType()->isIntegerType() && !RHS.get()->isValueDependent() &&
1654016542
// Don't warn in macros or template instantiations.
@@ -18250,7 +18252,8 @@ static bool needsConversionOfHalfVec(bool OpRequiresConversion, ASTContext &Ctx,
1825018252

1825118253
ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
1825218254
BinaryOperatorKind Opc, Expr *LHSExpr,
18253-
Expr *RHSExpr, bool ForFoldExpression) {
18255+
Expr *RHSExpr, bool ForFoldExpression,
18256+
bool ForBoundsCheckExpression) {
1825418257
if (getLangOpts().CPlusPlus11 && isa<InitListExpr>(RHSExpr)) {
1825518258
// The syntax only allows initializer lists on the RHS of assignment,
1825618259
// so we don't need to worry about accepting invalid code for
@@ -18404,7 +18407,8 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
1840418407
case BO_LAnd:
1840518408
case BO_LOr:
1840618409
ConvertHalfVec = true;
18407-
ResultTy = CheckLogicalOperands(LHS, RHS, OpLoc, Opc);
18410+
ResultTy = CheckLogicalOperands(LHS, RHS, OpLoc, Opc,
18411+
/*Diagnose=*/!ForBoundsCheckExpression);
1840818412
break;
1840918413
case BO_MulAssign:
1841018414
case BO_DivAssign:
@@ -26188,7 +26192,9 @@ ExprResult BoundsCheckBuilder::BuildImplicitPointerArith(Sema &S,
2618826192
Pointer = CastResult.get();
2618926193
}
2619026194

26191-
return S.CreateBuiltinBinOp(SourceLocation(), Opc, Pointer, RHS);
26195+
return S.CreateBuiltinBinOp(SourceLocation(), Opc, Pointer, RHS,
26196+
/*ForFoldExpression=*/false,
26197+
/*ForBoundsCheckExpression=*/true);
2619226198
}
2619326199

2619426200
OpaqueValueExpr *BoundsCheckBuilder::OpaqueWrap(Sema &S, Expr *E) {
@@ -26207,7 +26213,9 @@ bool BoundsCheckBuilder::BuildIndexBounds(Sema &S, Expr *Min, Expr *Max,
2620726213
if (!(Max = Upper.get()))
2620826214
return false;
2620926215
}
26210-
ExprResult Count = S.CreateBuiltinBinOp(Max->getBeginLoc(), BO_Sub, Max, Min);
26216+
ExprResult Count = S.CreateBuiltinBinOp(Max->getBeginLoc(), BO_Sub, Max, Min,
26217+
/*ForFoldExpression=*/false,
26218+
/*ForBoundsCheckExpression=*/true);
2621126219
if (!Count.get())
2621226220
return false;
2621326221

@@ -26230,7 +26238,8 @@ bool BoundsCheckBuilder::BuildIndexBounds(Sema &S, Expr *Min, Expr *Max,
2623026238
R.push_back(OpaqueStart);
2623126239
IsSigned |= OpaqueStart->getType()->isSignedIntegerOrEnumerationType();
2623226240
ExprResult CountTotal = S.CreateBuiltinBinOp(
26233-
OpaqueStart->getBeginLoc(), BO_Add, OpaqueStart, AccessCount);
26241+
OpaqueStart->getBeginLoc(), BO_Add, OpaqueStart, AccessCount,
26242+
/*ForFoldExpression=*/false, /*ForBoundsCheckExpression=*/true);
2623426243
if (!(AccessCount = CountTotal.get()))
2623526244
return false;
2623626245
}
@@ -26415,7 +26424,9 @@ ExprResult BoundsCheckBuilder::Build(Sema &S, Expr *GuardedValue) {
2641526424
if (NullCheck.isInvalid())
2641626425
return ExprError();
2641726426
// !Ptr || 0 <= Count <= (Upper - Ptr)
26418-
Cond = S.CreateBuiltinBinOp(Loc, BO_LOr, NullCheck.get(), Cond.get());
26427+
Cond = S.CreateBuiltinBinOp(Loc, BO_LOr, NullCheck.get(), Cond.get(),
26428+
/*ForFoldExpression=*/false,
26429+
/*ForBoundsCheckExpression=*/true);
2641926430
if (Cond.isInvalid())
2642026431
return ExprError();
2642126432
}
@@ -26430,8 +26441,10 @@ ExprResult BoundsCheckBuilder::Build(Sema &S, Expr *GuardedValue) {
2643026441
ExprResult BasePtrBoundsCheck = BuildLEChecks(S, Loc, Bounds, OVEs);
2643126442
if (!BasePtrBoundsCheck.get())
2643226443
return ExprError();
26433-
Cond = S.CreateBuiltinBinOp(
26434-
Loc, BO_LAnd, BasePtrBoundsCheck.get(), Cond.get());
26444+
Cond =
26445+
S.CreateBuiltinBinOp(Loc, BO_LAnd, BasePtrBoundsCheck.get(), Cond.get(),
26446+
/*ForFoldExpression=*/false,
26447+
/*ForBoundsCheckExpression=*/true);
2643526448
if (!Cond.get())
2643626449
return ExprError();
2643726450
}
@@ -26471,15 +26484,18 @@ ExprResult BoundsCheckBuilder::BuildLEChecks(
2647126484
Bound = OVEs.back();
2647226485
}
2647326486

26474-
ExprResult LEBounds = S.CreateBuiltinBinOp(Loc, BO_LE, Bound, Next);
26487+
ExprResult LEBounds = S.CreateBuiltinBinOp(
26488+
Loc, BO_LE, Bound, Next, /*ForFoldExpression=*/false,
26489+
/*ForBoundsCheckExpression=*/true);
2647526490
if (LEBounds.isInvalid())
2647626491
return ExprError();
2647726492

2647826493
if (!CondAccum) {
2647926494
CondAccum = LEBounds.get();
2648026495
} else {
26481-
ExprResult CondAnd = S.CreateBuiltinBinOp(Loc, BO_LAnd,
26482-
CondAccum, LEBounds.get());
26496+
ExprResult CondAnd = S.CreateBuiltinBinOp(
26497+
Loc, BO_LAnd, CondAccum, LEBounds.get(), /*ForFoldExpression=*/false,
26498+
/*ForBoundsCheckExpression=*/true);
2648326499
if (CondAnd.isInvalid())
2648426500
return ExprError();
2648526501
CondAccum = CondAnd.get();

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 5
2+
3+
// RUN: %clang_cc1 -O2 -triple arm64-apple-ios -emit-llvm -fbounds-safety -o - %s | FileCheck %s
4+
5+
void foo(char tag[3]);
6+
7+
// CHECK-LABEL: define void @bar(
8+
// CHECK-SAME: ) local_unnamed_addr #[[ATTR0:[0-9]+]] {
9+
// CHECK-NEXT: [[ENTRY:.*:]]
10+
// CHECK-NEXT: [[DOTCOMPOUNDLITERAL:%.*]] = alloca [3 x i8], align 1
11+
// CHECK-NEXT: store i8 65, ptr [[DOTCOMPOUNDLITERAL]], align 1, !tbaa [[TBAA2:![0-9]+]]
12+
// CHECK-NEXT: [[ARRAYINIT_ELEMENT:%.*]] = getelementptr inbounds nuw i8, ptr [[DOTCOMPOUNDLITERAL]], i64 1
13+
// CHECK-NEXT: store i8 66, ptr [[ARRAYINIT_ELEMENT]], align 1, !tbaa [[TBAA2]]
14+
// CHECK-NEXT: [[ARRAYINIT_ELEMENT1:%.*]] = getelementptr inbounds nuw i8, ptr [[DOTCOMPOUNDLITERAL]], i64 2
15+
// CHECK-NEXT: store i8 67, ptr [[ARRAYINIT_ELEMENT1]], align 1, !tbaa [[TBAA2]]
16+
// CHECK-NEXT: call void @foo(ptr noundef nonnull [[DOTCOMPOUNDLITERAL]]) #[[ATTR2:[0-9]+]]
17+
// CHECK-NEXT: ret void
18+
//
19+
void bar(void) {
20+
foo((char[3]){'A', 'B', 'C'});
21+
}
22+
//.
23+
// CHECK: [[TBAA2]] = !{[[META3:![0-9]+]], [[META3]], i64 0}
24+
// CHECK: [[META3]] = !{!"omnipotent char", [[META4:![0-9]+]], i64 0}
25+
// CHECK: [[META4]] = !{!"Simple C/C++ TBAA"}
26+
//.

0 commit comments

Comments
 (0)