Skip to content

Commit bcbf5c5

Browse files
committed
[region-isolation] Emit an error when we assign a non-disconnected value into a sending result.
rdar://127318392
1 parent ae797d4 commit bcbf5c5

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
@@ -972,6 +972,9 @@ ERROR(regionbasedisolation_arg_passed_to_strongly_transferred_param, none,
972972
ERROR(regionbasedisolation_named_transfer_yields_race, none,
973973
"sending %0 risks causing data races",
974974
(Identifier))
975+
NOTE(regionbasedisolation_type_is_non_sendable, none,
976+
"%0 is a non-Sendable type",
977+
(Type))
975978

976979
ERROR(regionbasedisolation_inout_sending_cannot_be_actor_isolated, none,
977980
"'inout sending' parameter %0 cannot be %1at end of function",
@@ -980,6 +983,20 @@ NOTE(regionbasedisolation_inout_sending_cannot_be_actor_isolated_note, none,
980983
"%1%0 risks causing races in between %1uses and caller uses since caller assumes value is not actor isolated",
981984
(Identifier, StringRef))
982985

986+
ERROR(regionbasedisolation_out_sending_cannot_be_actor_isolated_type, none,
987+
"returning a %1 %0 value as a 'sending' result risks causing data races",
988+
(Type, StringRef))
989+
NOTE(regionbasedisolation_out_sending_cannot_be_actor_isolated_note_type, none,
990+
"returning a %1 %0 value risks causing races since the caller assumes the value can be safely sent to other isolation domains",
991+
(Type, StringRef))
992+
993+
ERROR(regionbasedisolation_out_sending_cannot_be_actor_isolated_named, none,
994+
"returning %1 %0 as a 'sending' result risks causing data races",
995+
(Identifier, StringRef))
996+
NOTE(regionbasedisolation_out_sending_cannot_be_actor_isolated_note_named, none,
997+
"returning %1 %0 risks causing data races since the caller assumes that %0 can be safely sent to other isolation domains",
998+
(Identifier, StringRef))
999+
9831000
NOTE(regionbasedisolation_named_info_transfer_yields_race, none,
9841001
"sending %1%0 to %2 callee risks causing data races between %2 and local %3 uses",
9851002
(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");
@@ -1431,10 +1496,19 @@ struct PartitionOpEvaluatorBaseImpl : PartitionOpEvaluator<Subclass> {
14311496
const PartitionOp &op, Element elt,
14321497
SILDynamicMergedIsolationInfo regionInfo) const {}
14331498

1499+
/// Please see documentation on the CRTP version of this call for information
1500+
/// about this entrypoint.
14341501
void handleTransferNonTransferrable(
14351502
const PartitionOp &op, Element elt, Element otherElement,
14361503
SILDynamicMergedIsolationInfo isolationRegionInfo) const {}
14371504

1505+
/// Please see documentation on the CRTP version of this call for information
1506+
/// about this entrypoint.
1507+
void handleAssignTransferNonTransferrableIntoSendingResult(
1508+
const PartitionOp &partitionOp, Element destElement,
1509+
SILFunctionArgument *destValue, Element srcElement, SILValue srcValue,
1510+
SILDynamicMergedIsolationInfo srcIsolationRegionInfo) const {}
1511+
14381512
/// Used to signify an "unknown code pattern" has occured while performing
14391513
/// dataflow.
14401514
///

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)