Bug 41393 - [WatchOS 2] Incorrect calling convention for P/Invokes taking structures
Summary: [WatchOS 2] Incorrect calling convention for P/Invokes taking structures
Status: RESOLVED FIXED
Alias: None
Product: iOS
Classification: Xamarin
Component: Mono runtime / AOT compiler (show other bugs)
Version: master
Hardware: PC Mac OS
: --- normal
Target Milestone: Untriaged
Assignee: Zoltan Varga
URL:
Depends on:
Blocks:
 
Reported: 2016-05-31 11:55 UTC by Rolf Bjarne Kvinge [MSFT]
Modified: 2016-06-02 10:09 UTC (History)
2 users (show)

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

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:
RESOLVED FIXED

Description Rolf Bjarne Kvinge [MSFT] 2016-05-31 11:55:22 UTC
This function: CGAffineTransformInvert has this signature:

    CGAffineTransform CGAffineTransformInvert (CGAffineTransform transform);

CGAffineTransform is a struct with 6 floats.

On armv7k, the calling convention seems to be like this:

    CGAffineTransformInvert (CGAffineTransform* returnValue, CGAffineTransform* transform);

i.e. both the argument and the return value are passed as pointers.

However mono seems to generate code that places the individual float values in registers.

Disassembly of CGAffineTransformInvert:

    (lldb) disass -n CGAffineTransformInvert
    CoreGraphics`CGAffineTransformInvert:
        0x264584d4 <+0>:   push   {r4, r5, r7, lr}
        0x264584d6 <+2>:   add    r7, sp, #0x8
        0x264584d8 <+4>:   mov    r5, r1
        0x264584da <+6>:   mov    r4, r0
        0x264584dc <+8>:   vldr   s0, [r5]
        0x264584e0 <+12>:  vldr   s4, [r5, #4]
        0x264584e4 <+16>:  vldr   s8, [r5, #8]
        0x264584e8 <+20>:  vldr   s12, [r5, #12]

shows that r1 is a pointer.

and this is how we call the function:

    0x86e574 <+176>: ldr    r1, [r11, #0x5c]
    0x86e578 <+180>: ldr    r2, [r11, #0x60]
    0x86e57c <+184>: ldr    r3, [r11, #0x64]
    0x86e580 <+188>: ldr    r12, [r11, #0x68]
    0x86e584 <+192>: str    r12, [sp]
    0x86e588 <+196>: ldr    r12, [r11, #0x6c]
    0x86e58c <+200>: str    r12, [sp, #0x4]
    0x86e590 <+204>: ldr    r12, [r11, #0x70]
    0x86e594 <+208>: str    r12, [sp, #0x8]
    0x86e598 <+212>: bl     0x154bff4                 ; symbol stub for: CGAffineTransformInvert

Mono is clearly loading something into r1-r3.

Inspecting the registers shows the values I put into the first argument (new CGAffineTransform (1, 2, 3, 4, 5, 6))

    (lldb) p (float&) $r1
    (float) $0 = 1
    (lldb) p (float&) $r2
    (float) $1 = 2
    (lldb) p (float&) $r3
    (float) $2 = 3
Comment 1 Rolf Bjarne Kvinge [MSFT] 2016-05-31 13:00:53 UTC
Looks like structs bigger than 128 bites are passed by reference.

https://github.com/llvm-mirror/clang/blob/82f6d5c9ae84c04d6e7b402f72c33638d1fb6bc8/lib/CodeGen/TargetInfo.cpp#L5248-L5250:

    // WatchOS is adopting the 64-bit AAPCS rule on composite types: if they're
    // bigger than 128-bits, they get placed in space allocated by the caller,
    // and a pointer is passed.

https://github.com/llvm-mirror/clang/blob/82f6d5c9ae84c04d6e7b402f72c33638d1fb6bc8/lib/CodeGen/TargetInfo.cpp#L5542-L5543:

    // ARMv7k passes structs bigger than 16 bytes indirectly, in space
    // allocated by the caller.
Comment 2 Rolf Bjarne Kvinge [MSFT] 2016-05-31 14:14:13 UTC
There seems to be a related problem: Mono seems to think functions returning structs smaller than 16 bytes expects a pointer to the return value as the first parameter, when that's not correct.

Example:

    0x90debc <+140>: str    r0, [sp, #0x30]
    0x90dec0 <+144>: add    r0, sp, #8, #30
    0x90dec4 <+148>: ldr    r1, [sp, #0x28]
    0x90dec8 <+152>: ldr    r2, [sp, #0x2c]
    0x90decc <+156>: mov    r3, r6
    0x90ded0 <+160>: blx    0x1bd23e                  ; xamarin_pinvoke_wrapper_NSRange_objc_msgSend_UInt32 at pinvokes.m:1900

the function signature is:

    NSRange func (id self, SEL sel, unsigned int p2);

NSRange is 2 floats.

Mono converts that into this:

    void func (NSRange* return_value, id self, SEL sel, unsigned int p2);
Comment 3 Zoltan Varga 2016-06-01 01:05:56 UTC
What test can be used to reproduce this ?
Comment 4 Rolf Bjarne Kvinge [MSFT] 2016-06-01 07:29:09 UTC
I ran into this when running monotouch-test on the watch.

Here is a list of tests that ran into this issue: https://github.com/rolfbjarne/xamarin-macios/commit/d2cab72c1a3f794ba1afe5c2465cc3eb35913647

However this code should reproduce this as well:

For the first issue:

    var transform = new CGAffineTransform (1, 2, 3, 4, 5, 6).Invert ();

For the issue in the comment #2:

    var cal = new NSCalendar (NSCalendarType.Gregorian);
    var range = cal.MinimumRange (NSCalendarUnit.Hour);
Comment 5 Zoltan Varga 2016-06-02 04:53:27 UTC
As for the NSRange problem, the pinvoke method seems to be defined as:

  .method public hidebysig static pinvokeimpl("__Internal" as "xamarin_pinvoke_wrapper_NSRange_objc_msgSend_stret_UInt32" winapi) 
          void  NSRange_objc_msgSend_stret_UInt32([out] valuetype Foundation.NSRange& retval,
                                                  native int receiver,
                                                  native int selector,
                                                  uint32 arg1) cil managed preservesig
  {
  }

i.e. it receives an explicit return argument.
Comment 6 Rolf Bjarne Kvinge [MSFT] 2016-06-02 09:16:59 UTC
Sorry, I forgot I had a local fix for the P/Invoke signature: https://github.com/xamarin/xamarin-macios/pull/122

This can be used to reproduce instead:

	[System.Runtime.InteropServices.DllImport ("/usr/lib/libobjc.dylib")]
	static extern NSRange objc_msgSend (IntPtr sender, IntPtr selector, uint arg1);
	void Crash ()
	{
		var cal = new NSCalendar (NSCalendarType.Gregorian);
		var range = objc_msgSend (cal.Handle, ObjCRuntime.Selector.GetHandle ("minimumRangeOfUnit:"), (uint) NSCalendarUnit.Hour);
	}
Comment 7 Zoltan Varga 2016-06-02 10:09:56 UTC
Should be fixed by the mono bump in 301fa47c57a4778d2b55892dccedc95b735248c2.