[PATCH] Cygwin: remove %esp from asm clobber list

[Johannes Schindelin]

Hi,

On Fri, 28 Feb 2020, Jon Turney wrote:

> Mentioning the stack pointer in the clobber list is now a gcc warning.
>
> We never wanted gcc to try to restore %esp after this (x86-specific)
> asm, since the whole point of the inline asm here is to adjust %esp to
> satisfy alignment, so remove %esp from the asm clobber list.
>
> Of more concern is the alleged requirement that %esp must be unchanged
> over an asm statement (which makes what this code is trying to do
> impossible to write as a C function), although on x86 we are probably ok
> in this particular instance.
>
> ../../../../winsup/cygwin/init.cc: In function 'void threadfunc_fe(void*)':
> ../../../../winsup/cygwin/init.cc:33:46: error: listing the stack pointer register '%esp' in a clobber list is deprecated [-Werror=deprecated]
> ../../../../winsup/cygwin/init.cc:33:46: note: the value of the stack pointer after an 'asm' statement must be the same as it was before the statement
>
> Also, because we now using gcc's "basic" rather than "extended" asm
> syntax we don't need to escape the '%' in '%esp' as '%%esp'.
> ---
>
> Notes:
>     N.B: This comes with a 'this should be ok, but I haven't actually
>     tested that x86 Cygwin works after this' caveat.

We run with this patch in Git for Windows for quite a while, and I have
yet to hear complaints:

https://github.com/git-for-windows/msys2-runtime/commit/dd5aa267f6f28aef2a241cc7a01bb71a434b403c

As far as I can tell, MSYS2 does the same.

So there is at least some confirmation to be had ;-)

Ciao,
Dscho

>
>  winsup/cygwin/crt0.c  | 2 +-
>  winsup/cygwin/init.cc | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/winsup/cygwin/crt0.c b/winsup/cygwin/crt0.c
> index fee4b2e24..9fcebd8fa 100644
> --- a/winsup/cygwin/crt0.c
> +++ b/winsup/cygwin/crt0.c
> @@ -27,7 +27,7 @@ mainCRTStartup ()
>  #if __GNUC_PREREQ(6,0)
>  #pragma GCC diagnostic pop
>  #endif
> -  asm volatile ("andl $-16,%%esp" ::: "%esp");
> +  asm volatile ("andl $-16,%esp");
>  #endif
>
>    cygwin_crt0 (main);
> diff --git a/winsup/cygwin/init.cc b/winsup/cygwin/init.cc
> index 851a7ffed..7ae7d08fe 100644
> --- a/winsup/cygwin/init.cc
> +++ b/winsup/cygwin/init.cc
> @@ -30,7 +30,7 @@ threadfunc_fe (VOID *arg)
>  #if __GNUC_PREREQ(6,0)
>  #pragma GCC diagnostic pop
>  #endif
> -  asm volatile ("andl $-16,%%esp" ::: "%esp");
> +  asm volatile ("andl $-16,%esp");
>  #endif
>    _cygtls::call ((DWORD (*)  (void *, void *)) TlsGetValue (_my_oldfunc), arg);
>  }
> --
> 2.21.0
>
>