aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorlaanwj <126646+laanwj@users.noreply.github.com>2022-04-14 09:13:55 +0200
committerlaanwj <126646+laanwj@users.noreply.github.com>2022-04-14 09:18:16 +0200
commit6c9bc14a3f2cfa50144607c820ebab5288f9571c (patch)
tree2e6ebffe7c64a045a2ee843b208dea6bb044fe3c
parent8e3c266a4f02093d57d563f32ba73d3ab4b5f208 (diff)
parent457148a803cee02897b7428fa7b3eb93eed71e4c (diff)
downloadbitcoin-6c9bc14a3f2cfa50144607c820ebab5288f9571c.tar.xz
Merge bitcoin/bitcoin#24842: guix: fix GCC 10.3.0 + mingw-w64 setjmp/longjmp issues
457148a803cee02897b7428fa7b3eb93eed71e4c guix: fix GCC 10.3.0 + mingw-w64 setjmp/longjmp issues (fanquake) Pull request description: This commit backports [a patch](https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=e8d1ca7d2c344a411779892616c423e157f4aea8) to the GCC 10.3.0 we build for Windows cross-compilation in Guix. The commit has been [backported to the GCC releases/gcc-10 branch](https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=e3abcc56d2604b9d2652b615ff9e68981cb7f79e), but hasn't yet made it into a 10.x release. The patch corrects a regression from an earlier GCC commit, see: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=357c4350680bf29f0c7a115424e3da11c53b5582 and https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=074226d5aa86cd3de517014acfe34c7f69a2ccc7, related to the way newer versions of mingw-w64 implement setjmp/longjmp. Ultimately this was causing a crash for us when Windows users were viewing the network traffic tab inside the GUI. After some period, long enough that a buffer would need reallocating, a call into FreeTypes [`gray_record_cell()`](https://github.com/ImageMagick/freetype/blob/a18906091cd17c623a6819661589df5566958918/src/smooth/ftgrays.c#L526) would result in a call to [`ft_longjmp` (longjmp)](https://github.com/ImageMagick/freetype/blob/a18906091cd17c623a6819661589df5566958918/src/smooth/ftgrays.c#L165), which would then trigger a crash. Fixes: https://github.com/bitcoin-core/gui/issues/582. See also: https://bugreports.qt.io/browse/QTBUG-93476 - very similar issue reported to Qt. Guix Build (on x86_64): ```bash 62172df3089e7bca3fd00f63acc9c8d3678a35bfb2bb5a0af905e61e9d8def52 guix-build-457148a803ce/output/dist-archive/bitcoin-457148a803ce.tar.gz f8318d16d0418e0e790efd94527a5be374ac50f51df53e05a6d54cc8c08a8633 guix-build-457148a803ce/output/x86_64-w64-mingw32/SHA256SUMS.part 72076e6896297a36beec6c62065b3d8aeeeb87fed407df947261cefdc81cdb93 guix-build-457148a803ce/output/x86_64-w64-mingw32/bitcoin-457148a803ce-win64-debug.zip c617d2347f50d2706bbdcc2b3b97f2ecaf59243747f4c81d7747a22e64cb9d76 guix-build-457148a803ce/output/x86_64-w64-mingw32/bitcoin-457148a803ce-win64-setup-unsigned.exe 8b1e7821e495121bea8a70f09ea6a0b703503b054d831b0dd86a0fe29cece457 guix-build-457148a803ce/output/x86_64-w64-mingw32/bitcoin-457148a803ce-win64-unsigned.tar.gz c8d2c0e68e3bf21ed7cfe08df64925bfa54ce6225c6d29bb710f9d9d4474caee guix-build-457148a803ce/output/x86_64-w64-mingw32/bitcoin-457148a803ce-win64.zip ``` ACKs for top commit: jonatack: Approach and review-only ACK 457148a803cee02897b7428fa7b3eb93eed71e4c laanwj: Concept and code review ACK 457148a803cee02897b7428fa7b3eb93eed71e4c gruve-p: ACK https://github.com/bitcoin/bitcoin/pull/24842/commits/457148a803cee02897b7428fa7b3eb93eed71e4c hebasto: ACK 457148a803cee02897b7428fa7b3eb93eed71e4c, tested `bitcoin-457148a803ce-win64.zip` on Windows 11 Pro 21H2. Confirming that bitcoin-core/gui#582 is fixed. jarolrod: ACK 457148a803cee02897b7428fa7b3eb93eed71e4c Tree-SHA512: dfb832ce93d72827009458cebbbdd408175b90b98d3eb546f9bbd21efe7bdd4ceca6ed13f5f6ce8e8e15d1c5d613f3a10399847a3589e4e7cc113ac0196d4010
-rw-r--r--contrib/guix/manifest.scm7
-rw-r--r--contrib/guix/patches/gcc-broken-longjmp.patch68
2 files changed, 72 insertions, 3 deletions
diff --git a/contrib/guix/manifest.scm b/contrib/guix/manifest.scm
index fcec592c2c..708d2e698d 100644
--- a/contrib/guix/manifest.scm
+++ b/contrib/guix/manifest.scm
@@ -162,9 +162,10 @@ desirable for building Bitcoin Core release binaries."
(define (make-gcc-with-pthreads gcc)
(package-with-extra-configure-variable gcc "--enable-threads" "posix"))
-(define (make-mingw-w64-cross-gcc-vmov-alignment cross-gcc)
+(define (make-mingw-w64-cross-gcc cross-gcc)
(package-with-extra-patches cross-gcc
- (search-our-patches "vmov-alignment.patch")))
+ (search-our-patches "vmov-alignment.patch"
+ "gcc-broken-longjmp.patch")))
(define (make-mingw-pthreads-cross-toolchain target)
"Create a cross-compilation toolchain package for TARGET"
@@ -172,7 +173,7 @@ desirable for building Bitcoin Core release binaries."
(pthreads-xlibc mingw-w64-x86_64-winpthreads)
(pthreads-xgcc (make-gcc-with-pthreads
(cross-gcc target
- #:xgcc (make-ssp-fixed-gcc (make-mingw-w64-cross-gcc-vmov-alignment base-gcc))
+ #:xgcc (make-ssp-fixed-gcc (make-mingw-w64-cross-gcc base-gcc))
#:xbinutils xbinutils
#:libc pthreads-xlibc))))
;; Define a meta-package that propagates the resulting XBINUTILS, XLIBC, and
diff --git a/contrib/guix/patches/gcc-broken-longjmp.patch b/contrib/guix/patches/gcc-broken-longjmp.patch
new file mode 100644
index 0000000000..1cfc0918b0
--- /dev/null
+++ b/contrib/guix/patches/gcc-broken-longjmp.patch
@@ -0,0 +1,68 @@
+commit eb5698897c52702498938592d7f76e67d126451f
+Author: Eric Botcazou <ebotcazou@adacore.com>
+Date: Wed May 5 22:48:51 2021 +0200
+
+ Fix PR target/100402
+
+ This is a regression for 64-bit Windows present from mainline down to the 9
+ branch and introduced by the fix for PR target/99234. Again SEH, but with
+ a twist related to the way MinGW implements setjmp/longjmp, which turns out
+ to be piggybacked on SEH with recent versions of MinGW, i.e. the longjmp
+ performs a bona-fide unwinding of the stack, because it calls RtlUnwindEx
+ with the second argument initially passed to setjmp, which is the result of
+ __builtin_frame_address (0) in the MinGW header file:
+
+ define setjmp(BUF) _setjmp((BUF), __builtin_frame_address (0))
+
+ This means that we directly expose the frame pointer to the SEH machinery
+ here (unlike with regular exception handling where we use an intermediate
+ CFA) and thus that we cannot do whatever we want with it. The old code
+ would leave it unaligned, i.e. not multiple of 16, whereas the new code
+ aligns it, but this breaks for some reason; at least it appears that a
+ .seh_setframe directive with 0 as second argument always works, so the
+ fix aligns it this way.
+
+ gcc/
+ PR target/100402
+ * config/i386/i386.c (ix86_compute_frame_layout): For a SEH target,
+ always return the establisher frame for __builtin_frame_address (0).
+ gcc/testsuite/
+ * gcc.c-torture/execute/20210505-1.c: New test.
+
+diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
+index 2f838840e96..06ad1b2274e 100644
+--- a/gcc/config/i386/i386.c
++++ b/gcc/config/i386/i386.c
+@@ -6356,12 +6356,29 @@ ix86_compute_frame_layout (void)
+ area, see the SEH code in config/i386/winnt.c for the rationale. */
+ frame->hard_frame_pointer_offset = frame->sse_reg_save_offset;
+
+- /* If we can leave the frame pointer where it is, do so. Also, return
++ /* If we can leave the frame pointer where it is, do so; however return
+ the establisher frame for __builtin_frame_address (0) or else if the
+- frame overflows the SEH maximum frame size. */
++ frame overflows the SEH maximum frame size.
++
++ Note that the value returned by __builtin_frame_address (0) is quite
++ constrained, because setjmp is piggybacked on the SEH machinery with
++ recent versions of MinGW:
++
++ # elif defined(__SEH__)
++ # if defined(__aarch64__) || defined(_ARM64_)
++ # define setjmp(BUF) _setjmp((BUF), __builtin_sponentry())
++ # elif (__MINGW_GCC_VERSION < 40702)
++ # define setjmp(BUF) _setjmp((BUF), mingw_getsp())
++ # else
++ # define setjmp(BUF) _setjmp((BUF), __builtin_frame_address (0))
++ # endif
++
++ and the second argument passed to _setjmp, if not null, is forwarded
++ to the TargetFrame parameter of RtlUnwindEx by longjmp (after it has
++ built an ExceptionRecord on the fly describing the setjmp buffer). */
+ const HOST_WIDE_INT diff
+ = frame->stack_pointer_offset - frame->hard_frame_pointer_offset;
+- if (diff <= 255)
++ if (diff <= 255 && !crtl->accesses_prior_frames)
+ {
+ /* The resulting diff will be a multiple of 16 lower than 255,
+ i.e. at most 240 as required by the unwind data structure. */