Skip to content

Commit 6649fac

Browse files
committed
[region-isolation] Emit an error when we assign a non-disconnected value into a sending result.
rdar://127318392 (cherry picked from commit bcbf5c5)
1 parent 753f06e commit 6649fac

File tree

7 files changed

+450
-23
lines changed

7 files changed

+450
-23
lines changed

include/swift/AST/DiagnosticsSIL.def

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -984,6 +984,9 @@ NOTE(regionbasedisolation_isolated_since_in_same_region_string, none,
984984
ERROR(regionbasedisolation_named_transfer_yields_race, none,
985985
"sending %0 risks causing data races",
986986
(Identifier))
987+
NOTE(regionbasedisolation_type_is_non_sendable, none,
988+
"%0 is a non-Sendable type",
989+
(Type))
987990

988991
ERROR(regionbasedisolation_inout_sending_cannot_be_actor_isolated, none,
989992
"'inout sending' parameter %0 cannot be %1at end of function",
@@ -992,6 +995,20 @@ NOTE(regionbasedisolation_inout_sending_cannot_be_actor_isolated_note, none,
992995
"%1%0 risks causing races in between %1uses and caller uses since caller assumes value is not actor isolated",
993996
(Identifier, StringRef))
994997

998+
ERROR(regionbasedisolation_out_sending_cannot_be_actor_isolated_type, none,
999+
"returning a %1 %0 value as a 'sending' result risks causing data races",
1000+
(Type, StringRef))
1001+
NOTE(regionbasedisolation_out_sending_cannot_be_actor_isolated_note_type, none,
1002+
"returning a %1 %0 value risks causing races since the caller assumes the value can be safely sent to other isolation domains",
1003+
(Type, StringRef))
1004+
1005+
ERROR(regionbasedisolation_out_sending_cannot_be_actor_isolated_named, none,
1006+
"returning %1 %0 as a 'sending' result risks causing data races",
1007+
(Identifier, StringRef))
1008+
NOTE(regionbasedisolation_out_sending_cannot_be_actor_isolated_note_named, none,
1009+
"returning %1 %0 risks causing data races since the caller assumes that %0 can be safely sent to other isolation domains",
1010+
(Identifier, StringRef))
1011+
9951012
NOTE(regionbasedisolation_named_info_transfer_yields_race, none,
9961013
"sending %1%0 to %2 callee risks causing data races between %2 and local %3 uses",
9971014
(Identifier, StringRef, ActorIsolation, ActorIsolation))

include/swift/SILOptimizer/Utils/PartitionUtils.h

Lines changed: 76 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -996,6 +996,16 @@ struct PartitionOpEvaluator {
996996
isolationRegionInfo);
997997
}
998998

999+
/// Just call our CRTP subclass.
1000+
void handleAssignTransferNonTransferrableIntoSendingResult(
1001+
const PartitionOp &op, Element destElement,
1002+
SILFunctionArgument *destValue, Element srcElement, SILValue srcValue,
1003+
SILDynamicMergedIsolationInfo srcIsolationRegionInfo) const {
1004+
return asImpl().handleAssignTransferNonTransferrableIntoSendingResult(
1005+
op, destElement, destValue, srcElement, srcValue,
1006+
srcIsolationRegionInfo);
1007+
}
1008+
9991009
/// Call our CRTP subclass.
10001010
void handleInOutSendingNotInitializedAtExitError(
10011011
const PartitionOp &op, Element elt, Operand *transferringOp) const {
@@ -1112,13 +1122,41 @@ struct PartitionOpEvaluator {
11121122
p.pushHistorySequenceBoundary(loc);
11131123

11141124
switch (op.getKind()) {
1115-
case PartitionOpKind::Assign:
1125+
case PartitionOpKind::Assign: {
11161126
assert(op.getOpArgs().size() == 2 &&
11171127
"Assign PartitionOp should be passed 2 arguments");
11181128
assert(p.isTrackingElement(op.getOpArgs()[1]) &&
11191129
"Assign PartitionOp's source argument should be already tracked");
1130+
1131+
// See if we are assigning an a non-disconnected value into a 'out
1132+
// sending' parameter. In such a case, we emit a diagnostic.
1133+
if (op.getSourceInst()
1134+
->getFunction()
1135+
->getLoweredFunctionType()
1136+
->hasSendingResult()) {
1137+
if (auto instance = getRepresentativeValue(op.getOpArgs()[0])) {
1138+
if (auto value = instance.maybeGetValue()) {
1139+
if (auto *fArg = dyn_cast<SILFunctionArgument>(value)) {
1140+
if (fArg->getArgumentConvention().isIndirectOutParameter()) {
1141+
Region srcRegion = p.getRegion(op.getOpArgs()[1]);
1142+
auto dynamicRegionIsolation = getIsolationRegionInfo(srcRegion);
1143+
// We can unconditionally getValue here since we can never
1144+
// assign an actor introducing inst.
1145+
auto rep = getRepresentativeValue(op.getOpArgs()[1]).getValue();
1146+
if (!dynamicRegionIsolation.isDisconnected()) {
1147+
handleAssignTransferNonTransferrableIntoSendingResult(
1148+
op, op.getOpArgs()[0], fArg, op.getOpArgs()[1], rep,
1149+
dynamicRegionIsolation);
1150+
}
1151+
}
1152+
}
1153+
}
1154+
}
1155+
}
1156+
11201157
p.assignElement(op.getOpArgs()[0], op.getOpArgs()[1]);
11211158
return;
1159+
}
11221160
case PartitionOpKind::AssignFresh:
11231161
assert(op.getOpArgs().size() == 1 &&
11241162
"AssignFresh PartitionOp should be passed 1 argument");
@@ -1199,15 +1237,42 @@ struct PartitionOpEvaluator {
11991237
p.undoTransfer(op.getOpArgs()[0]);
12001238
return;
12011239
}
1202-
case PartitionOpKind::Merge:
1240+
case PartitionOpKind::Merge: {
12031241
assert(op.getOpArgs().size() == 2 &&
12041242
"Merge PartitionOp should be passed 2 arguments");
12051243
assert(p.isTrackingElement(op.getOpArgs()[0]) &&
12061244
p.isTrackingElement(op.getOpArgs()[1]) &&
12071245
"Merge PartitionOp's arguments should already be tracked");
12081246

1247+
// See if we are assigning an a non-disconnected value into a 'out
1248+
// sending' parameter. In such a case, we emit a diagnostic.
1249+
if (op.getSourceInst()
1250+
->getFunction()
1251+
->getLoweredFunctionType()
1252+
->hasSendingResult()) {
1253+
if (auto instance = getRepresentativeValue(op.getOpArgs()[0])) {
1254+
if (auto value = instance.maybeGetValue()) {
1255+
if (auto *fArg = dyn_cast<SILFunctionArgument>(value)) {
1256+
if (fArg->getArgumentConvention().isIndirectOutParameter()) {
1257+
Region srcRegion = p.getRegion(op.getOpArgs()[1]);
1258+
auto dynamicRegionIsolation = getIsolationRegionInfo(srcRegion);
1259+
// We can unconditionally getValue here since we can never
1260+
// assign an actor introducing inst.
1261+
auto rep = getRepresentativeValue(op.getOpArgs()[1]).getValue();
1262+
if (!dynamicRegionIsolation.isDisconnected()) {
1263+
handleAssignTransferNonTransferrableIntoSendingResult(
1264+
op, op.getOpArgs()[0], fArg, op.getOpArgs()[1], rep,
1265+
dynamicRegionIsolation);
1266+
}
1267+
}
1268+
}
1269+
}
1270+
}
1271+
}
1272+
12091273
p.merge(op.getOpArgs()[0], op.getOpArgs()[1]);
12101274
return;
1275+
}
12111276
case PartitionOpKind::Require:
12121277
assert(op.getOpArgs().size() == 1 &&
12131278
"Require PartitionOp should be passed 1 argument");
@@ -1424,10 +1489,19 @@ struct PartitionOpEvaluatorBaseImpl : PartitionOpEvaluator<Subclass> {
14241489
const PartitionOp &op, Element elt,
14251490
SILDynamicMergedIsolationInfo regionInfo) const {}
14261491

1492+
/// Please see documentation on the CRTP version of this call for information
1493+
/// about this entrypoint.
14271494
void handleTransferNonTransferrable(
14281495
const PartitionOp &op, Element elt, Element otherElement,
14291496
SILDynamicMergedIsolationInfo isolationRegionInfo) const {}
14301497

1498+
/// Please see documentation on the CRTP version of this call for information
1499+
/// about this entrypoint.
1500+
void handleAssignTransferNonTransferrableIntoSendingResult(
1501+
const PartitionOp &partitionOp, Element destElement,
1502+
SILFunctionArgument *destValue, Element srcElement, SILValue srcValue,
1503+
SILDynamicMergedIsolationInfo srcIsolationRegionInfo) const {}
1504+
14311505
/// Used to signify an "unknown code pattern" has occured while performing
14321506
/// dataflow.
14331507
///

include/swift/SILOptimizer/Utils/VariableNameUtils.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,13 @@ class VariableNameInferrer {
201201
static std::optional<std::pair<Identifier, SILValue>>
202202
inferNameAndRoot(SILValue value);
203203

204+
/// Given a specific decl \p d, come up with a name for it.
205+
///
206+
/// This is used internally for translating all decls to names. This is
207+
/// exposed in case someone wants to wrap VariableNameUtils and needs to use
208+
/// the same internal mapping that VariableNameUtils uses.
209+
static StringRef getNameFromDecl(Decl *d);
210+
204211
private:
205212
void drainVariableNamePath();
206213
void popSingleVariableName();

0 commit comments

Comments
 (0)