Skip to content

Conversation

@davidwrighton
Copy link
Member

@davidwrighton davidwrighton commented Jun 27, 2022

  • The version resilience test had 2 major bugs in it
    1. It was not properly specifying references, so references to other dlls were not being handled
    2. It was generating .ni.dll files, which are not actually loaded by the runtime anymore, so even for the code it did generate, the cross module references were not in use.

Version resiliency bugs also fixed

  • Fix assertion when handling fields of sequential/explicit types from another module, where we did not generate a correct cross module version resilient fixup. (That's what the fix is in CorInfoImpl.ReadyToRun.cs)
  • Disable cross version bubble p/invoke inlining, as we don't currently generate valid cross module tokens for that scenario

- The version resilience test had 2 major bugs in it
  1. It was not properly specifying references, so references to other dlls were not being handled
  2. It was generating .ni.dll files, which are not actually loaded by the runtime anymore, so even for the code it did generate, the cross module references were not in use.
- In addition, fixing the build, caused us to generate an assertion when handling fields of sequential/explicit types from another module, where we did not generate a correct cross module version resilient fixup. (That's what the fix is in CorInfoImpl.ReadyToRun.cs)
@davidwrighton davidwrighton requested a review from trylek June 27, 2022 23:38
@ghost ghost assigned davidwrighton Jun 27, 2022
Copy link
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 David for fixing this!

@davidwrighton davidwrighton merged commit 31164f4 into dotnet:main Jul 1, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jul 31, 2022
@davidwrighton davidwrighton deleted the fix_crossgen2_version_resilience_test branch April 13, 2023 18:53
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