[[ Bug 20644 ]] Fix calls to struct-returning obj-c methods

livecodeali · livecodeali · commit c45076b5b27b · 2017-11-07T16:29:16.000Z
Sometimes structs are returned from Objective-C methods in
registers instead of using the address provided as a parameter. In
the latter case `objc_msgSend_stret` should be used instead of
`objc_msgSend`. The rules for when this happens are architecture and
struct-size dependent and FFI calls will crash if the wrong form
is used. This patch updates the code to use `objc_msgSend_stret`
just in case the method returns a struct and
- on arm64, the struct size &gt; 64 bytes
- on arm, the struct size &gt; 4 bytes
- on x86_64, the struct size &gt; 16 bytes
- on i386, the struct size &gt; 8 bytes or a power of 2.
diff --git a/docs/notes/bugfix-20644.md b/docs/notes/bugfix-20644.md
@@ -0,0 +1 @@
+# Fix calls to struct-returning obj-c methods
diff --git a/libscript/src/script-execute.cpp b/libscript/src/script-execute.cpp
@@ -268,7 +268,11 @@ class MCScriptForeignInvocation
         /* There are different variants of objc_msgSend we need to use
          * depending on architecture and return type:
          *   struct return
-         *     all but arm64: _stret
+         *     arm64: _stret if struct size > 64 bytes
+         *     arm: _stret if struct size > 4 bytes
+         *     x86-64: _stret if struct size > 16 bytes
+         *     i386: _stret if struct size >= 8 bytes or
+         *           exactly 1, 2, or 4 bytes
          *   float return
          *     arm: no difference
          *     i386: _fpret
@@ -289,22 +293,31 @@ class MCScriptForeignInvocation
          
         ffi_cif *t_cif = (ffi_cif *)p_handler->objc.function_cif;
         void (*t_objc_msgSend)() = nullptr;
+        size_t t_rsize = t_cif->rtype->size;
 #if defined(__ARM64__)
-        t_objc_msgSend = (void(*)())objc_msgSend;
+        if (t_cif->rtype->type == FFI_TYPE_STRUCT &&
+            t_rsize > 64)
+            t_objc_msgSend = (void(*)())objc_msgSend_stret;
+        else
+            t_objc_msgSend = (void(*)())objc_msgSend;
 #elif defined(__ARM__)
-        if (t_cif->rtype->type == FFI_TYPE_STRUCT)
+        if (t_cif->rtype->type == FFI_TYPE_STRUCT &&
+            t_rsize > 4)
             t_objc_msgSend = (void(*)())objc_msgSend_stret;
         else
             t_objc_msgSend = (void(*)())objc_msgSend;
 #elif defined(__X86_64__)
-        if (t_cif->rtype->type == FFI_TYPE_STRUCT)
+        if (t_cif->rtype->type == FFI_TYPE_STRUCT &&
+            t_rsize > 16)
             t_objc_msgSend = (void(*)())objc_msgSend_stret;
         else if (t_cif->rtype->type == FFI_TYPE_LONGDOUBLE)
             t_objc_msgSend = (void(*)())objc_msgSend_fpret;
         else
             t_objc_msgSend = (void(*)())objc_msgSend;
 #elif defined(__I386__)
-        if (t_cif->rtype->type == FFI_TYPE_STRUCT)
+        if (t_cif->rtype->type == FFI_TYPE_STRUCT &&
+            (t_rsize >= 8 ||
+             (t_rsize & (t_rsize - 1)) == 0))
             t_objc_msgSend = (void(*)())objc_msgSend_stret;
         else if (t_cif->rtype->type == FFI_TYPE_FLOAT ||
                  t_cif->rtype->type == FFI_TYPE_DOUBLE ||
diff --git a/tests/lcb/vm/interop-objc.lcb b/tests/lcb/vm/interop-objc.lcb
@@ -121,4 +121,31 @@ end handler
 
 ---------
 
+// NSRange is a pair of (natural sized) unsigned integers
+public foreign type NSRange binds to "MCAggregateTypeInfo:pp"
+
+// NSRange is 8 bytes on 64-bit arch, 4 bytes on 32-bit
+// so will use objc_msgSend_stret on 32-bit arch, but objc_msgSend on
+// 64 bit.
+foreign handler RangeOfString(in pHaystack as ObjcId, in pNeedle as ObjcId) returns NSRange binds to "objc:NSString.-rangeOfString:"
+
+public handler TestReturnNSRange()
+	if not the operating system is in ["mac", "ios"] then
+		skip test "return NSRange struct succeeds" because "not implemented on" && the operating system
+		return
+	end if
+	
+	variable tRange as NSRange
+	unsafe
+		put RangeOfString(StringToNSString("cartoon"), \
+						  StringToNSString("art")) into tRange
+	end unsafe
+	
+	variable tRangeList as List
+	put tRange into tRangeList
+	
+	test "return NSRange struct succeeds" when \
+		tRangeList is [1,3]
+end handler
+
 end module

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+# Fix calls to struct-returning obj-c methods`