[release/6.0] [metadata] Skip null vtable entries when checking covariant return overrides

[github-actions]

Backport of #76323 to release/6.0

/cc @lambdageek

Customer Impact

Customers using code that uses C# covariant returns in mobile or WebAssembly applications experienced crashes when using classes that re-abstracted virtual methods that had covariant returns in child subclasses. At least one commercial NuGet package used this pattern which prevented their customers from deploying the library in Android applications.

See dotnet/maui#6811

Testing

New CI test and manual testing

Risk

Low. The change affects validation done by the runtime while loading classes. A null pointer dereference is avoided. No existing checks are removed, no new checks are added. Code generation is unchanged. Any code that previously worked will continue to work.

[lewing]

approved in email

[lewing]

@carlossanlop I would really like a way to land changes between branding and lock down, It feels like we're creating additional servicing compression

[carlossanlop]

@carlossanlop I would really like a way to land changes between branding and lock down, It feels like we're creating additional servicing compression

I see some problems if we start doing this:

• If there's a build respin, we could end up including an extra change that wasn't contemplated before. There is always a non-zero chance this new commit introduces additional unexpected problems.
• Especially because of the previous point, now determining which changes made it into what servicing release would become harder. My main guidance is this window between branding and lock down.

Having said that, I am open to changing the rules if we have enough counter arguments to outweigh the disadvantages.

@ViktorHofer @ericstj @mmitche what is your opinion on this? Are there additional concerns I am unaware of? Do you know if repos merge whenever?

[mmitche]

@carlossanlop I would really like a way to land changes between branding and lock down, It feels like we're creating additional servicing compression

I see some problems if we start doing this:

• If there's a build respin, we could end up including an extra change that wasn't contemplated before. There is always a non-zero chance this new commit introduces additional unexpected problems.
• Especially because of the previous point, now determining which changes made it into what servicing release would become harder. My main guidance is this window between branding and lock down.

Having said that, I am open to changing the rules if we have enough counter arguments to outweigh the disadvantages.

@ViktorHofer @ericstj @mmitche what is your opinion on this? Are there additional concerns I am unaware of? Do you know if repos merge whenever?

The only real requirement is that we do branding before we start merging fixes. Branding happens Tuesday. The main problem with checking in before we do branding is that we do incremental servicing for some repos. If you start checking into runtime, new versions of packages that will not ship in the next release start flowing into downstream repos and this tends to cause a ton of confusion and clean-up.

Basically, it gets more complicated to allow changes whenever. The other option, is to branch for each release (6.0.9, 6.0.10, etc.). But it's been shown over time that it's really difficult to coordinate these activities (we do this for previews, of course), and they're difficult to automate because the repository owners need to be aware and in control of the content of those newly created branches.

So we've tried to arrive at a happy medium for now where branding happens a week before release (around signoff) and we get a week or two where we can take new fixes.

[carlossanlop]

Approved, signed-off, CI green. No OOB package authoring changes needed. Ready to merge.