Skip to content

Commit 9ce9dce

Browse files
committed
[SILOptimizer] Fix let constant of non-class protocol being mutable
Extend the checks in `LifetimeChecker` in `SILOptimizer/Mandatory/DefiniteInitialization.cpp` to catch when the memory object corresponding to a let constant is used as an inout parameter in a protocol witness method. Initializing a let constant separate from its declaration requires write access. Therefore it is treated as `@lvalue TestProtocol` in the AST for a certain scope. Checking that the constant is not written to after being initialized is supposed to happen in the Mandatory SILOptimizer phase instead. On loads, the Optimizer checks, that a variable is fully initialized, but perviously did not validate what happens to it after it is loaded. This allowed loading the memory object through an open_existential_addr instruction and applying the result as an Operand to a mutating witness method.
1 parent 2ab53b5 commit 9ce9dce

File tree

2 files changed

+57
-3
lines changed

2 files changed

+57
-3
lines changed

include/swift/AST/DiagnosticsSIL.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,9 @@ ERROR(immutable_value_passed_inout,none,
173173
ERROR(assignment_to_immutable_value,none,
174174
"immutable value '%0' may not be assigned to",
175175
(StringRef))
176+
ERROR(mutating_protocol_witness_method_on_let_constant,none,
177+
"cannot perform mutating operation: '%0' is a 'let' constant",
178+
(StringRef))
176179

177180

178181
// Control flow diagnostics.

lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -518,6 +518,7 @@ namespace {
518518

519519

520520
void handleStoreUse(unsigned UseID);
521+
void handleLoadUse(unsigned UseID);
521522
void handleInOutUse(const DIMemoryUse &Use);
522523
void handleEscapeUse(const DIMemoryUse &Use);
523524

@@ -807,15 +808,16 @@ void LifetimeChecker::doIt() {
807808
handleStoreUse(i);
808809
break;
809810

810-
case DIUseKind::IndirectIn:
811-
case DIUseKind::Load: {
811+
case DIUseKind::IndirectIn: {
812812
bool IsSuperInitComplete, FailedSelfUse;
813813
// If the value is not definitively initialized, emit an error.
814814
if (!isInitializedAtUse(Use, &IsSuperInitComplete, &FailedSelfUse))
815815
handleLoadUseFailure(Use, IsSuperInitComplete, FailedSelfUse);
816816
break;
817817
}
818-
818+
case DIUseKind::Load:
819+
handleLoadUse(i);
820+
break;
819821
case DIUseKind::InOutUse:
820822
handleInOutUse(Use);
821823
break;
@@ -855,6 +857,55 @@ void LifetimeChecker::doIt() {
855857
handleConditionalDestroys(ControlVariable);
856858
}
857859

860+
void LifetimeChecker::handleLoadUse(unsigned UseID) {
861+
DIMemoryUse &Use = Uses[UseID];
862+
SILInstruction *LoadInst = Use.Inst;
863+
864+
bool IsSuperInitComplete, FailedSelfUse;
865+
// If the value is not definitively initialized, emit an error.
866+
if (!isInitializedAtUse(Use, &IsSuperInitComplete, &FailedSelfUse))
867+
return handleLoadUseFailure(Use, IsSuperInitComplete, FailedSelfUse);
868+
869+
// If this is an OpenExistentialAddrInst in preparation for applying
870+
// a witness method, analyze its use to make sure, that no mutation of
871+
// lvalue let constants occurs.
872+
auto* OEAI = dyn_cast<OpenExistentialAddrInst>(LoadInst);
873+
if (OEAI != nullptr && TheMemory.isElementLetProperty(Use.FirstElement)) {
874+
for (auto OEAUse : OEAI->getUses()) {
875+
auto* AI = dyn_cast<ApplyInst>(OEAUse->getUser());
876+
877+
if (AI == nullptr)
878+
// User is not an ApplyInst
879+
continue;
880+
881+
unsigned OperandNumber = OEAUse->getOperandNumber();
882+
if (OperandNumber < 1 || OperandNumber > AI->getNumCallArguments())
883+
// Not used as a call argument
884+
continue;
885+
886+
unsigned ArgumentNumber = OperandNumber - 1;
887+
888+
CanSILFunctionType calleeType = AI->getSubstCalleeType();
889+
SILParameterInfo parameterInfo = calleeType->getParameters()[ArgumentNumber];
890+
891+
if (!parameterInfo.isIndirectMutating() ||
892+
parameterInfo.getType().isAnyClassReferenceType())
893+
continue;
894+
895+
if (!shouldEmitError(LoadInst))
896+
continue;
897+
898+
std::string PropertyName;
899+
auto *VD = TheMemory.getPathStringToElement(Use.FirstElement, PropertyName);
900+
diagnose(Module, LoadInst->getLoc(),
901+
diag::mutating_protocol_witness_method_on_let_constant, PropertyName);
902+
903+
if (auto *Var = dyn_cast<VarDecl>(VD)) {
904+
Var->emitLetToVarNoteIfSimple(nullptr);
905+
}
906+
}
907+
}
908+
}
858909

859910
void LifetimeChecker::handleStoreUse(unsigned UseID) {
860911
DIMemoryUse &InstInfo = Uses[UseID];

0 commit comments

Comments
 (0)