Bug 36317 - ObjCRuntime.Runtime.GetNSObject<T> returns wrong object in [MonoPInvokeCallback]-attributed method on 64-bit devices when "Enable device-specific builds" is unchecked
Summary: ObjCRuntime.Runtime.GetNSObject<T> returns wrong object in [MonoPInvokeCallba...
Status: VERIFIED FIXED
Alias: None
Product: iOS
Classification: Xamarin
Component: XI runtime ()
Version: XI 9.2
Hardware: Macintosh Mac OS
: Normal normal
Target Milestone: C6SR1
Assignee: Rolf Bjarne Kvinge [MSFT]
URL:
Depends on:
Blocks:
 
Reported: 2015-11-27 18:00 UTC by Brad Umbaugh
Modified: 2015-12-16 17:47 UTC (History)
5 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:


Attachments
Sample Xamarin.iOS project demonstrating bug (59.46 KB, application/zip)
2015-11-27 18:00 UTC, Brad Umbaugh
Details


Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on Developer Community or GitHub with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
VERIFIED FIXED

Description Brad Umbaugh 2015-11-27 18:00:37 UTC
Created attachment 14021 [details]
Sample Xamarin.iOS project demonstrating bug

In the attached sample, inside a method ("PopViewControllerAnimated(...)") attributed with [MonoPInvokeCallback(...)], the following line of code returns an __NSMallocBlock__ instead of a UIViewController, but only when "Enable-device specific builds" is unchecked, and only on 64-bit physical devices (tested on iPhone 6s). On 32-bit devices (tested on iPod Touch 5th Gen), the code returns the expected UINavigationController instance:


ObjCRuntime.Runtime.GetNSObject<UINavigationController>(self);

To trigger this behavior in the sample, tap the button on the first screen, which navigates to a second screen; then hit the back button to navigate back to the first screen, and the exception will occur. Bug described in comments in Hook.cs, too.
Comment 2 Rolf Bjarne Kvinge [MSFT] 2015-11-30 16:22:36 UTC
There is a bug in BlockLiteral.SetupBlock that causes iOS to think it's a block with a stret signature, effectively causing imp_implementationWithBlock to create a wrapper method that puts the arguments in the wrong place for all architectures but ARM64 (since ARM64 doesn't have any stret signatures).

This has been fixed (maccore/master: 20b932ed52f61b73a872951fd20df7fede91b532).

In any case your implementation is incorrect, if you're using imp_implementationWithBlock the first argument to the method is the block, and then the rest comes after (and this is how it will behave once we've released the fix).

Fortunately you don't have to create a block and use imp_implementationWithBlock, you can just create a method with the proper signature for the direct call instead, and that works just fine: https://gist.github.com/rolfbjarne/1115b3e1aa14ee7acfd2
Comment 3 Mark Woollard 2015-11-30 17:29:51 UTC
Thanks for info, I was using the SetupBlock approach from stack overflow article indicating bugs with the non-block approach in your re-work of the example. I prefer your approach. Maybe a bug in this area causing issues in the past has been addressed already.
Comment 4 Rolf Bjarne Kvinge [MSFT] 2015-11-30 17:35:37 UTC
Which stackoverflow post is that?
Comment 5 Mark Woollard 2015-11-30 17:43:50 UTC
I based my original approach from this article 

http://stackoverflow.com/questions/14127453/how-to-port-method-getimplementation-and-method-setimplementation-to-monotou
Comment 6 Rolf Bjarne Kvinge [MSFT] 2015-11-30 18:00:17 UTC
@Mark, thanks, that stackoverflow post looks right (except that it doesn't currently work due to this bug).
Comment 8 Abhishek 2015-12-01 19:18:24 UTC
I am able to reproduce this issue with 32 bit device. Here is the screencast for the same: http://www.screencast.com/t/is1KkOodKBB

To verify this issue I have checked this issue with the master build on 32 bit device: monotouch-9.3.2.477_20b932ed52f61b73a872951fd20df7fede91b532. Now this issue is working fine. Here is the screencast for the same: http://www.screencast.com/t/MAP3GxQOMUT9
Comment 9 Abhishek 2015-12-16 17:47:50 UTC
I have checked this issue with the latest C6SR1 build on 32 bit device:
monotouch-9.4.1.8_f146d47db0476e3519dc6e8229af74a4e867f386. Now this issue is working fine. Here is the screencast for the same: http://www.screencast.com/t/9LgNjCU9JUp

Thanks!