Upstream arm64e ptrauth changes from Apple's libffi #565

jeremyhu · 2020-05-29T18:27:21Z

This reverts a recent incomplete change for arm64e support (#548) and includes all potentially upstreamable changes to libffi made in macOS 10.15.5 and iOS 13.4, including Apple's arm64e support.

Note: Please do not merge this yet. It is here for comments first as I also want to hear from @oleavr and others from #548 .

I have not tested these changes against the current master, I have only tested them with our libffi-26 tag based on c2a6859

Note that I have omitted our Xcode project changes, but if there is interest in upstreaming those as well, please let me know.

oleavr

Yay, love it! 😍 My PR obviously took the lazy route, so I'm super-excited to see a proper implementation! 👏

src/aarch64/ffi.c

src/aarch64/internal.h

src/aarch64/sysv.S

oleavr · 2020-05-29T19:12:21Z

include/ffi.h.in

@@ -357,6 +357,11 @@ ffi_prep_closure_loc (ffi_closure*,
 		      void *user_data,
 		      void*codeloc);

+#if defined(__x86_64__) || defined(__arm64__)


Would it make sense to make this available on all architectures?

If this seems valuable outside of our internal use case, then I certainly can. One of the main reasons I'm putting all this up is to see if any of this work is valuable to the larger OSS community or should remain Apple-internal changes for our specific cases.

We only wrote the implementation in src/aarch64/ffi.c and arc/x86/ffi64.c since that is all we use, but it should be usable elsewhere. This was necessitated by the trampoline changes, and I think we ended up using it in python3 and a few other places where there were assumptions that codeloc == closure.

Ahh I see. That's a good question for the maintainers. /cc @atgreen @tromey

Leave it for now. We may come back to this as we need to do something for the Linux architectures.

Ok. I'll break that out into a separate PR that adds the API to all archs.

@atgreen What are your opinions on the API naming? Is ffi_find_closure_for_code() ok, or would you prefer something different?

oleavr · 2020-05-29T19:16:11Z

src/aarch64/trampoline.S

+    .endr
+
+#endif /* FFI_EXEC_TRAMPOLINE_TABLE */
+#endif /* __arm64 */


Build system changes seem to be missing for this file.

Yeah, the build system changes were specific to our Xcode project. I'm not looking forward to figuring that out for autotools.

Ah no worries, I can take a stab at it and post a suggestion here.

Just pushed a rough draft here.

Here's how I tested it:

1. Generated a build environment

$ git clone https://github.com/frida/frida.git $ cd frida $ make build/frida-env-ios-arm64e.rc

Piggybacking on the autotools legacy build environment in Frida, which we still use for building a few dependencies that haven't yet moved to Meson.

2. Bootstrapped and built for iOS/arm64e

$ . build/frida-env-ios-arm64e.rc $ cd ~/src/libffi $ ./autogen.sh $ ./configure --prefix=/usr --enable-shared $ make $ make install DESTDIR="$(echo ~/Desktop/libffi-ios-arm64e)"

--enable-shared is only needed because Frida's $CONFIG_SITE defaults to enable_shared=no

3. Built for macOS as a sanity check

$ . ~/src/frida/build/frida-env-macos-x86_64.rc $ ./configure --prefix=/usr --enable-shared $ make $ make install DESTDIR="$(echo ~/Desktop/libffi-macos-x86_64)"

Cool, thanks for the help. FWIW, I am crazy busy with WWDC work and taking care of kids at home, but I plan to circle back to this after WWDC, break out some of the smaller less offensive pieces into individual PRs, and then polish up the rest.

oleavr · 2020-05-29T19:24:37Z

src/closures.c

+  static void *ffi_closure_trampoline_table_page;
+
+  dispatch_once(&trampoline_template_init_once, ^{
+    void * const trampoline_handle = dlopen("/usr/lib/libffi-trampolines.dylib", RTLD_NOW | RTLD_LOCAL | RTLD_FIRST);


This should take libdir into account instead of hardcoding it.

Yep, good point.

oleavr · 2020-05-29T19:26:04Z

src/closures.c

+    void * const trampoline_handle = dlopen("/usr/lib/libffi-trampolines.dylib", RTLD_NOW | RTLD_LOCAL | RTLD_FIRST);
+    assert(trampoline_handle);
+
+    ffi_closure_trampoline_table_page = dlsym(trampoline_handle, "ffi_closure_trampoline_table_page");


Suggested change

ffi_closure_trampoline_table_page = dlsym(trampoline_handle, "ffi_closure_trampoline_table_page");

ffi_closure_trampoline_table_page = dlsym (trampoline_handle, "ffi_closure_trampoline_table_page");

Here and elsewhere. It looks like the coding style has eroded somewhat since the early days, but this seems to be the most prevalent style.

oleavr · 2020-05-29T19:53:50Z

src/closures.c

-extern void *ffi_closure_trampoline_table_page;
+#if __has_feature(ptrauth_calls)
+#include <ptrauth.h>
+#define sign_ptr(p) ptrauth_sign_unauthenticated(p, ptrauth_key_function_pointer, 0)


This appears to be an unused macro.

jeremyhu · 2020-05-29T18:33:20Z

src/aarch64/ffi.c

@@ -31,6 +31,10 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.  */
 #include <windows.h> /* FlushInstructionCache */
 #endif

+#if __has_feature(ptrauth_calls)


We're just using __has_feature() unconditionally. Before requesting merge, I will change this to use HAVE_PTRAUTH.

Thanks. I just noticed this caused a build failure here: https://travis-ci.org/github/libffi/libffi/jobs/692670895#L652

Cool! I love CI that works =) I've updated the title to make it more clear that I'm soliciting feedback on what you think is interesting to upstream rather than merge this in wholesale. Parts that you find interesting, I'll definitely work to get integrated with autotools builds and cross-platform (as best I can at least) and spin off separate PRs for isolated changes.

I put a lot of effort into getting the CI to work for lots of platforms, even implementing a new tool (https://rl.gl , you can see the libffi policy @ https://github.com/libffi/rlgl-policy/blob/master/XFAIL ), HOWEVER one big gap is around iOS testing. I've never really participated in the iOS ecosystem. If there was some way to run the testsuite on travis using a simulator, that would be wonderful. Do you know if this is possible?

jeremyhu · 2020-05-29T18:34:12Z

configure.ac

@@ -176,28 +176,6 @@ case "$TARGET" in
    ;;
 esac

-AC_CACHE_CHECK([whether compiler supports pointer authentication],


This will be resurrected to allow better portability. I just wanted to quickly get Apple's changes into a PR for discussion and reverted the conflicting change wholesale.

atgreen

Thank you, @jeremyhu and @oleavr for your work. It would be great to get these changes upstream once the final bits have been cleaned up (including the build machinery).

atgreen · 2020-06-11T12:22:49Z

src/prep_cif.c

@@ -234,7 +234,7 @@ ffi_status ffi_prep_cif_var(ffi_cif *cif,
  return ffi_prep_cif_core(cif, abi, 1, nfixedargs, ntotalargs, rtype, atypes);
 }

-#if FFI_CLOSURES
+#if FFI_CLOSURES && FFI_LEGACY_CLOSURE_API


What is FFI_LEGACY_CLOSURE_API?

In our build, it's defined in our ffitarget.hs as:

darwin/include/ffitarget_arm64.h
54:#define FFI_LEGACY_CLOSURE_API 0

darwin/include/ffitarget_armv7.h
68:#define FFI_LEGACY_CLOSURE_API 0

darwin/include/ffitarget_x86.h
125:#define FFI_LEGACY_CLOSURE_API 1

We just didn't want to make ffi_prep_closure() available on arm since it won't work there... better to catch issues at build time than at runtime. We should omit these bits from the final PR.

atgreen · 2020-06-11T12:34:40Z

include/ffi.h.in

@@ -357,6 +357,11 @@ ffi_prep_closure_loc (ffi_closure*,
 		      void *user_data,
 		      void*codeloc);

+#if defined(__x86_64__) || defined(__arm64__)


Leave it for now. We may come back to this as we need to do something for the Linux architectures.

jeremyhu · 2020-09-02T04:57:38Z

Sorry, time's been a bit more scarce for me recently. I haven't had a chance to circle back to this. I'm hoping to have some time this month to at least start forking off some of these changes into their own PRs and then look into the autoconf build system work.

jeremyhu · 2020-10-01T21:44:16Z

#586 and #587 filed for some of the simpler changes made in Apple's libffi. I'm hoping to break this out into other incremental PRs over the coming week or two. Sorry this took longer to get back to than expected.

hjelmn · 2021-01-04T04:53:15Z

@jeremyhu I can help finish the autotools changes if you like.

hjelmn · 2021-01-04T05:52:58Z

Accidentally posted one extra comment (now deleted). Ran into the build system integration issue while trying to get libffi to work within ghc 8.10.3 on an Apple Silicon Mac.

The PR adds a patch that brings in improvements from Apple's libffi (specifically libffi-26). These improvements are needed to use libffi with ghc. Commits: libffi/libffi@4c7bde3 libffi/libffi#565 I added additional configure logic to ensure that FFI_TRAMPOLINE_WHOLE_DYLIB is set on Apple Silicon (it is hard-coded to 1 in libffi-26 for aarch64). Signed-off-by: Nathan Hjelm <hjelmn@cs.unm.edu>

jeremyhu · 2021-01-19T19:46:08Z

I split off some more preprocessing changes in #620, and I removed the whole-dylib trampoline changes such that this can focus on the arm64e changes. I'll file a subsequent pull request with the FFI_TRAMPOLINE_WHOLE_DYLIB changes.

jeremyhu · 2021-01-19T19:52:06Z

@jeremyhu I can help finish the autotools changes if you like.

That would be greatly appreciated. I've cleaned up the patches a bit. This one and #620 are ready to merge. The remaining piece is the trampoline changes. I've split that out into #621. Please feel free to jump on that. I hope I'll have time in the next few weeks to look at this again, but not sure.

…i port NOTES: This changes the ptrauth support from libffi#548 to match what Apple is shipping in its libffi-27 tag. Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>

jeremyhu · 2021-03-12T20:48:20Z

Rebased to address merge conflicts with #624.

@atgreen Could we get this merged to avoid future merge conflicts? Thanks. The pieces that were not ready were split out into #621.

Currently, upstream libffi doesn't support Apple Silicon. However, there are several PRs from Apple employees: * libffi/libffi#565 * libffi/libffi#621 Homebrew formulae for libffi contains patch that is based on these pull requests. We use this patch here to build our own version of libffi.

jeremyhu changed the title ~~Merge apple arm64e ptrauth changes~~ Merge apple arm64e ptrauth changes (and other misc fixes from Apple's libffi-26) May 29, 2020

jeremyhu mentioned this pull request May 29, 2020

Port to iOS/arm64e #548

Merged

oleavr reviewed May 29, 2020

View reviewed changes

jeremyhu commented May 29, 2020

View reviewed changes

atgreen reviewed Jun 11, 2020

View reviewed changes

jeremyhu changed the title ~~Merge apple arm64e ptrauth changes (and other misc fixes from Apple's libffi-26)~~ [RFC] Upstreaming apple arm64e ptrauth changes (and other misc fixes from Apple's libffi-26) Jun 12, 2020

ekurutepe mentioned this pull request Jun 30, 2020

pod install on DTK crashes in ethon CocoaPods/CocoaPods#9890

Closed

1 task

mistydemeo mentioned this pull request Jul 16, 2020

libffi: fix build on Apple Silicon Homebrew/homebrew-core#58092

Merged

5 tasks

claui mentioned this pull request Jul 19, 2020

macOS 11 Big Sur compatibility on Apple Silicon Homebrew/brew#7857

Closed

9 tasks

kou mentioned this pull request Sep 9, 2020

Build error with --enable-bundled-libffi on Apple DTK(arm64) ruby/fiddle#46

Closed

jeremyhu force-pushed the apple-arm64e branch from ba480d9 to 11ed359 Compare October 1, 2020 21:37

jeremyhu force-pushed the apple-arm64e branch 2 times, most recently from 61179c3 to 6aa41be Compare October 1, 2020 23:01

hjelmn mentioned this pull request Jan 5, 2021

ghc: add support for Apple Silicon Homebrew/homebrew-core#65997

Closed

5 tasks

hjelmn mentioned this pull request Jan 5, 2021

Add a patch to improve libffi support for Apple Silicon Homebrew/formula-patches#340

Merged

jeremyhu force-pushed the apple-arm64e branch from 6aa41be to e066ba1 Compare January 19, 2021 19:44

jeremyhu force-pushed the apple-arm64e branch from e066ba1 to 7bd776a Compare January 19, 2021 19:48

jeremyhu mentioned this pull request Jan 19, 2021

[RFC WIP] Apple whole dylib trampoline #621

Open

jeremyhu force-pushed the apple-arm64e branch 2 times, most recently from 0ad45a7 to 81d487a Compare January 19, 2021 20:43

jeremyhu changed the title ~~[RFC] Upstreaming apple arm64e ptrauth changes (and other misc fixes from Apple's libffi-26)~~ Upstream arm64e ptrauth changes from Apple's libffi Mar 12, 2021

arm64e: Pull in pointer authentication code from Apple's arm64e libff…

83502ff

…i port NOTES: This changes the ptrauth support from libffi#548 to match what Apple is shipping in its libffi-27 tag. Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>

jeremyhu force-pushed the apple-arm64e branch from 81d487a to 83502ff Compare March 12, 2021 20:45

jeremyhu mentioned this pull request Mar 22, 2021

Add some missing #if conditionals from Apple's code drop #620

Merged

atgreen merged commit eafab23 into libffi:master Mar 24, 2021

fanninpm mentioned this pull request Oct 22, 2021

Outdated version of vendored libffi causes build error on macOS Big Sur with M1 processor tov/libffi-rs#30

Closed

This was referenced May 30, 2022

Trampoline creation fails on iOS devices #717

Closed

"ObjC Class 'b'WrappedPyObject'' couldn't be found." when running on iOS device beeware/rubicon-objc#226

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upstream arm64e ptrauth changes from Apple's libffi #565

Upstream arm64e ptrauth changes from Apple's libffi #565

jeremyhu commented May 29, 2020 •

edited

Loading

oleavr left a comment •

edited

Loading

oleavr May 29, 2020

jeremyhu May 29, 2020

oleavr May 29, 2020

atgreen Jun 11, 2020

jeremyhu Jun 12, 2020

oleavr May 29, 2020

jeremyhu May 29, 2020

oleavr May 30, 2020

oleavr May 30, 2020 •

edited

Loading

jeremyhu Jun 4, 2020

oleavr May 29, 2020

jeremyhu May 29, 2020

oleavr May 29, 2020

oleavr May 29, 2020 •

edited

Loading

jeremyhu May 29, 2020

atgreen Jun 11, 2020

jeremyhu Jun 12, 2020

atgreen Jun 12, 2020

jeremyhu May 29, 2020

atgreen left a comment

atgreen Jun 11, 2020

jeremyhu Jun 12, 2020

atgreen Jun 11, 2020

jeremyhu commented Sep 2, 2020

jeremyhu commented Oct 1, 2020

hjelmn commented Jan 4, 2021

hjelmn commented Jan 4, 2021 •

edited

Loading

jeremyhu commented Jan 19, 2021

jeremyhu commented Jan 19, 2021

jeremyhu commented Mar 12, 2021

	ffi_closure_trampoline_table_page = dlsym(trampoline_handle, "ffi_closure_trampoline_table_page");
	ffi_closure_trampoline_table_page = dlsym (trampoline_handle, "ffi_closure_trampoline_table_page");

Upstream arm64e ptrauth changes from Apple's libffi #565

Upstream arm64e ptrauth changes from Apple's libffi #565

Conversation

jeremyhu commented May 29, 2020 • edited Loading

oleavr left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oleavr May 30, 2020 • edited Loading

Choose a reason for hiding this comment

1. Generated a build environment

2. Bootstrapped and built for iOS/arm64e

3. Built for macOS as a sanity check

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oleavr May 29, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

atgreen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeremyhu commented Sep 2, 2020

jeremyhu commented Oct 1, 2020

hjelmn commented Jan 4, 2021

hjelmn commented Jan 4, 2021 • edited Loading

jeremyhu commented Jan 19, 2021

jeremyhu commented Jan 19, 2021

jeremyhu commented Mar 12, 2021

jeremyhu commented May 29, 2020 •

edited

Loading

oleavr left a comment •

edited

Loading

oleavr May 30, 2020 •

edited

Loading

oleavr May 29, 2020 •

edited

Loading

hjelmn commented Jan 4, 2021 •

edited

Loading