-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Check return types in isCompatibleMethodGDV #118027
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
Conversation
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
That seems unlikely, this check should never fail with dynamic PGO, only with potentially outdated static PGO. |
Yeah, but I think even for Dynamic PGO we should be resilient there (I know there is a debug assert there, but who knows). |
|
Still, the issue might be the Static PGO? The one that we ship BCL with,. |
How did this issue surface? The point of the check is just to ensure that the JIT does not blow up. If the signatures don't match then the guard will never pass at runtime, so I don't think it would ever result in bad codegen. |
Makes sense, but in my case it just threw a bunch of JIT asserts here and there |
|
Does it look good? @jakobbotsch cc @dotnet/jit-contrib |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds return type validation to the isCompatibleMethodGDV function in the JIT compiler to prevent incorrect delegate inlining optimizations. The change addresses a bug discovered through stress testing where Guarded Devirtualization (GDV) for delegate calls was not properly validating that the candidate method's return type matches the call site's expected return type.
Key changes:
- Added return type compatibility checks using both implicit coercion validation and exact type matching
- Added specialized handling for struct return types with layout and passing convention validation
- Enhanced logging for debugging incompatible method scenarios
|
Reverted to getMethodSig because |
I've hacked a stress mode for GDV delegate inlining and found an issue that we don't validate return types.
Might fix #117867