diff mbox series

[v2,2/4] sysemu/os-win32: fix setjmp/longjmp on windows-arm64

Message ID 20230216134911.6803-3-pierrick.bouvier@linaro.org
State Superseded
Headers show
Series Adds support for running QEMU natively on windows-arm64 | expand

Commit Message

Pierrick Bouvier Feb. 16, 2023, 1:49 p.m. UTC
Windows implementation of setjmp/longjmp is done in
C:/WINDOWS/system32/ucrtbase.dll. Alas, on arm64, it seems to *always*
perform stack unwinding, which crashes from generated code.

By using alternative implementation built in mingw, we avoid doing stack
unwinding and this fixes crash when calling longjmp.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 include/sysemu/os-win32.h | 21 +++++++++++++++++++--
 meson.build               | 22 ++++++++++++++++++++++
 2 files changed, 41 insertions(+), 2 deletions(-)

Comments

Richard Henderson Feb. 16, 2023, 8:02 p.m. UTC | #1
On 2/16/23 03:49, Pierrick Bouvier wrote:
> Windows implementation of setjmp/longjmp is done in
> C:/WINDOWS/system32/ucrtbase.dll. Alas, on arm64, it seems to *always*
> perform stack unwinding, which crashes from generated code.
> 
> By using alternative implementation built in mingw, we avoid doing stack
> unwinding and this fixes crash when calling longjmp.
> 
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   include/sysemu/os-win32.h | 21 +++++++++++++++++++--
>   meson.build               | 22 ++++++++++++++++++++++
>   2 files changed, 41 insertions(+), 2 deletions(-)

Ugly, but workable.

Acked-by: Richard Henderson <richard.henderson@linaro.org>

Ideally we'd interact properly with system unwinding.  It looks like we'd use 
RtlAddFunctionTable, but the documentation is spread out and I've not found all of the bits.

We already do something similar for gdb -- see tcg/tcg.c, tcg_register_jit_int, and 
tcg/aarch64/tcg-target.c.inc, debug_frame.


r~
Pierrick Bouvier Feb. 20, 2023, 9:53 a.m. UTC | #2
On 2/16/23 21:02, Richard Henderson wrote:
> On 2/16/23 03:49, Pierrick Bouvier wrote:
>> Windows implementation of setjmp/longjmp is done in
>> C:/WINDOWS/system32/ucrtbase.dll. Alas, on arm64, it seems to *always*
>> perform stack unwinding, which crashes from generated code.
>>
>> By using alternative implementation built in mingw, we avoid doing stack
>> unwinding and this fixes crash when calling longjmp.
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>    include/sysemu/os-win32.h | 21 +++++++++++++++++++--
>>    meson.build               | 22 ++++++++++++++++++++++
>>    2 files changed, 41 insertions(+), 2 deletions(-)
> 
> Ugly, but workable.
> 
> Acked-by: Richard Henderson <richard.henderson@linaro.org>
> 
> Ideally we'd interact properly with system unwinding.  It looks like we'd use
> RtlAddFunctionTable, but the documentation is spread out and I've not found all of the bits.
> 
> We already do something similar for gdb -- see tcg/tcg.c, tcg_register_jit_int, and
> tcg/aarch64/tcg-target.c.inc, debug_frame.
> 

Thanks for the idea.
For the sake of completeness, using RtlInstallFunctionTableCallback 
could be a better strategy, as it allows to have a callback called only 
during stack unwinding [1].

Meanwhile, I'll ask to our contact in MSFT if it's possible to perform a 
setjmp/longjmp that does not trigger stack unwinding on aarch64.

[1] 
https://learn.microsoft.com/en-us/windows/win32/api/winnt/nf-winnt-rtlinstallfunctiontablecallback

> 
> r~
Richard Henderson Feb. 20, 2023, 5:50 p.m. UTC | #3
On 2/19/23 23:53, Pierrick Bouvier wrote:
> Thanks for the idea.
> For the sake of completeness, using RtlInstallFunctionTableCallback could be a better 
> strategy, as it allows to have a callback called only during stack unwinding [1].

Interesting idea.

I suspect that is of more use when the JIT is more complicated than ours.  In particular, 
notice the OutOfProcessCallbackDll member (for use by the debugger). Whereas our unwind 
info is static, and can exist in read-only memory.


r~
diff mbox series

Patch

diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index 5b38c7bd04..634931a387 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -51,14 +51,31 @@  typedef struct sockaddr_un {
 extern "C" {
 #endif
 
-#if defined(_WIN64)
+#if defined(__aarch64__)
+#ifndef CONFIG_MINGW64_HAS_SETJMP_LONGJMP
+#error mingw must provide setjmp/longjmp for windows-arm64
+#endif
+/* On windows-arm64, setjmp is available in only one variant, and longjmp always
+ * does stack unwinding. This crash with generated code.
+ * Thus, we use another implementation of setjmp (not windows one), coming from
+ * mingw, which never performs stack unwinding. */
+#undef setjmp
+#undef longjmp
+/* These functions are not declared in setjmp.h because __aarch64__ defines
+ * setjmp to _setjmpex instead. However, they are still defined in libmingwex.a,
+ * which gets linked automatically. */
+extern int __mingw_setjmp(jmp_buf);
+extern void __attribute__((noreturn)) __mingw_longjmp(jmp_buf, int);
+#define setjmp(env) __mingw_setjmp(env)
+#define longjmp(env, val) __mingw_longjmp(env, val)
+#elif defined(_WIN64)
 /* On w64, setjmp is implemented by _setjmp which needs a second parameter.
  * If this parameter is NULL, longjump does no stack unwinding.
  * That is what we need for QEMU. Passing the value of register rsp (default)
  * lets longjmp try a stack unwinding which will crash with generated code. */
 # undef setjmp
 # define setjmp(env) _setjmp(env, NULL)
-#endif
+#endif /* __aarch64__ */
 /* QEMU uses sigsetjmp()/siglongjmp() as the portable way to specify
  * "longjmp and don't touch the signal masks". Since we know that the
  * savemask parameter will always be zero we can safely define these
diff --git a/meson.build b/meson.build
index 4ba3bf3431..7f55205437 100644
--- a/meson.build
+++ b/meson.build
@@ -2450,6 +2450,28 @@  if targetos == 'windows'
     }''', name: '_lock_file and _unlock_file'))
 endif
 
+if targetos == 'windows'
+  mingw_has_setjmp_longjmp = cc.links('''
+    #include <setjmp.h>
+    int main(void) {
+      // these functions are not available in setjmp header, but may be
+      // available at link time, from libmingwex.a
+      extern int __mingw_setjmp(jmp_buf);
+      extern void __attribute__((noreturn)) __mingw_longjmp(jmp_buf, int);
+      jmp_buf env;
+      __mingw_setjmp(env);
+      __mingw_longjmp(env, 0);
+    }
+  ''', name: 'mingw setjmp and longjmp')
+
+  config_host_data.set('CONFIG_MINGW64_HAS_SETJMP_LONGJMP',
+                       mingw_has_setjmp_longjmp)
+
+  if cpu == 'aarch64' and not mingw_has_setjmp_longjmp
+    error('mingw must provide setjmp/longjmp for windows-arm64')
+  endif
+endif
+
 ########################
 # Target configuration #
 ########################