bug#73167: [PATCH] Fix setjmp/longjmp-related crashes on Windows

["Michael Käppler via Bug reports for GUILE, GNU's Ubiquitous Extension Language" <bug-guile@gnu.org>]

[-- Attachment #1: Type: text/plain, Size: 1500 bytes --]

Hi all,
recently an user reported a bug to the LilyPond project, where he tried
to print
a data structure with (pretty-print), which silently failed at a certain
point inside the data structure:

https://gitlab.com/lilypond/lilypond/-/issues/6737#note_2049515997

The error did only affect LilyPond mingw builds, compiled against Guile
3.0.10, 3.0.9 was (seemingly) fine.
Bisecting showed that the error first showed up with commit
https://git.savannah.gnu.org/cgit/guile.git/commit/?id=29a9f26a36035d5425b173d101628ecc62f5a46b

which substantially reworked the pretty-print implementation to use
delimited continuations.
As it turned out, the problems only arises with the JIT turned on.

Further debugging revealed that the crash happens inside the MSVCRT
function 'RtlUnwindEx', which detects
a stack corruption and throws "STATUS_BAD_STACK". The reason is that
'setjmp' on mingw expands to
'_setjmp' that takes the current frame address as second parameter.
After a 'longjmp' call it tries to unwind
the stack up to this particular frame address. IIUC, this will fail
because the JIT'ed code does not follow
the Windows x64 calling conventions. Setting the second parameter to
NULL prevents unwinding and fixes
the issue.

Guile is not the only project using JIT compilation that faces this
Windows peculiarity.
See
https://blog.lazym.io/2020/09/21/Unicorn-Devblog-setjmp-longjmp-on-Windows/
for a nice summary.

Please consider the attached patch.

Michael

[-- Attachment #2: 0001-Fix-setjmp-longjmp-related-crashes-on-Windows.patch --]
[-- Type: text/plain, Size: 4112 bytes --]

From f9222ec96209c59c9a9a409c019ff59c0c20917c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20K=C3=A4ppler?= <xmichael-k@web.de>
Date: Sat, 7 Sep 2024 22:52:22 +0200
Subject: [PATCH] Fix setjmp/longjmp-related crashes on Windows

* libguile/Makefile.am: add new header file setjump-win.h
* libguile/continuations.h, libguile/dynstack.c, libguile/dynstack.h,
  libguile/intrinsics.h, libguile/vm.h:
  supply custom `setjmp` macro on Windows

Mingw implements `setjmp (env)` as a macro that expands to

 _setjmp (env, faddr)

where `faddr` is set to the current frame address.

This address is then stored as first element in the jump buffer `env`.
When `longjmp` is called, it tries to unwind the stack up
to the saved address by calling `RtlUnwindEx` from MSVCRT,
which will fail, if the stack frames are interwoven with
JIT-generated code, that violate the Windows x64 calling conventions.

Thus implement the macro ourselves as

_setjmp (env, NULL)

which will toggle a code path in `longjmp` that does no unwinding.
---
 libguile/Makefile.am     |  1 +
 libguile/continuations.h |  4 ++++
 libguile/dynstack.c      |  5 +++++
 libguile/dynstack.h      |  5 +++++
 libguile/intrinsics.h    |  4 ++++
 libguile/setjump-win.h   | 17 +++++++++++++++++
 libguile/vm.h            |  4 ++++
 7 files changed, 40 insertions(+)
 create mode 100644 libguile/setjump-win.h

diff --git a/libguile/Makefile.am b/libguile/Makefile.am
index 0890acc18..bf8893cf2 100644
--- a/libguile/Makefile.am
+++ b/libguile/Makefile.am
@@ -682,6 +682,7 @@ modinclude_HEADERS =				\
 	rw.h					\
 	scmsigs.h				\
 	script.h				\
+	setjump-win.h                           \
 	simpos.h				\
 	smob.h					\
 	snarf.h					\
diff --git a/libguile/continuations.h b/libguile/continuations.h
index d83bed9b7..ac636512e 100644
--- a/libguile/continuations.h
+++ b/libguile/continuations.h
@@ -22,7 +22,11 @@
 
 \f
 
+#ifndef _WIN64
 #include <setjmp.h>
+#else
+#include "libguile/setjump-win.h"
+#endif
 
 #include "libguile/programs.h"
 #include "libguile/throw.h"
diff --git a/libguile/dynstack.c b/libguile/dynstack.c
index 2eec7a7eb..e4ed878c2 100644
--- a/libguile/dynstack.c
+++ b/libguile/dynstack.c
@@ -25,7 +25,12 @@
 #endif
 
 #include <assert.h>
+
+#ifndef _WIN64
 #include <setjmp.h>
+#else
+#include "setjump-win.h"
+#endif
 
 #include "control.h"
 #include "eval.h"
diff --git a/libguile/dynstack.h b/libguile/dynstack.h
index 4c32a0943..6f0775e40 100644
--- a/libguile/dynstack.h
+++ b/libguile/dynstack.h
@@ -22,7 +22,12 @@
 
 \f
 
+#ifndef _WIN64
 #include <setjmp.h>
+#else
+#include "libguile/setjump-win.h"
+#endif
+
 #include <signal.h>
 
 #include "libguile/scm.h"
diff --git a/libguile/intrinsics.h b/libguile/intrinsics.h
index d2ffc847e..8a1c7c04e 100644
--- a/libguile/intrinsics.h
+++ b/libguile/intrinsics.h
@@ -24,7 +24,11 @@
 #error intrinsics.h is private and uninstalled
 #endif
 
+#ifndef _WIN64
 #include <setjmp.h>
+#else
+#include "libguile/setjump-win.h"
+#endif
 
 #include <libguile/scm.h>
 
diff --git a/libguile/setjump-win.h b/libguile/setjump-win.h
new file mode 100644
index 000000000..bf06e868b
--- /dev/null
+++ b/libguile/setjump-win.h
@@ -0,0 +1,17 @@
+#ifndef _SCM_SETJUMP_WIN_H_
+#define _SCM_SETJUMP_WIN_H_
+
+#include <setjmp.h>
+
+/* On Windows, `setjmp` expands to _setjmp, which takes a second
+   parameter that is set to the current frame address by default.
+   The address is then stored as first element in the jump buffer.
+   When `longjmp` is called, it tries to unwind the stack up
+   to the saved address, which will fail, if the stack frames are
+   interwoven with JIT-generated code.
+   Set the second parameter to NULL to prevent unwinding. */
+#undef setjmp
+#define setjmp(env) _setjmp(env, NULL)
+
+#endif /* _SCM_SETJUMP_WIN_H_ */
+
diff --git a/libguile/vm.h b/libguile/vm.h
index 9681188bd..d5b7138d3 100644
--- a/libguile/vm.h
+++ b/libguile/vm.h
@@ -20,7 +20,11 @@
 #ifndef _SCM_VM_H_
 #define _SCM_VM_H_
 
+#ifndef _WIN64
 #include <setjmp.h>
+#else
+#include "libguile/setjump-win.h"
+#endif
 
 #include <libguile/gc.h>
 #include <libguile/programs.h>
-- 
2.34.1