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

Yao Qi
2011-06-20T04:13:44Z

[-- Attachment #1: Type: text/plain, Size: 1784 bytes --]

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?

--
Yao (é½å°§)

[-- Attachment #2: savedregs_sigsegv_to_sigalrm.patch --]
[-- Type: text/x-patch, Size: 1180 bytes --]

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 }