Skip to content

Fix crossgen2 handling of direct calls to abstract methods#215

Merged
janvorli merged 2 commits intodotnet:masterfrom
janvorli:fix-abstractcalls-test
Nov 22, 2019
Merged

Fix crossgen2 handling of direct calls to abstract methods#215
janvorli merged 2 commits intodotnet:masterfrom
janvorli:fix-abstractcalls-test

Conversation

@janvorli
Copy link
Copy Markdown
Member

This change fixes two differences between old and new crossgen in the
getCallInfo method. Direct calls to abstract methods were being compiled
as if they were possible and lead to runtime crash in the call chain
from the PreStubWorker.

This change fixes two differences between old and new crossgen in the
getCallInfo method. Direct calls to abstract methods were being compiled
as if they were possible and lead to runtime crash in the call chain
from the PreStubWorker.
@janvorli janvorli added this to the 5.0 milestone Nov 22, 2019
@janvorli janvorli requested a review from trylek November 22, 2019 11:03
@janvorli janvorli self-assigned this Nov 22, 2019
if (originalMethod.Signature.IsStatic && (flags & CORINFO_CALLINFO_FLAGS.CORINFO_CALLINFO_CALLVIRT) != 0)
{
throw new BadImageFormatException();
throw new RequiresRuntimeJitException("CallVirt to static method");
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is not related to the fix, but it seems that we should not fail the whole build for this case, just the function compilation.

It seems that the InvalidProgramException is more appropriate.
Copy link
Copy Markdown
Member

@trylek trylek left a comment

Choose a reason for hiding this comment

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

Looks great to me, thanks for fixing this!

@janvorli janvorli merged commit d996a91 into dotnet:master Nov 22, 2019
@janvorli janvorli deleted the fix-abstractcalls-test branch November 22, 2019 15:13
@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants