* [patch, testsuite] gdb.base/savedregs.exp: SIGSEGV -> SIGILL
@ 2011-06-09 9:28 Yao Qi
2011-06-09 10:08 ` Andreas Schwab
2011-06-09 11:19 ` Mark Kettenis
0 siblings, 2 replies; 14+ messages in thread
From: Yao Qi @ 2011-06-09 9:28 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 520 bytes --]
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.
FAILs go away on my non-mmu uclinux system, and no regression on
i686-pc-linux-gnu/x86_64-unknown-linux-gnu/armv7l-unknown-linux-gnueabi.
OK to apply?
--
Yao (é½å°§)
[-- Attachment #2: savedregs_sigsegv_to_sigill.patch --]
[-- Type: text/x-patch, Size: 1283 bytes --]
2011-06-09 Yao Qi <yao@codesourcery.com>
gdb/testsuite/
* gdb.base/savedregs.c (catcher): Add an invalid instruction
to trigger SIGILL.
(main): Install catcher for signal SIGILL.
* gdb.base/savedregs.exp (process_saved_regs): Handle SIGILL.
diff --git a/gdb/testsuite/gdb.base/savedregs.c b/gdb/testsuite/gdb.base/savedregs.c
index 9c4ce87..9af2c6c 100644
--- a/gdb/testsuite/gdb.base/savedregs.c
+++ b/gdb/testsuite/gdb.base/savedregs.c
@@ -45,11 +45,12 @@ catcher (int sig)
static void
thrower (void)
{
- *(char *)0 = 0;
+ /* Assume 0xffff is an invalid instruction on all ports. */
+ asm (".word 0xffff");
}
main ()
{
- signal (SIGSEGV, catcher);
+ signal (SIGILL, catcher);
thrower ();
}
diff --git a/gdb/testsuite/gdb.base/savedregs.exp b/gdb/testsuite/gdb.base/savedregs.exp
index 6434512..5d9634e 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"
+gdb_test "handle SIGILL pass print nostop"
gdb_test "advance catcher" "catcher .* at .*"
process_saved_regs catcher { sigtramp thrower } { main }
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch, testsuite] gdb.base/savedregs.exp: SIGSEGV -> SIGILL
2011-06-09 9:28 [patch, testsuite] gdb.base/savedregs.exp: SIGSEGV -> SIGILL Yao Qi
@ 2011-06-09 10:08 ` Andreas Schwab
2011-06-09 10:53 ` Yao Qi
2011-06-09 11:19 ` Mark Kettenis
1 sibling, 1 reply; 14+ messages in thread
From: Andreas Schwab @ 2011-06-09 10:08 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
Yao Qi <yao@codesourcery.com> writes:
> 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.
Does (char*)-1 work instead?
Andreas.
--
Andreas Schwab, schwab@redhat.com
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84 5EC7 45C6 250E 6F00 984E
"And now for something completely different."
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch, testsuite] gdb.base/savedregs.exp: SIGSEGV -> SIGILL
2011-06-09 10:08 ` Andreas Schwab
@ 2011-06-09 10:53 ` Yao Qi
0 siblings, 0 replies; 14+ messages in thread
From: Yao Qi @ 2011-06-09 10:53 UTC (permalink / raw)
To: Andreas Schwab; +Cc: gdb-patches
On 06/09/2011 06:07 PM, Andreas Schwab wrote:
> Yao Qi <yao@codesourcery.com> writes:
>
>> 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.
>
> Does (char*)-1 work instead?
>
Andreas,
Nope. :(
(gdb) advance thrower
thrower () at gdb/testsuite/gdb.base/savedregs.c:48
48 *(char *)-1 = 0;
(gdb) n
49 }
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch, testsuite] gdb.base/savedregs.exp: SIGSEGV -> SIGILL
2011-06-09 9:28 [patch, testsuite] gdb.base/savedregs.exp: SIGSEGV -> SIGILL Yao Qi
2011-06-09 10:08 ` Andreas Schwab
@ 2011-06-09 11:19 ` Mark Kettenis
2011-06-09 11:41 ` Pedro Alves
2011-06-09 13:10 ` Yao Qi
1 sibling, 2 replies; 14+ messages in thread
From: Mark Kettenis @ 2011-06-09 11:19 UTC (permalink / raw)
To: yao; +Cc: gdb-patches
> 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
there is no guarantee that 0xffff is an invalid instruction. Heck
most platforms don't even have 16-bit instructions.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch, testsuite] gdb.base/savedregs.exp: SIGSEGV -> SIGILL
2011-06-09 11:19 ` Mark Kettenis
@ 2011-06-09 11:41 ` Pedro Alves
2011-06-09 13:25 ` Yao Qi
2011-06-09 13:10 ` Yao Qi
1 sibling, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2011-06-09 11:41 UTC (permalink / raw)
To: gdb-patches; +Cc: Mark Kettenis, yao
On Thursday 09 June 2011 12:17:27, 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
> there is no guarantee that 0xffff is an invalid instruction. Heck
> most platforms don't even have 16-bit instructions.
If backtracing through 0 is important to this test (haven't looked),
and replacing the write to 0 by raise(SIGSEGV) won't cut it,
then you can use the same trick sigbpt.exp, signest.exp, signull.exp
use to skip the test on targets without an MMU.
--
Pedro Alves
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch, testsuite] gdb.base/savedregs.exp: SIGSEGV -> SIGILL
2011-06-09 11:19 ` Mark Kettenis
2011-06-09 11:41 ` Pedro Alves
@ 2011-06-09 13:10 ` Yao Qi
2011-06-09 14:25 ` Pedro Alves
2011-06-20 4:13 ` [patch V2, testsuite] gdb.base/savedregs.exp: SIGSEGV -> SIGALRM Yao Qi
1 sibling, 2 replies; 14+ messages in thread
From: Yao Qi @ 2011-06-09 13:10 UTC (permalink / raw)
To: Mark Kettenis; +Cc: gdb-patches
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.
> 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.
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch, testsuite] gdb.base/savedregs.exp: SIGSEGV -> SIGILL
2011-06-09 11:41 ` Pedro Alves
@ 2011-06-09 13:25 ` Yao Qi
0 siblings, 0 replies; 14+ messages in thread
From: Yao Qi @ 2011-06-09 13:25 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Mark Kettenis
On 06/09/2011 07:41 PM, Pedro Alves wrote:
> On Thursday 09 June 2011 12:17:27, 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
>> there is no guarantee that 0xffff is an invalid instruction. Heck
>> most platforms don't even have 16-bit instructions.
>
> If backtracing through 0 is important to this test (haven't looked),
> and replacing the write to 0 by raise(SIGSEGV) won't cut it,
> then you can use the same trick sigbpt.exp, signest.exp, signull.exp
> use to skip the test on targets without an MMU.
>
Pedro,
Thanks for this trick, which I don't know before. SIGSEGV usage in
savedregs.exp is a little bit different from them. In savedregs.exp,
SIGSEGV is used to trigger invocation of signal handler, and check
registers contents in signal trampoline frame. If my understand is
correct, I can't see any reason that we can't replace SIGSEGV by SIGILL,
or other signal.
Of course, we can skip savedregs.exp with the same trick, but it is
imperfect that we skip a test which can be run naturally.
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch, testsuite] gdb.base/savedregs.exp: SIGSEGV -> SIGILL
2011-06-09 13:10 ` Yao Qi
@ 2011-06-09 14:25 ` Pedro Alves
2011-06-20 4:13 ` [patch V2, testsuite] gdb.base/savedregs.exp: SIGSEGV -> SIGALRM Yao Qi
1 sibling, 0 replies; 14+ messages in thread
From: Pedro Alves @ 2011-06-09 14:25 UTC (permalink / raw)
To: gdb-patches; +Cc: Yao Qi, Mark Kettenis
On Thursday 09 June 2011 14:09:39, 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.
You made me go look for the original explanation behind the
test :-)
<http://sourceware.org/ml/gdb-patches/2004-10/msg00475.html>
--
Pedro Alves
^ permalink raw reply [flat|nested] 14+ messages in thread
* [patch V2, testsuite] gdb.base/savedregs.exp: SIGSEGV -> SIGALRM
2011-06-09 13:10 ` Yao Qi
2011-06-09 14:25 ` Pedro Alves
@ 2011-06-20 4:13 ` Yao Qi
2011-06-20 7:03 ` Mark Kettenis
1 sibling, 1 reply; 14+ messages in thread
From: Yao Qi @ 2011-06-20 4:13 UTC (permalink / raw)
To: Mark Kettenis; +Cc: gdb-patches
[-- 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 }
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch V2, testsuite] gdb.base/savedregs.exp: SIGSEGV -> SIGALRM
2011-06-20 4:13 ` [patch V2, testsuite] gdb.base/savedregs.exp: SIGSEGV -> SIGALRM Yao Qi
@ 2011-06-20 7:03 ` Mark Kettenis
2011-06-20 8:26 ` Yao Qi
0 siblings, 1 reply; 14+ messages in thread
From: Mark Kettenis @ 2011-06-20 7:03 UTC (permalink / raw)
To: yao; +Cc: gdb-patches
> 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--
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch V2, testsuite] gdb.base/savedregs.exp: SIGSEGV -> SIGALRM
2011-06-20 7:03 ` Mark Kettenis
@ 2011-06-20 8:26 ` Yao Qi
2011-06-20 11:12 ` Mark Kettenis
0 siblings, 1 reply; 14+ messages in thread
From: Yao Qi @ 2011-06-20 8:26 UTC (permalink / raw)
To: Mark Kettenis; +Cc: gdb-patches
On 06/20/2011 03:01 PM, Mark Kettenis wrote:
> 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.
>
What system doesn't support SIGALRM? I noticed that SIGALRM is widely
used in gdb testsuite, so I assume that it is safer to use SIGALRM than
SIGILL here.
> 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.
I am afraid it is not a good idea. There are many MMU-less processor,
and shall we duplicate this test case all over under gdb.arch/ for each
MMU-less processor?
As I pointed out before, this test case has nothing to do with the
difference of MMU system and MMU-less system. Originally, SIGSEGV was
used here to trigger an invocation to signal handler. The key point of
this case is "to trigger an invocation to a handler, and check the frame
in signal handler", so handler of what signal doesn't matter here.
Then, we should choose a signal which exists on all systems that gdb
supports. Firstly, SIGSEGV is chosen, but it doesn't work on MMU-less
system, then SIGILL and SIGALRM is proposed in my two patches
respectively, which you don't like.
Maybe, another option is to define invalid instruction for each targets
in test case.
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch V2, testsuite] gdb.base/savedregs.exp: SIGSEGV -> SIGALRM
2011-06-20 8:26 ` Yao Qi
@ 2011-06-20 11:12 ` Mark Kettenis
2011-06-20 15:07 ` Yao Qi
0 siblings, 1 reply; 14+ messages in thread
From: Mark Kettenis @ 2011-06-20 11:12 UTC (permalink / raw)
To: yao; +Cc: mark.kettenis, gdb-patches
> Date: Mon, 20 Jun 2011 16:26:00 +0800
> From: Yao Qi <yao@codesourcery.com>
> Organization: CodeSourcery
> CC: gdb-patches@sourceware.org
> X-XS4ALL-DNSBL-Checked: mxdrop157.xs4all.nl checked 38.113.113.100 against DNS blacklists
> X-CNFS-Analysis: v=1.1 cv=0zc6fmG9YcuPB4Yp6G+9JUp7sX0X0uIJmZE+jPYAbEE= c=1
> sm=0 a=BqqcDBkZDkoA:10 a=JR3SvU1DSgIA:10 a=zbTXIsBlecMA:10
> a=BLceEmwcHowA:10 a=IkcTkHD0fZMA:10 a=gA6+7WRReeCfZX6hXOPmZA==:17
> a=WZgqnTsFiMBE2RNew4wA:9 a=MWeep4gfJk6IGiYzYrIA:7 a=QEXdDO2ut3YA:10
> a=gA6+7WRReeCfZX6hXOPmZA==:117
> X-Virus-Scanned: by XS4ALL Virus Scanner
> X-XS4ALL-Spam-Score: -0.0 () SPF_PASS
> X-XS4ALL-Spam: NO
> Envelope-To: mark.kettenis@xs4all.nl
>
> On 06/20/2011 03:01 PM, Mark Kettenis wrote:
> > 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.
> >
>
> What system doesn't support SIGALRM? I noticed that SIGALRM is widely
> used in gdb testsuite, so I assume that it is safer to use SIGALRM than
> SIGILL here.
>
> > 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.
>
> I am afraid it is not a good idea. There are many MMU-less processor,
> and shall we duplicate this test case all over under gdb.arch/ for each
> MMU-less processor?
>
> As I pointed out before, this test case has nothing to do with the
> difference of MMU system and MMU-less system. Originally, SIGSEGV was
> used here to trigger an invocation to signal handler. The key point of
> this case is "to trigger an invocation to a handler, and check the frame
> in signal handler", so handler of what signal doesn't matter here.
> Then, we should choose a signal which exists on all systems that gdb
> supports. Firstly, SIGSEGV is chosen, but it doesn't work on MMU-less
> system, then SIGILL and SIGALRM is proposed in my two patches
> respectively, which you don't like.
The fundamental problem with SIGALRM is that it is non-deterministic.
The point where the progrem gets interrupted by the signal can be
different for each run of the test. That point determines which
unwinder gets used, which may affect the test result. Using SIGALRM
really is a bad idea for this test. It might be a bad idea for those
other tests as well.
My concern with using SIGILL (apart from generating an instruction
that forces SIGALL on all architectures we support) is that you're
going to end up testing a different unwinder as well. Typically in
the SIGSEGV case you'll end up at the faulting instruction, which is
defenitely in the function body, where we should be using the DWARF
CFI unwinder. But for SIGILL you could end up at the instruction
after the trapping instruction, which is likely to be in the function
epilogue which may be handled by an epilogue unwinder.
> Maybe, another option is to define invalid instruction for each targets
> in test case.
Perhaps a reasonable compromise is to do something like:
static void
thrower (void)
{
*(char *)0 = 0;
#ifdef __arm__
asm(".word 0xffff");
#endif
}
and then handle both SIGSEGV and SIGILL.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch V2, testsuite] gdb.base/savedregs.exp: SIGSEGV -> SIGALRM
2011-06-20 11:12 ` Mark Kettenis
@ 2011-06-20 15:07 ` Yao Qi
2011-06-20 15:14 ` Paul Koning
0 siblings, 1 reply; 14+ messages in thread
From: Yao Qi @ 2011-06-20 15:07 UTC (permalink / raw)
To: Mark Kettenis; +Cc: gdb-patches
On 06/20/2011 07:10 PM, Mark Kettenis wrote:
> My concern with using SIGILL (apart from generating an instruction
> that forces SIGALL on all architectures we support) is that you're
> going to end up testing a different unwinder as well. Typically in
> the SIGSEGV case you'll end up at the faulting instruction, which is
> defenitely in the function body, where we should be using the DWARF
> CFI unwinder. But for SIGILL you could end up at the instruction
> after the trapping instruction, which is likely to be in the function
> epilogue which may be handled by an epilogue unwinder.
Oh, I don't know PC could be the next instruction of that illegal
instruction. At least, some years ago, when I was working on PowerPC,
PC still points to the illegal instruction when SIGILL is triggered.
>
>> > Maybe, another option is to define invalid instruction for each targets
>> > in test case.
> Perhaps a reasonable compromise is to do something like:
>
> static void
> thrower (void)
> {
> *(char *)0 = 0;
> #ifdef __arm__
> asm(".word 0xffff");
> #endif
> }
>
> and then handle both SIGSEGV and SIGILL.
Yes, that looks good to me in general. It can be like this,
#ifdef __UCLIBC__
#if !(defined(__UCLIBC_HAS_MMU__) || defined(__ARCH_HAS_MMU__))
#define HAS_NOMMU
#endif
#endif
static void
thrower (void)
{
#if defined(HAS_NOMMU)
#if defined(__arm__)
asm(".word 0xffff");
#elif defined(__foo__)
asm(".word 0xeeeee"); // invalid instruction for port foo.
#else
#error Please write an invalid instruction here for your target
#endif
#else
*(char *)0 = 0;
#endif
}
I'll write a new patch later.
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch V2, testsuite] gdb.base/savedregs.exp: SIGSEGV -> SIGALRM
2011-06-20 15:07 ` Yao Qi
@ 2011-06-20 15:14 ` Paul Koning
0 siblings, 0 replies; 14+ messages in thread
From: Paul Koning @ 2011-06-20 15:14 UTC (permalink / raw)
To: Yao Qi; +Cc: Mark Kettenis, gdb-patches
On Jun 20, 2011, at 11:07 AM, Yao Qi wrote:
> On 06/20/2011 07:10 PM, Mark Kettenis wrote:
>> My concern with using SIGILL (apart from generating an instruction
>> that forces SIGALL on all architectures we support) is that you're
>> going to end up testing a different unwinder as well. Typically in
>> the SIGSEGV case you'll end up at the faulting instruction, which is
>> defenitely in the function body, where we should be using the DWARF
>> CFI unwinder. But for SIGILL you could end up at the instruction
>> after the trapping instruction, which is likely to be in the function
>> epilogue which may be handled by an epilogue unwinder.
>
> Oh, I don't know PC could be the next instruction of that illegal
> instruction. At least, some years ago, when I was working on PowerPC,
> PC still points to the illegal instruction when SIGILL is triggered.
Same for MIPS.
More in general, where the PC points for any given exception is very much an architecture dependent question. On some architectures, PC points to the next instruction in a SEGV. On some architecture, PC points to the next instruction on SIGILL. Some architectures have imprecise exceptions where any number of these have a PC pointing somewhere in the vicinity (usually) of the offending instruction, but not a single well-known distance from it.
paul
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2011-06-20 15:14 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-09 9:28 [patch, testsuite] gdb.base/savedregs.exp: SIGSEGV -> SIGILL Yao Qi
2011-06-09 10:08 ` Andreas Schwab
2011-06-09 10:53 ` Yao Qi
2011-06-09 11:19 ` Mark Kettenis
2011-06-09 11:41 ` Pedro Alves
2011-06-09 13:25 ` Yao Qi
2011-06-09 13:10 ` Yao Qi
2011-06-09 14:25 ` Pedro Alves
2011-06-20 4:13 ` [patch V2, testsuite] gdb.base/savedregs.exp: SIGSEGV -> SIGALRM Yao Qi
2011-06-20 7:03 ` Mark Kettenis
2011-06-20 8:26 ` Yao Qi
2011-06-20 11:12 ` Mark Kettenis
2011-06-20 15:07 ` Yao Qi
2011-06-20 15:14 ` Paul Koning
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox