Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

configure.ac: add --disable-exec-static-tramp flag #647

Merged
merged 1 commit into from Jun 27, 2021
Merged

configure.ac: add --disable-exec-static-tramp flag #647

merged 1 commit into from Jun 27, 2021

Conversation

trofi
Copy link
Contributor

@trofi trofi commented Jun 27, 2021

Some projects like GHC (Glasgow Haskell Compiler) and
gobject-introspection use ffi_closure_alloc() as a way
to allocate executable memory. exec static tramp
interferes with it (unclear how exactly yet).

GHC symptom: ffi closure freeing cimplains about unexpected
trampoline (GHC manually fills one):

$ ghci
GHCi, version 8.10.5: https://www.haskell.org/ghc/  :? for help
ghc: freeHaskellFunctionPtr: not for me, guv! 0x7f0417a1efe8
ghc: freeHaskellFunctionPtr: not for me, guv! 0x7f0417a1efc8

gobject-introspection symptom:

$ meld
Segmentation fault (core dumped)

$ gdb --args /usr/bin/python3.9 /usr/bin/meld
(gdb) run
...
Thread 1 "python3.9" received signal SIGSEGV, Segmentation fault.
0x00007fffe9ac1ae8 in g_callable_info_free_closure (
  callable_info=0x555555d45990, closure=0x7fffe9e70c20)
    at ../gobject-introspection-1.68.0/girepository/girffi.c:428
428       g_free (wrapper->ffi_closure.cif->arg_types);
(gdb) bt
  callable_info=0x555555d45990, closure=0x7fffe9e70c20)
    at ../gobject-introspection-1.68.0/girepository/girffi.c:428
  data=0x555555d252d0)
    at ../pygobject-3.40.1/gi/pygi-closure.c:635
...

To ease downstreams narrowing down the actual problem let's
provide a knob to disable exec static trampolines.

The change for not affect current default.

Some projects like GHC (Glasgow Haskell Compiler) and
gobject-introspection use `ffi_closure_alloc()` as a way
to allocate executable memory. exec static tramp
interferes with it (unclear how exactly yet).

GHC symptom: ffi closure freeing cimplains about unexpected
trampoline (GHC manually fills one):

```
$ ghci
GHCi, version 8.10.5: https://www.haskell.org/ghc/  :? for help
ghc: freeHaskellFunctionPtr: not for me, guv! 0x7f0417a1efe8
ghc: freeHaskellFunctionPtr: not for me, guv! 0x7f0417a1efc8
```

gobject-introspection symptom:

```
$ meld
Segmentation fault (core dumped)

$ gdb --args /usr/bin/python3.9 /usr/bin/meld
(gdb) run
...
Thread 1 "python3.9" received signal SIGSEGV, Segmentation fault.
0x00007fffe9ac1ae8 in g_callable_info_free_closure (
  callable_info=0x555555d45990, closure=0x7fffe9e70c20)
    at ../gobject-introspection-1.68.0/girepository/girffi.c:428
428       g_free (wrapper->ffi_closure.cif->arg_types);
(gdb) bt
  callable_info=0x555555d45990, closure=0x7fffe9e70c20)
    at ../gobject-introspection-1.68.0/girepository/girffi.c:428
  data=0x555555d252d0)
    at ../pygobject-3.40.1/gi/pygi-closure.c:635
...
```

To ease downstreams narrowing down the actual problem let's
provide a knob to disable exec static trampolines.

The change for not affect current default.

Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
@atgreen
Copy link
Member

atgreen commented Jun 27, 2021

This is disappointing, but I agree with the change. So my understanding is that we can do the 3.4 release and distros can initially build with --disable-exec-static-tramp. Once the problems are solved we can enable that feature because the ABI doesn't change. Does this sound right?

@atgreen atgreen merged commit 132699b into libffi:master Jun 27, 2021
@trofi
Copy link
Contributor Author

trofi commented Jun 27, 2021

Once the problems are solved we can enable that feature because the ABI doesn't change. Does this sound right?

Yes.

@atgreen
Copy link
Member

atgreen commented Jun 27, 2021

Related.. I just noticed that none of the test cases exercise ffi_closure_free()

@trofi
Copy link
Contributor Author

trofi commented Jun 27, 2021

Filed ghc bug as https://gitlab.haskell.org/ghc/ghc/-/issues/20051. Tl;DR of it: ghc relies on ffi_closure_alloc()'s data and code layout too much. ghc will have to adapt and libffi can't do much about it's strong assumptions.

Will look at gobject-interospection's failure mechanics.

@atgreen
Copy link
Member

atgreen commented Jun 27, 2021 via email

@trofi
Copy link
Contributor Author

trofi commented Jun 27, 2021

I think I found gobject-introspection failure as well. It used code pointer and closure returned by ffi_closure_alloc() interchangeably.

ffi_closure * g_callable_info_prepare_closure (...) {
    void * exec_ptr;
    ...
    closure = ffi_closure_alloc (..., &exec_ptr);
    status = ffi_prep_closure_loc (closure, cif, callback, user_data, exec_ptr);
    ...
    return exec_ptr; // whoops, should have been `closure`
}

void
g_callable_info_free_closure (..., ffi_closure    *closure)
{
    ffi_closure_free (closure);
}

Proposed an upstream fix as https://gitlab.gnome.org/GNOME/gobject-introspection/-/merge_requests/283

@justinkb
Copy link

@trofi tried your fix because gjs segfaulted with libffi 3.4.2. it fixed the segfault in gjs, but one happened one level higher in gnome-shell instead, backtrace ended in libffi itself, see the gnome gitlab

@trofi
Copy link
Contributor Author

trofi commented Jul 15, 2021

Perhaps you mean https://gitlab.gnome.org/GNOME/gjs/-/issues/428 report.

@justinkb
Copy link

justinkb commented Jul 15, 2021 via email

@trofi
Copy link
Contributor Author

trofi commented Jul 15, 2021

Aha, I reproduced the same crash locally. liferea also seems to exhibit the same crash around gjs callbacks from within ffi calls. I'll have a look.

My gut feeling is that https://gitlab.gnome.org/GNOME/gobject-introspection/-/merge_requests/283 exposes invalid assumption that ffi_closure* is really used interchangeably with the trampoline code in gjs.

@Mindavi Mindavi mentioned this pull request Aug 13, 2021
11 tasks
dicta added a commit to dicta/libffi-feedstock that referenced this pull request Sep 22, 2021
This new feature was introduced in libffi 3.4 and is known to break
some downstream packages, notably ghc and gobject-introspection.
Upstream also added a configure-time option in this release to
disable the feature until all downstream users can be fixed to be
compatible with it, at which point it can be safely re-enabled.

See also: libffi/libffi#647
Signed-off-by: Dan Bryant <dan@deepwavedigital.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants