On 02/04/2019 22:18, Martin Storsjö wrote:
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?
No, that was intentional, note that it's defined in
msvcrt-common.def.in. Now that I read it again, ucrtbase.def.in could
also be tweaked to use the version from msvcrt-common.def.in, I will
send a new version tomorrow.
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.
Thank you for the great explanation! It makes perfect sense now.
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).
Agreed. Your patch is a nice way for supporting that, it looks good to me.
While we're on this subject, is there a reason for providing our own
implementation for ARM? Could we use imported version as well?
Thanks,
Jacek
_______________________________________________
Mingw-w64-public mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
