Skip to content
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

[release/7.0] Generate diagnosable failfast in GVM resolution #78936

Merged
merged 1 commit into from
Jan 4, 2023

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Nov 28, 2022

Backport of #78904 to release/7.0

/cc @MichalStrehovsky

Customer Impact

This was reported by a customer in #78904

We have a bug where some static generic virtual methods might not be possible to create at runtime. The bug itself is a difficult and risky fix and likely won't meet the bar for 7.0 servicing. But it's possible for users to self-workaround the bug with RD.XML. But the failfast message when the bug is hit doesn't make it easy to construct the RD.XML. This improves the failfast when customer run into the bug. See #78882 (comment) and #78904 for details.

Testing

Manual. This only occurs when we run into the bug.

Risk

Low, this is in a failfast.

If the program hits the conditions in #77070, generate a failfast message that makes it possible to create a workaround.

Instead of:

```
Process terminated. Generic virtual method pointer lookup failure.

Declaring type handle: MethodTable:0x00007FF66E8587B8
Target type handle: MethodTable:0x00007FF66E858810
Method name: Serialize
Instantiation:
  Argument 00000000: MethodTable:0x00007FF66E85DA08
```

Generate:

```
Process terminated. Generic virtual method pointer lookup failure.

Declaring type handle: EETypeRva:0x005438B8(MemoryPackFormatter2`1[MemPackObject])
Target type handle: EETypeRva:0x00543910(MemoryPackableFormatter2`1[MemPackObject])
Method name: Serialize
Instantiation:
  Argument 00000000: EETypeRva:0x00529B38(System.Buffers.ArrayBufferWriter`1[System.Byte])
```

The workaround is then:

```xml
<Directives>
  <Application>
    <Assembly Name="repro">
      <Type Name="MemoryPackableFormatter2`1[[MemPackObject]]">
        <Method Name="Serialize" Dynamic="Required All">
          <GenericArgument Name="System.Buffers.ArrayBufferWriter`1[[System.Byte, mscorlib]],System.Memory" />
        </Method>
      </Type>
    </Assembly>
  </Application>
</Directives>
```
@ghost
Copy link

ghost commented Nov 28, 2022

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #78904 to release/7.0

/cc @MichalStrehovsky

Customer Impact

Testing

Risk

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

Author: github-actions[bot]
Assignees: -
Labels:

area-NativeAOT-coreclr

Milestone: -

Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

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

approved. we will take for consideration in 7.0.x

@jeffschwMSFT jeffschwMSFT added the Servicing-consider Issue for next servicing release review label Nov 28, 2022
@jeffschwMSFT jeffschwMSFT added this to the 7.0.x milestone Nov 28, 2022
@rbhanda rbhanda added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Nov 29, 2022
@rbhanda rbhanda modified the milestones: 7.0.x, 7.0.3 Nov 29, 2022
@carlossanlop
Copy link
Member

Approved by Tactics.
Signed off by area owners.
CI green.
Milestone 7.0.3 applied.
No OOB changes needed.
Ready to merge. :shipit:

@carlossanlop carlossanlop merged commit aa8eb01 into release/7.0 Jan 4, 2023
@carlossanlop carlossanlop deleted the backport/pr-78904-to-release/7.0 branch January 4, 2023 20:17
@ghost ghost locked as resolved and limited conversation to collaborators Feb 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-NativeAOT-coreclr Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants