Skip to content

Commit 0e9396b

Browse files
Merge pull request swiftlang#74373 from nate-chandler/cherrypick/release/6.0/rdar127518559
6.0: [ConsumeAddrChecker] Diagnose consumes of borrows.
2 parents ca8e023 + f3a2da9 commit 0e9396b

File tree

3 files changed

+109
-9
lines changed

3 files changed

+109
-9
lines changed

include/swift/SIL/MemAccessUtils.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -887,10 +887,10 @@ namespace swift {
887887
/// For convenience, encapsulate and AccessStorage value along with its
888888
/// accessed base address.
889889
struct AccessStorageWithBase {
890-
/// Identical to AccessStorage::compute but preserves the access base.
890+
/// Identical to AccessStorage::computeInScope but walks through begin_access.
891891
static AccessStorageWithBase compute(SILValue sourceAddress);
892892

893-
/// Identical to AccessStorage::computeInScope but preserves the base.
893+
/// Identical to AccessStorage::compute but stops at begin_access
894894
static AccessStorageWithBase computeInScope(SILValue sourceAddress);
895895

896896
AccessStorage storage;

lib/SILOptimizer/Mandatory/ConsumeOperatorCopyableAddressesChecker.cpp

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2079,12 +2079,98 @@ bool ConsumeOperatorCopyableAddressesChecker::performClosureDataflow(
20792079
closureConsumes);
20802080
}
20812081

2082+
struct MoveConstraint {
2083+
enum Value : uint8_t {
2084+
None,
2085+
RequiresReinit,
2086+
Illegal,
2087+
} value;
2088+
2089+
operator Value() { return value; }
2090+
MoveConstraint(Value value) : value(value) {}
2091+
2092+
static MoveConstraint forGuaranteed(bool guaranteed) {
2093+
return guaranteed ? Illegal : None;
2094+
}
2095+
2096+
bool isIllegal() { return value == Illegal; }
2097+
};
2098+
2099+
static MoveConstraint getMoveConstraint(SILValue addr) {
2100+
assert(addr->getType().isAddress());
2101+
auto access = AccessPathWithBase::computeInScope(addr);
2102+
auto base = access.getAccessBase();
2103+
switch (access.accessPath.getStorage().getKind()) {
2104+
case AccessRepresentation::Kind::Box:
2105+
// Even if the box is guaranteed, it may be permitted to consume its
2106+
// storage.
2107+
return MoveConstraint::None;
2108+
case AccessRepresentation::Kind::Stack: {
2109+
// An alloc_stack is guaranteed if it's a "store_borrow destination".
2110+
auto *asi = cast<AllocStackInst>(base.getBaseAddress());
2111+
return MoveConstraint::forGuaranteed(
2112+
!asi->getUsersOfType<StoreBorrowInst>().empty());
2113+
}
2114+
case AccessRepresentation::Kind::Global:
2115+
// A global can be consumed if it's reinitialized.
2116+
return MoveConstraint::RequiresReinit;
2117+
case AccessRepresentation::Kind::Class:
2118+
// A class field can be consumed if it's reinitialized.
2119+
return MoveConstraint::RequiresReinit;
2120+
case AccessRepresentation::Kind::Tail:
2121+
// A class field can be consumed if it's reinitialized.
2122+
return MoveConstraint::RequiresReinit;
2123+
case AccessRepresentation::Kind::Argument: {
2124+
// An indirect argument is guaranteed if it's @in_guaranteed.
2125+
auto *arg = base.getArgument();
2126+
return MoveConstraint::forGuaranteed(
2127+
arg->getArgumentConvention().isGuaranteedConvention());
2128+
}
2129+
case AccessRepresentation::Kind::Yield: {
2130+
auto baseAddr = base.getBaseAddress();
2131+
auto *bai = cast<BeginApplyInst>(
2132+
cast<MultipleValueInstructionResult>(baseAddr)->getParent());
2133+
auto index = *bai->getIndexOfResult(baseAddr);
2134+
auto info = bai->getSubstCalleeConv().getYieldInfoForOperandIndex(index);
2135+
return MoveConstraint::forGuaranteed(!info.isConsumed());
2136+
}
2137+
case AccessRepresentation::Kind::Nested: {
2138+
auto *bai = cast<BeginAccessInst>(base.getBaseAddress());
2139+
if (bai->getAccessKind() == SILAccessKind::Init ||
2140+
bai->getAccessKind() == SILAccessKind::Read)
2141+
return MoveConstraint::Illegal;
2142+
// Allow moves from both modify and deinit.
2143+
return MoveConstraint::None;
2144+
}
2145+
case AccessRepresentation::Kind::Unidentified:
2146+
// Conservatively reject for now.
2147+
return MoveConstraint::Illegal;
2148+
}
2149+
}
2150+
20822151
// Returns true if we emitted a diagnostic and handled the single block
20832152
// case. Returns false if we visited all of the uses and seeded the UseState
20842153
// struct with the information needed to perform our interprocedural dataflow.
20852154
bool ConsumeOperatorCopyableAddressesChecker::performSingleBasicBlockAnalysis(
20862155
SILValue address, DebugVarCarryingInst addressDebugInst,
20872156
MarkUnresolvedMoveAddrInst *mvi) {
2157+
if (getMoveConstraint(mvi->getSrc()).isIllegal()) {
2158+
auto &astCtx = mvi->getFunction()->getASTContext();
2159+
StringRef name = getDebugVarName(address);
2160+
diagnose(astCtx, getSourceLocFromValue(address),
2161+
diag::sil_movechecking_guaranteed_value_consumed, name);
2162+
diagnose(astCtx, mvi->getLoc().getSourceLoc(),
2163+
diag::sil_movechecking_consuming_use_here);
2164+
2165+
// Replace the marker instruction with a copy_addr to avoid subsequent
2166+
// diagnostics.
2167+
SILBuilderWithScope builder(mvi);
2168+
builder.createCopyAddr(mvi->getLoc(), mvi->getSrc(), mvi->getDest(),
2169+
IsNotTake, IsInitialization);
2170+
mvi->eraseFromParent();
2171+
2172+
return true;
2173+
}
20882174
// First scan downwards to make sure we are move out of this block.
20892175
auto &useState = dataflowState.useState;
20902176
auto &applySiteToPromotedArgIndices =

test/SILOptimizer/consume_operator_kills_copyable_addressonly_lets.swift

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-frontend -verify %s -parse-stdlib -emit-sil -o /dev/null
1+
// RUN: %target-swift-frontend -sil-verify-all -verify %s -parse-stdlib -emit-sil -o /dev/null
22

33

44
import Swift
@@ -146,8 +146,8 @@ public func conditionalBadConsumingUseLoop2<T>(_ x: T) {
146146
// Parameters
147147

148148
// This is ok, no uses after.
149-
public func simpleMoveOfParameter<T>(_ x: T) -> () {
150-
let _ = consume x
149+
public func simpleMoveOfParameter<T>(_ x: T) -> () { // expected-error {{'x' is borrowed and cannot be consumed}}
150+
let _ = consume x // expected-note {{consumed here}}
151151
}
152152

153153
public func simpleMoveOfOwnedParameter<T>(_ x: __owned T) -> () {
@@ -214,12 +214,12 @@ func consumeOwned<T>(_ k: __owned T) {
214214
_ = consume k
215215
}
216216

217-
func consumeShared<T>(_ k: __shared T) {
218-
_ = consume k
217+
func consumeShared<T>(_ k: __shared T) { // expected-error {{'k' is borrowed and cannot be consumed}}
218+
_ = consume k // expected-note {{consumed here}}
219219
}
220220

221-
func consumeBare<T>(_ k: T) {
222-
_ = consume k
221+
func consumeBare<T>(_ k: T) { // expected-error {{'k' is borrowed and cannot be consumed}}
222+
_ = consume k // expected-note {{consumed here}}
223223
}
224224

225225
////////////////////////
@@ -384,6 +384,20 @@ public func castTestIfLet2(_ x : __owned EnumWithKlass) { // expected-error {{'x
384384
}
385385
}
386386

387+
enum rdar125817827<A, B> {
388+
case a(A)
389+
case b(B)
390+
}
391+
392+
extension rdar125817827 {
393+
func foo() { // expected-error {{'self' is borrowed and cannot be consumed}}
394+
switch consume self { // expected-note {{consumed here}}
395+
case let .a(a): print(a)
396+
case let .b(b): print(b)
397+
}
398+
}
399+
}
400+
387401
/////////////////////////
388402
// Partial Apply Tests //
389403
/////////////////////////

0 commit comments

Comments
 (0)