Skip to content

[cxx-interop] Fix runtime crash when casting from an existential to a foreign reference type #78223

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.

Sign up for GitHub

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 1 commit into from
Dec 17, 2024
Merged

[cxx-interop] Fix runtime crash when casting from an existential to a foreign reference type #78223

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions stdlib/public/runtime/DynamicCast.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,16 @@ tryCastToForeignClass(
return DynamicCastResult::Failure;
}

static DynamicCastResult tryCastToForeignReferenceType(
OpaqueValue *destLocation, const Metadata *destType, OpaqueValue *srcValue,
const Metadata *srcType, const Metadata *&destFailureType,
const Metadata *&srcFailureType, bool takeOnSuccess, bool mayDeferChecks) {
assert(srcType != destType);
assert(destType->getKind() == MetadataKind::ForeignReferenceType);

return DynamicCastResult::Failure;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Despite this returning Failure unconditionally, the cast eventually succeeds in this code path:

swift/stdlib/public/runtime/DynamicCast.cpp

Lines 2373 to 2381 in 84ae5fb

case MetadataKind::Existential: {
auto subcastResult = tryCastUnwrappingExistentialSource(
destLocation, destType, srcValue, srcType,
destFailureType, srcFailureType, takeOnSuccess, mayDeferChecks);
if (isSuccess(subcastResult)) {
return subcastResult;
}
break;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Yeah, I think I remember there are other cases where an always-failing conversion is the right first step to unblock later progress. It might help people to think of the dynamic caster as a search process that evaluates a variety of paths to find a feasible conversion.

I suppose we could consider having selectCasterForDest fall back to returning a tryCastUnknown that unconditionally fails, instead of falling through to an unreachable assertion. That would allow many of the general engines (optional unwrapping, existential unwrapping) to work even for otherwise-unrecognized types.

}

/******************************************************************************/
/***************************** Enum Destination *******************************/
/******************************************************************************/
Expand Down Expand Up @@ -2187,6 +2197,8 @@ static tryCastFunctionType *selectCasterForDest(const Metadata *destType) {
return tryCastToOptional;
case MetadataKind::ForeignClass:
return tryCastToForeignClass;
case MetadataKind::ForeignReferenceType:
return tryCastToForeignReferenceType;
case MetadataKind::Opaque:
return tryCastToOpaque;
case MetadataKind::Tuple:
Expand Down
9 changes: 9 additions & 0 deletions test/Interop/Cxx/foreign-reference/witness-table.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,15 @@ WitnessTableTestSuite.test("As a Sequence") {
expectEqual(count, 3)
}

WitnessTableTestSuite.test("As an existential") {
let existential: any ListNode = makeLinkedList()
let cast: CxxLinkedList? = existential as? CxxLinkedList
expectNotNil(cast)
expectEqual(cast?.value, 0)
expectEqual(cast?.next()?.value, 1)
expectEqual(cast?.next()?.next()?.value, 2)
}

}

runAllTests()
Expand Down