Start using UnmanagedCallersOnly in CoreLib

[jkotas]

No description provided.

[Dotnet-GitSync-Bot]

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

[jkotas]

This is crashing in IL linker with:

 Mono.Linker.LinkerFatalErrorException: ILLinker: error IL1005: .Interop.Kernel32.EnumSystemLocalesEx(method,UInt32,Void*,IntPtr): Error processing method '.Interop.Kernel32.EnumSystemLocalesEx(method,UInt32,Void*,IntPtr)' in assembly 'System.Private.CoreLib.dll'
   ---> System.NullReferenceException: Object reference not set to an instance of an object.
     at Mono.Linker.Steps.MarkStep.ProcessInteropMethod(MethodDefinition method)
     at Mono.Linker.Steps.MarkStep.ProcessMethod(MethodDefinition method, DependencyInfo& reason)
     at Mono.Linker.Steps.MarkStep.ProcessQueue()
     --- End of inner exception stack trace ---
     at Mono.Linker.Steps.MarkStep.ProcessQueue()
     at Mono.Linker.Steps.MarkStep.ProcessPrimaryQueue()
     at Mono.Linker.Steps.MarkStep.Process()
     at Mono.Linker.Steps.MarkStep.Process(LinkContext context)
     at Mono.Linker.Pipeline.ProcessStep(LinkContext context, IStep step)
     at Mono.Linker.Pipeline.Process(LinkContext context)
     at Mono.Linker.Driver.Run(ILogger customLogger)

I have opened dotnet/linker#1345
@marek-safar @AaronRobinsonMSFT

[stephentoub]

Neat.

[AaronRobinsonMSFT]

@jkotas Is this something we want to run some benchmarks on? At least so we all have a general idea on the benefit of doing this work or at the very least to get a baseline of what we may expect for wins.

[jkotas]

The public entrypoints for what I am touching in this PR are pretty bulky methods. I do not expect to see measurable throughput wins. I expect to see delegate allocation and JITing of the reverse delegate interop interop to disappear with this change. I will check that it is what is happening once it actually builds.

My primary motivation for this change was to validate that the feature actually works end-to-end and shake stress bugs if there are any

[jkotas]

@vargaz UnmanagedCallersOnly function pointers seems to be broken on Mono. Could you please take look?

Also, PInvokeTable generator has problems with this change, but I should be able to fix that.

[jkotas]

I have reverted the Icu change for now to see whether there is anything else that breaks. (The full change is at https://github.com/jkotas/runtime/tree/unmanaged-callers-only-full.)

[jkotas]

I expect to see delegate allocation and JITing of the reverse delegate interop interop to disappear with this change

I have verified that this does actually happen.

[jkotas]

x86 failing with:

System.Globalization.Tests.DateTimeFormatInfoLongTimePattern.LongTimePattern_CheckReadingTimeFormatWithSingleQuotes_ICU [SKIP]
      Condition(s) not met: "IsIcuGlobalization"

Assert failure(PID 5328 [0x000014d0], Thread: 4784 [0x12b0]): !pMethod->HasUnmanagedCallersOnlyAttribute()

[vargaz]

How does UnmanagedCallersOnly fail on mono ? Does it fail on wasm or on desktop ?

[am11]

@vargaz, I think it failed on arm64 (desktop). Here are the job runs with ICU interop change. It was this change delegate* <char*, IntPtr, void> callback, which is now reverted. example log:

• Assertion at /__w/1/s/src/mono/mono/metadata/class-init.c:2241, condition `klass->instance_size == instance_size' not met

[jkotas]

Yes, this was the failure. You can reproduce it on Linux by checking out https://github.com/jkotas/runtime/tree/unmanaged-callers-only-full and running globalization tests.

[vargaz]

Should be fixed by #39517
.

[jkotas]

Thanks. Does #39517 fix it on wasm as well?

[jkotas]

The new Mono failure pattern is:

Unhandled Exception:\nSystem.InvalidProgramException: Invalid IL due to: method System.Globalization.CalendarData:EnumCalendarInfoCallback (char*,intptr) with UnmanagedCallersOnlyAttribute has non-blittable parameters or return type 

[vargaz]

Can't reproduce that failure.

[jkotas]

CoreCLR test failure is:

  Starting:    Microsoft.Extensions.Logging.Console.Tests (parallel test collections = on, max threads = 4)

Assert failure(PID 22 [0x00000016], Thread: 40 [0x0028]): m_callCountingInfoByCodeVersionHash.Lookup(codeVersion) == nullptr
    File: /__w/1/s/src/coreclr/src/vm/callcounting.cpp Line: 477
    Image: /root/helix/work/correlation/dotnet

[jkotas]

Blocked on #39604

[jkotas]

All failures are due to #39082

[jkotas]

Unhandled Exception:\nSystem.InvalidProgramException: Invalid IL due to: method System.Globalization.CalendarData:EnumCalendarInfoCallback (char*,intptr) with UnmanagedCallersOnlyAttribute has non-blittable parameters or return type

I have extended the Wasm workaround to apply to all Mono targets to work around this. @vargaz You should be able to reproduce this by changing #if MONO back to #if TARGETS_WASM.

[vargaz]

Can't reproduce the mono failures.

[jkotas]

I have submitted #39695 . It should reproduce the failures.