Skip to content

[6.0][region-isolation] Emit an error when we assign a non-disconnected value into a sending result. #75377

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jul 20, 2024
Merged
17 changes: 17 additions & 0 deletions include/swift/AST/DiagnosticsSIL.def
Original file line number Diff line number Diff line change
Expand Up @@ -984,6 +984,9 @@ NOTE(regionbasedisolation_isolated_since_in_same_region_string, none,
ERROR(regionbasedisolation_named_transfer_yields_race, none,
"sending %0 risks causing data races",
(Identifier))
NOTE(regionbasedisolation_type_is_non_sendable, none,
"%0 is a non-Sendable type",
(Type))

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

ERROR(regionbasedisolation_out_sending_cannot_be_actor_isolated_type, none,
"returning a %1 %0 value as a 'sending' result risks causing data races",
(Type, StringRef))
NOTE(regionbasedisolation_out_sending_cannot_be_actor_isolated_note_type, none,
"returning a %1 %0 value risks causing races since the caller assumes the value can be safely sent to other isolation domains",
(Type, StringRef))

ERROR(regionbasedisolation_out_sending_cannot_be_actor_isolated_named, none,
"returning %1 %0 as a 'sending' result risks causing data races",
(Identifier, StringRef))
NOTE(regionbasedisolation_out_sending_cannot_be_actor_isolated_note_named, none,
"returning %1 %0 risks causing data races since the caller assumes that %0 can be safely sent to other isolation domains",
(Identifier, StringRef))

NOTE(regionbasedisolation_named_info_transfer_yields_race, none,
"sending %1%0 to %2 callee risks causing data races between %2 and local %3 uses",
(Identifier, StringRef, ActorIsolation, ActorIsolation))
Expand Down
19 changes: 19 additions & 0 deletions include/swift/SIL/InstWrappers.h
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,13 @@ class SelectEnumOperation {
return sei->getCase(i);
return value.get<SelectEnumAddrInst *>()->getCase(i);
}

std::pair<EnumElementDecl *, Operand *> getCaseOperand(unsigned i) const {
if (auto *sei = value.dyn_cast<SelectEnumInst *>())
return sei->getCaseOperand(i);
return value.get<SelectEnumAddrInst *>()->getCaseOperand(i);
}

/// Return the value that will be used as the result for the specified enum
/// case.
SILValue getCaseResult(EnumElementDecl *D) {
Expand All @@ -229,6 +236,12 @@ class SelectEnumOperation {
return value.get<SelectEnumAddrInst *>()->getCaseResult(D);
}

Operand *getCaseResultOperand(EnumElementDecl *D) {
if (auto *sei = value.dyn_cast<SelectEnumInst *>())
return sei->getCaseResultOperand(D);
return value.get<SelectEnumAddrInst *>()->getCaseResultOperand(D);
}

/// If the default refers to exactly one case decl, return it.
NullablePtr<EnumElementDecl> getUniqueCaseForDefault();

Expand All @@ -243,6 +256,12 @@ class SelectEnumOperation {
return sei->getDefaultResult();
return value.get<SelectEnumAddrInst *>()->getDefaultResult();
}

Operand *getDefaultResultOperand() const {
if (auto *sei = value.dyn_cast<SelectEnumInst *>())
return sei->getDefaultResultOperand();
return value.get<SelectEnumAddrInst *>()->getDefaultResultOperand();
}
};

class ForwardingOperation {
Expand Down
24 changes: 24 additions & 0 deletions include/swift/SIL/SILInstruction.h
Original file line number Diff line number Diff line change
Expand Up @@ -7018,6 +7018,12 @@ class SelectEnumInstBase : public BaseTy {
getAllOperands()[i+1].get());
}

std::pair<EnumElementDecl *, Operand *> getCaseOperand(unsigned i) const {
auto *self = const_cast<SelectEnumInstBase *>(this);
return std::make_pair(getEnumElementDeclStorage()[i],
&self->getAllOperands()[i + 1]);
}

/// Return the value that will be used as the result for the specified enum
/// case.
SILValue getCaseResult(EnumElementDecl *D) {
Expand All @@ -7030,6 +7036,18 @@ class SelectEnumInstBase : public BaseTy {
return getDefaultResult();
}

Operand *getCaseResultOperand(EnumElementDecl *D) {
for (unsigned i = 0, e = getNumCases(); i != e; ++i) {
auto Entry = getCaseOperand(i);
if (Entry.first == D)
return Entry.second;
}

// select_enum is required to be fully covered, so return the default if we
// didn't find anything.
return getDefaultResultOperand();
}

bool hasDefault() const {
return sharedUInt8().SelectEnumInstBase.hasDefault;
}
Expand All @@ -7039,6 +7057,12 @@ class SelectEnumInstBase : public BaseTy {
return getAllOperands().back().get();
}

Operand *getDefaultResultOperand() const {
assert(hasDefault() && "doesn't have a default");
auto *self = const_cast<SelectEnumInstBase *>(this);
return &self->getAllOperands().back();
}

unsigned getNumCases() const {
return getAllOperands().size() - 1 - hasDefault();
}
Expand Down
86 changes: 4 additions & 82 deletions include/swift/SILOptimizer/Analysis/RegionAnalysis.h
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ class BlockPartitionState {

class TrackableValue;
class TrackableValueState;
class RepresentativeValue;

enum class TrackableValueFlag {
/// Base value that says a value is uniquely represented and is
Expand Down Expand Up @@ -208,53 +207,6 @@ class regionanalysisimpl::TrackableValueState {
}
};

/// The representative value of the equivalence class that makes up a tracked
/// value.
///
/// We use a wrapper struct here so that we can inject "fake" actor isolated
/// values into the regions of values that become merged into an actor by
/// calling a function without a non-sendable result.
class regionanalysisimpl::RepresentativeValue {
friend llvm::DenseMapInfo<RepresentativeValue>;

using InnerType = PointerUnion<SILValue, SILInstruction *>;

/// If this is set to a SILValue then it is the actual represented value. If
/// it is set to a SILInstruction, then this is a "fake" representative value
/// used to inject actor isolatedness. The instruction stored is the
/// instruction that introduced the actor isolated-ness.
InnerType value;

public:
RepresentativeValue() : value() {}
RepresentativeValue(SILValue value) : value(value) {}
RepresentativeValue(SILInstruction *actorRegionInst)
: value(actorRegionInst) {}

operator bool() const { return bool(value); }

void print(llvm::raw_ostream &os) const {
if (auto *inst = value.dyn_cast<SILInstruction *>()) {
os << "ActorRegionIntroducingInst: " << *inst;
return;
}

os << *value.get<SILValue>();
}

SILValue getValue() const { return value.get<SILValue>(); }
SILValue maybeGetValue() const { return value.dyn_cast<SILValue>(); }
bool hasRegionIntroducingInst() const { return value.is<SILInstruction *>(); }
SILInstruction *getActorRegionIntroducingInst() const {
return value.get<SILInstruction *>();
}

SWIFT_DEBUG_DUMP { print(llvm::dbgs()); }

private:
RepresentativeValue(InnerType value) : value(value) {}
};

/// A tuple consisting of a base value and its value state.
///
/// DISCUSSION: We are computing regions among equivalence classes of values
Expand Down Expand Up @@ -336,7 +288,6 @@ class RegionAnalysisValueMap {
using Region = PartitionPrimitives::Region;
using TrackableValue = regionanalysisimpl::TrackableValue;
using TrackableValueState = regionanalysisimpl::TrackableValueState;
using RepresentativeValue = regionanalysisimpl::RepresentativeValue;

private:
/// A map from the representative of an equivalence class of values to their
Expand Down Expand Up @@ -365,6 +316,10 @@ class RegionAnalysisValueMap {
/// value" returns an empty SILValue.
SILValue maybeGetRepresentative(Element trackableValueID) const;

/// Returns the value for this instruction if it isn't a fake "represenative
/// value" to inject actor isolatedness. Asserts in such a case.
RepresentativeValue getRepresentativeValue(Element trackableValueID) const;

/// Returns the fake "representative value" for this element if it
/// exists. Returns nullptr otherwise.
SILInstruction *maybeGetActorIntroducingInst(Element trackableValueID) const;
Expand Down Expand Up @@ -576,37 +531,4 @@ class RegionAnalysis final

} // namespace swift

namespace llvm {

inline llvm::raw_ostream &
operator<<(llvm::raw_ostream &os,
const swift::regionanalysisimpl::RepresentativeValue &value) {
value.print(os);
return os;
}

template <>
struct DenseMapInfo<swift::regionanalysisimpl::RepresentativeValue> {
using RepresentativeValue = swift::regionanalysisimpl::RepresentativeValue;
using InnerType = RepresentativeValue::InnerType;
using InnerDenseMapInfo = DenseMapInfo<InnerType>;

static RepresentativeValue getEmptyKey() {
return RepresentativeValue(InnerDenseMapInfo::getEmptyKey());
}
static RepresentativeValue getTombstoneKey() {
return RepresentativeValue(InnerDenseMapInfo::getTombstoneKey());
}

static unsigned getHashValue(RepresentativeValue value) {
return InnerDenseMapInfo::getHashValue(value.value);
}

static bool isEqual(RepresentativeValue LHS, RepresentativeValue RHS) {
return InnerDenseMapInfo::isEqual(LHS.value, RHS.value);
}
};

} // namespace llvm

#endif
Loading