Bug 2313598
| Summary: | closures with fork() don't work on ppc64le / s390x | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Vít Ondruch <vondruch> |
| Component: | libffi | Assignee: | Carlos O'Donell <codonell> |
| Status: | NEW --- | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | unspecified | ||
| Version: | 42 | CC: | bergner, codonell, dj, fweimer, green, lkundrak, mcermak, uweigand |
| Target Milestone: | --- | ||
| Target Release: | --- | ||
| Hardware: | Unspecified | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | libffi-3.4.7-2.fc43 | Doc Type: | If docs needed, set a value |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | Type: | --- | |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
| Attachments: | |||
|
Description
Vít Ondruch
2024-09-19 17:48:13 UTC
If you're on a platform that uses shared memory (anonymous or file-backed) for its closures, note that fork() does *not* unshare that memory. Freeing a closure in one thread frees it for all threads! As each platform may have a different way of storing closures, this problem may not exist on all platforms. (In reply to DJ Delorie from comment #1) > If you're on a platform that uses shared memory (anonymous or file-backed) > for its closures, note that fork() does *not* unshare that memory. Freeing > a closure in one thread frees it for all threads! > As each platform may have a different way of storing closures, this problem > may not exist on all platforms. Do I understand correctly that this is feature not a bug and there is no way to improve the situation? Actually, looking at [1, 2], it seems that static trampolines were not implemented for ppc64le / s390x, so that is likely part of the issue. [1]: https://github.com/libffi/libffi/pull/624 [2]: https://github.com/libffi/libffi/blob/084f36903f56b280283f3f4473a80b8e77727a29/configure.ac#L375-L390 (In reply to Vít Ondruch from comment #2) > Actually, looking at [1, 2], it seems that static trampolines were not > implemented for ppc64le / s390x, so that is likely part of the issue. I'm going to raise this with our IBM colleagues to see what we can do to improve the implementation. (In reply to Carlos O'Donell from comment #3) Thx a lot 👍 The IBM teams fixed this by implementing static tramplines for s390x: https://github.com/libffi/libffi/commit/458b2ae2829f1916ea3a3e07c944b4668732290f So once we include this it will fix the issue for s390x. I'll have to check back on the ppc64le status. This bug appears to have been reported against 'rawhide' during the Fedora Linux 42 development cycle. Changing version to 42. Created attachment 2079176 [details]
Patch to add static trampoline support to powerpc*-linux
Here's a WIP patch that adds static trampoline support for powerpc-linux, powerpc64-linux and powerpc64le-linux. It passes the libffi testsuite with no errors. Can someone please give this a try on the Ruby issue reported in this bugzilla?
It that comes back clean, I'll work on getting it upstream.
This patch seems to be missing src/powerpc/internal.h Created attachment 2079219 [details]
Updated patch to add static trampoline support to powerpc*-linux, with internal.h
Sorry about that. Here's an updated patch which does include internal.h as well as a few other cleanups.
bergner@begna:~$ diffstat libffi-ppc64.v5.diff
Makefile.am | 3 ++-
configure.ac | 3 ++-
src/powerpc/ffi.c | 13 +++++++++++++
src/powerpc/ffi_linux64.c | 45 ++++++++++++++++++++++++++-------------------
src/powerpc/ffi_sysv.c | 42 ++++++++++++++++++++++++++----------------
src/powerpc/internal.h | 6 ++++++
src/powerpc/linux64_closure.S | 39 +++++++++++++++++++++++++++++++++++++++
src/powerpc/ppc_closure.S | 24 ++++++++++++++++++++++++
8 files changed, 138 insertions(+), 37 deletions(-)
(In reply to Peter Bergner from comment #8) > Can someone please give this a try on the Ruby issue reported in > this bugzilla? To test rubygem-ffi, it should be enough to remove this line from .spec file: https://src.fedoraproject.org/rpms/rubygem-ffi/blob/c3e875b6594ace69c2ce4798748d4c6dbe75e666/f/rubygem-ffi.spec#_49 DJ has a Rawhide build up with the changes: https://bodhi.fedoraproject.org/updates/FEDORA-2025-c66c3c672e Tried a scratch build as suggested, still fails: build/BUILD/rubygem-ffi-1.17.0-build/ffi-1.17.0/usr/share/gems/gems/ffi-1.17.0/spec/ffi/async_callback_spec.rb:57: warning: Ractor is experimental, and the behavior may change in future versions of Ruby! Also there are many implementation issues. ......................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................../var/tmp/rpm-tmp.LKXb6s: line 55: 3424 Aborted (core dumped) RUBYOPT="-I$(dirs +1)/usr/lib64/gems/ruby/ffi-1.17.0" rspec spec https://koji.fedoraproject.org/koji/taskinfo?taskID=129947312 According root.log, the new build was used: DEBUG util.py:461: libffi ppc64le 3.4.7-2.fc43 build 90.3 KiB Looks like configure.ac was patched, but configure was not regenerated. The old configure doesn't enable static tramapolines for powerpc64-linux-gnu. (In reply to Florian Weimer from comment #14) > Looks like configure.ac was patched, but configure was not regenerated. The > old configure doesn't enable static tramapolines for powerpc64-linux-gnu. So I created the patch against current upstream libffi sources and that doesn't have a configure script. Rather it has you run autogen.sh which creates it for you. (In reply to Florian Weimer from comment #14) > Looks like configure.ac was patched, but configure was not regenerated. The > old configure doesn't enable static tramapolines for powerpc64-linux-gnu. Can someone regen that configure or just apply the configure.ac changes to configure and rerun the test? If it passes, I'll work on pushing the changes to upstream libffi. My bad, please try https://koji.fedoraproject.org/koji/taskinfo?taskID=130825673 If that works I'll push it officially. Created attachment 2082475 [details] Final patch to add static trampoline support to powerpc*-linux, with internal.h (In reply to DJ Delorie from comment #17) > My bad, please try https://koji.fedoraproject.org/koji/taskinfo?taskID=130825673 > > If that works I'll push it officially. I don't really have the ability to run that ruby test case against the updated libffi, so hopefully one of you can do that for us? That said, I've updated the patch again and attached it here. This is the "final" version of the patch. It adds pcrel support in the case you're compiling for power10 and it cleans up the newly added asm code to use relocations against the offsets, rather than just using the offsets as is, in the event someone changes them and they need real relocation handling. I did another scratch build with the "final" patch: https://koji.fedoraproject.org/koji/taskinfo?taskID=130860329 (In reply to DJ Delorie from comment #19) > I did another scratch build with the "final" patch: > https://koji.fedoraproject.org/koji/taskinfo?taskID=130860329 We need a non-scratch build before we can run a rubygem-ffi scratch build. (In reply to Peter Bergner from comment #18) > That said, I've updated the patch again and attached it here. This is the > "final" version of the patch. It adds pcrel support in the case you're > compiling for power10 and it cleans up the newly added asm code to use > relocations against the offsets, rather than just using the offsets as is, > in the event someone changes them and they need real relocation handling. I'll note that I built and regtested libffi using the patch on powerpc64le-linux using --with-gcc-arch= from power8 thru power10 and on powerpc64-linux and powrepc-linux from power4 thru power10 with no regressions. libffi-3.4.7-3 is in rawhide and has the latest patch. Is there any status on doing a rubygem-ffi scratch build with the updated libffi-3.4.7-3 in rawhide? Sorry, commented on the wrong (bug 2344471 comment 5): A scratch build of rubygem-ffi succeeded: https://koji.fedoraproject.org/koji/taskinfo?taskID=130949254 I even removed the compatibility patch. (In reply to Florian Weimer from comment #24) > Sorry, commented on the wrong (bug 2344471 comment 5): > > A scratch build of rubygem-ffi succeeded: > https://koji.fedoraproject.org/koji/taskinfo?taskID=130949254 > > I even removed the compatibility patch. Excellent, thanks for confirming it fixes the rubygem-ffi issue! DJ, did you say you could push the patch to upstream libffi? If not, I can work on doing that. (In reply to Peter Bergner from comment #25) > DJ, did you say you could push the patch to upstream libffi? > If not, I can work on doing that. I talked with DJ offline and his "push officially" comment above meant pushing into Fedora officially. I'll work on pushing the patch into the upstream libffi sources. The patch was just pulled into the upstream libffi sources. |