Skip to content

Commit 6ed43af

Browse files
committed
Fix checks and assertions in -enable-verify-exclusivity mode.
Now that access marker verification is strict and exhaustive, adjust some code to handle the extra markers and extra checks produced by -enable-verify-exclusivity.
1 parent 5b1965d commit 6ed43af

File tree

4 files changed

+49
-1
lines changed

4 files changed

+49
-1
lines changed

lib/SILOptimizer/IPO/GlobalOpt.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -678,6 +678,10 @@ replaceLoadsByKnownValue(BuiltinInst *CallToOnce, SILFunction *AddrF,
678678
auto *GetterRef = B.createFunctionRef(Call->getLoc(), GetterF);
679679
auto *NewAI = B.createApply(Call->getLoc(), GetterRef, Args, false);
680680

681+
// FIXME: This is asserting that a specific SIL sequence follows an
682+
// addressor! SIL passes should never do this without first specifying a
683+
// structural SIL property independent of the SILOptimizer and enforced by
684+
// the SILVerifier.
681685
for (auto Use : Call->getUses()) {
682686
auto *PTAI = dyn_cast<PointerToAddressInst>(Use->getUser());
683687
assert(PTAI && "All uses should be pointer_to_address");

lib/SILOptimizer/Mandatory/DiagnoseStaticExclusivity.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1076,7 +1076,16 @@ static void checkAccessedAddress(Operand *memOper, StorageMap &Accesses) {
10761076
// initialization, not a formal memory access. The strength of
10771077
// verification rests on the completeness of the opcode list inside
10781078
// findAccessedStorage.
1079-
if (!storage || !isPossibleFormalAccessBase(storage, memInst->getFunction()))
1079+
//
1080+
// For the purpose of verification, an unidentified access is
1081+
// unenforced. These occur in cases like global addressors and local buffers
1082+
// that make use of RawPointers.
1083+
if (!storage || storage.getKind() == AccessedStorage::Unidentified)
1084+
return;
1085+
1086+
// Some identifiable addresses can also be recognized as local initialization
1087+
// or other patterns that don't qualify as formal access.
1088+
if (!isPossibleFormalAccessBase(storage, memInst->getFunction()))
10801089
return;
10811090

10821091
// A box or stack variable may represent lvalues, but they can only conflict

lib/SILOptimizer/Utils/Local.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1409,6 +1409,10 @@ swift::analyzeStaticInitializer(SILValue V,
14091409
/// The sequence is traversed inside out, i.e.
14101410
/// starting with the innermost struct_element_addr
14111411
/// Move into utils.
1412+
///
1413+
/// FIXME: this utility does not make sense as an API. How can the caller
1414+
/// guarantee that the only uses of `I` are struct_element_addr and
1415+
/// tuple_element_addr?
14121416
void swift::replaceLoadSequence(SILInstruction *I,
14131417
SILValue Value,
14141418
SILBuilder &B) {
@@ -1434,6 +1438,18 @@ void swift::replaceLoadSequence(SILInstruction *I,
14341438
return;
14351439
}
14361440

1441+
if (auto *BA = dyn_cast<BeginAccessInst>(I)) {
1442+
for (auto Use : BA->getUses()) {
1443+
replaceLoadSequence(Use->getUser(), Value, B);
1444+
}
1445+
return;
1446+
}
1447+
1448+
// Incidental uses of an addres are meaningless with regard to the loaded
1449+
// value.
1450+
if (isIncidentalUse(I) || isa<BeginUnpairedAccessInst>(I))
1451+
return;
1452+
14371453
llvm_unreachable("Unknown instruction sequence for reading from a global");
14381454
}
14391455

test/SILOptimizer/access_marker_verify.swift

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,3 +1022,22 @@ public func testInitBox() throws {
10221022
// CHECK: store %{{.*}} to [trivial] [[PROJ]] : $*SomeError
10231023
// CHECK: throw [[BOXALLOC]] : $Error
10241024
// CHECK-LABEL: } // end sil function '$S20access_marker_verify11testInitBoxyyKF'
1025+
1026+
public final class HasStaticProp {
1027+
public static let empty: HasStaticProp = HasStaticProp()
1028+
}
1029+
1030+
// A global addressor produces an unenforced RawPointer. This looke
1031+
// like an Unidentified access with no access marker. Ensure that
1032+
// verification doesn't assert.
1033+
public func getStaticProp() -> HasStaticProp {
1034+
return .empty
1035+
}
1036+
1037+
// CHECK-LABEL: sil @$S20access_marker_verify13getStaticPropAA03HaseF0CyF : $@convention(thin) () -> @owned HasStaticProp {
1038+
// function_ref HasStaticProp.empty.unsafeMutableAddressor
1039+
// CHECK: [[F:%.*]] = function_ref @$S20access_marker_verify13HasStaticPropC5emptyACvau : $@convention(thin) () -> Builtin.RawPointer
1040+
// CHECK: [[RP:%.*]] = apply [[F]]() : $@convention(thin) () -> Builtin.RawPointer
1041+
// CHECK: [[ADR:%.*]] = pointer_to_address [[RP]] : $Builtin.RawPointer to [strict] $*HasStaticProp
1042+
// CHECK: load [copy] [[ADR]] : $*HasStaticProp
1043+
// CHECK-LABEL: } // end sil function '$S20access_marker_verify13getStaticPropAA03HaseF0CyF'

0 commit comments

Comments
 (0)