Skip to content

Commit 0089ca9

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 (cherry picked from commit 80d3756)
1 parent 57b2922 commit 0089ca9

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
@@ -7901,7 +7901,8 @@ class Sema final : public SemaBase {
79017901
/// built-in operations; ActOnBinOp handles overloaded operators.
79027902
ExprResult CreateBuiltinBinOp(SourceLocation OpLoc, BinaryOperatorKind Opc,
79037903
Expr *LHSExpr, Expr *RHSExpr,
7904-
bool ForFoldExpression = false);
7904+
bool ForFoldExpression = false,
7905+
bool ForBoundsCheckExpression = false);
79057906
void LookupBinOp(Scope *S, SourceLocation OpLoc, BinaryOperatorKind Opc,
79067907
UnresolvedSetImpl &Functions);
79077908

@@ -8612,7 +8613,7 @@ class Sema final : public SemaBase {
86128613
BinaryOperatorKind Opc);
86138614
QualType CheckLogicalOperands( // C99 6.5.[13,14]
86148615
ExprResult &LHS, ExprResult &RHS, SourceLocation Loc,
8615-
BinaryOperatorKind Opc);
8616+
BinaryOperatorKind Opc, bool Diagnose = true);
86168617
// CheckAssignmentOperands is used for both simple and compound assignment.
86178618
// For simple assignment, pass both expressions and a null converted type.
86188619
// 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
@@ -9358,6 +9358,20 @@ LValueExprEvaluator::VisitCompoundLiteralExpr(const CompoundLiteralExpr *E) {
93589358
*Lit = APValue();
93599359
} else {
93609360
assert(!Info.getLangOpts().CPlusPlus);
9361+
9362+
// TO_UPSTREAM(BoundsSafety) ON
9363+
// BoundsSafety can reuse the same CLE when emitting a bounds-check
9364+
// condition. If the state already exist, reset it and evaluate again.
9365+
// FIXME: Same as above, we could re-use the previously evaluated value.
9366+
APValue *Val;
9367+
if (Info.getLangOpts().BoundsSafety &&
9368+
(Val = Info.CurrentCall->getCurrentTemporary(E))) {
9369+
Lit = Val;
9370+
Result.set(E);
9371+
*Lit = APValue();
9372+
} else
9373+
// TO_UPSTREAM(BoundsSafety) OFF
9374+
93619375
Lit = &Info.CurrentCall->createTemporary(E, E->getInitializer()->getType(),
93629376
ScopeKind::Block, Result);
93639377
}

‎clang/lib/Sema/SemaChecking.cpp‎

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

14132+
void VisitMaterializeSequenceExpr(const MaterializeSequenceExpr *MSE) {
14133+
Visit(MSE->getWrappedExpr());
14134+
}
14135+
14136+
void VisitBoundsCheckExpr(const BoundsCheckExpr *BCE) {
14137+
Visit(BCE->getGuardedExpr());
14138+
}
14139+
1413214140
private:
1413314141
void SequenceExpressionsInOrder(ArrayRef<const Expr *> ExpressionList) {
1413414142
SmallVector<SequenceTree::Seq, 32> Elts;

‎clang/lib/Sema/SemaExpr.cpp‎

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16371,7 +16371,8 @@ inline QualType Sema::CheckBitwiseOperands(ExprResult &LHS, ExprResult &RHS,
1637116371
// C99 6.5.[13,14]
1637216372
inline QualType Sema::CheckLogicalOperands(ExprResult &LHS, ExprResult &RHS,
1637316373
SourceLocation Loc,
16374-
BinaryOperatorKind Opc) {
16374+
BinaryOperatorKind Opc,
16375+
bool Diagnose) {
1637516376
// Check vector operands differently.
1637616377
if (LHS.get()->getType()->isVectorType() ||
1637716378
RHS.get()->getType()->isVectorType())
@@ -16402,7 +16403,8 @@ inline QualType Sema::CheckLogicalOperands(ExprResult &LHS, ExprResult &RHS,
1640216403
// Diagnose cases where the user write a logical and/or but probably meant a
1640316404
// bitwise one. We do this when the LHS is a non-bool integer and the RHS
1640416405
// is a constant.
16405-
if (!EnumConstantInBoolContext && LHS.get()->getType()->isIntegerType() &&
16406+
if (Diagnose && !EnumConstantInBoolContext &&
16407+
LHS.get()->getType()->isIntegerType() &&
1640616408
!LHS.get()->getType()->isBooleanType() &&
1640716409
RHS.get()->getType()->isIntegerType() && !RHS.get()->isValueDependent() &&
1640816410
// Don't warn in macros or template instantiations.
@@ -18107,7 +18109,8 @@ static bool needsConversionOfHalfVec(bool OpRequiresConversion, ASTContext &Ctx,
1810718109

1810818110
ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
1810918111
BinaryOperatorKind Opc, Expr *LHSExpr,
18110-
Expr *RHSExpr, bool ForFoldExpression) {
18112+
Expr *RHSExpr, bool ForFoldExpression,
18113+
bool ForBoundsCheckExpression) {
1811118114
if (getLangOpts().CPlusPlus11 && isa<InitListExpr>(RHSExpr)) {
1811218115
// The syntax only allows initializer lists on the RHS of assignment,
1811318116
// so we don't need to worry about accepting invalid code for
@@ -18264,7 +18267,8 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
1826418267
case BO_LAnd:
1826518268
case BO_LOr:
1826618269
ConvertHalfVec = true;
18267-
ResultTy = CheckLogicalOperands(LHS, RHS, OpLoc, Opc);
18270+
ResultTy = CheckLogicalOperands(LHS, RHS, OpLoc, Opc,
18271+
/*Diagnose=*/!ForBoundsCheckExpression);
1826818272
break;
1826918273
case BO_MulAssign:
1827018274
case BO_DivAssign:
@@ -26025,7 +26029,9 @@ ExprResult BoundsCheckBuilder::BuildImplicitPointerArith(Sema &S,
2602526029
Pointer = CastResult.get();
2602626030
}
2602726031

26028-
return S.CreateBuiltinBinOp(SourceLocation(), Opc, Pointer, RHS);
26032+
return S.CreateBuiltinBinOp(SourceLocation(), Opc, Pointer, RHS,
26033+
/*ForFoldExpression=*/false,
26034+
/*ForBoundsCheckExpression=*/true);
2602926035
}
2603026036

2603126037
OpaqueValueExpr *BoundsCheckBuilder::OpaqueWrap(Sema &S, Expr *E) {
@@ -26044,7 +26050,9 @@ bool BoundsCheckBuilder::BuildIndexBounds(Sema &S, Expr *Min, Expr *Max,
2604426050
if (!(Max = Upper.get()))
2604526051
return false;
2604626052
}
26047-
ExprResult Count = S.CreateBuiltinBinOp(Max->getBeginLoc(), BO_Sub, Max, Min);
26053+
ExprResult Count = S.CreateBuiltinBinOp(Max->getBeginLoc(), BO_Sub, Max, Min,
26054+
/*ForFoldExpression=*/false,
26055+
/*ForBoundsCheckExpression=*/true);
2604826056
if (!Count.get())
2604926057
return false;
2605026058

@@ -26067,7 +26075,8 @@ bool BoundsCheckBuilder::BuildIndexBounds(Sema &S, Expr *Min, Expr *Max,
2606726075
R.push_back(OpaqueStart);
2606826076
IsSigned |= OpaqueStart->getType()->isSignedIntegerOrEnumerationType();
2606926077
ExprResult CountTotal = S.CreateBuiltinBinOp(
26070-
OpaqueStart->getBeginLoc(), BO_Add, OpaqueStart, AccessCount);
26078+
OpaqueStart->getBeginLoc(), BO_Add, OpaqueStart, AccessCount,
26079+
/*ForFoldExpression=*/false, /*ForBoundsCheckExpression=*/true);
2607126080
if (!(AccessCount = CountTotal.get()))
2607226081
return false;
2607326082
}
@@ -26252,7 +26261,9 @@ ExprResult BoundsCheckBuilder::Build(Sema &S, Expr *GuardedValue) {
2625226261
if (NullCheck.isInvalid())
2625326262
return ExprError();
2625426263
// !Ptr || 0 <= Count <= (Upper - Ptr)
26255-
Cond = S.CreateBuiltinBinOp(Loc, BO_LOr, NullCheck.get(), Cond.get());
26264+
Cond = S.CreateBuiltinBinOp(Loc, BO_LOr, NullCheck.get(), Cond.get(),
26265+
/*ForFoldExpression=*/false,
26266+
/*ForBoundsCheckExpression=*/true);
2625626267
if (Cond.isInvalid())
2625726268
return ExprError();
2625826269
}
@@ -26267,8 +26278,10 @@ ExprResult BoundsCheckBuilder::Build(Sema &S, Expr *GuardedValue) {
2626726278
ExprResult BasePtrBoundsCheck = BuildLEChecks(S, Loc, Bounds, OVEs);
2626826279
if (!BasePtrBoundsCheck.get())
2626926280
return ExprError();
26270-
Cond = S.CreateBuiltinBinOp(
26271-
Loc, BO_LAnd, BasePtrBoundsCheck.get(), Cond.get());
26281+
Cond =
26282+
S.CreateBuiltinBinOp(Loc, BO_LAnd, BasePtrBoundsCheck.get(), Cond.get(),
26283+
/*ForFoldExpression=*/false,
26284+
/*ForBoundsCheckExpression=*/true);
2627226285
if (!Cond.get())
2627326286
return ExprError();
2627426287
}
@@ -26308,15 +26321,18 @@ ExprResult BoundsCheckBuilder::BuildLEChecks(
2630826321
Bound = OVEs.back();
2630926322
}
2631026323

26311-
ExprResult LEBounds = S.CreateBuiltinBinOp(Loc, BO_LE, Bound, Next);
26324+
ExprResult LEBounds = S.CreateBuiltinBinOp(
26325+
Loc, BO_LE, Bound, Next, /*ForFoldExpression=*/false,
26326+
/*ForBoundsCheckExpression=*/true);
2631226327
if (LEBounds.isInvalid())
2631326328
return ExprError();
2631426329

2631526330
if (!CondAccum) {
2631626331
CondAccum = LEBounds.get();
2631726332
} else {
26318-
ExprResult CondAnd = S.CreateBuiltinBinOp(Loc, BO_LAnd,
26319-
CondAccum, LEBounds.get());
26333+
ExprResult CondAnd = S.CreateBuiltinBinOp(
26334+
Loc, BO_LAnd, CondAccum, LEBounds.get(), /*ForFoldExpression=*/false,
26335+
/*ForBoundsCheckExpression=*/true);
2632026336
if (CondAnd.isInvalid())
2632126337
return ExprError();
2632226338
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)