Skip to content

Commit 6521d0c

Browse files
atrickatrick
committed
[exclusivity] Make KeyPath enforcement an error in Swift 3 mode.
Modify IRGen to emit builtin access markers with an error flag in Swift 3 mode. KeyPath enforcement is required by user code in Swift 4+ mode, but is implemented within the standard library. A [builtin] flag marks the special case for access generated by Builtins so that they are always enforced as an error regardless of the language mode. This is necessary for Swift 4.2 because the standard library continues to build in Swift 3 mode. Once the standard library build migrates, this is all irrelevant. This does not actually affect existing Swift 3 code, since the KeyPath feature wasn't introduced until Swift 4. <rdar://problem/40115738> [Exclusivity] Enforce Keypath access as an error, not a warning in 4.2.
1 parent 8155a01 commit 6521d0c

File tree

6 files changed

+70
-152
lines changed

6 files changed

+70
-152
lines changed

‎lib/IRGen/IRGenSIL.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4228,11 +4228,12 @@ static ExclusivityFlags getExclusivityAction(SILAccessKind kind) {
42284228

42294229
static ExclusivityFlags getExclusivityFlags(SILModule &M,
42304230
SILAccessKind kind,
4231-
bool noNestedConflict) {
4231+
bool noNestedConflict,
4232+
bool fromBuiltin) {
42324233
auto flags = getExclusivityAction(kind);
42334234

42344235
// In old Swift compatibility modes, downgrade this to a warning.
4235-
if (M.getASTContext().LangOpts.isSwiftVersion3())
4236+
if (!fromBuiltin && M.getASTContext().LangOpts.isSwiftVersion3())
42364237
flags |= ExclusivityFlags::WarningOnly;
42374238

42384239
if (!noNestedConflict)
@@ -4264,7 +4265,7 @@ static SILAccessEnforcement getEffectiveEnforcement(IRGenFunction &IGF,
42644265
template <class BeginAccessInst>
42654266
static ExclusivityFlags getExclusivityFlags(BeginAccessInst *i) {
42664267
return getExclusivityFlags(i->getModule(), i->getAccessKind(),
4267-
i->hasNoNestedConflict());
4268+
i->hasNoNestedConflict(), i->isFromBuiltin());
42684269
}
42694270

42704271
void IRGenSILFunction::visitBeginAccessInst(BeginAccessInst *access) {

‎lib/SILOptimizer/Mandatory/AccessMarkerElimination.cpp‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,12 @@ bool AccessMarkerElimination::shouldPreserveAccess(
112112
// updated the SIL, short of erasing the marker itself, and return true.
113113
bool AccessMarkerElimination::checkAndEliminateMarker(SILInstruction *inst) {
114114
if (auto beginAccess = dyn_cast<BeginAccessInst>(inst)) {
115+
// Builtins used by the standard library must emit markers regardless of the
116+
// current compiler options so that any user code that initiates access via
117+
// the standard library is fully enforced.
118+
if (beginAccess->isFromBuiltin())
119+
return false;
120+
115121
// Leave dynamic accesses in place, but delete all others.
116122
if (shouldPreserveAccess(beginAccess->getEnforcement()))
117123
return false;
@@ -126,6 +132,11 @@ bool AccessMarkerElimination::checkAndEliminateMarker(SILInstruction *inst) {
126132
// begin_unpaired_access instructions will be directly removed and
127133
// simply replaced with their operand.
128134
if (auto BUA = dyn_cast<BeginUnpairedAccessInst>(inst)) {
135+
// Builtins used by the standard library must emit markers regardless of the
136+
// current compiler options.
137+
if (BUA->isFromBuiltin())
138+
return false;
139+
129140
if (shouldPreserveAccess(BUA->getEnforcement()))
130141
return false;
131142

@@ -134,6 +145,11 @@ bool AccessMarkerElimination::checkAndEliminateMarker(SILInstruction *inst) {
134145
// end_unpaired_access instructions will be directly removed and
135146
// simply replaced with their operand.
136147
if (auto EUA = dyn_cast<EndUnpairedAccessInst>(inst)) {
148+
// Builtins used by the standard library must emit markers regardless of the
149+
// current compiler options.
150+
if (EUA->isFromBuiltin())
151+
return false;
152+
137153
if (shouldPreserveAccess(EUA->getEnforcement()))
138154
return false;
139155

‎test/IRGen/preserve_exclusivity.swift‎

Lines changed: 28 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
1-
// RUN: %target-swift-frontend -parse-stdlib -emit-ir -Onone %s | %FileCheck --check-prefix=IR-Onone %s
1+
// RUN: %target-swift-frontend -swift-version 3 -parse-stdlib -Xllvm -sil-disable-pass=FunctionSignatureOpts -Xllvm -sil-disable-pass=GenericSpecializer -emit-ir -O %s | %FileCheck %s
2+
// RUN: %target-swift-frontend -swift-version 4 -parse-stdlib -Xllvm -sil-disable-pass=FunctionSignatureOpts -Xllvm -sil-disable-pass=GenericSpecializer -emit-ir -O %s | %FileCheck %s
23
//
3-
// Check that access markers in @_semantics("optimize.sil.preserve_exclusivity") functions generate runtime calls.
4-
5-
// RUN: %target-swift-frontend -parse-stdlib -Xllvm -sil-disable-pass=FunctionSignatureOpts -Xllvm -sil-disable-pass=GenericSpecializer -emit-ir -O %s | %FileCheck --check-prefix=IR-O %s
6-
//
7-
// Check that the -O pipeline preserves the runtime calls for @_semantics("optimize.sil.preserve_exclusivity") functions.
4+
// Check that the -O pipeline always preserves the runtime calls for Builtin access markers and that the KeyPath implementation is fully inlined.
85

96
@_silgen_name("marker1")
107
func marker1() -> ()
@@ -15,122 +12,44 @@ func marker2() -> ()
1512
@_silgen_name("marker3")
1613
func marker3() -> ()
1714

18-
@_silgen_name("marker4")
19-
func marker4() -> ()
20-
21-
@_silgen_name("marker5")
22-
func marker5() -> ()
23-
24-
@_silgen_name("marker6")
25-
func marker6() -> ()
15+
// IR-LABEL: define {{.*}}swiftcc void @"$S20preserve_exclusivity11beginAccessyyBp_BpxmtlF"(i8*, i8*, %swift.type*{{.*}}, %swift.type*{{.*}} %T1)
16+
// IR: call void @swift_beginAccess
17+
// IR-NEXT: ret void
2618

27-
// IR-Onone-LABEL: define {{.*}}swiftcc void @"$S20preserve_exclusivity10beginNoOptyyBp_BpxmtlF"(i8*, i8*, %swift.type*, %swift.type* %T1)
28-
// IR-Onone: call void @swift_beginAccess
29-
// IR-Onone-NEXT: ret void
30-
31-
// IR-O-LABEL: define {{.*}}swiftcc void @"$S20preserve_exclusivity10beginNoOptyyBp_BpxmtlF"(i8*, i8*, %swift.type*{{.*}}, %swift.type*{{.*}} %T1)
32-
// IR-O: call void @swift_beginAccess
33-
// IR-O-NEXT: ret void
34-
35-
@_semantics("optimize.sil.preserve_exclusivity")
36-
public func beginNoOpt<T1>(_ address: Builtin.RawPointer, _ scratch: Builtin.RawPointer, _ ty1: T1.Type) {
19+
public func beginAccess<T1>(_ address: Builtin.RawPointer, _ scratch: Builtin.RawPointer, _ ty1: T1.Type) {
3720
marker1()
3821
Builtin.beginUnpairedModifyAccess(address, scratch, ty1);
3922
}
4023

41-
// IR-Onone-LABEL: define {{.*}}swiftcc void @"$S20preserve_exclusivity8endNoOptyyBpF"(i8*)
42-
// IR-Onone: call void @swift_endAccess
43-
// IR-Onone-NEXT: ret void
44-
45-
// IR-O-LABEL: define {{.*}}swiftcc void @"$S20preserve_exclusivity8endNoOptyyBpF"(i8*)
46-
// IR-O: call void @swift_endAccess
47-
// IR-O-NEXT: ret void
48-
@_semantics("optimize.sil.preserve_exclusivity")
49-
public func endNoOpt(_ address: Builtin.RawPointer) {
24+
// CHECK-LABEL: define {{.*}}swiftcc void @"$S20preserve_exclusivity9endAccessyyBpF"(i8*{{.*}})
25+
// CHECK: call void @swift_endAccess
26+
// CHECK-NEXT: ret void
27+
public func endAccess(_ address: Builtin.RawPointer) {
5028
marker2()
5129
Builtin.endUnpairedAccess(address)
5230
}
5331

54-
// IR-Onone-LABEL: define {{.*}}swiftcc void @"$S20preserve_exclusivity9readNoOptyyBp_xmtlF"(i8*, %swift.type*, %swift.type* %T1)
55-
// IR-Onone: call void @swift_beginAccess
56-
// IR-Onone: ret void
57-
58-
// IR-O-LABEL: define {{.*}}swiftcc void @"$S20preserve_exclusivity9readNoOptyyBp_xmtlF"(i8*, %swift.type*{{.*}}, %swift.type*{{.*}} %T1)
59-
// IR-O: call void @swift_beginAccess
60-
// IR-O: ret void
32+
// CHECK-LABEL: define {{.*}}swiftcc void @"$S20preserve_exclusivity10readAccessyyBp_xmtlF"(i8*, %swift.type*{{.*}}, %swift.type*{{.*}} %T1)
33+
// CHECK: call void @swift_beginAccess
34+
// CHECK: ret void
6135
@_semantics("optimize.sil.preserve_exclusivity")
62-
public func readNoOpt<T1>(_ address: Builtin.RawPointer, _ ty1: T1.Type) {
36+
public func readAccess<T1>(_ address: Builtin.RawPointer, _ ty1: T1.Type) {
6337
marker3()
6438
Builtin.performInstantaneousReadAccess(address, ty1);
6539
}
6640

67-
// Make sure testNoOpt properly inlines in our functions.
68-
//
69-
// IR-O-LABEL: define {{.*}}swiftcc void @"$S20preserve_exclusivity9testNoOptyyBpF"(i8*)
70-
// IR-O: call swiftcc void @marker1
71-
// IR-O: call void @swift_beginAccess
72-
// IR-O: call swiftcc void @marker2
73-
// IR-O: call void @swift_endAccess
74-
// IR-O: call swiftcc void @marker3
75-
// IR-O: call void @swift_beginAccess
76-
// IR-O: ret void
77-
public func testNoOpt(_ k1: Builtin.RawPointer) {
78-
beginNoOpt(k1, k1, Builtin.RawPointer.self)
79-
endNoOpt(k1)
80-
readNoOpt(k1, Builtin.RawPointer.self)
81-
}
82-
83-
// IR-Onone-LABEL: define {{.*}}swiftcc void @"$S20preserve_exclusivity8beginOptyyBp_BpxmtlF"(i8*, i8*, %swift.type*, %swift.type* %T1)
84-
// IR-Onone: call void @swift_beginAccess
85-
// IR-Onone-NEXT: ret void
86-
87-
// IR-O-LABEL: define {{.*}}swiftcc void @"$S20preserve_exclusivity8beginOptyyBp_BpxmtlF"(i8*{{.*}}, i8*{{.*}}, %swift.type*{{.*}}, %swift.type*{{.*}} %T1)
88-
// IR-O-NEXT: entry
89-
// IR-O-NEXT: call swiftcc void @marker4
90-
// IR-O-NEXT: ret void
91-
92-
public func beginOpt<T1>(_ address: Builtin.RawPointer, _ scratch: Builtin.RawPointer, _ ty1: T1.Type) {
93-
marker4()
94-
Builtin.beginUnpairedModifyAccess(address, scratch, ty1);
95-
}
96-
97-
// IR-Onone-LABEL: define {{.*}}swiftcc void @"$S20preserve_exclusivity6endOptyyBpF"(i8*)
98-
// IR-Onone: call void @swift_endAccess
99-
// IR-Onone-NEXT: ret void
100-
101-
// IR-O-LABEL: define {{.*}}swiftcc void @"$S20preserve_exclusivity6endOptyyBpF"(i8*{{.*}})
102-
// IR-O-NEXT: entry
103-
// IR-O-NEXT: call swiftcc void @marker5
104-
// IR-O-NEXT: ret void
105-
106-
public func endOpt(_ address: Builtin.RawPointer) {
107-
marker5()
108-
Builtin.endUnpairedAccess(address)
109-
}
110-
111-
// IR-Onone-LABEL: define {{.*}}swiftcc void @"$S20preserve_exclusivity7readOptyyBp_xmtlF"(i8*, %swift.type*, %swift.type* %T1)
112-
// IR-Onone: call void @swift_beginAccess
113-
// IR-Onone: ret void
114-
115-
// IR-O-LABEL: define {{.*}}swiftcc void @"$S20preserve_exclusivity7readOptyyBp_xmtlF"(i8*{{.*}}, %swift.type*{{.*}}, %swift.type*{{.*}} %T1)
116-
// IR-O-NEXT: entry
117-
// IR-O-NEXT: call swiftcc void @marker6
118-
// IR-O-NEXT: ret void
119-
120-
public func readOpt<T1>(_ address: Builtin.RawPointer, _ ty1: T1.Type) {
121-
marker6()
122-
Builtin.performInstantaneousReadAccess(address, ty1);
123-
}
124-
125-
// Make sure testOpt properly inlines in our functions.
41+
// Make sure testAccess properly inlines in our functions.
12642
//
127-
// IR-O-LABEL: define {{.*}}swiftcc void @"$S20preserve_exclusivity7testOptyyBpF"(i8*{{.*}})
128-
// IR-O: call swiftcc void @marker4
129-
// IR-O: call swiftcc void @marker5
130-
// IR-O: call swiftcc void @marker6
131-
// IR-O: ret void
132-
public func testOpt(_ k1: Builtin.RawPointer) {
133-
beginOpt(k1, k1, Builtin.RawPointer.self)
134-
endOpt(k1)
135-
readOpt(k1, Builtin.RawPointer.self)
43+
// CHECK-LABEL: define {{.*}}swiftcc void @"$S20preserve_exclusivity10testAccessyyBpF"(i8*)
44+
// CHECK: call swiftcc void @marker1
45+
// CHECK: call void @swift_beginAccess
46+
// CHECK: call swiftcc void @marker2
47+
// CHECK: call void @swift_endAccess
48+
// CHECK: call swiftcc void @marker3
49+
// CHECK: call void @swift_beginAccess
50+
// CHECK: ret void
51+
public func testAccess(_ k1: Builtin.RawPointer) {
52+
beginAccess(k1, k1, Builtin.RawPointer.self)
53+
endAccess(k1)
54+
readAccess(k1, Builtin.RawPointer.self)
13655
}

‎test/Interpreter/enforce_exclusive_access.swift‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -308,24 +308,37 @@ ExclusiveAccessTestSuite.test("SequentialKeyPathWritesDontOverlap") {
308308
c[keyPath: getF] += c[keyPath: getF] + 1 // no-trap
309309
}
310310

311-
// This does not trap, for now, because the standard library (and thus KeyPath) is
312-
// compiled in Swift 3 mode and we currently log rather than trap in Swift mode.
313311
ExclusiveAccessTestSuite.test("KeyPathInoutKeyPathWriteClassStoredProp")
312+
.skip(.custom(
313+
{ _isFastAssertConfiguration() },
314+
reason: "this trap is not guaranteed to happen in -Ounchecked"))
315+
.crashOutputMatches("Previous access (a modification) started at")
316+
.crashOutputMatches("Current access (a modification) started at")
317+
.code
314318
{
315319
let getF = \ClassWithStoredProperty.f
316320
let c = ClassWithStoredProperty()
317321

322+
expectCrashLater()
318323
modifyAndPerform(&c[keyPath: getF]) {
319324
c[keyPath: getF] = 12
320325
}
321326
}
322327

323328
// This does not currently trap because the standard library is compiled in Swift 3 mode,
324329
// which logs.
325-
ExclusiveAccessTestSuite.test("KeyPathInoutKeyPathReadClassStoredProp") {
330+
ExclusiveAccessTestSuite.test("KeyPathInoutKeyPathReadClassStoredProp")
331+
.skip(.custom(
332+
{ _isFastAssertConfiguration() },
333+
reason: "this trap is not guaranteed to happen in -Ounchecked"))
334+
.crashOutputMatches("Previous access (a modification) started at")
335+
.crashOutputMatches("Current access (a read) started at")
336+
.code
337+
{
326338
let getF = \ClassWithStoredProperty.f
327339
let c = ClassWithStoredProperty()
328340

341+
expectCrashLater()
329342
modifyAndPerform(&c[keyPath: getF]) {
330343
let y = c[keyPath: getF]
331344
_blackHole(y)

‎test/Interpreter/enforce_exclusive_access_backtrace.swift‎

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -44,34 +44,3 @@ class C {
4444
var c = C()
4545
withUnsafePointer(to: &c.f) { _ = fputs(String(format: "c.f: 0x%lx\n", Int(bitPattern: $0)), stderr) }
4646
// CHECK: c.f: [[C_F_ADDR:0x.*]]
47-
48-
// Key path accesses are performed in the Standard Library. The Standard Library
49-
// is currently compiled in Swift 3 mode and the compiler currently logs conflicts
50-
// (rather than trapping on them) code compiled in Swift 3 mode. For this reason
51-
// conflicts where the second conflicting access is via a key path will log rather than
52-
// trap.
53-
54-
fputs("Overlapping Key Path Write Accesses\n", stderr);
55-
writeAndPerform(&c[keyPath: \C.f]) {
56-
c[keyPath: \C.f] = 8
57-
// CHECK-LABEL: Overlapping Key Path Write Accesses
58-
// CHECK: Simultaneous accesses to [[C_F_ADDR]], but modification requires exclusive access.
59-
// CHECK: Previous access (a modification)
60-
// CHECK: Current access (a modification)
61-
// CHECK: a.out {{.*}} closure
62-
// CHECK: a.out {{.*}} writeAndPerform
63-
// CHECK: a.out {{.*}} main
64-
}
65-
66-
fputs("Overlapping Key Path Write Access and Key Path Read Access\n", stderr);
67-
writeAndPerform(&c[keyPath: \C.f]) {
68-
let x = c[keyPath: \C.f]
69-
_blackHole(x)
70-
// CHECK-LABEL: Overlapping Key Path Write Access and Key Path Read Access
71-
// CHECK: Simultaneous accesses to [[C_F_ADDR]], but modification requires exclusive access.
72-
// CHECK: Previous access (a modification)
73-
// CHECK: Current access (a read)
74-
// CHECK: a.out {{.*}} closure
75-
// CHECK: a.out {{.*}} writeAndPerform
76-
// CHECK: a.out {{.*}} main
77-
}

‎test/SILOptimizer/preserve_exclusivity.swift‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,11 @@ public func f4__testNoOpt(_ k1: Builtin.RawPointer) {
9696
}
9797

9898
// CHECK-LABEL: sil @$S20preserve_exclusivity13f5_beginDoOptyyBp_BpxmtlF : $@convention(thin) <T1> (Builtin.RawPointer, Builtin.RawPointer, @thick T1.Type) -> () {
99-
// CHECK-NOT: begin_unpaired_access
99+
// CHECK: begin_unpaired_access
100100
// CHECK: } // end sil function '$S20preserve_exclusivity13f5_beginDoOptyyBp_BpxmtlF'
101101

102102
// DESERIALIZED-LABEL: sil [serialized] [canonical] @$S20preserve_exclusivity13f5_beginDoOptyyBp_BpxmtlF : $@convention(thin) <T1> (Builtin.RawPointer, Builtin.RawPointer, @thick T1.Type) -> () {
103-
// DESERIALIZED-NOT: begin_unpaired_access
103+
// DESERIALIZED: begin_unpaired_access
104104
// DESERIALIZED: } // end sil function '$S20preserve_exclusivity13f5_beginDoOptyyBp_BpxmtlF'
105105

106106
@inlinable
@@ -110,11 +110,11 @@ public func f5_beginDoOpt<T1>(_ address: Builtin.RawPointer, _ scratch: Builtin.
110110
}
111111

112112
// CHECK-LABEL: sil @$S20preserve_exclusivity13f6___endDoOptyyBpF : $@convention(thin) (Builtin.RawPointer) -> () {
113-
// CHECK-NOT: end_unpaired_access
113+
// CHECK: end_unpaired_access
114114
// CHECK: } // end sil function '$S20preserve_exclusivity13f6___endDoOptyyBpF'
115115

116116
// DESERIALIZED-LABEL: sil [serialized] [canonical] @$S20preserve_exclusivity13f6___endDoOptyyBpF : $@convention(thin) (Builtin.RawPointer) -> () {
117-
// DESERIALIZED-NOT: end_unpaired_access
117+
// DESERIALIZED: end_unpaired_access
118118
// DESERIALIZED: } // end sil function '$S20preserve_exclusivity13f6___endDoOptyyBpF'
119119

120120
@inlinable
@@ -124,11 +124,11 @@ public func f6___endDoOpt(_ address: Builtin.RawPointer) {
124124
}
125125

126126
// CHECK-LABEL: sil @$S20preserve_exclusivity13f7__readDoOptyyBp_BpmtF : $@convention(thin) (Builtin.RawPointer, @thin Builtin.RawPointer.Type) -> () {
127-
// CHECK-NOT: begin_unpaired_access
127+
// CHECK: begin_unpaired_access
128128
// CHECK: } // end sil function '$S20preserve_exclusivity13f7__readDoOptyyBp_BpmtF'
129129

130130
// DESERIALIZED-LABEL: sil [serialized] [canonical] @$S20preserve_exclusivity13f7__readDoOptyyBp_BpmtF : $@convention(thin) (Builtin.RawPointer, @thin Builtin.RawPointer.Type) -> () {
131-
// DESERIALIZED-NOT: begin_unpaired_access
131+
// DESERIALIZED: begin_unpaired_access
132132
// DESERIALIZED: } // end sil function '$S20preserve_exclusivity13f7__readDoOptyyBp_BpmtF'
133133

134134
@inlinable

0 commit comments

Comments
 (0)