Skip to content

Conversation

jakobbotsch
Copy link
Member

For unmanaged calli crossgen2 was not properly checking all necessary
conditions for a pinvoke being required. In particular it did not check
for managed byrefs. Unify the MethodDesc/MethodSignature logic to get
all the checks.

Fix #58259

cc @dotnet/crossgen-contrib

For unmanaged calli crossgen2 was not properly checking all necessary
conditions for a pinvoke being required. In particular it did not check
for managed byrefs. Unify the MethodDesc/MethodSignature logic to get
all the checks.

Fix dotnet#58259
@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Aug 31, 2021
@ghost
Copy link

ghost commented Aug 31, 2021

Tagging subscribers to this area: @JulieLeeMSFT
See info in area-owners.md if you want to be subscribed.

Issue Details

For unmanaged calli crossgen2 was not properly checking all necessary
conditions for a pinvoke being required. In particular it did not check
for managed byrefs. Unify the MethodDesc/MethodSignature logic to get
all the checks.

Fix #58259

cc @dotnet/crossgen-contrib

Author: jakobbotsch
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@jakobbotsch jakobbotsch added area-crossgen2-coreclr and removed area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels Aug 31, 2021
@jakobbotsch jakobbotsch changed the title Unify MethodDesc/MethodSignature IsMarshallingRequired logic Fix crossgen2 calli GC hole by unifying MethodDesc/MethodSignature IsMarshallingRequired logic Aug 31, 2021
@jakobbotsch
Copy link
Member Author

This one should probably be backported to 6.0.

@jakobbotsch
Copy link
Member Author

/azp run runtime-coreclr outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jakobbotsch
Copy link
Member Author

/azp run runtime-coreclr outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jakobbotsch
Copy link
Member Author

Test failure looks like #58481

@JulieLeeMSFT
Copy link
Member

@davidwrighton and @MichalStrehovsky Please review the code.

Copy link
Member

@davidwrighton davidwrighton left a comment

Choose a reason for hiding this comment

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

:shipit:

@jakobbotsch jakobbotsch merged commit cd771fb into dotnet:main Sep 3, 2021
@jakobbotsch
Copy link
Member Author

/backport to release/6.0

@github-actions
Copy link
Contributor

github-actions bot commented Sep 3, 2021

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1197309432

@jakobbotsch jakobbotsch deleted the fix-58259 branch September 3, 2021 17:39
@ghost ghost locked as resolved and limited conversation to collaborators Oct 3, 2021
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.

Assertion failed '!varTypeIsGC(varDsc) in Checked crossgen over Microsoft.Diagnostics.Runtime.dll

5 participants