guix: fix GCC 10.3.0 + mingw-w64 setjmp/longjmp issues #24842

pull fanquake wants to merge 1 commits into bitcoin:master from fanquake:guix_fix_windows_longjmp changing 2 files +72 −3
  1. fanquake commented at 12:36 pm on April 13, 2022: member

    This commit backports a patch 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, 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() would result in a call to ft_longjmp (longjmp), 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):

    062172df3089e7bca3fd00f63acc9c8d3678a35bfb2bb5a0af905e61e9d8def52  guix-build-457148a803ce/output/dist-archive/bitcoin-457148a803ce.tar.gz
1f8318d16d0418e0e790efd94527a5be374ac50f51df53e05a6d54cc8c08a8633  guix-build-457148a803ce/output/x86_64-w64-mingw32/SHA256SUMS.part
272076e6896297a36beec6c62065b3d8aeeeb87fed407df947261cefdc81cdb93  guix-build-457148a803ce/output/x86_64-w64-mingw32/bitcoin-457148a803ce-win64-debug.zip
3c617d2347f50d2706bbdcc2b3b97f2ecaf59243747f4c81d7747a22e64cb9d76  guix-build-457148a803ce/output/x86_64-w64-mingw32/bitcoin-457148a803ce-win64-setup-unsigned.exe
48b1e7821e495121bea8a70f09ea6a0b703503b054d831b0dd86a0fe29cece457  guix-build-457148a803ce/output/x86_64-w64-mingw32/bitcoin-457148a803ce-win64-unsigned.tar.gz
5c8d2c0e68e3bf21ed7cfe08df64925bfa54ce6225c6d29bb710f9d9d4474caee  guix-build-457148a803ce/output/x86_64-w64-mingw32/bitcoin-457148a803ce-win64.zip
  2. guix: fix GCC 10.3.0 + mingw-w64 setjmp/longjmp issues 
    This commit backports a patch 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, but hasn't yet made it into a 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() would result in a call to ft_longjmp (longjmp), which
would then trigger a crash.

Fixes: https://github.com/bitcoin-core/gui/issues/582.

See also:
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=e8d1ca7d2c344a411779892616c423e157f4aea8.
https://bugreports.qt.io/browse/QTBUG-93476.
    457148a803
  3. fanquake added the label Windows on Apr 13, 2022
  4. fanquake added the label Build system on Apr 13, 2022
  5. fanquake added the label Needs backport (23.x) on Apr 13, 2022
  6. fanquake added this to the milestone 23.0 on Apr 13, 2022
  7. jonatack commented at 12:48 pm on April 13, 2022: member
    Approach and review-only ACK 457148a803cee02897b7428fa7b3eb93eed71e4c
  8. fanquake referenced this in commit a75b8ec836 on Apr 13, 2022
  9. laanwj commented at 1:21 pm on April 13, 2022: member
    Concept and code review ACK 457148a803cee02897b7428fa7b3eb93eed71e4c This is the right fix for https://github.com/bitcoin-core/gui/issues/582.
  10. hebasto commented at 1:26 pm on April 13, 2022: member

    Guix build on x86_64:

    0$ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
162172df3089e7bca3fd00f63acc9c8d3678a35bfb2bb5a0af905e61e9d8def52  guix-build-457148a803ce/output/dist-archive/bitcoin-457148a803ce.tar.gz
2f8318d16d0418e0e790efd94527a5be374ac50f51df53e05a6d54cc8c08a8633  guix-build-457148a803ce/output/x86_64-w64-mingw32/SHA256SUMS.part
372076e6896297a36beec6c62065b3d8aeeeb87fed407df947261cefdc81cdb93  guix-build-457148a803ce/output/x86_64-w64-mingw32/bitcoin-457148a803ce-win64-debug.zip
4c617d2347f50d2706bbdcc2b3b97f2ecaf59243747f4c81d7747a22e64cb9d76  guix-build-457148a803ce/output/x86_64-w64-mingw32/bitcoin-457148a803ce-win64-setup-unsigned.exe
58b1e7821e495121bea8a70f09ea6a0b703503b054d831b0dd86a0fe29cece457  guix-build-457148a803ce/output/x86_64-w64-mingw32/bitcoin-457148a803ce-win64-unsigned.tar.gz
6c8d2c0e68e3bf21ed7cfe08df64925bfa54ce6225c6d29bb710f9d9d4474caee  guix-build-457148a803ce/output/x86_64-w64-mingw32/bitcoin-457148a803ce-win64.zip
  12. hebasto approved
  13. hebasto commented at 1:37 pm on April 13, 2022: member

    ACK 457148a803cee02897b7428fa7b3eb93eed71e4c, tested bitcoin-457148a803ce-win64.zip on Windows 11 Pro 21H2. Confirming that bitcoin-core/gui#582 is fixed.

    Also checked that this PR patch fixes bitcoin-core/gui#582 for the 23.x branch.

  14. jarolrod commented at 2:37 pm on April 13, 2022: member

    GUIX hashes on x86, mine match hebasto and fanquake:

    0find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
1
262172df3089e7bca3fd00f63acc9c8d3678a35bfb2bb5a0af905e61e9d8def52  guix-build-457148a803ce/output/dist-archive/bitcoin-457148a803ce.tar.gz
3f8318d16d0418e0e790efd94527a5be374ac50f51df53e05a6d54cc8c08a8633  guix-build-457148a803ce/output/x86_64-w64-mingw32/SHA256SUMS.part
472076e6896297a36beec6c62065b3d8aeeeb87fed407df947261cefdc81cdb93  guix-build-457148a803ce/output/x86_64-w64-mingw32/bitcoin-457148a803ce-win64-debug.zip
5c617d2347f50d2706bbdcc2b3b97f2ecaf59243747f4c81d7747a22e64cb9d76  guix-build-457148a803ce/output/x86_64-w64-mingw32/bitcoin-457148a803ce-win64-setup-unsigned.exe
68b1e7821e495121bea8a70f09ea6a0b703503b054d831b0dd86a0fe29cece457  guix-build-457148a803ce/output/x86_64-w64-mingw32/bitcoin-457148a803ce-win64-unsigned.tar.gz
7c8d2c0e68e3bf21ed7cfe08df64925bfa54ce6225c6d29bb710f9d9d4474caee  guix-build-457148a803ce/output/x86_64-w64-mingw32/bitcoin-457148a803ce-win64.zip
  15. jarolrod approved
  16. jarolrod commented at 2:53 pm on April 13, 2022: member

    ACK 457148a803cee02897b7428fa7b3eb93eed71e4c

    I guix built both master and this PR and tested the resulting binary on Windows 11. Confirming this fixes the linked issue.

    Obviously, this should have been caught earlier through testing. When I perform manual testing of the GUI, it’s never a standard set of routines and actions. And I assume that for other reviewers, it’s a similar situation. What this means is that all actions the GUI can perform aren’t reviewed. I’m going to put together a checklist of actions that should be performed when manually reviewing the GUI, and hopefully it will catch issues like this much earlier in the release process.

  17. laanwj commented at 7:16 am on April 14, 2022: member

    Obviously, this should have been caught earlier through testing.

    We should be especially careful with testing on compiler updates, especially the Windows one. It’s a low tier platform for GCC and time after time it’s clear that it cannot be assumed even basic things keep working with a new version.

  18. laanwj merged this on Apr 14, 2022
  19. laanwj closed this on Apr 14, 2022
  20. fanquake removed the label Needs backport (23.x) on Apr 14, 2022
  21. fanquake deleted the branch on Apr 14, 2022
  22. fanquake commented at 7:35 am on April 14, 2022: member
    Being backported in #24843.
  23. fanquake commented at 7:50 am on April 14, 2022: member
    cc @skitt. This patch might be something to backport for Debian mingw-w64, which currently builds with 10.3.0?
  24. skitt commented at 8:10 am on April 14, 2022: none
    Thanks for the heads-up @fanquake; target/100402 was included in the gcc-10 10.3.0-3 package, so it’s included in the current 10.3 builds of gcc-mingw-w64. I might need to backport the fix to the stable builds however (10.2.1).
  25. laanwj referenced this in commit dabac355c8 on Apr 14, 2022
  26. sidhujag referenced this in commit 794f18eb6a on Apr 14, 2022
  27. DrahtBot locked this on Apr 14, 2023


fanquake jonatack laanwj hebasto gruve-p jarolrod skitt

Labels
Windows Build system

Milestone
23.0

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-09-29 01:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me