On Tue, 2 Apr 2019, Jacek Caban wrote:
We currently have pretty useless chunk of assembly in mingwex that recently
caused problems with Wine mingw builds. The problem is that we have longjmp
symbol defined in mingwex which in fact forwards the call to the imported
function on x64 non-SEH and x86. I did some digging in git history, but it
doesn't explain much about why it's needed.
The one thing I'm not sure is non-SEH x64 build. In this case, it sets frame
stored in jmp_buf to 0 before calling the actual implementation. There is
absolutely no explanation for it. Maybe it was needed at some point for sjlj
exception handling? Even if that's the case, it would be a pretty broken
design. If such nulling is needed, it should be done before calling longjmp;
intercepting an existing function is definitely not the right way to do it.
Maybe it's not really needed?
And yet another question is, is non-SEH x64 build still something we need to
care about?
If we still care about non-SEH x64 builds and really need to carry this hack,
that could be preserved at the cost of complexity. But my guess is that we
don't need it and that's what this patch does.
Regarding the patch itself, it looks like you're losing the longjmp
function altogether from msvcrt.def.in, instead of making it available for
all arches?
I believe the reason for clearing the frame field of jmp_buf, is that the
msvcrt setjmp/longjmp actually does unwinding with SEH, instead of just
restoring the stored context from jmp_buf. The exact details of this seems
to differ a bit between architectures (and potentially between msvcrt
versions).
When doing the setjmp, setjmp is expanded into _setjmp (or a variation
thereof), where the second parameter is a pointer to the current stack
frame. When doing longjmp, it does a SEH unwind of the stack until it
finds a frame that matches this pointer. This parameter afaik is stored in
the frame field of jmp_buf.
What actually happens if SEH information is unavailable varies quite a
bit. When testing on ARM and ARM64, the longjmp will crash if SEH is
unavailable, when _setjmpex is called with a non-null frame pointer. On
ARM, if the second parameter to _setjmpex is NULL though, the longjmp does
succeed, but on ARM64 this doesn't help, SEH information simply is
mandatory. (And for ARM64, the exact form of the frame pointer needed is a
bit more tricky; llvm had to add a new intrinsic function for getting it.)
Given this, it seems like the clearing of the frame field is to make sure
that the longjmp works as a plain longjmp without SEH unwinding (like it
worked for me on ARM, unlike ARM64). However, I just tested building a
toolchain with SJLJ, i.e. without SEH, for x86_64, and setjmp/longjmp does
seem to work (on Windows 10 18.09, with ucrt) even if the frame pointer is
left non-null. In this case, the executable contained no SEH
information altogether. But in a more mixed build, where the executable
does contain some SEH information (but incomplete, unable to do the unwind
using it), things fail unless the field is cleared.
However, instead of intercepting the longjmp function, it's probably
better to just pass NULL for this parameter to begin with, if SEH isn't
available. With the attached patch, this seems to work fine for me.
So in general, I'd still like to keep x86_64 builds without SEH usable.
(For llvm-mingw, this was the only configuration of x86_64 up until last
summer.) But with the patch I just sent, your patch should be fine (modulo
the misedit in msvcrt.def.in).
// Martin
From 70244dca36207da95170d8f67e21696d0567390b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?= <[email protected]>
Date: Tue, 2 Apr 2019 23:13:59 +0300
Subject: [PATCH] headers: Only pass a frame pointer to setjmp if SEH is
enabled
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This should avoid the need for clearing the frame pointer in the
longjmp wrapper.
Signed-off-by: Martin Storsjö <[email protected]>
---
mingw-w64-headers/crt/setjmp.h | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/mingw-w64-headers/crt/setjmp.h b/mingw-w64-headers/crt/setjmp.h
index fc79f1091..547dfde2f 100644
--- a/mingw-w64-headers/crt/setjmp.h
+++ b/mingw-w64-headers/crt/setjmp.h
@@ -217,18 +217,20 @@ void * __cdecl __attribute__ ((__nothrow__)) mingw_getsp (void);
# define setjmp(BUF) __mingw_setjmp((BUF))
# define longjmp __mingw_longjmp
int __cdecl __attribute__ ((__nothrow__,__returns_twice__)) __mingw_setjmp(jmp_buf _Buf);
-# else
+# elif defined(__SEH__)
# if (__MINGW_GCC_VERSION < 40702)
# define setjmp(BUF) _setjmp((BUF), mingw_getsp())
# else
# define setjmp(BUF) _setjmp((BUF), __builtin_frame_address (0))
# endif
+# else
+# define setjmp(BUF) _setjmp((BUF), NULL)
# endif
int __cdecl __attribute__ ((__nothrow__,__returns_twice__)) _setjmp(jmp_buf _Buf, void *_Ctx);
int __cdecl __attribute__ ((__nothrow__,__returns_twice__)) _setjmp3(jmp_buf _Buf, void *_Ctx);
# else
# undef setjmp
-# ifdef _WIN64
+# ifdef __SEH__
# if (__MINGW_GCC_VERSION < 40702)
# define setjmp(BUF) _setjmpex((BUF), mingw_getsp())
# define setjmpex(BUF) _setjmpex((BUF), mingw_getsp())
--
2.17.1
_______________________________________________
Mingw-w64-public mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
