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

Mark Kettenis
2011-06-20T07:03:31Z

> 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--
>