Skip to content

Commit 6a48b2d

Browse files
atrickatrick
authored
Merge pull request #16491 from atrick/enforce-keypath-exclusivity-as-error
Enforce keypath exclusivity as an error.
2 parents 57d7c0b + 5b1b730 commit 6a48b2d

28 files changed

+242
-523
lines changed

‎include/swift/SIL/SILBuilder.h‎

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

678679
EndAccessInst *createEndAccess(SILLocation loc, SILValue address,
@@ -685,18 +686,19 @@ class SILBuilder {
685686
createBeginUnpairedAccess(SILLocation loc, SILValue address, SILValue buffer,
686687
SILAccessKind accessKind,
687688
SILAccessEnforcement enforcement,
688-
bool noNestedConflict) {
689+
bool noNestedConflict,
690+
bool fromBuiltin) {
689691
return insert(new (getModule()) BeginUnpairedAccessInst(
690692
getSILDebugLocation(loc), address, buffer, accessKind, enforcement,
691-
noNestedConflict));
693+
noNestedConflict, fromBuiltin));
692694
}
693695

694-
EndUnpairedAccessInst *createEndUnpairedAccess(SILLocation loc,
695-
SILValue buffer,
696-
SILAccessEnforcement enforcement,
697-
bool aborted) {
696+
EndUnpairedAccessInst *
697+
createEndUnpairedAccess(SILLocation loc, SILValue buffer,
698+
SILAccessEnforcement enforcement, bool aborted,
699+
bool fromBuiltin) {
698700
return insert(new (getModule()) EndUnpairedAccessInst(
699-
getSILDebugLocation(loc), buffer, enforcement, aborted));
701+
getSILDebugLocation(loc), buffer, enforcement, aborted, fromBuiltin));
700702
}
701703

702704
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
@@ -809,7 +809,8 @@ void SILCloner<ImplClass>::visitBeginAccessInst(BeginAccessInst *Inst) {
809809
getOpValue(Inst->getOperand()),
810810
Inst->getAccessKind(),
811811
Inst->getEnforcement(),
812-
Inst->hasNoNestedConflict()));
812+
Inst->hasNoNestedConflict(),
813+
Inst->isFromBuiltin()));
813814
}
814815

815816
template <typename ImplClass>
@@ -831,18 +832,19 @@ void SILCloner<ImplClass>::visitBeginUnpairedAccessInst(
831832
getOpValue(Inst->getBuffer()),
832833
Inst->getAccessKind(),
833834
Inst->getEnforcement(),
834-
Inst->hasNoNestedConflict()));
835+
Inst->hasNoNestedConflict(),
836+
Inst->isFromBuiltin()));
835837
}
836838

837839
template <typename ImplClass>
838840
void SILCloner<ImplClass>::visitEndUnpairedAccessInst(
839841
EndUnpairedAccessInst *Inst) {
840842
getBuilder().setCurrentDebugScope(getOpScope(Inst->getDebugScope()));
841-
doPostProcess(
842-
Inst, getBuilder().createEndUnpairedAccess(getOpLocation(Inst->getLoc()),
843-
getOpValue(Inst->getOperand()),
844-
Inst->getEnforcement(),
845-
Inst->isAborting()));
843+
doPostProcess(Inst,
844+
getBuilder().createEndUnpairedAccess(
845+
getOpLocation(Inst->getLoc()),
846+
getOpValue(Inst->getOperand()), Inst->getEnforcement(),
847+
Inst->isAborting(), Inst->isFromBuiltin()));
846848
}
847849

848850
template <typename ImplClass>

‎include/swift/SIL/SILInstruction.h‎

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

32303230
BeginAccessInst(SILDebugLocation loc, SILValue lvalue,
32313231
SILAccessKind accessKind, SILAccessEnforcement enforcement,
3232-
bool noNestedConflict)
3232+
bool noNestedConflict, bool fromBuiltin)
32333233
: UnaryInstructionBase(loc, lvalue, lvalue->getType()) {
32343234
SILInstruction::Bits.BeginAccessInst.AccessKind = unsigned(accessKind);
32353235
SILInstruction::Bits.BeginAccessInst.Enforcement = unsigned(enforcement);
32363236
SILInstruction::Bits.BeginAccessInst.NoNestedConflict =
32373237
unsigned(noNestedConflict);
3238+
SILInstruction::Bits.BeginAccessInst.FromBuiltin =
3239+
unsigned(fromBuiltin);
32383240

32393241
static_assert(unsigned(SILAccessKind::Last) < (1 << 2),
32403242
"reserve sufficient bits for serialized SIL");
@@ -3278,6 +3280,13 @@ class BeginAccessInst
32783280
SILInstruction::Bits.BeginAccessInst.NoNestedConflict = noNestedConflict;
32793281
}
32803282

3283+
/// Return true if this access marker was emitted for a user-controlled
3284+
/// Builtin. Return false if this access marker was auto-generated by the
3285+
/// compiler to enforce formal access that derives from the language.
3286+
bool isFromBuiltin() const {
3287+
return SILInstruction::Bits.BeginAccessInst.FromBuiltin;
3288+
}
3289+
32813290
SILValue getSource() const {
32823291
return getOperand();
32833292
}
@@ -3358,14 +3367,17 @@ class BeginUnpairedAccessInst
33583367
BeginUnpairedAccessInst(SILDebugLocation loc, SILValue addr, SILValue buffer,
33593368
SILAccessKind accessKind,
33603369
SILAccessEnforcement enforcement,
3361-
bool noNestedConflict)
3370+
bool noNestedConflict,
3371+
bool fromBuiltin)
33623372
: InstructionBase(loc), Operands(this, addr, buffer) {
33633373
SILInstruction::Bits.BeginUnpairedAccessInst.AccessKind =
33643374
unsigned(accessKind);
33653375
SILInstruction::Bits.BeginUnpairedAccessInst.Enforcement =
33663376
unsigned(enforcement);
33673377
SILInstruction::Bits.BeginUnpairedAccessInst.NoNestedConflict =
33683378
unsigned(noNestedConflict);
3379+
SILInstruction::Bits.BeginUnpairedAccessInst.FromBuiltin =
3380+
unsigned(fromBuiltin);
33693381
}
33703382

33713383
public:
@@ -3400,6 +3412,13 @@ class BeginUnpairedAccessInst
34003412
noNestedConflict;
34013413
}
34023414

3415+
/// Return true if this access marker was emitted for a user-controlled
3416+
/// Builtin. Return false if this access marker was auto-generated by the
3417+
/// compiler to enforce formal access that derives from the language.
3418+
bool isFromBuiltin() const {
3419+
return SILInstruction::Bits.BeginUnpairedAccessInst.FromBuiltin;
3420+
}
3421+
34033422
SILValue getSource() const {
34043423
return Operands[0].get();
34053424
}
@@ -3428,11 +3447,13 @@ class EndUnpairedAccessInst
34283447

34293448
private:
34303449
EndUnpairedAccessInst(SILDebugLocation loc, SILValue buffer,
3431-
SILAccessEnforcement enforcement, bool aborting = false)
3450+
SILAccessEnforcement enforcement, bool aborting,
3451+
bool fromBuiltin)
34323452
: UnaryInstructionBase(loc, buffer) {
34333453
SILInstruction::Bits.EndUnpairedAccessInst.Enforcement
34343454
= unsigned(enforcement);
34353455
SILInstruction::Bits.EndUnpairedAccessInst.Aborting = aborting;
3456+
SILInstruction::Bits.EndUnpairedAccessInst.FromBuiltin = fromBuiltin;
34363457
}
34373458

34383459
public:
@@ -3458,6 +3479,13 @@ class EndUnpairedAccessInst
34583479
unsigned(enforcement);
34593480
}
34603481

3482+
/// Return true if this access marker was emitted for a user-controlled
3483+
/// Builtin. Return false if this access marker was auto-generated by the
3484+
/// compiler to enforce formal access that derives from the language.
3485+
bool isFromBuiltin() const {
3486+
return SILInstruction::Bits.EndUnpairedAccessInst.FromBuiltin;
3487+
}
3488+
34613489
SILValue getBuffer() const {
34623490
return getOperand();
34633491
}

‎include/swift/SIL/SILNode.h‎

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

259259
SWIFT_INLINE_BITFIELD(BeginAccessInst, SingleValueInstruction,
260260
NumSILAccessKindBits+NumSILAccessEnforcementBits
261-
+ 1,
261+
+ 1 + 1,
262262
AccessKind : NumSILAccessKindBits,
263263
Enforcement : NumSILAccessEnforcementBits,
264-
NoNestedConflict : 1
264+
NoNestedConflict : 1,
265+
FromBuiltin : 1
265266
);
266267
SWIFT_INLINE_BITFIELD(BeginUnpairedAccessInst, NonValueInstruction,
267-
NumSILAccessKindBits + NumSILAccessEnforcementBits + 1,
268+
NumSILAccessKindBits + NumSILAccessEnforcementBits
269+
+ 1 + 1,
268270
AccessKind : NumSILAccessKindBits,
269271
Enforcement : NumSILAccessEnforcementBits,
270-
NoNestedConflict : 1);
272+
NoNestedConflict : 1,
273+
FromBuiltin : 1);
271274

272275
SWIFT_INLINE_BITFIELD(EndAccessInst, NonValueInstruction, 1,
273276
Aborting : 1
274277
);
275278
SWIFT_INLINE_BITFIELD(EndUnpairedAccessInst, NonValueInstruction,
276-
NumSILAccessEnforcementBits + 1,
279+
NumSILAccessEnforcementBits + 1 + 1,
277280
Enforcement : NumSILAccessEnforcementBits,
278-
Aborting : 1);
281+
Aborting : 1,
282+
FromBuiltin : 1);
279283

280284
SWIFT_INLINE_BITFIELD(StoreInst, NonValueInstruction,
281285
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 = 417; // Last change: revert @usableFromInline imports
58+
const uint16_t VERSION_MINOR = 418; // Last change: add begin_access [builtin].
5959

6060
using DeclIDField = BCFixed<31>;
6161

‎include/swift/Strings.h‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,6 @@
1313
#ifndef SWIFT_STRINGS_H
1414
#define SWIFT_STRINGS_H
1515

16-
#include "swift/Basic/LLVM.h"
17-
#include "llvm/ADT/StringRef.h"
18-
1916
namespace swift {
2017

2118
/// The extension for serialized modules.
@@ -92,10 +89,6 @@ constexpr static const char BUILTIN_TYPE_NAME_VEC[] = "Builtin.Vec";
9289
constexpr static const char BUILTIN_TYPE_NAME_SILTOKEN[] = "Builtin.SILToken";
9390
/// The name of the Builtin type for Word
9491
constexpr static const char BUILTIN_TYPE_NAME_WORD[] = "Builtin.Word";
95-
96-
constexpr static StringLiteral OPTIMIZE_SIL_PRESERVE_EXCLUSIVITY =
97-
"optimize.sil.preserve_exclusivity";
98-
9992
} // end namespace swift
10093

10194
#endif // SWIFT_STRINGS_H

‎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/IRGen/LoadableByAddress.cpp‎

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

‎lib/ParseSIL/ParseSIL.cpp‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3447,6 +3447,7 @@ bool SILParser::parseSILInstruction(SILBuilder &B) {
34473447
ParsedEnum<SILAccessEnforcement> enforcement;
34483448
ParsedEnum<bool> aborting;
34493449
ParsedEnum<bool> noNestedConflict;
3450+
ParsedEnum<bool> fromBuiltin;
34503451

34513452
bool isBeginAccess = (Opcode == SILInstructionKind::BeginAccessInst ||
34523453
Opcode == SILInstructionKind::BeginUnpairedAccessInst);
@@ -3478,6 +3479,10 @@ bool SILParser::parseSILInstruction(SILBuilder &B) {
34783479
auto setNoNestedConflict = [&](bool value) {
34793480
maybeSetEnum(isBeginAccess, noNestedConflict, value, attr, identLoc);
34803481
};
3482+
auto setFromBuiltin = [&](bool value) {
3483+
maybeSetEnum(Opcode != SILInstructionKind::EndAccessInst, fromBuiltin,
3484+
value, attr, identLoc);
3485+
};
34813486

34823487
if (attr == "unknown") {
34833488
setEnforcement(SILAccessEnforcement::Unknown);
@@ -3499,6 +3504,8 @@ bool SILParser::parseSILInstruction(SILBuilder &B) {
34993504
setAborting(true);
35003505
} else if (attr == "no_nested_conflict") {
35013506
setNoNestedConflict(true);
3507+
} else if (attr == "builtin") {
3508+
setFromBuiltin(true);
35023509
} else {
35033510
P.diagnose(identLoc, diag::unknown_attribute, attr);
35043511
}
@@ -3523,6 +3530,9 @@ bool SILParser::parseSILInstruction(SILBuilder &B) {
35233530
if (isBeginAccess && !noNestedConflict.isSet())
35243531
noNestedConflict.Value = false;
35253532

3533+
if (!fromBuiltin.isSet())
3534+
fromBuiltin.Value = false;
3535+
35263536
SILValue addrVal;
35273537
SourceLoc addrLoc;
35283538
if (parseTypedValueRef(addrVal, addrLoc, B))
@@ -3547,16 +3557,16 @@ bool SILParser::parseSILInstruction(SILBuilder &B) {
35473557
if (Opcode == SILInstructionKind::BeginAccessInst) {
35483558
ResultVal =
35493559
B.createBeginAccess(InstLoc, addrVal, *kind, *enforcement,
3550-
*noNestedConflict);
3560+
*noNestedConflict, *fromBuiltin);
35513561
} else if (Opcode == SILInstructionKind::EndAccessInst) {
35523562
ResultVal = B.createEndAccess(InstLoc, addrVal, *aborting);
35533563
} else if (Opcode == SILInstructionKind::BeginUnpairedAccessInst) {
35543564
ResultVal = B.createBeginUnpairedAccess(InstLoc, addrVal, bufferVal,
35553565
*kind, *enforcement,
3556-
*noNestedConflict);
3566+
*noNestedConflict, *fromBuiltin);
35573567
} else {
3558-
ResultVal = B.createEndUnpairedAccess(InstLoc, addrVal,
3559-
*enforcement, *aborting);
3568+
ResultVal = B.createEndUnpairedAccess(InstLoc, addrVal, *enforcement,
3569+
*aborting, *fromBuiltin);
35603570
}
35613571
break;
35623572
}

‎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) {

0 commit comments

Comments
 (0)