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)

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


Attachments

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.

Note You need to log in before you can comment on or make changes to this bug.