Objective-C msgSend* support for pending exceptions in Release

[AaronRobinsonMSFT]

Fixes #52737
Fixes #52962

The current logic was assuming the NDirectMethodDesc was always fully populated when the call to MarshalingRequired was performed – This is not the case, comment was added. The fix here is to attempt to populate the NDirectMethodDesc when we get to a point where it is needed.

It was also determined that a fundamental bug existed in the experience since the JIT does call MarshalingRequired prior to the method being run which means detecting if a method is overloaded could be detected in the scenario below. This means we are changing the logic so that no calls to overridable functions (i.e., msgSend*) in /usr/lib/libobjc.dylib will be inlinable and will always check for a pending exception.

unsafe class Program
{
    private const string LibName = "/usr/lib/libobjc.dylib";
    [DllImport(LibName)]
    public static extern IntPtr objc_msgSend(IntPtr self, IntPtr selector);
    private sealed class PendingException : Exception
    {
        public PendingException(string message) : base(message) { }
    }
    [UnmanagedCallersOnly]
    private static IntPtr MsgSend(IntPtr inst, IntPtr sel) => SetPendingException();
    private static IntPtr SetPendingException([CallerMemberName] string callerName = "")
    {
        ObjectiveCMarshal.SetMessageSendPendingException(new PendingException(callerName));
        return IntPtr.Zero;
    }
    static void Main(string[] doNotUse)
    {
        var func = (IntPtr)(delegate* unmanaged<IntPtr, IntPtr, IntPtr>)&MsgSend;
        ObjectiveCMarshal.SetMessageSendCallback(MessageSendFunction.MsgSend, func);
        IntPtr inst = IntPtr.Zero;
        IntPtr sel = IntPtr.Zero;
        objc_msgSend(inst, sel); // Should throw PendingException but doesn’t.
    }
}

/cc @jkotas @jkoritzinsky @elinor-fung

[jkotas]

LGTM. Thanks!

[elinor-fung]

This means we are changing the logic so that no calls to overridable functions (i.e., msgSend*) in /usr/lib/libobjc.dylib will be inlinable.

This is not just disabling inlining for objc_msgSend*, but also changing the emitted IL to always check for a pending exception object. Is that intentional? I'm not seeing why that would be necessary for this.

[AaronRobinsonMSFT]

This is not just disabling inlining for objc_msgSend*, but also changing the emitted IL to always check for a pending exception object. Is that intentional?

Yes. This aligns with the current Mono contract.

I'm not seeing why that would be necessary for this.

Is your interpretation that only overriding a msgSend* function should require a check?

[elinor-fung]

Chatted offline.

If the intention is that all msgSend invokes (not just overridden ones) will actually check for a pending exceptions, we should update the API docs. Could we also add a test for this behaviour?

[elinor-fung]

Thanks for the clarifications.

[AaronRobinsonMSFT]

I've one more commit to add but will wait to see if the current CI is green. This final commit only contains comment code so if the CI is green, I will push the commits and then merge.

[AaronRobinsonMSFT]

Issue is #52799.

[AaronRobinsonMSFT]

The failure in crossgen2 is #49365.

[AaronRobinsonMSFT]

Failures in x64 Debug are infrastructure failures.

[filipnavara]

Any plans to backport this to preview5?

[AaronRobinsonMSFT]

@filipnavara No. You can workaround this issue easily by making the P/Invoke signature non-blittable. An easy way to do this would be to set DllImportAttribute.SetLastError to true. This would skip the inlining and be minimal overhead that could be removed once Preview 6 is released.

[filipnavara]

I was actually more concerned about consuming the API on the Xamarin side but I supposed it can wait one more preview or I can use custom builds.