Re: [PATCH v3 2/4] sysemu/os-win32: fix setjmp/longjmp on windows-arm64

[Pierrick Bouvier]

On 2/21/23 11:33, Philippe Mathieu-Daudé wrote:
> On 21/2/23 10:47, Pierrick Bouvier wrote:
> > @Philippe Mathieu-Daudé, is that version satisfying for you, regarding
> > your initial comments about setjmp/longjmp detection in meson?
>
> Yes, the meson check is what I had in mind.
>
> > I can integrate more changes if needed.
> >
> > Thanks,
> > Pierrick
> >
> > On 2/20/23 16:16, Pierrick Bouvier wrote:
> > > Acked-by: Richard Henderson <richard.henderson@linaro.org>
> > >
> > > On 2/20/23 12:12, 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 | 25 +++++++++++++++++++++++--
> > > >     meson.build               | 24 ++++++++++++++++++++++++
> > > >     2 files changed, 47 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
> > > > index 5b38c7bd04..1f6c141d39 100644
> > > > --- a/include/sysemu/os-win32.h
> > > > +++ b/include/sysemu/os-win32.h
> > > > @@ -51,14 +51,35 @@ 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
>
> Per the meson error [*], this now seems impossible, thus we can
> simply drop the CONFIG_MINGW64_HAS_SETJMP_LONGJMP definition?

I think it's redundant too.

> > > > +#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__ */
>
> This comment doesn't seem accurate. Maybe "64-bit"?

I'd like to use windows-x64. IMHO,  it's clearer and on par with 
windows-arm64. But I know some people don't like x64 nomenclature.

Alas, several projects are using w32 and w64 names, and there is no 
consensus yet on how win-arm64 should be named.

> > > >     /* 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..e968ed9e7a 100644
> > > > --- a/meson.build
> > > > +++ b/meson.build
> > > > @@ -2450,6 +2450,30 @@ 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')
>
> (This is the [*] error I mentioned earlier).
>
> > > > +  endif
> > > > +endif
> > > > +
> > > >     ########################
> > > >     # Target configuration #
> > > >     ########################