Skip to content

Commit

Permalink
[Blocks] Remove a block callback validation that's apparently too eag…
Browse files Browse the repository at this point in the history
…er. (#20625)

The validation verifies that the function pointer for a block callback is the
same the return value from the method's
MethodInfo.MethodHandle.GetFunctionPointer() method.

In actual code, it's equivalent to validating that the following always prints "Equal: true":

```cs
[UnmanagedCallersOnly]
public static void Invoke () {}

static void Test ()
{
    delegate* unmanaged<void> fptr1 = &Invoke;
    IntPtr fptr2 = GetType ().GetMethod ("Invoke").MethodHandle.GetFunctionPointer ();
    Console.WriteLine ($"fptr1: 0x{((IntPtr) fptr1).ToString ("x")}");
    Console.WriteLine ($"fptr2: 0x{fptr2.ToString ("x")}");
    Console.WriteLine ($"Equal: ((IntPtr) fptr1) == fptr2}"); // prints "Equal: true" for me
}
```

However, this isn't documented, and some feedback indicates it's certainly not
a valid assumption for CoreCLR:

https://discord.com/channels/732297728826277939/732582981163548703/1242473425759633488

And there's also a customer running into this validation, apparently without cause:
https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2054534

So just remove it.
  • Loading branch information
rolfbjarne committed May 23, 2024
1 parent 0f91fcd commit 76cef6c
Show file tree
Hide file tree
Showing 3 changed files with 1 addition and 17 deletions.
5 changes: 0 additions & 5 deletions src/ObjCRuntime/Blocks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -172,11 +172,6 @@ static string GetBlockSignature (void* trampoline, MethodInfo trampolineMethod)
if (!Runtime.DynamicRegistrationSupported)
throw ErrorHelper.CreateError (8050, Errors.MX8050 /* BlockLiteral.GetBlockSignature is not supported when the dynamic registrar has been linked away. */);

// Verify that the function pointer matches the trampoline
var functionPointer = trampolineMethod.MethodHandle.GetFunctionPointer ();
if (functionPointer != (IntPtr) trampoline)
throw ErrorHelper.CreateError (8047, Errors.MX8047 /* The trampoline method {0} does not match the function pointer 0x{1} for the trampolineMethod argument (they're don't refer to the same method) */, trampolineMethod.DeclaringType.FullName + "." + trampolineMethod.Name, ((IntPtr) trampoline).ToString ("x"));

// Verify that there's at least one parameter, and it must be System.IntPtr, void* or ObjCRuntime.BlockLiteral*.
var parameters = trampolineMethod.GetParameters ();
if (parameters.Length < 1)
Expand Down
9 changes: 0 additions & 9 deletions tools/mtouch/Errors.designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 1 addition & 3 deletions tools/mtouch/Errors.resx
Original file line number Diff line number Diff line change
Expand Up @@ -2227,9 +2227,7 @@
<value>Unable to find the method '{0}' in the type '{1}'</value>
</data>

<data name="MX8047" xml:space="preserve">
<value>The trampoline method {0} does not match the function pointer 0x{1} for the trampolineMethod argument (they're don't refer to the same method).</value>
</data>
<!-- MX8047: unused -->

<data name="MX8048" xml:space="preserve">
<value>The trampoline method {0} must have at least one parameter.</value>
Expand Down

16 comments on commit 76cef6c

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

Please sign in to comment.