Fix build with latest GCC 9.0 tree

  A recent patch [1] to fix a GCC PR [2] actually broke the GDB build.
To fix, remove the stack pointer clobber. GCC will ignore the clobber
marker, and will not save or restore the stack pointer.

I ran "make check-gdb" on x86_64 to ensure there are no regressions.

gdb/ChangeLog:

2018-12-17  Dimitar Dimitrov  <dimitar@dinux.eu>

	* nat/linux-ptrace.c (linux_ptrace_test_ret_to_nx): Remove sp clobbers.

[1] https://gcc.gnu.org/ml/gcc-patches/2018-12/msg00532.html
[2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52813

Signed-off-by: Dimitar Dimitrov <dimitar@dinux.eu>
---
 gdb/nat/linux-ptrace.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
  

On 2018-12-17 14:30, Dimitar Dimitrov wrote:
> A recent patch [1] to fix a GCC PR [2] actually broke the GDB build.
> To fix, remove the stack pointer clobber. GCC will ignore the clobber
> marker, and will not save or restore the stack pointer.
> 
> I ran "make check-gdb" on x86_64 to ensure there are no regressions.
> 
> gdb/ChangeLog:
> 
> 2018-12-17  Dimitar Dimitrov  <dimitar@dinux.eu>
> 
> 	* nat/linux-ptrace.c (linux_ptrace_test_ret_to_nx): Remove sp 
> clobbers.
> 
> [1] https://gcc.gnu.org/ml/gcc-patches/2018-12/msg00532.html
> [2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52813
> 
> Signed-off-by: Dimitar Dimitrov <dimitar@dinux.eu>
> ---
>  gdb/nat/linux-ptrace.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/nat/linux-ptrace.c b/gdb/nat/linux-ptrace.c
> index 1f21ef03a3..58aed199bf 100644
> --- a/gdb/nat/linux-ptrace.c
> +++ b/gdb/nat/linux-ptrace.c
> @@ -128,14 +128,14 @@ linux_ptrace_test_ret_to_nx (void)
>  			".globl linux_ptrace_test_ret_to_nx_instr;"
>  			"linux_ptrace_test_ret_to_nx_instr:"
>  			"ret"
> -			: : "r" (return_address) : "%esp", "memory");
> +			: : "r" (return_address) : "memory");
>  #elif defined __x86_64__
>  	  asm volatile ("pushq %0;"
>  			".globl linux_ptrace_test_ret_to_nx_instr;"
>  			"linux_ptrace_test_ret_to_nx_instr:"
>  			"ret"
>  			: : "r" ((uint64_t) (uintptr_t) return_address)
> -			: "%rsp", "memory");
> +			: "memory");
>  #else
>  # error "!__i386__ && !__x86_64__"
>  #endif

LGTM:

1. We push a return address and ret, so in the end the stack point has 
not changed.
2. This is executed in a child process, which immediately stops on a 
breakpoint and never continues, even if the stack pointer was messed up, 
we wouldn't notice.

Do you have push access, or would you like me to push the patch for you?

Simon
  

On понеделник, 17 декември 2018 г. 22:05:11 EET Simon Marchi wrote:
> On 2018-12-17 14:30, Dimitar Dimitrov wrote:
> > A recent patch [1] to fix a GCC PR [2] actually broke the GDB build.
> > To fix, remove the stack pointer clobber. GCC will ignore the clobber
> > marker, and will not save or restore the stack pointer.
> > 
> > I ran "make check-gdb" on x86_64 to ensure there are no regressions.
> > 
> > gdb/ChangeLog:
> > 
> > 2018-12-17  Dimitar Dimitrov  <dimitar@dinux.eu>
> > 
> > 	* nat/linux-ptrace.c (linux_ptrace_test_ret_to_nx): Remove sp
> > 
> > clobbers.
> > 
> > [1] https://gcc.gnu.org/ml/gcc-patches/2018-12/msg00532.html
> > [2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52813
> > 
> > Signed-off-by: Dimitar Dimitrov <dimitar@dinux.eu>
> > ---
> > 
> >  gdb/nat/linux-ptrace.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/gdb/nat/linux-ptrace.c b/gdb/nat/linux-ptrace.c
> > index 1f21ef03a3..58aed199bf 100644
> > --- a/gdb/nat/linux-ptrace.c
> > +++ b/gdb/nat/linux-ptrace.c
> > @@ -128,14 +128,14 @@ linux_ptrace_test_ret_to_nx (void)
> > 
> >  			".globl linux_ptrace_test_ret_to_nx_instr;"
> >  			"linux_ptrace_test_ret_to_nx_instr:"
> >  			"ret"
> > 
> > -			: : "r" (return_address) : "%esp", "memory");
> > +			: : "r" (return_address) : "memory");
> > 
> >  #elif defined __x86_64__
> >  
> >  	  asm volatile ("pushq %0;"
> >  	  
> >  			".globl linux_ptrace_test_ret_to_nx_instr;"
> >  			"linux_ptrace_test_ret_to_nx_instr:"
> >  			"ret"
> >  			
> >  			: : "r" ((uint64_t) (uintptr_t) return_address)
> > 
> > -			: "%rsp", "memory");
> > +			: "memory");
> > 
> >  #else
> >  # error "!__i386__ && !__x86_64__"
> >  #endif
> 
> LGTM:
> 
> 1. We push a return address and ret, so in the end the stack point has
> not changed.
> 2. This is executed in a child process, which immediately stops on a
> breakpoint and never continues, even if the stack pointer was messed up,
> we wouldn't notice.
> 
> Do you have push access, or would you like me to push the patch for you?
I do not have push access for GDB. Please push it for me.

Please note that discussion on GCC mailing list is still ongoing. I still 
think that this GDB patch should be merged, though.

https://gcc.gnu.org/ml/gcc-patches/2018-12/msg01250.html
https://gcc.gnu.org/ml/gcc-patches/2018-12/msg01308.html

Thanks,
Dimitar
  

On 2018-12-19 01:13, Dimitar Dimitrov wrote:
> I do not have push access for GDB. Please push it for me.
> 
> Please note that discussion on GCC mailing list is still ongoing. I 
> still
> think that this GDB patch should be merged, though.
> 
> https://gcc.gnu.org/ml/gcc-patches/2018-12/msg01250.html
> https://gcc.gnu.org/ml/gcc-patches/2018-12/msg01308.html

Thanks, I pushed it.

Simon