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.

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

Conversation

egorzhdan
Copy link
Contributor

When a C++ foreign reference type is conformed to a Swift protocol via a Swift extension, trying to cast any MyProtocol to the foreign reference type crashes the runtime.

This was because selectCasterForDest wasn't handling C++ foreign reference types, and we were hitting swift_unreachable.

This change makes sure the runtime doesn't crash for such casts.

Notably, Swift doesn't have enough metadata to determine if the conditional cast actually succeeded. This is also a problem for CF types. Casting CF types in a similar fashion triggers a typechecker diagnostic. That diagnostic will be amended in a follow-up patch to also trigger for foreign reference types.

rdar://141227849

… foreign reference type

When a C++ foreign reference type is conformed to a Swift protocol via a Swift extension, trying to cast `any MyProtocol` to the foreign reference type crashes the runtime.

This was because `selectCasterForDest` wasn't handling C++ foreign reference types, and we were hitting `swift_unreachable`.

This change makes sure the runtime doesn't crash for such casts.

Notably, Swift doesn't have enough metadata to determine if the conditional cast actually succeeded. This is also a problem for CF types. Casting CF types in a similar fashion triggers a typechecker diagnostic. That diagnostic will be amended in a follow-up patch to also trigger for foreign reference types.

rdar://141227849
@egorzhdan egorzhdan added the c++ interop Feature: Interoperability with C++ label Dec 16, 2024
@egorzhdan
Copy link
Contributor Author

@swift-ci please test

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:

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.

Copy link
Contributor

@mikeash mikeash left a comment

Choose a reason for hiding this comment

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

LGTM. /cc @tbkka in case you're interested.

@egorzhdan egorzhdan requested a review from tbkka December 16, 2024 20:30
@egorzhdan egorzhdan merged commit f2ad9f3 into main Dec 17, 2024
5 checks passed
@egorzhdan egorzhdan deleted the egorzhdan/frt-as-existential-crash branch December 17, 2024 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ interop Feature: Interoperability with C++
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants