Re: [patch V2, testsuite] gdb.base/savedregs.exp: SIGSEGV -> SIGALRM

[Mark Kettenis <mark dot kettenis at xs4all dot nl>]

> Date: Mon, 20 Jun 2011 12:13:12 +0800
> From: Yao Qi <yao@codesourcery.com>
> 
> On 06/09/2011 09:09 PM, Yao Qi wrote:
> > On 06/09/2011 07:17 PM, Mark Kettenis wrote:
> >>> Date: Thu, 09 Jun 2011 17:28:09 +0800
> >>> From: Yao Qi <yao@codesourcery.com>
> >>>
> >>> In current gdb.base/savedregs.exp, signal handler is installed for
> >>> signal SIGSEGV, and SIGSEGV is trigger by `*(char *)0 = 0;'.  However,
> >>> on non-mmu uclinux system, writing to an address 0x0 doesn't trigger
> >>> SIGSEGV.
> >>>
> >>> In my patch, SIGILL is chosen to replace SIGSEGV.  One assumption here
> >>> is that 0xffff is an invalid instruction on all ports.
> >>
> >> Please don't do this.  You're changing the test significantly.  And
> > 
> > I don't think the test is changed *significantly*.  The purpose of
> > writing to zero, at least in this case, is to trigger a signal, and
> > check the register in signal trampoline frame.  Either SIGSEGV or SIGILL
> > meets this need.
> > 
> 
> Mark,
> I still believe my explanation above is correct.  The original patch's
> explanation is
> <http://sourceware.org/ml/gdb-patches/2004-10/msg00475.html> and "The
> attached checks that "info frame" doesn't change as the stack evolves."
> shows that SIGSEGV is used here to trigger calling to signal handler.
> 
> Please let me know if you still believe that replacing SIGSEGV by other
> signals change the test significantly.
> 
> >> there is no guarantee that 0xffff is an invalid instruction.  Heck
> >> most platforms don't even have 16-bit instructions.
> > 
> > It is possible to find a `common' invalid instruction over all ports,
> > even 0xffff may not be.
> > 
> 
> This part is weak, I think.  In my second version of this patch, I am
> using SIGALRM to replace SIGSEGV.
> 
> Tested this new patch on
> i686-pc-linux-gnu/x86_64-unknown-linux-gnu/armv7l-unknown-linux-gnu-eabi/my-uclinux-new-port/.
>  No new fails.
> 
> OK for mainline?

That's an even bigger change.  And I don't think that SIGALRM is even
guaranteed to happen before the program terminates.  And if there
MMU-less systems that effectively don't support SIGSEGV, there
certainly are timer-less systems that don't support SIGALRM.

Really, just skip this test on MMU-less systems.  If you're worried
about test coverage on your MMU-less ARM systems, add an additional
test in gdb.arch/ that uses an undefined instruction to generate
SIGILL.

> --------------020203020005010203000004
> Content-Type: text/x-patch;
>  name="savedregs_sigsegv_to_sigalrm.patch"
> Content-Transfer-Encoding: 7bit
> Content-Disposition: attachment;
>  filename="savedregs_sigsegv_to_sigalrm.patch"
> 
> 2011-06-20  Yao Qi  <yao@codesourcery.com>
> 
> 	gdb/testsuite/
> 	* gdb.base/savedregs.c (thrower): Remove write to address zero.
> 	(main): Install handler to SIGALRM instead of SIGSEGV.
> 	* gdb.base/savedregs.exp: Sleep for a while to make signal is delivered.
> 
> diff --git a/gdb/testsuite/gdb.base/savedregs.c b/gdb/testsuite/gdb.base/savedregs.c
> index 9c4ce87..0a804cd 100644
> --- a/gdb/testsuite/gdb.base/savedregs.c
> +++ b/gdb/testsuite/gdb.base/savedregs.c
> @@ -45,11 +45,11 @@ catcher (int sig)
>  static void
>  thrower (void)
>  {
> -  *(char *)0 = 0;
>  }
>  
>  main ()
>  {
> -  signal (SIGSEGV, catcher);
> +  signal (SIGALRM, catcher);
> +  alarm (1);
>    thrower ();
>  }
> diff --git a/gdb/testsuite/gdb.base/savedregs.exp b/gdb/testsuite/gdb.base/savedregs.exp
> index 6434512..0de0db8 100644
> --- a/gdb/testsuite/gdb.base/savedregs.exp
> +++ b/gdb/testsuite/gdb.base/savedregs.exp
> @@ -142,7 +142,7 @@ process_saved_regs thrower { main } { }
>  
>  # Continue to the signal catcher, check main's saved-reg info, capture
>  # catcher's saved-reg info.
> -gdb_test "handle SIGSEGV pass print nostop"
> +sleep 2
>  gdb_test "advance catcher" "catcher .* at .*"
>  process_saved_regs catcher { sigtramp thrower } { main }
>  
> 
> --------------020203020005010203000004--
>