dotnet / linker

388 stars 126 forks source link

[release/7.0] Fixes 3072 handling intrinsics on TypeInfo #3109

Closed vitek-karas closed 1 year ago

vitek-karas commented 1 year ago

The problem is that GetIntrinsicIdForMethod operates on MethodDefinition but some parts of HandleCall operate on MethodReference. If the calling code references for example TypeInfo.GetMethods intrinsics detection finds this as being the GetMethods intrinsics but HandleCall doesn't and falls through to the default case which should basically never happen.

The fix is to use MethodDefinition in both places.

Added a test which needed to be done in IL since the last time TypeInfo.GetMethods existed as a method (and not inherited from Type.GetMethods) was in netstandard1.2. So in the IL the call is made to a MethodReference TypeInfo.GetMethods which reproes the problem.

This is a servicing fix for https://github.com/dotnet/linker/issues/3072 into 7.0.

Triage template:

Customer Impact

Apps which contain components compiled against netstandard1.2 (or earlier) which call some of the TypeInfo.Get... methods and the app is trimmed will trigger this problem. When it happens it's a fatal error which prevents the app from publishing. Workarounds are only:

Risk

Low - the change is only in code related to recognizing Type/TypeInfo.Get... methods and the change is "simple" (instead of operating on the reference, operate on the resolved definition - the resolution happens already anyway, so it will not be triggered by this change).

Testing

Verified that the reported repro for the problem is fixed. Added a regression test which covers all of the affected TypeInfo.Get... methods.

vitek-karas commented 1 year ago

Once approved I'll port the tests over to main, where the bug doesn't repro since the affected code has been changed. But we still want the tests.