Skip to content

Commit 58a7c9f

Browse files
authored
[BoundsSafety] Apply started_by on PointerType before qualifiers so that getUnqualifiedType doesn't strip started_by (#10046)
* [BoundsSafety] Apply started_by on PointerType before qualifiers so that getUnqualifiedType doesn't strip started_by All other bounds annotations are applied directly to bare PointerType via recursive TypeVisitor but started_by. This caused started_by to be stripped wheneter there is a type query to get unqualified type and the pointer type has a qualifier. This bug caused the started_by pointer to be treated as a single pointer, leading to a run-time trap when it gets the upper bound which has zero memory available.. The fix is to apply started_by at the bare PointerType just like any other bounds annotated types. Also, this strips PtrAutoAttr added to started_by because it's no longer an implicit single pointer. rdar://137701968 * Fix build failure caused by TransformAttributedType * [BoundsSafety] SaveAndRestore MakeStartedByPointerType::Done in TransformPointerType PointerType can be visited multiple times if it's wrapped by AttributedType in order to build ModifiedType and EquivalentType. This fixes the issue where started_by wasn't attached properly in EquivalentType.
1 parent d7de9bd commit 58a7c9f

File tree

5 files changed

+339
-20
lines changed

5 files changed

+339
-20
lines changed

clang/lib/Sema/SemaDeclAttr.cpp

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5774,20 +5774,7 @@ class MakeStartedByPointerType
57745774
return QualType();
57755775
}
57765776

5777-
llvm::SaveAndRestore<bool> DoneLocal(Done, true);
5778-
5779-
T = BaseClass::TransformType(TLB, TL);
5780-
5781-
// Create a new started_by() pointer.
5782-
5783-
Expr *StartPtr = BuildStartPtrExpr();
5784-
if (!StartPtr)
5785-
return QualType();
5786-
5787-
QualType NewTy = SemaRef.Context.getDynamicRangePointerType(
5788-
T, StartPtr, /*EndPtr=*/nullptr, {NewStartPtrInfo}, {});
5789-
TLB.push<DynamicRangePointerTypeLoc>(NewTy);
5790-
return NewTy;
5777+
return BaseClass::TransformType(TLB, TL);
57915778
}
57925779

57935780
QualType TransformType(QualType T) { return BaseClass::TransformType(T); }
@@ -5829,8 +5816,42 @@ class MakeStartedByPointerType
58295816
return NewTy;
58305817
}
58315818

5819+
using TransformModifiedFnTy =
5820+
std::function<QualType(TypeLocBuilder &TLB, TypeLoc TL)>;
5821+
QualType TransformAttributedType(TypeLocBuilder &TLB, AttributedTypeLoc TL,
5822+
TransformModifiedFnTy TransformModifiedTypeFn) {
5823+
// Strip attr::PtrAutoAttr because the pointer is now marked __started_by
5824+
// as it is referred to by __ended_by.
5825+
const AttributedType *oldType = TL.getTypePtr();
5826+
if (oldType->getAttrKind() == attr::PtrAutoAttr && Level == 0 && !Done) {
5827+
return TransformModifiedTypeFn(TLB, TL.getModifiedLoc());
5828+
}
5829+
return BaseClass::TransformAttributedType(TLB, TL, TransformModifiedTypeFn);
5830+
}
5831+
5832+
QualType TransformAttributedType(TypeLocBuilder &TLB, AttributedTypeLoc TL) {
5833+
return TransformAttributedType(
5834+
TLB, TL, [&](TypeLocBuilder &TLB, TypeLoc ModifiedLoc) -> QualType {
5835+
return TransformType(TLB, ModifiedLoc);
5836+
});
5837+
}
5838+
58325839
QualType TransformPointerType(TypeLocBuilder &TLB, PointerTypeLoc TL) {
58335840
llvm::SaveAndRestore<unsigned> LevelLocal(Level);
5841+
llvm::SaveAndRestore<bool> DoneLocal(Done);
5842+
if (!Done && Level == 0) {
5843+
Expr *StartPtr = BuildStartPtrExpr();
5844+
if (!StartPtr)
5845+
return QualType();
5846+
Done = true;
5847+
5848+
QualType T = BaseClass::TransformPointerType(TLB, TL);
5849+
5850+
QualType NewTy = SemaRef.Context.getDynamicRangePointerType(
5851+
T, StartPtr, /*EndPtr=*/nullptr, {NewStartPtrInfo}, {});
5852+
TLB.push<DynamicRangePointerTypeLoc>(NewTy);
5853+
return NewTy;
5854+
}
58345855
if (!Done) {
58355856
assert(Level > 0);
58365857
--Level;
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
// FileCheck lines automatically generated using make-ast-dump-check-v2.py
2+
// REQUIRES: apple-disclosure-ios
3+
4+
// RUN: %clang_cc1 -triple x86_64-apple-mac -ast-dump -fbounds-safety %s 2>&1 | FileCheck %s
5+
// RUN: %clang_cc1 -triple x86_64-apple-mac -ast-dump -fbounds-safety -x objective-c -fbounds-attributes-objc-experimental %s 2>&1 | FileCheck %s
6+
#include <ptrcheck.h>
7+
8+
struct S {
9+
int *end;
10+
int *__ended_by(end) iter;
11+
};
12+
13+
// CHECK-LABEL:|-FunctionDecl {{.+}} used foo 'void (int *__single __ended_by(end)const, int *__single /* __started_by(start) */ const)'
14+
// CHECK-NEXT: | |-ParmVarDecl {{.+}} used start 'int *__single __ended_by(end)const':'int *__singleconst'
15+
// CHECK-NEXT: | |-ParmVarDecl {{.+}} used end 'int *__single /* __started_by(start) */ const':'int *__singleconst'
16+
// CHECK-NEXT: | `-CompoundStmt {{.+}}
17+
// CHECK-NEXT: | `-DeclStmt {{.+}}
18+
// CHECK-NEXT: | `-VarDecl {{.+}} local 'int *__bidi_indexable' cinit
19+
// CHECK-NEXT: | `-FirebloomPointerPromotionExpr {{.+}} 'int *__bidi_indexable'
20+
// CHECK-NEXT: | |-DeclRefExpr {{.+}} 'int *__single /* __started_by(start) */ const':'int *__singleconst' lvalue ParmVar {{.+}} 'end' 'int *__single /* __started_by(start) */ const':'int *__singleconst'
21+
// CHECK-NEXT: | |-ImplicitCastExpr {{.+}} 'int *__single /* __started_by(start) */ const':'int *__singleconst' <LValueToRValue>
22+
// CHECK-NEXT: | | `-DeclRefExpr {{.+}} 'int *__single /* __started_by(start) */ const':'int *__singleconst' lvalue ParmVar {{.+}} 'end' 'int *__single /* __started_by(start) */ const':'int *__singleconst'
23+
// CHECK-NEXT: | `-ImplicitCastExpr {{.+}} <<invalid sloc>> 'int *__single __ended_by(end)const':'int *__singleconst' <LValueToRValue>
24+
// CHECK-NEXT: | `-DeclRefExpr {{.+}} <<invalid sloc>> 'int *__single __ended_by(end)const':'int *__singleconst' lvalue ParmVar {{.+}} 'start' 'int *__single __ended_by(end)const':'int *__singleconst'
25+
void foo(int * const __ended_by(end) start, int* const end) {
26+
int *local = end;
27+
}
28+
29+
// CHECK-LABEL:`-FunctionDecl {{.+}} bar 'void (void)'
30+
// CHECK-NEXT: `-CompoundStmt {{.+}}
31+
// CHECK-NEXT: |-DeclStmt {{.+}}
32+
// CHECK-NEXT: | `-VarDecl {{.+}} used arr 'int[40]'
33+
// CHECK-NEXT: `-MaterializeSequenceExpr {{.+}} 'void' <Unbind>
34+
// CHECK-NEXT: |-MaterializeSequenceExpr {{.+}} 'void' <Bind>
35+
// CHECK-NEXT: | |-BoundsCheckExpr {{.+}} 'void' 'arr + 40 <= __builtin_get_pointer_upper_bound(arr) && arr <= arr + 40'
36+
// CHECK-NEXT: | | |-CallExpr {{.+}} 'void'
37+
// CHECK-NEXT: | | | |-ImplicitCastExpr {{.+}} 'void (*__single)(int *__single __ended_by(end)const, int *__single /* __started_by(start) */ const)' <FunctionToPointerDecay>
38+
// CHECK-NEXT: | | | | `-DeclRefExpr {{.+}} 'void (int *__single __ended_by(end)const, int *__single /* __started_by(start) */ const)' Function {{.+}} 'foo' 'void (int *__single __ended_by(end)const, int *__single /* __started_by(start) */ const)'
39+
// CHECK-NEXT: | | | |-ImplicitCastExpr {{.+}} 'int *__single __ended_by(end)':'int *__single' <FirebloomPointerCast>
40+
// CHECK-NEXT: | | | | `-OpaqueValueExpr {{.+}} 'int *__bidi_indexable'
41+
// CHECK-NEXT: | | | | `-ImplicitCastExpr {{.+}} 'int *__bidi_indexable' <ArrayToPointerDecay>
42+
// CHECK-NEXT: | | | | `-DeclRefExpr {{.+}} 'int[40]' lvalue Var {{.+}} 'arr' 'int[40]'
43+
// CHECK-NEXT: | | | `-ImplicitCastExpr {{.+}} 'int *__single /* __started_by(start) */ ':'int *__single' <FirebloomPointerCast>
44+
// CHECK-NEXT: | | | `-OpaqueValueExpr {{.+}} 'int *__bidi_indexable'
45+
// CHECK-NEXT: | | | `-BinaryOperator {{.+}} 'int *__bidi_indexable' '+'
46+
// CHECK-NEXT: | | | |-ImplicitCastExpr {{.+}} 'int *__bidi_indexable' <ArrayToPointerDecay>
47+
// CHECK-NEXT: | | | | `-DeclRefExpr {{.+}} 'int[40]' lvalue Var {{.+}} 'arr' 'int[40]'
48+
// CHECK-NEXT: | | | `-IntegerLiteral {{.+}} 'int' 40
49+
// CHECK-NEXT: | | `-BinaryOperator {{.+}} 'int' '&&'
50+
// CHECK-NEXT: | | |-BinaryOperator {{.+}} 'int' '<='
51+
// CHECK-NEXT: | | | |-ImplicitCastExpr {{.+}} 'int *' <FirebloomPointerCast>
52+
// CHECK-NEXT: | | | | `-OpaqueValueExpr {{.+}} 'int *__bidi_indexable'
53+
// CHECK-NEXT: | | | | `-BinaryOperator {{.+}} 'int *__bidi_indexable' '+'
54+
// CHECK-NEXT: | | | | |-ImplicitCastExpr {{.+}} 'int *__bidi_indexable' <ArrayToPointerDecay>
55+
// CHECK-NEXT: | | | | | `-DeclRefExpr {{.+}} 'int[40]' lvalue Var {{.+}} 'arr' 'int[40]'
56+
// CHECK-NEXT: | | | | `-IntegerLiteral {{.+}} 'int' 40
57+
// CHECK-NEXT: | | | `-ImplicitCastExpr {{.+}} 'int *' <FirebloomPointerCast>
58+
// CHECK-NEXT: | | | `-GetBoundExpr {{.+}} 'int *__bidi_indexable' upper
59+
// CHECK-NEXT: | | | `-OpaqueValueExpr {{.+}} 'int *__bidi_indexable'
60+
// CHECK-NEXT: | | | `-ImplicitCastExpr {{.+}} 'int *__bidi_indexable' <ArrayToPointerDecay>
61+
// CHECK-NEXT: | | | `-DeclRefExpr {{.+}} 'int[40]' lvalue Var {{.+}} 'arr' 'int[40]'
62+
// CHECK-NEXT: | | `-BinaryOperator {{.+}} 'int' '<='
63+
// CHECK-NEXT: | | |-ImplicitCastExpr {{.+}} 'int *' <FirebloomPointerCast>
64+
// CHECK-NEXT: | | | `-OpaqueValueExpr {{.+}} 'int *__bidi_indexable'
65+
// CHECK-NEXT: | | | `-ImplicitCastExpr {{.+}} 'int *__bidi_indexable' <ArrayToPointerDecay>
66+
// CHECK-NEXT: | | | `-DeclRefExpr {{.+}} 'int[40]' lvalue Var {{.+}} 'arr' 'int[40]'
67+
// CHECK-NEXT: | | `-ImplicitCastExpr {{.+}} 'int *' <FirebloomPointerCast>
68+
// CHECK-NEXT: | | `-OpaqueValueExpr {{.+}} 'int *__bidi_indexable'
69+
// CHECK-NEXT: | | `-BinaryOperator {{.+}} 'int *__bidi_indexable' '+'
70+
// CHECK-NEXT: | | |-ImplicitCastExpr {{.+}} 'int *__bidi_indexable' <ArrayToPointerDecay>
71+
// CHECK-NEXT: | | | `-DeclRefExpr {{.+}} 'int[40]' lvalue Var {{.+}} 'arr' 'int[40]'
72+
// CHECK-NEXT: | | `-IntegerLiteral {{.+}} 'int' 40
73+
// CHECK-NEXT: | |-OpaqueValueExpr {{.+}} 'int *__bidi_indexable'
74+
// CHECK-NEXT: | | `-ImplicitCastExpr {{.+}} 'int *__bidi_indexable' <ArrayToPointerDecay>
75+
// CHECK-NEXT: | | `-DeclRefExpr {{.+}} 'int[40]' lvalue Var {{.+}} 'arr' 'int[40]'
76+
// CHECK-NEXT: | `-OpaqueValueExpr {{.+}} 'int *__bidi_indexable'
77+
// CHECK-NEXT: | `-BinaryOperator {{.+}} 'int *__bidi_indexable' '+'
78+
// CHECK-NEXT: | |-ImplicitCastExpr {{.+}} 'int *__bidi_indexable' <ArrayToPointerDecay>
79+
// CHECK-NEXT: | | `-DeclRefExpr {{.+}} 'int[40]' lvalue Var {{.+}} 'arr' 'int[40]'
80+
// CHECK-NEXT: | `-IntegerLiteral {{.+}} 'int' 40
81+
// CHECK-NEXT: |-OpaqueValueExpr {{.+}} 'int *__bidi_indexable'
82+
// CHECK-NEXT: | `-ImplicitCastExpr {{.+}} 'int *__bidi_indexable' <ArrayToPointerDecay>
83+
// CHECK-NEXT: | `-DeclRefExpr {{.+}} 'int[40]' lvalue Var {{.+}} 'arr' 'int[40]'
84+
// CHECK-NEXT: `-OpaqueValueExpr {{.+}} 'int *__bidi_indexable'
85+
// CHECK-NEXT: `-BinaryOperator {{.+}} 'int *__bidi_indexable' '+'
86+
// CHECK-NEXT: |-ImplicitCastExpr {{.+}} 'int *__bidi_indexable' <ArrayToPointerDecay>
87+
// CHECK-NEXT: | `-DeclRefExpr {{.+}} 'int[40]' lvalue Var {{.+}} 'arr' 'int[40]'
88+
// CHECK-NEXT: `-IntegerLiteral {{.+}} 'int' 40
89+
void bar(void) {
90+
int arr[40];
91+
foo(arr, arr + 40);
92+
}

clang/test/BoundsSafety/AST/typedef-function-attrs-late-parsed-param.c

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,8 @@ void foo(
5656
// CHECK: | | `-PointerType {{.+}} 'void *__single'
5757
// CHECK: | | `-BuiltinType {{.+}} 'void'
5858
// CHECK: | `-DynamicRangePointerType {{.+}} 'void *__single /* __started_by(start) */ ' sugar
59-
// CHECK: | `-AttributedType {{.+}} 'void *__single' sugar
60-
// CHECK: | `-PointerType {{.+}} 'void *__single'
61-
// CHECK: | `-BuiltinType {{.+}} 'void'
59+
// CHECK: | `-PointerType {{.+}} 'void *__single'
60+
// CHECK: | `-BuiltinType {{.+}} 'void'
6261
// CHECK: |-TypedefDecl {{.+}} cb_ptr_t 'void (*)(int *__single __counted_by(count), int)'
6362
// CHECK: | `-PointerType {{.+}} 'void (*)(int *__single __counted_by(count), int)'
6463
// CHECK: | `-ParenType {{.+}} 'void (int *__single __counted_by(count), int)' sugar
@@ -86,9 +85,8 @@ void foo(
8685
// CHECK: | | `-PointerType {{.+}} 'void *__single'
8786
// CHECK: | | `-BuiltinType {{.+}} 'void'
8887
// CHECK: | `-DynamicRangePointerType {{.+}} 'void *__single /* __started_by(start) */ ' sugar
89-
// CHECK: | `-AttributedType {{.+}} 'void *__single' sugar
90-
// CHECK: | `-PointerType {{.+}} 'void *__single'
91-
// CHECK: | `-BuiltinType {{.+}} 'void'
88+
// CHECK: | `-PointerType {{.+}} 'void *__single'
89+
// CHECK: | `-BuiltinType {{.+}} 'void'
9290
// CHECK: |-VarDecl {{.+}} g_cb_ptr 'void (*__single)(int *__single __counted_by(count), int)'
9391
// CHECK: |-VarDecl {{.+}} g_sb_ptr 'void (*__single)(void *__single __sized_by(size), int)'
9492
// CHECK: |-VarDecl {{.+}} g_eb_ptr 'void (*__single)(void *__single __ended_by(end), void *__single /* __started_by(start) */ )'
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 5
2+
// REQUIRES: apple-disclosure-ios
3+
// RUN: %clang_cc1 -O2 -triple arm64-apple-iphoneos -fbounds-safety -emit-llvm %s -o - | FileCheck %s
4+
5+
#include <ptrcheck.h>
6+
7+
struct S {
8+
int *end;
9+
int *__ended_by(end) iter;
10+
};
11+
12+
void foo(int * const __ended_by(end) start, int* const end);
13+
14+
15+
// CHECK-LABEL: define dso_local void @good(
16+
// CHECK-SAME: ) local_unnamed_addr #[[ATTR0:[0-9]+]] {
17+
// CHECK-NEXT: [[ENTRY:.*:]]
18+
// CHECK-NEXT: [[ARR:%.*]] = alloca [40 x i32], align 4
19+
// CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 160, ptr nonnull [[ARR]]) #[[ATTR5:[0-9]+]]
20+
// CHECK-NEXT: [[BOUND_PTR_ARITH:%.*]] = getelementptr inbounds i8, ptr [[ARR]], i64 160
21+
// CHECK-NEXT: call void @foo(ptr noundef nonnull [[ARR]], ptr noundef nonnull [[BOUND_PTR_ARITH]]) #[[ATTR5]]
22+
// CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 160, ptr nonnull [[ARR]]) #[[ATTR5]]
23+
// CHECK-NEXT: ret void
24+
//
25+
void good(void) {
26+
int arr[40];
27+
foo(arr, arr + 40);
28+
}
29+
30+
// CHECK-LABEL: define dso_local void @oob_upper(
31+
// CHECK-SAME: ) local_unnamed_addr #[[ATTR0]] {
32+
// CHECK-NEXT: [[ENTRY:.*:]]
33+
// CHECK-NEXT: [[ARR:%.*]] = alloca [40 x i32], align 4
34+
// CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 160, ptr nonnull [[ARR]]) #[[ATTR5]]
35+
// CHECK-NEXT: [[UPPER:%.*]] = getelementptr inbounds i8, ptr [[ARR]], i64 160
36+
// CHECK-NEXT: [[BOUND_PTR_ARITH:%.*]] = getelementptr i8, ptr [[ARR]], i64 164
37+
// CHECK-NEXT: [[CMP_NOT:%.*]] = icmp ugt ptr [[BOUND_PTR_ARITH]], [[UPPER]], !annotation [[META2:![0-9]+]]
38+
// CHECK-NEXT: [[CMP28_NOT:%.*]] = icmp ugt ptr [[ARR]], [[BOUND_PTR_ARITH]], !annotation [[META2]]
39+
// CHECK-NEXT: [[OR_COND:%.*]] = or i1 [[CMP_NOT]], [[CMP28_NOT]], !annotation [[META2]]
40+
// CHECK-NEXT: br i1 [[OR_COND]], label %[[TRAP:.*]], label %[[CONT:.*]], !annotation [[META2]]
41+
// CHECK: [[TRAP]]:
42+
// CHECK-NEXT: call void @llvm.ubsantrap(i8 25) #[[ATTR6:[0-9]+]], !annotation [[META2]]
43+
// CHECK-NEXT: unreachable, !annotation [[META2]]
44+
// CHECK: [[CONT]]:
45+
// CHECK-NEXT: call void @foo(ptr noundef nonnull [[ARR]], ptr noundef [[BOUND_PTR_ARITH]]) #[[ATTR5]]
46+
// CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 160, ptr nonnull [[ARR]]) #[[ATTR5]]
47+
// CHECK-NEXT: ret void
48+
//
49+
void oob_upper(void) {
50+
int arr[40];
51+
foo(arr, arr + 41);
52+
}
53+
54+
// CHECK-LABEL: define dso_local void @oob_lower(
55+
// CHECK-SAME: ) local_unnamed_addr #[[ATTR0]] {
56+
// CHECK-NEXT: [[ENTRY:.*:]]
57+
// CHECK-NEXT: [[ARR:%.*]] = alloca [40 x i32], align 4
58+
// CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 160, ptr nonnull [[ARR]]) #[[ATTR5]]
59+
// CHECK-NEXT: [[UPPER:%.*]] = getelementptr inbounds i8, ptr [[ARR]], i64 160
60+
// CHECK-NEXT: [[BOUND_PTR_ARITH:%.*]] = getelementptr i8, ptr [[ARR]], i64 -4
61+
// CHECK-NEXT: [[CMP_NOT:%.*]] = icmp ugt ptr [[BOUND_PTR_ARITH]], [[UPPER]], !annotation [[META2]]
62+
// CHECK-NEXT: [[CMP28_NOT:%.*]] = icmp ugt ptr [[ARR]], [[BOUND_PTR_ARITH]], !annotation [[META2]]
63+
// CHECK-NEXT: [[OR_COND:%.*]] = or i1 [[CMP_NOT]], [[CMP28_NOT]], !annotation [[META2]]
64+
// CHECK-NEXT: br i1 [[OR_COND]], label %[[TRAP:.*]], label %[[CONT:.*]], !annotation [[META2]]
65+
// CHECK: [[TRAP]]:
66+
// CHECK-NEXT: call void @llvm.ubsantrap(i8 25) #[[ATTR6]], !annotation [[META2]]
67+
// CHECK-NEXT: unreachable, !annotation [[META2]]
68+
// CHECK: [[CONT]]:
69+
// CHECK-NEXT: call void @foo(ptr noundef nonnull [[ARR]], ptr noundef [[BOUND_PTR_ARITH]]) #[[ATTR5]]
70+
// CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 160, ptr nonnull [[ARR]]) #[[ATTR5]]
71+
// CHECK-NEXT: ret void
72+
//
73+
void oob_lower(void) {
74+
int arr[40];
75+
foo(arr, arr - 1);
76+
}
77+
78+
// CHECK-LABEL: define dso_local void @oob_order(
79+
// CHECK-SAME: ) local_unnamed_addr #[[ATTR4:[0-9]+]] {
80+
// CHECK-NEXT: [[ENTRY:.*:]]
81+
// CHECK-NEXT: tail call void @llvm.ubsantrap(i8 25) #[[ATTR6]], !annotation [[META2]]
82+
// CHECK-NEXT: unreachable, !annotation [[META2]]
83+
//
84+
void oob_order(void) {
85+
int arr[40];
86+
foo(arr + 2, arr + 1);
87+
}
88+
//.
89+
// CHECK: [[META2]] = !{!"firebloom-generic"}
90+
//.

0 commit comments

Comments
 (0)