Improved Apple Silicon support (variadic functions) + add tests on apple-silicon-m1 (self hosted runner) (#85)

* Add tests on apple-silicon-m1 (Self hosted runner)

* Remove (unnecessary?) dylib and fix Makefile to build them as universal2

* Fixes variadic function call on ARM64

* IMP*(void*, SEL, ...) is failing on Apple Silicon but is not our fault

* ffi_prep_cif_var should be guarded via __builtin_available(macOS 10.15, *)

* On iOS we're using a non-apple version of ffi that should support ffi_prep_cif_var

Author: misl6

.ci/osx_ci.sh

@@ -0,0 +1,13 @@
+#!/bin/bash
+set -e -x
+
+arm64_set_path_and_python_version(){
+  python_version="$1"
+  if [[ $(/usr/bin/arch) = arm64 ]]; then
+      export PATH=/opt/homebrew/bin:$PATH
+      eval "$(pyenv init --path)"
+      pyenv install $python_version -s
+      pyenv global $python_version
+      export PATH=$(pyenv prefix)/bin:$PATH
+  fi
+}

.github/workflows/python-package.yml

@@ -5,25 +5,48 @@ on: [push, pull_request]
 jobs:
   build:
 
-    runs-on: macOs-latest
+    name: "build (${{ matrix.runs_on }}, ${{ matrix.python }})"
+    defaults:
+      run:
+        shell: ${{ matrix.run_wrapper || 'bash --noprofile --norc -eo pipefail {0}' }}
+    runs-on: ${{ matrix.runs_on || 'macos-latest' }}
     strategy:
       matrix:
-        python-version: ["3.6", "3.7", "3.8", "3.9", "3.10"]
-
+        include:
+          - runs_on: macos-latest
+            python: "3.7"
+          - runs_on: macos-latest
+            python: "3.8"
+          - runs_on: macos-latest
+            python: "3.9"
+          - runs_on: macos-latest
+            python: "3.10"
+          - runs_on: apple-silicon-m1
+            run_wrapper: arch -arm64 bash --noprofile --norc -eo pipefail {0}
+            python: "3.9.11"
+          - runs_on: apple-silicon-m1
+            run_wrapper: arch -arm64 bash --noprofile --norc -eo pipefail {0}
+            python: "3.10.3"
     steps:
     - uses: actions/checkout@v2
-    - name: Set up Python ${{ matrix.python-version }}
+    - name: Set up Python ${{ matrix.python }}
+      # Needs to be skipped on our self-hosted runners tagged as 'apple-silicon-m1'
+      if: ${{ matrix.runs_on  != 'apple-silicon-m1' }}
       uses: actions/setup-python@v2
       with:
-        python-version: ${{ matrix.python-version }}
+        python-version: ${{ matrix.python }}
 
     - name: Install project
       run: |
+        source .ci/osx_ci.sh
+        arm64_set_path_and_python_version ${{ matrix.python }}
         pip install cython pytest
         pip install .
 
     - name: Test with pytest
       run: |
+        source .ci/osx_ci.sh
+        arm64_set_path_and_python_version ${{ matrix.python }}
         make test_lib
         make
         make tests

Makefile

@@ -4,8 +4,9 @@ build_ext:
 	env CFLAGS="-O0" python setup.py build_ext --inplace -g
 
 test_lib:
-	rm -rf objc_classes/test/usrlib.dylib
-	clang objc_classes/test/testlib.m -o objc_classes/test/testlib.dylib -dynamiclib -framework Foundation
+	rm -rf objc_classes/test/testlib.dylib objc_classes/test/CArrayTestlib.dylib
+	clang objc_classes/test/testlib.m -o objc_classes/test/testlib.dylib -dynamiclib -framework Foundation -arch arm64 -arch x86_64
+	clang objc_classes/test/CArrayTestlib.m -o objc_classes/test/CArrayTestlib.dylib -dynamiclib -framework Foundation -arch arm64 -arch x86_64
 
 tests: build_ext
 	cd tests && env PYTHONPATH=..:$(PYTHONPATH) python -m pytest -v

objc_classes/test/CArrayTestlib.dylib

@@ -146,10 +146,22 @@ - (IMP) getImp {
     return [self methodForSelector:@selector(getSumOf:and:)];
 }
 
-- (int) useImp:(IMP*(void*, SEL, ...))imp withA:(int)a andB:(int)b {
+/*
+    Variadic Functions are managed differently on ARM64!
+
+    IMP*(void*, SEL, ...) is failing on Apple Silicon, but not due to pyobjus.
+    - getandUseImpWithDefaultValues is here to demonstrate is not pyobjus fault,
+    in fact, directly calling getandUseImpWithDefaultValues with `IMP*(void*, SEL, ...)`
+    instead of `IMP*(void*, SEL, int, int)` will lead to unexpected results.
+*/
+- (int) useImp:(IMP*(void*, SEL, int, int))imp withA:(int)a andB:(int)b {
     return (int)imp(self, @selector(getSumOf:and:), a, b);
 }
 
+- (int) getandUseImpWithDefaultValues {
+    return (int)[self useImp: [self getImp] withA: 7 andB: 5];
+}
+
 /******************** </UNKNOWN TYPE TESTS> ***********************/
 
 /******************** <IVARS TESTS> ***********************/

objc_classes/test/testlib.dylib

@@ -1,5 +1,6 @@
 #include <objc/runtime.h>
 #include <objc/message.h>
+#include <ffi/ffi.h>
 #include <stdio.h>
 #include <dlfcn.h>
 #include <string.h>
@@ -52,4 +53,21 @@ id objc_msgSend_custom(id obj, SEL sel){
   }
 
   bool MACOS_HAVE_OBJMSGSEND_STRET = false;
-#endif
+#endif
+
+  ffi_status guarded_ffi_prep_cif_var(ffi_cif *_Nonnull cif, ffi_abi abi, unsigned int nfixedargs,
+                                      unsigned int ntotalargs, ffi_type *_Nonnull rtype, ffi_type *_Nonnull *_Nonnull atypes)
+  {
+    if (ntotalargs > nfixedargs)
+    {
+      #if TARGET_OS_OSX
+      if (__builtin_available(macOS 10.15, *))
+      {
+        return ffi_prep_cif_var(cif, abi, nfixedargs, ntotalargs, rtype, atypes);
+      }
+      #elif TARGET_OS_IOS
+        return ffi_prep_cif_var(cif, abi, nfixedargs, ntotalargs, rtype, atypes);
+      #endif
+    }
+    return ffi_prep_cif(cif, abi, ntotalargs, rtype, atypes);
+  }

objc_classes/test/testlib.m

@@ -148,9 +148,6 @@ cdef extern from "ffi/ffi.h":
     cdef ffi_type ffi_type_longdouble
     cdef ffi_type ffi_type_pointer
 
-    cdef ffi_status  ffi_prep_cif(ffi_cif *cif, ffi_abi abi,
-                        unsigned int nargs,ffi_type *rtype, ffi_type **atypes)
-
     cdef void        ffi_call(ffi_cif *cif, void (*fn)(), void *rvalue,
                         void **avalue)
 
@@ -161,3 +158,6 @@ cdef extern from "_runtime.h":
     id    objc_msgSend_custom(id obj, SEL sel)
     void  objc_msgSend_stret__safe(id self, SEL selector, ...)
     bool  MACOS_HAVE_OBJMSGSEND_STRET
+    ffi_status guarded_ffi_prep_cif_var(ffi_cif *cif, ffi_abi abi,
+                                    unsigned int nfixedargs, unsigned int ntotalargs,
+                                    ffi_type *rtype, ffi_type **atypes)

pyobjus/_runtime.h

@@ -294,6 +294,7 @@ cdef class ObjcMethod(object):
 
         # casting is needed here because otherwise we will get warning at compile
         cdef unsigned int num_args = <unsigned int>len(signature_args)
+        cdef unsigned int num_fixed_args = <unsigned int>len(self.signature_args)
         cdef unsigned int size = sizeof(ffi_type) * num_args
 
         # allocate memory to hold ffi_type* of arguments
@@ -317,8 +318,17 @@ cdef class ObjcMethod(object):
 
         # FFI PREP
         cdef ffi_status f_status
-        f_status = ffi_prep_cif(&self.f_cif, FFI_DEFAULT_ABI,
-                num_args, self.f_result_type, self.f_arg_types)
+
+        f_status = guarded_ffi_prep_cif_var(
+            &self.f_cif,
+            FFI_DEFAULT_ABI,
+            num_fixed_args,
+            num_args,
+            self.f_result_type,
+            self.f_arg_types,
+        )
+
+
         if f_status != FFI_OK:
             raise ObjcException(
                     'Unable to prepare the method {0!r}'.format(self.name))
@@ -348,7 +358,8 @@ cdef class ObjcMethod(object):
         cdef size_t result_size = <size_t>int(self.signature_return[1])
 
         # check that we have at least the same number of arguments as the
-        # signature want.
+        # signature want (having more than expected could signify that the called
+        # function is variadic).
         if len(args) < len(self.signature_args) - 2:
             raise ObjcException('Not enough parameters for {}'.format(
                 self.name))