Skip to content

Commit 8155a01

Browse files
atrickatrick
committed
[exclusivity] Add a [builtin] flag to begin_[unpaired_]access.
This flag supports promoting KeyPath access violations to an error in Swift 4+, while building the standard library in Swift 3 mode. This is only necessary as long as the standard library continues to build in Swift 3 mode. Once the standard library build migrates, it can all be ripped out. <rdar://problem/40115738> [Exclusivity] Enforce Keypath access as an error, not a warning in 4.2.
1 parent c2cf0e1 commit 8155a01

File tree

18 files changed

+173
-56
lines changed

18 files changed

+173
-56
lines changed

‎include/swift/SIL/SILBuilder.h‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -670,10 +670,11 @@ class SILBuilder {
670670
BeginAccessInst *createBeginAccess(SILLocation loc, SILValue address,
671671
SILAccessKind accessKind,
672672
SILAccessEnforcement enforcement,
673-
bool noNestedConflict) {
673+
bool noNestedConflict,
674+
bool fromBuiltin) {
674675
return insert(new (getModule()) BeginAccessInst(
675676
getSILDebugLocation(loc), address, accessKind, enforcement,
676-
noNestedConflict));
677+
noNestedConflict, fromBuiltin));
677678
}
678679

679680
EndAccessInst *createEndAccess(SILLocation loc, SILValue address,
@@ -686,18 +687,19 @@ class SILBuilder {
686687
createBeginUnpairedAccess(SILLocation loc, SILValue address, SILValue buffer,
687688
SILAccessKind accessKind,
688689
SILAccessEnforcement enforcement,
689-
bool noNestedConflict) {
690+
bool noNestedConflict,
691+
bool fromBuiltin) {
690692
return insert(new (getModule()) BeginUnpairedAccessInst(
691693
getSILDebugLocation(loc), address, buffer, accessKind, enforcement,
692-
noNestedConflict));
694+
noNestedConflict, fromBuiltin));
693695
}
694696

695-
EndUnpairedAccessInst *createEndUnpairedAccess(SILLocation loc,
696-
SILValue buffer,
697-
SILAccessEnforcement enforcement,
698-
bool aborted) {
697+
EndUnpairedAccessInst *
698+
createEndUnpairedAccess(SILLocation loc, SILValue buffer,
699+
SILAccessEnforcement enforcement, bool aborted,
700+
bool fromBuiltin) {
699701
return insert(new (getModule()) EndUnpairedAccessInst(
700-
getSILDebugLocation(loc), buffer, enforcement, aborted));
702+
getSILDebugLocation(loc), buffer, enforcement, aborted, fromBuiltin));
701703
}
702704

703705
AssignInst *createAssign(SILLocation Loc, SILValue Src, SILValue DestAddr) {

‎include/swift/SIL/SILCloner.h‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -792,7 +792,8 @@ void SILCloner<ImplClass>::visitBeginAccessInst(BeginAccessInst *Inst) {
792792
getOpValue(Inst->getOperand()),
793793
Inst->getAccessKind(),
794794
Inst->getEnforcement(),
795-
Inst->hasNoNestedConflict()));
795+
Inst->hasNoNestedConflict(),
796+
Inst->isFromBuiltin()));
796797
}
797798

798799
template <typename ImplClass>
@@ -814,18 +815,19 @@ void SILCloner<ImplClass>::visitBeginUnpairedAccessInst(
814815
getOpValue(Inst->getBuffer()),
815816
Inst->getAccessKind(),
816817
Inst->getEnforcement(),
817-
Inst->hasNoNestedConflict()));
818+
Inst->hasNoNestedConflict(),
819+
Inst->isFromBuiltin()));
818820
}
819821

820822
template <typename ImplClass>
821823
void SILCloner<ImplClass>::visitEndUnpairedAccessInst(
822824
EndUnpairedAccessInst *Inst) {
823825
getBuilder().setCurrentDebugScope(getOpScope(Inst->getDebugScope()));
824-
doPostProcess(
825-
Inst, getBuilder().createEndUnpairedAccess(getOpLocation(Inst->getLoc()),
826-
getOpValue(Inst->getOperand()),
827-
Inst->getEnforcement(),
828-
Inst->isAborting()));
826+
doPostProcess(Inst,
827+
getBuilder().createEndUnpairedAccess(
828+
getOpLocation(Inst->getLoc()),
829+
getOpValue(Inst->getOperand()), Inst->getEnforcement(),
830+
Inst->isAborting(), Inst->isFromBuiltin()));
829831
}
830832

831833
template <typename ImplClass>

‎include/swift/SIL/SILInstruction.h‎

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3258,12 +3258,14 @@ class BeginAccessInst
32583258

32593259
BeginAccessInst(SILDebugLocation loc, SILValue lvalue,
32603260
SILAccessKind accessKind, SILAccessEnforcement enforcement,
3261-
bool noNestedConflict)
3261+
bool noNestedConflict, bool fromBuiltin)
32623262
: UnaryInstructionBase(loc, lvalue, lvalue->getType()) {
32633263
SILInstruction::Bits.BeginAccessInst.AccessKind = unsigned(accessKind);
32643264
SILInstruction::Bits.BeginAccessInst.Enforcement = unsigned(enforcement);
32653265
SILInstruction::Bits.BeginAccessInst.NoNestedConflict =
32663266
unsigned(noNestedConflict);
3267+
SILInstruction::Bits.BeginAccessInst.FromBuiltin =
3268+
unsigned(fromBuiltin);
32673269

32683270
static_assert(unsigned(SILAccessKind::Last) < (1 << 2),
32693271
"reserve sufficient bits for serialized SIL");
@@ -3307,6 +3309,13 @@ class BeginAccessInst
33073309
SILInstruction::Bits.BeginAccessInst.NoNestedConflict = noNestedConflict;
33083310
}
33093311

3312+
/// Return true if this access marker was emitted for a user-controlled
3313+
/// Builtin. Return false if this access marker was auto-generated by the
3314+
/// compiler to enforce formal access that derives from the language.
3315+
bool isFromBuiltin() const {
3316+
return SILInstruction::Bits.BeginAccessInst.FromBuiltin;
3317+
}
3318+
33103319
SILValue getSource() const {
33113320
return getOperand();
33123321
}
@@ -3387,14 +3396,17 @@ class BeginUnpairedAccessInst
33873396
BeginUnpairedAccessInst(SILDebugLocation loc, SILValue addr, SILValue buffer,
33883397
SILAccessKind accessKind,
33893398
SILAccessEnforcement enforcement,
3390-
bool noNestedConflict)
3399+
bool noNestedConflict,
3400+
bool fromBuiltin)
33913401
: InstructionBase(loc), Operands(this, addr, buffer) {
33923402
SILInstruction::Bits.BeginUnpairedAccessInst.AccessKind =
33933403
unsigned(accessKind);
33943404
SILInstruction::Bits.BeginUnpairedAccessInst.Enforcement =
33953405
unsigned(enforcement);
33963406
SILInstruction::Bits.BeginUnpairedAccessInst.NoNestedConflict =
33973407
unsigned(noNestedConflict);
3408+
SILInstruction::Bits.BeginUnpairedAccessInst.FromBuiltin =
3409+
unsigned(fromBuiltin);
33983410
}
33993411

34003412
public:
@@ -3429,6 +3441,13 @@ class BeginUnpairedAccessInst
34293441
noNestedConflict;
34303442
}
34313443

3444+
/// Return true if this access marker was emitted for a user-controlled
3445+
/// Builtin. Return false if this access marker was auto-generated by the
3446+
/// compiler to enforce formal access that derives from the language.
3447+
bool isFromBuiltin() const {
3448+
return SILInstruction::Bits.BeginUnpairedAccessInst.FromBuiltin;
3449+
}
3450+
34323451
SILValue getSource() const {
34333452
return Operands[0].get();
34343453
}
@@ -3457,11 +3476,13 @@ class EndUnpairedAccessInst
34573476

34583477
private:
34593478
EndUnpairedAccessInst(SILDebugLocation loc, SILValue buffer,
3460-
SILAccessEnforcement enforcement, bool aborting = false)
3479+
SILAccessEnforcement enforcement, bool aborting,
3480+
bool fromBuiltin)
34613481
: UnaryInstructionBase(loc, buffer) {
34623482
SILInstruction::Bits.EndUnpairedAccessInst.Enforcement
34633483
= unsigned(enforcement);
34643484
SILInstruction::Bits.EndUnpairedAccessInst.Aborting = aborting;
3485+
SILInstruction::Bits.EndUnpairedAccessInst.FromBuiltin = fromBuiltin;
34653486
}
34663487

34673488
public:
@@ -3487,6 +3508,13 @@ class EndUnpairedAccessInst
34873508
unsigned(enforcement);
34883509
}
34893510

3511+
/// Return true if this access marker was emitted for a user-controlled
3512+
/// Builtin. Return false if this access marker was auto-generated by the
3513+
/// compiler to enforce formal access that derives from the language.
3514+
bool isFromBuiltin() const {
3515+
return SILInstruction::Bits.EndUnpairedAccessInst.FromBuiltin;
3516+
}
3517+
34903518
SILValue getBuffer() const {
34913519
return getOperand();
34923520
}

‎include/swift/SIL/SILNode.h‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -263,24 +263,28 @@ class alignas(8) SILNode {
263263

264264
SWIFT_INLINE_BITFIELD(BeginAccessInst, SingleValueInstruction,
265265
NumSILAccessKindBits+NumSILAccessEnforcementBits
266-
+ 1,
266+
+ 1 + 1,
267267
AccessKind : NumSILAccessKindBits,
268268
Enforcement : NumSILAccessEnforcementBits,
269-
NoNestedConflict : 1
269+
NoNestedConflict : 1,
270+
FromBuiltin : 1
270271
);
271272
SWIFT_INLINE_BITFIELD(BeginUnpairedAccessInst, NonValueInstruction,
272-
NumSILAccessKindBits + NumSILAccessEnforcementBits + 1,
273+
NumSILAccessKindBits + NumSILAccessEnforcementBits
274+
+ 1 + 1,
273275
AccessKind : NumSILAccessKindBits,
274276
Enforcement : NumSILAccessEnforcementBits,
275-
NoNestedConflict : 1);
277+
NoNestedConflict : 1,
278+
FromBuiltin : 1);
276279

277280
SWIFT_INLINE_BITFIELD(EndAccessInst, NonValueInstruction, 1,
278281
Aborting : 1
279282
);
280283
SWIFT_INLINE_BITFIELD(EndUnpairedAccessInst, NonValueInstruction,
281-
NumSILAccessEnforcementBits + 1,
284+
NumSILAccessEnforcementBits + 1 + 1,
282285
Enforcement : NumSILAccessEnforcementBits,
283-
Aborting : 1);
286+
Aborting : 1,
287+
FromBuiltin : 1);
284288

285289
SWIFT_INLINE_BITFIELD(StoreInst, NonValueInstruction,
286290
NumStoreOwnershipQualifierBits,

‎include/swift/Serialization/ModuleFormat.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ const uint16_t VERSION_MAJOR = 0;
5555
/// describe what change you made. The content of this comment isn't important;
5656
/// it just ensures a conflict if two people change the module format.
5757
/// Don't worry about adhering to the 80-column limit for this line.
58-
const uint16_t VERSION_MINOR = 411; // Last change: copy_block_without_escaping
58+
const uint16_t VERSION_MINOR = 412; // Last change: add begin_access [builtin].
5959

6060
using DeclIDField = BCFixed<31>;
6161

‎lib/IRGen/LoadableByAddress.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2087,7 +2087,8 @@ static void rewriteFunction(StructLoweringState &pass,
20872087
auto *convInstr = cast<BeginAccessInst>(instr);
20882088
newInstr = resultTyBuilder.createBeginAccess(
20892089
Loc, convInstr->getOperand(), convInstr->getAccessKind(),
2090-
convInstr->getEnforcement(), convInstr->hasNoNestedConflict());
2090+
convInstr->getEnforcement(), convInstr->hasNoNestedConflict(),
2091+
convInstr->isFromBuiltin());
20912092
break;
20922093
}
20932094
case SILInstructionKind::EnumInst: {

‎lib/ParseSIL/ParseSIL.cpp‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3434,6 +3434,7 @@ bool SILParser::parseSILInstruction(SILBuilder &B) {
34343434
ParsedEnum<SILAccessEnforcement> enforcement;
34353435
ParsedEnum<bool> aborting;
34363436
ParsedEnum<bool> noNestedConflict;
3437+
ParsedEnum<bool> fromBuiltin;
34373438

34383439
bool isBeginAccess = (Opcode == SILInstructionKind::BeginAccessInst ||
34393440
Opcode == SILInstructionKind::BeginUnpairedAccessInst);
@@ -3465,6 +3466,10 @@ bool SILParser::parseSILInstruction(SILBuilder &B) {
34653466
auto setNoNestedConflict = [&](bool value) {
34663467
maybeSetEnum(isBeginAccess, noNestedConflict, value, attr, identLoc);
34673468
};
3469+
auto setFromBuiltin = [&](bool value) {
3470+
maybeSetEnum(Opcode != SILInstructionKind::EndAccessInst, fromBuiltin,
3471+
value, attr, identLoc);
3472+
};
34683473

34693474
if (attr == "unknown") {
34703475
setEnforcement(SILAccessEnforcement::Unknown);
@@ -3486,6 +3491,8 @@ bool SILParser::parseSILInstruction(SILBuilder &B) {
34863491
setAborting(true);
34873492
} else if (attr == "no_nested_conflict") {
34883493
setNoNestedConflict(true);
3494+
} else if (attr == "builtin") {
3495+
setFromBuiltin(true);
34893496
} else {
34903497
P.diagnose(identLoc, diag::unknown_attribute, attr);
34913498
}
@@ -3510,6 +3517,9 @@ bool SILParser::parseSILInstruction(SILBuilder &B) {
35103517
if (isBeginAccess && !noNestedConflict.isSet())
35113518
noNestedConflict.Value = false;
35123519

3520+
if (!fromBuiltin.isSet())
3521+
fromBuiltin.Value = false;
3522+
35133523
SILValue addrVal;
35143524
SourceLoc addrLoc;
35153525
if (parseTypedValueRef(addrVal, addrLoc, B))
@@ -3534,16 +3544,16 @@ bool SILParser::parseSILInstruction(SILBuilder &B) {
35343544
if (Opcode == SILInstructionKind::BeginAccessInst) {
35353545
ResultVal =
35363546
B.createBeginAccess(InstLoc, addrVal, *kind, *enforcement,
3537-
*noNestedConflict);
3547+
*noNestedConflict, *fromBuiltin);
35383548
} else if (Opcode == SILInstructionKind::EndAccessInst) {
35393549
ResultVal = B.createEndAccess(InstLoc, addrVal, *aborting);
35403550
} else if (Opcode == SILInstructionKind::BeginUnpairedAccessInst) {
35413551
ResultVal = B.createBeginUnpairedAccess(InstLoc, addrVal, bufferVal,
35423552
*kind, *enforcement,
3543-
*noNestedConflict);
3553+
*noNestedConflict, *fromBuiltin);
35443554
} else {
3545-
ResultVal = B.createEndUnpairedAccess(InstLoc, addrVal,
3546-
*enforcement, *aborting);
3555+
ResultVal = B.createEndUnpairedAccess(InstLoc, addrVal, *enforcement,
3556+
*aborting, *fromBuiltin);
35473557
}
35483558
break;
35493559
}

‎lib/SIL/SILInstruction.cpp‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,8 @@ namespace {
388388
auto left = cast<BeginAccessInst>(LHS);
389389
return left->getAccessKind() == right->getAccessKind()
390390
&& left->getEnforcement() == right->getEnforcement()
391-
&& left->hasNoNestedConflict() == right->hasNoNestedConflict();
391+
&& left->hasNoNestedConflict() == right->hasNoNestedConflict()
392+
&& left->isFromBuiltin() == right->isFromBuiltin();
392393
}
393394

394395
bool visitEndAccessInst(const EndAccessInst *right) {
@@ -400,13 +401,15 @@ namespace {
400401
auto left = cast<BeginUnpairedAccessInst>(LHS);
401402
return left->getAccessKind() == right->getAccessKind()
402403
&& left->getEnforcement() == right->getEnforcement()
403-
&& left->hasNoNestedConflict() == right->hasNoNestedConflict();
404+
&& left->hasNoNestedConflict() == right->hasNoNestedConflict()
405+
&& left->isFromBuiltin() == right->isFromBuiltin();
404406
}
405407

406408
bool visitEndUnpairedAccessInst(const EndUnpairedAccessInst *right) {
407409
auto left = cast<EndUnpairedAccessInst>(LHS);
408410
return left->getEnforcement() == right->getEnforcement()
409-
&& left->isAborting() == right->isAborting();
411+
&& left->isAborting() == right->isAborting()
412+
&& left->isFromBuiltin() == right->isFromBuiltin();
410413
}
411414

412415
bool visitStrongReleaseInst(const StrongReleaseInst *RHS) {

‎lib/SIL/SILPrinter.cpp‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1842,6 +1842,7 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
18421842
*this << '[' << getSILAccessKindName(BAI->getAccessKind()) << "] ["
18431843
<< getSILAccessEnforcementName(BAI->getEnforcement()) << "] "
18441844
<< (BAI->hasNoNestedConflict() ? "[no_nested_conflict] " : "")
1845+
<< (BAI->isFromBuiltin() ? "[builtin] " : "")
18451846
<< getIDAndType(BAI->getOperand());
18461847
}
18471848
void visitEndAccessInst(EndAccessInst *EAI) {
@@ -1852,12 +1853,14 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
18521853
*this << '[' << getSILAccessKindName(BAI->getAccessKind()) << "] ["
18531854
<< getSILAccessEnforcementName(BAI->getEnforcement()) << "] "
18541855
<< (BAI->hasNoNestedConflict() ? "[no_nested_conflict] " : "")
1856+
<< (BAI->isFromBuiltin() ? "[builtin] " : "")
18551857
<< getIDAndType(BAI->getSource()) << ", "
18561858
<< getIDAndType(BAI->getBuffer());
18571859
}
18581860
void visitEndUnpairedAccessInst(EndUnpairedAccessInst *EAI) {
1859-
*this << (EAI->isAborting() ? "[abort] " : "")
1860-
<< '[' << getSILAccessEnforcementName(EAI->getEnforcement()) << "] "
1861+
*this << (EAI->isAborting() ? "[abort] " : "") << '['
1862+
<< getSILAccessEnforcementName(EAI->getEnforcement()) << "] "
1863+
<< (EAI->isFromBuiltin() ? "[builtin] " : "")
18611864
<< getIDAndType(EAI->getOperand());
18621865
}
18631866

‎lib/SILGen/SILGenBuiltin.cpp‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -581,7 +581,8 @@ static ManagedValue emitBuiltinBeginUnpairedModifyAccess(SILGenFunction &SGF,
581581
/*invariant*/ false);
582582
SGF.B.createBeginUnpairedAccess(loc, addr, buffer, SILAccessKind::Modify,
583583
SILAccessEnforcement::Dynamic,
584-
/*noNestedConflict*/ false);
584+
/*noNestedConflict*/ false,
585+
/*fromBuiltin*/ true);
585586

586587
return ManagedValue::forUnmanaged(SGF.emitEmptyTuple(loc));
587588
}
@@ -617,7 +618,8 @@ static ManagedValue emitBuiltinPerformInstantaneousReadAccess(
617618
// use will be trivially optimized away.
618619
SGF.B.createBeginUnpairedAccess(loc, addr, unusedBuffer, SILAccessKind::Read,
619620
SILAccessEnforcement::Dynamic,
620-
/*noNestedConflict*/ true);
621+
/*noNestedConflict*/ true,
622+
/*fromBuiltin*/ true);
621623

622624
return ManagedValue::forUnmanaged(SGF.emitEmptyTuple(loc));
623625
}
@@ -641,7 +643,8 @@ static ManagedValue emitBuiltinEndUnpairedAccess(SILGenFunction &SGF,
641643
/*strict*/ true,
642644
/*invariant*/ false);
643645
SGF.B.createEndUnpairedAccess(loc, buffer, SILAccessEnforcement::Dynamic,
644-
/*aborted*/ false);
646+
/*aborted*/ false,
647+
/*fromBuiltin*/ true);
645648

646649
return ManagedValue::forUnmanaged(SGF.emitEmptyTuple(loc));
647650
}

0 commit comments

Comments
 (0)