public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Fix for the gdb.base/sigstep.exp test.
@ 2021-11-08 18:26 Carl Love
  2021-11-10 23:17 ` Tom de Vries
  0 siblings, 1 reply; 6+ messages in thread
From: Carl Love @ 2021-11-08 18:26 UTC (permalink / raw)
  To: gdb-patches, cel, Will Schmidt, Rogerio Alves




GDB maintainers:

The gdb.base/sigstep.exp has 6 fails on the PowerPC platforms.  Th
following patch fixes the minor differences of how the test runs on
PowerPC versus X86.  The differences have to do with the number of
assembly instructions generated for the two systems and where gdb stops
at the beginning of a function.  On PowerPC, gdb stops in the prolog
rather than at the first line of code.  

The fix adds an addtional stepi in one case and an additional step in
the other case to get to the expected location in the code.

The patch only consists of changes to the test.  No GDB internal
changes are made.

The patch was also tested on X86 with no regression failures. 

Please let me know if the patch is acceptable for mainline.  Thanks.

                 Carl Love

---------------------------------------------------------------------

This patch fixes the test for a few differences in how the test runs on
PPC.  The firs fix adds a gdb_test_multiple to check for PPC.  The assembly
sequence on PPC reqires two stepi commands to get into the handler where as
it only takes one stepi on x86.

The second change in proc advance is due to gdb stopping int the prolog of
the function on PPC.  Specificially it stops at the opening brace for the
function not with the first line of code.  The patch adds an extra step for
PPC to get gdb to the expected location.
---
 gdb/testsuite/gdb.base/sigstep.exp | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/sigstep.exp b/gdb/testsuite/gdb.base/sigstep.exp
index ea254af5297..dca9d39d1b6 100644
--- a/gdb/testsuite/gdb.base/sigstep.exp
+++ b/gdb/testsuite/gdb.base/sigstep.exp
@@ -93,7 +93,17 @@ proc advance { enter_cmd in_handler_prefix in_handler exit_cmd } {
 	    gdb_test "handle SIGVTALRM print pass stop"
 	    gdb_test "continue" "Program received signal.*" "continue to signal"
 	}
-	gdb_test "$enter_cmd" ".*handler .*" "$enter_cmd to handler"
+
+	set test_cmd ".*handler .*"
+	gdb_test_multiple "$enter_cmd" "$enter_cmd to handler" {
+	    -re "handler .*sig=.*" {
+		pass "$test_cmd"
+	    }
+	    -re ".*signal handler called.*" {
+	    #ppc is at branch instruction, need one more stepi to get to handler
+	    gdb_test "$enter_cmd" ".*handler .*" "$enter_cmd to handler ppc"
+	    }
+	}
 
 	delete_breakpoints
 
@@ -113,6 +123,11 @@ proc advance { enter_cmd in_handler_prefix in_handler exit_cmd } {
 		setup_kfail gdb/8841 "nios2*-*-linux*"
 		fail "$test (could not insert single-step breakpoint)"
 	    }
+	    -re ".*\{.*${gdb_prompt} $" {
+		#ppc stops at the opening brace, step to get to the first done=1
+		send_gdb "step\n"
+		exp_continue -continue_timer
+	    }
 	    -re "done = 1;.*${gdb_prompt} $" {
 		send_gdb "$exit_cmd\n"
 		exp_continue -continue_timer
-- 
2.30.2



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix for the gdb.base/sigstep.exp test.
  2021-11-08 18:26 [PATCH] Fix for the gdb.base/sigstep.exp test Carl Love
@ 2021-11-10 23:17 ` Tom de Vries
  2021-11-12  0:28   ` Carl Love
  0 siblings, 1 reply; 6+ messages in thread
From: Tom de Vries @ 2021-11-10 23:17 UTC (permalink / raw)
  To: Carl Love, gdb-patches, Will Schmidt, Rogerio Alves

On 11/8/21 7:26 PM, Carl Love via Gdb-patches wrote:
> 
> 
> 
> GDB maintainers:
> 
> The gdb.base/sigstep.exp has 6 fails on the PowerPC platforms.  Th
> following patch fixes the minor differences of how the test runs on
> PowerPC versus X86.  The differences have to do with the number of
> assembly instructions generated for the two systems and where gdb stops
> at the beginning of a function.  On PowerPC, gdb stops in the prolog
> rather than at the first line of code.  
> 
> The fix adds an addtional stepi in one case and an additional step in
> the other case to get to the expected location in the code.
> 
> The patch only consists of changes to the test.  No GDB internal
> changes are made.
> 
> The patch was also tested on X86 with no regression failures. 
> 
> Please let me know if the patch is acceptable for mainline.  Thanks.
> 
>                  Carl Love
> 
> ---------------------------------------------------------------------
> 

I ran the test-case on ppc64le-linux, and got 6 FAILs as well:
...
FAIL: gdb.base/sigstep.exp: stepi to handler, nothing in handler, step
from handler: leave handler
FAIL: gdb.base/sigstep.exp: stepi to handler, nothing in handler, next
from handler: leave handler
FAIL: gdb.base/sigstep.exp: stepi to handler, nothing in handler,
continue from handler: break clear done (got interactive prompt)
FAIL: gdb.base/sigstep.exp: nexti to handler, nothing in handler, step
from handler: leave handler
FAIL: gdb.base/sigstep.exp: nexti to handler, nothing in handler, next
from handler: leave handler
FAIL: gdb.base/sigstep.exp: nexti to handler, nothing in handler,
continue from handler: break clear done (got interactive prompt)
...

The first FAIL looks like this:
...
(gdb) PASS: gdb.base/sigstep.exp: stepi to handler, nothing in handler,
step from handler: continue to signal
stepi^M
<signal handler called>^M
1: x/i $pc^M
=> 0x7ffff7f804c0 <__kernel_start_sigtramp_rt64>:       bctrl^M
(gdb) PASS: gdb.base/sigstep.exp: stepi to handler, nothing in handler,
step from handler: stepi to handler
delete breakpoints^M
Delete all breakpoints? (y or n) y^M
(gdb) info breakpoints^M
No breakpoints or watchpoints.^M
(gdb) step^M
handler (sig=1685382480) at
/suse/tdevries/gdb/src/gdb/testsuite/gdb.base/sigstep.c:32^M
32      {^M
1: x/i $pc^M
=> 0x1000081c <handler>:        lis     r2,4098^M
(gdb) FAIL: gdb.base/sigstep.exp: stepi to handler, nothing in handler,
step from handler: leave handler
...

The first difference with x86_64 though is where the first stepi gets
us.  On x86_64-linux, it's:
...
gdb) PASS: gdb.base/sigstep.exp: stepi to handler, nothing in handler,
step from handler: continue to signal
stepi^M
handler (sig=0) at
/home/vries/gdb_versions/devel/src/gdb/testsuite/gdb.base/sigstep.c:32^M
32      {^M
1: x/i $pc^M
=> 0x400657 <handler>:  push   %rbp^M
(gdb) PASS: gdb.base/sigstep.exp: stepi to handler, nothing in handler,
step from handler: stepi to handler
...
while on ppc it gets us "<signal handler called>".

> This patch fixes the test for a few differences in how the test runs on
> PPC.  The firs fix adds a gdb_test_multiple to check for PPC.  The assembly
> sequence on PPC reqires two stepi commands to get into the handler where as
> it only takes one stepi on x86.

The description here does not tell us why two stepi commands are
required, while its obvious if you show it.  I'd include the relevant
part of the gdb log in the commit message to clarify why you're making
the change.

Anyway, using this change all 6 FAILs disappear.

> 
> The second change in proc advance is due to gdb stopping int the prolog of
> the function on PPC.  Specificially it stops at the opening brace for the
> function not with the first line of code.  The patch adds an extra step for
> PPC to get gdb to the expected location.

Do you really need this bit?  If so, can you show why?

> ---
>  gdb/testsuite/gdb.base/sigstep.exp | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/testsuite/gdb.base/sigstep.exp b/gdb/testsuite/gdb.base/sigstep.exp
> index ea254af5297..dca9d39d1b6 100644
> --- a/gdb/testsuite/gdb.base/sigstep.exp
> +++ b/gdb/testsuite/gdb.base/sigstep.exp
> @@ -93,7 +93,17 @@ proc advance { enter_cmd in_handler_prefix in_handler exit_cmd } {
>  	    gdb_test "handle SIGVTALRM print pass stop"
>  	    gdb_test "continue" "Program received signal.*" "continue to signal"
>  	}
> -	gdb_test "$enter_cmd" ".*handler .*" "$enter_cmd to handler"
> +
> +	set test_cmd ".*handler .*"

The second parameter of gdb_test is a regexp, not a command.

> +	gdb_test_multiple "$enter_cmd" "$enter_cmd to handler" {
> +	    -re "handler .*sig=.*" {

Both regexps are missing a -wrap, otherwise you're not consuming the prompt.

> +		pass "$test_cmd"

The name of the test is "$enter_cmd to handler", and it's available as
$gdb_test_name.

> +	    }
> +	    -re ".*signal handler called.*" {
> +	    #ppc is at branch instruction, need one more stepi to get to handler
> +	    gdb_test "$enter_cmd" ".*handler .*" "$enter_cmd to handler ppc"
> +	    }
> +	}

[ Missing indentation. ]

I guess it's easier to reuse the first regexp using exp_continue rather
than using another gdb_test.  If not, you can also assign the regexp to
a variable, and use that one twice.

So, this is what I got:
...
diff --git a/gdb/testsuite/gdb.base/sigstep.exp
b/gdb/testsuite/gdb.base/sigstep.exp
index ea254af5297..176918b67d6 100644
--- a/gdb/testsuite/gdb.base/sigstep.exp
+++ b/gdb/testsuite/gdb.base/sigstep.exp
@@ -79,6 +79,7 @@ validate_backtrace
 proc advance { enter_cmd in_handler_prefix in_handler exit_cmd } {
     global gdb_prompt inferior_exited_re
     global clear_done other_handler_location
+    global decimal

     set prefix "$enter_cmd to handler, $in_handler_prefix in handler,
$exit_cmd from handler"

@@ -93,7 +94,16 @@ proc advance { enter_cmd in_handler_prefix in_handler
exit_cmd } {
            gdb_test "handle SIGVTALRM print pass stop"
            gdb_test "continue" "Program received signal.*" "continue to
signal"
        }
-       gdb_test "$enter_cmd" ".*handler .*" "$enter_cmd to handler"
+
+       gdb_test_multiple "$enter_cmd" "$enter_cmd to handler" {
+           -re -wrap "\r\n<signal handler called>.*" {
+               send_gdb "$enter_cmd\n"
+               exp_continue
+           }
+           -re -wrap "\r\n(Breakpoint $decimal, )?handler \\(sig=.*" {
+               pass $gdb_test_name
+           }
+       }

        delete_breakpoints

...

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] Fix for the gdb.base/sigstep.exp test.
  2021-11-10 23:17 ` Tom de Vries
@ 2021-11-12  0:28   ` Carl Love
  2021-11-12  8:08     ` Tom de Vries
  0 siblings, 1 reply; 6+ messages in thread
From: Carl Love @ 2021-11-12  0:28 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches, Will Schmidt, Rogerio Alves

Tom:

> 
> The description here does not tell us why two stepi commands are
> required, while its obvious if you show it.  I'd include the relevant
> part of the gdb log in the commit message to clarify why you're
> making
> the change.
> 
> Anyway, using this change all 6 FAILs disappear.
> 
> > The second change in proc advance is due to gdb stopping int the
> > prolog of
> > the function on PPC.  Specificially it stops at the opening brace
> > for the
> > function not with the first line of code.  The patch adds an extra
> > step for
> > PPC to get gdb to the expected location.
> 
> Do you really need this bit?  If so, can you show why?

I actually made the "second change" discussed above.  It fixed 2 of the
errors.  Then I did the "first change" in the patch.  I went back and
is seems the "second change" is no longer needed.  The bottom line is
we only need one fix to take care of all failures.

> 
> > ---
> >  gdb/testsuite/gdb.base/sigstep.exp | 17 ++++++++++++++++-
> >  1 file changed, 16 insertions(+), 1 deletion(-)
> > 
> > diff --git a/gdb/testsuite/gdb.base/sigstep.exp
> > b/gdb/testsuite/gdb.base/sigstep.exp
> > index ea254af5297..dca9d39d1b6 100644
> > --- a/gdb/testsuite/gdb.base/sigstep.exp
> > +++ b/gdb/testsuite/gdb.base/sigstep.exp
> > @@ -93,7 +93,17 @@ proc advance { enter_cmd in_handler_prefix
> > in_handler exit_cmd } {
> >  	    gdb_test "handle SIGVTALRM print pass stop"
> >  	    gdb_test "continue" "Program received signal.*" "continue
> > to signal"
> >  	}
> > -	gdb_test "$enter_cmd" ".*handler .*" "$enter_cmd to handler"
> > +
> > +	set test_cmd ".*handler .*"
> 
> The second parameter of gdb_test is a regexp, not a command.
> 
> > +	gdb_test_multiple "$enter_cmd" "$enter_cmd to handler" {
> > +	    -re "handler .*sig=.*" {
> 
> Both regexps are missing a -wrap, otherwise you're not consuming the
> prompt.
> 
> > +		pass "$test_cmd"
> 
> The name of the test is "$enter_cmd to handler", and it's available
> as
> $gdb_test_name.
> 
> > +	    }
> > +	    -re ".*signal handler called.*" {
> > +	    #ppc is at branch instruction, need one more stepi to get
> > to handler
> > +	    gdb_test "$enter_cmd" ".*handler .*" "$enter_cmd to handler
> > ppc"
> > +	    }
> > +	}
> 
> [ Missing indentation. ]
> 
> I guess it's easier to reuse the first regexp using exp_continue
> rather
> than using another gdb_test.  If not, you can also assign the regexp
> to
> a variable, and use that one twice.

I like this fix better than what I had did.  

Below is the updated patch.  I has been tested on PPC and Intel.  The
test passes on both platforms without errors.

                          Carl 

-----------------------------------------------------
Fix for the sigstep.exp test.

The test stops at <signal_handler called> which is the call to the handler
rather than in the handler as intended.  This patch replaces the
gdb_test "$enter_cmd to handler" with a gdb_test_multiple test.  The multiple
test looks for the stop at <signal_handler called>.  If found, the command
is issued again.  The test passes if gdb stops in the handler as expected.


(gdb) PASS: gdb.base/sigstep.exp: stepi to handler, nothing in handler, step fr\
om handler: continue to signal
stepi
<signal handler called>
1: x/i $pc
=> 0x7ffff7f80440 <__kernel_start_sigtramp_rt64>:       bctrl
(gdb) stepi
handler (sig=551) at /home/carll/GDB/build-current/gdb/testsuite/../../../binut\
ils-gdb-current/gdb/testsuite/gdb.base/sigstep.c:32
32      {
1: x/i $pc
=> 0x10000097c <handler>:       addis   r2,r12,2
(gdb) PASS: gdb.base/sigstep.exp: stepi to handler, nothing in handler,
step from handler: stepi to handler
---
 gdb/testsuite/gdb.base/sigstep.exp | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/sigstep.exp b/gdb/testsuite/gdb.base/sigstep.exp
index ea254af5297..7bdadfd5823 100644
--- a/gdb/testsuite/gdb.base/sigstep.exp
+++ b/gdb/testsuite/gdb.base/sigstep.exp
@@ -79,6 +79,7 @@ validate_backtrace
 proc advance { enter_cmd in_handler_prefix in_handler exit_cmd } {
     global gdb_prompt inferior_exited_re
     global clear_done other_handler_location
+    global decimal
 
     set prefix "$enter_cmd to handler, $in_handler_prefix in handler, $exit_cmd from handler"
 
@@ -93,7 +94,17 @@ proc advance { enter_cmd in_handler_prefix in_handler exit_cmd } {
 	    gdb_test "handle SIGVTALRM print pass stop"
 	    gdb_test "continue" "Program received signal.*" "continue to signal"
 	}
-	gdb_test "$enter_cmd" ".*handler .*" "$enter_cmd to handler"
+
+	set test_cmd ".*handler .*"
+	gdb_test_multiple "$enter_cmd" "$enter_cmd to handler" {
+	    -re -wrap "\r\n<signal handler called>.*" {
+		send_gdb "$enter_cmd\n"
+		exp_continue
+	    }
+	    -re -wrap "\r\n(Breakpoint $decimal, )?handler \\(sig=.*" {
+		pass $gdb_test_name
+	    }
+	}
 
 	delete_breakpoints
 
-- 
2.30.2



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix for the gdb.base/sigstep.exp test.
  2021-11-12  0:28   ` Carl Love
@ 2021-11-12  8:08     ` Tom de Vries
  2021-11-12 16:52       ` Carl Love
  0 siblings, 1 reply; 6+ messages in thread
From: Tom de Vries @ 2021-11-12  8:08 UTC (permalink / raw)
  To: Carl Love, gdb-patches, Will Schmidt, Rogerio Alves

On 11/12/21 1:28 AM, Carl Love wrote:
> Tom:
> 
>>
>> The description here does not tell us why two stepi commands are
>> required, while its obvious if you show it.  I'd include the relevant
>> part of the gdb log in the commit message to clarify why you're
>> making
>> the change.
>>
>> Anyway, using this change all 6 FAILs disappear.
>>
>>> The second change in proc advance is due to gdb stopping int the
>>> prolog of
>>> the function on PPC.  Specificially it stops at the opening brace
>>> for the
>>> function not with the first line of code.  The patch adds an extra
>>> step for
>>> PPC to get gdb to the expected location.
>>
>> Do you really need this bit?  If so, can you show why?
> 
> I actually made the "second change" discussed above.  It fixed 2 of the
> errors.  Then I did the "first change" in the patch.  I went back and
> is seems the "second change" is no longer needed.  The bottom line is
> we only need one fix to take care of all failures.
> 
>>
>>> ---
>>>  gdb/testsuite/gdb.base/sigstep.exp | 17 ++++++++++++++++-
>>>  1 file changed, 16 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/gdb/testsuite/gdb.base/sigstep.exp
>>> b/gdb/testsuite/gdb.base/sigstep.exp
>>> index ea254af5297..dca9d39d1b6 100644
>>> --- a/gdb/testsuite/gdb.base/sigstep.exp
>>> +++ b/gdb/testsuite/gdb.base/sigstep.exp
>>> @@ -93,7 +93,17 @@ proc advance { enter_cmd in_handler_prefix
>>> in_handler exit_cmd } {
>>>  	    gdb_test "handle SIGVTALRM print pass stop"
>>>  	    gdb_test "continue" "Program received signal.*" "continue
>>> to signal"
>>>  	}
>>> -	gdb_test "$enter_cmd" ".*handler .*" "$enter_cmd to handler"
>>> +
>>> +	set test_cmd ".*handler .*"
>>
>> The second parameter of gdb_test is a regexp, not a command.
>>
>>> +	gdb_test_multiple "$enter_cmd" "$enter_cmd to handler" {
>>> +	    -re "handler .*sig=.*" {
>>
>> Both regexps are missing a -wrap, otherwise you're not consuming the
>> prompt.
>>
>>> +		pass "$test_cmd"
>>
>> The name of the test is "$enter_cmd to handler", and it's available
>> as
>> $gdb_test_name.
>>
>>> +	    }
>>> +	    -re ".*signal handler called.*" {
>>> +	    #ppc is at branch instruction, need one more stepi to get
>>> to handler
>>> +	    gdb_test "$enter_cmd" ".*handler .*" "$enter_cmd to handler
>>> ppc"
>>> +	    }
>>> +	}
>>
>> [ Missing indentation. ]
>>
>> I guess it's easier to reuse the first regexp using exp_continue
>> rather
>> than using another gdb_test.  If not, you can also assign the regexp
>> to
>> a variable, and use that one twice.
> 
> I like this fix better than what I had did.  
> 
> Below is the updated patch.  I has been tested on PPC and Intel.  The
> test passes on both platforms without errors.
> 
>                           Carl 
> 
> -----------------------------------------------------
> Fix for the sigstep.exp test.
> 
> The test stops at <signal_handler called> which is the call to the handler
> rather than in the handler as intended.  This patch replaces the
> gdb_test "$enter_cmd to handler" with a gdb_test_multiple test.  The multiple
> test looks for the stop at <signal_handler called>.  If found, the command
> is issued again.  The test passes if gdb stops in the handler as expected.
> 
> 
> (gdb) PASS: gdb.base/sigstep.exp: stepi to handler, nothing in handler, step fr\
> om handler: continue to signal

Hm, what happened here?

Is this an artifact of how you send the patch?  In that case, check out
this (
https://sourceware.org/gdb/wiki/ContributionChecklist#Submitting_patches
) on how to avoid this problem.

[ Otherwise, if you do want to wrap the line in the commit message,
break it outside of a word, f.i. like so:
...
(gdb) PASS: gdb.base/sigstep.exp: stepi to handler, \
  nothing in handler, step from handler: continue to signal...
... ]

> stepi
> <signal handler called>
> 1: x/i $pc
> => 0x7ffff7f80440 <__kernel_start_sigtramp_rt64>:       bctrl
> (gdb) stepi
> handler (sig=551) at /home/carll/GDB/build-current/gdb/testsuite/../../../binut\
> ils-gdb-current/gdb/testsuite/gdb.base/sigstep.c:32

FWIW, I tend to hand-edit this sort of line into:
...
> handler (sig=551) at sigstep.c:32
...
The dir name is not relevant to the problem at hand, and it makes it
easier to read the log.  But that's a personal preference.

> 32      {
> 1: x/i $pc
> => 0x10000097c <handler>:       addis   r2,r12,2
> (gdb) PASS: gdb.base/sigstep.exp: stepi to handler, nothing in handler,
> step from handler: stepi to handler
> ---
>  gdb/testsuite/gdb.base/sigstep.exp | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/testsuite/gdb.base/sigstep.exp b/gdb/testsuite/gdb.base/sigstep.exp
> index ea254af5297..7bdadfd5823 100644
> --- a/gdb/testsuite/gdb.base/sigstep.exp
> +++ b/gdb/testsuite/gdb.base/sigstep.exp
> @@ -79,6 +79,7 @@ validate_backtrace
>  proc advance { enter_cmd in_handler_prefix in_handler exit_cmd } {
>      global gdb_prompt inferior_exited_re
>      global clear_done other_handler_location
> +    global decimal
>  
>      set prefix "$enter_cmd to handler, $in_handler_prefix in handler, $exit_cmd from handler"
>  
> @@ -93,7 +94,17 @@ proc advance { enter_cmd in_handler_prefix in_handler exit_cmd } {
>  	    gdb_test "handle SIGVTALRM print pass stop"
>  	    gdb_test "continue" "Program received signal.*" "continue to signal"
>  	}
> -	gdb_test "$enter_cmd" ".*handler .*" "$enter_cmd to handler"
> +
> +	set test_cmd ".*handler .*"

Unused variable, please remove.

> +	gdb_test_multiple "$enter_cmd" "$enter_cmd to handler" {
> +	    -re -wrap "\r\n<signal handler called>.*" {
> +		send_gdb "$enter_cmd\n"
> +		exp_continue
> +	    }
> +	    -re -wrap "\r\n(Breakpoint $decimal, )?handler \\(sig=.*" {
> +		pass $gdb_test_name
> +	    }
> +	}
>  
>  	delete_breakpoints
>  
> 

Now, some nit-picking ...

About $subject.  Do I understand correctly that you changed $subject to
"Fix for the sigstep.exp test."?  I liked "Fix for the
gdb.base/sigstep.exp test." better, it has gdb in there, which is useful
for a repository shared with other projects.

Please drop the dot in $subject, it's counter-style.

You could also be more specific and mention ppc in there, f.i.:
...
Fix gdb.base/sigstep.exp test for ppc"
...
or some such.

Finally, add a line in the commit message mentioning the testing that
you did.  The format we use is specific about architecture and os.  So,
for example, for my setup I might mention:
...
Tested on x86_64-linux and ppc64le-linux.
...

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] Fix for the gdb.base/sigstep.exp test.
  2021-11-12  8:08     ` Tom de Vries
@ 2021-11-12 16:52       ` Carl Love
  2021-11-12 17:15         ` Tom de Vries
  0 siblings, 1 reply; 6+ messages in thread
From: Carl Love @ 2021-11-12 16:52 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches, Will Schmidt, Rogerio Alves

Tom:

I updated the commit log as suggested and removed the unused variable
set from the patch.  The patch has been retested on PowerPC and X86.

I beleve I have fixed all of your comments on the patch.  Thank you for
your feedback and input to improve the patch.

Please let me know if this is now acceptable for committing to
mainline.

                Carl 

-----------------------------------------------------------------

Fix gdb.base/sigstep.exp test for ppc

The test stops at <signal_handler called> which is the call to the handler
rather than in the handler as intended.  This patch replaces the
gdb_test "$enter_cmd to handler" with a gdb_test_multiple test.  The multiple
test looks for the stop at <signal_handler called>.  If found, the command
is issued again.  The test passes if gdb stops in the handler as expected.


(gdb) PASS: gdb.base/sigstep.exp: stepi to handler, nothing in handler, step
from handler: continue to signal
stepi
<signal handler called>
1: x/i $pc
=> 0x7ffff7f80440 <__kernel_start_sigtramp_rt64>:       bctrl
(gdb) stepi
handler (sig=551) at sigstep.c:32
32      {
1: x/i $pc
=> 0x10000097c <handler>:       addis   r2,r12,2
(gdb) PASS: gdb.base/sigstep.exp: stepi to handler, nothing in handler,
step from handler: stepi to handler

Patch has been tested on x86_64-linux and ppc64le-linux with no test failures.

---
 gdb/testsuite/gdb.base/sigstep.exp | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/sigstep.exp b/gdb/testsuite/gdb.base/sigstep.exp
index ea254af5297..176918b67d6 100644
--- a/gdb/testsuite/gdb.base/sigstep.exp
+++ b/gdb/testsuite/gdb.base/sigstep.exp
@@ -79,6 +79,7 @@ validate_backtrace
 proc advance { enter_cmd in_handler_prefix in_handler exit_cmd } {
     global gdb_prompt inferior_exited_re
     global clear_done other_handler_location
+    global decimal
 
     set prefix "$enter_cmd to handler, $in_handler_prefix in handler, $exit_cmd from handler"
 
@@ -93,7 +94,16 @@ proc advance { enter_cmd in_handler_prefix in_handler exit_cmd } {
 	    gdb_test "handle SIGVTALRM print pass stop"
 	    gdb_test "continue" "Program received signal.*" "continue to signal"
 	}
-	gdb_test "$enter_cmd" ".*handler .*" "$enter_cmd to handler"
+
+	gdb_test_multiple "$enter_cmd" "$enter_cmd to handler" {
+	    -re -wrap "\r\n<signal handler called>.*" {
+		send_gdb "$enter_cmd\n"
+		exp_continue
+	    }
+	    -re -wrap "\r\n(Breakpoint $decimal, )?handler \\(sig=.*" {
+		pass $gdb_test_name
+	    }
+	}
 
 	delete_breakpoints
 
-- 
2.30.2



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix for the gdb.base/sigstep.exp test.
  2021-11-12 16:52       ` Carl Love
@ 2021-11-12 17:15         ` Tom de Vries
  0 siblings, 0 replies; 6+ messages in thread
From: Tom de Vries @ 2021-11-12 17:15 UTC (permalink / raw)
  To: Carl Love, gdb-patches, Will Schmidt, Rogerio Alves

On 11/12/21 5:52 PM, Carl Love wrote:
> Tom:
> 
> I updated the commit log as suggested and removed the unused variable
> set from the patch.  The patch has been retested on PowerPC and X86.
> 
> I beleve I have fixed all of your comments on the patch.  Thank you for
> your feedback and input to improve the patch.
> 
> Please let me know if this is now acceptable for committing to
> mainline.
> 

Ok for committing to mainline, thanks for working on this.  I'm looking
forward to seeing the FAIL count drop yet further :)

Thanks,
- Tom


>                 Carl 
> 
> -----------------------------------------------------------------
> 
> Fix gdb.base/sigstep.exp test for ppc
> 
> The test stops at <signal_handler called> which is the call to the handler
> rather than in the handler as intended.  This patch replaces the
> gdb_test "$enter_cmd to handler" with a gdb_test_multiple test.  The multiple
> test looks for the stop at <signal_handler called>.  If found, the command
> is issued again.  The test passes if gdb stops in the handler as expected.
> 
> 
> (gdb) PASS: gdb.base/sigstep.exp: stepi to handler, nothing in handler, step
> from handler: continue to signal
> stepi
> <signal handler called>
> 1: x/i $pc
> => 0x7ffff7f80440 <__kernel_start_sigtramp_rt64>:       bctrl
> (gdb) stepi
> handler (sig=551) at sigstep.c:32
> 32      {
> 1: x/i $pc
> => 0x10000097c <handler>:       addis   r2,r12,2
> (gdb) PASS: gdb.base/sigstep.exp: stepi to handler, nothing in handler,
> step from handler: stepi to handler
> 
> Patch has been tested on x86_64-linux and ppc64le-linux with no test failures.
> 
> ---
>  gdb/testsuite/gdb.base/sigstep.exp | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/testsuite/gdb.base/sigstep.exp b/gdb/testsuite/gdb.base/sigstep.exp
> index ea254af5297..176918b67d6 100644
> --- a/gdb/testsuite/gdb.base/sigstep.exp
> +++ b/gdb/testsuite/gdb.base/sigstep.exp
> @@ -79,6 +79,7 @@ validate_backtrace
>  proc advance { enter_cmd in_handler_prefix in_handler exit_cmd } {
>      global gdb_prompt inferior_exited_re
>      global clear_done other_handler_location
> +    global decimal
>  
>      set prefix "$enter_cmd to handler, $in_handler_prefix in handler, $exit_cmd from handler"
>  
> @@ -93,7 +94,16 @@ proc advance { enter_cmd in_handler_prefix in_handler exit_cmd } {
>  	    gdb_test "handle SIGVTALRM print pass stop"
>  	    gdb_test "continue" "Program received signal.*" "continue to signal"
>  	}
> -	gdb_test "$enter_cmd" ".*handler .*" "$enter_cmd to handler"
> +
> +	gdb_test_multiple "$enter_cmd" "$enter_cmd to handler" {
> +	    -re -wrap "\r\n<signal handler called>.*" {
> +		send_gdb "$enter_cmd\n"
> +		exp_continue
> +	    }
> +	    -re -wrap "\r\n(Breakpoint $decimal, )?handler \\(sig=.*" {
> +		pass $gdb_test_name
> +	    }
> +	}
>  
>  	delete_breakpoints
>  
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-11-12 17:15 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-08 18:26 [PATCH] Fix for the gdb.base/sigstep.exp test Carl Love
2021-11-10 23:17 ` Tom de Vries
2021-11-12  0:28   ` Carl Love
2021-11-12  8:08     ` Tom de Vries
2021-11-12 16:52       ` Carl Love
2021-11-12 17:15         ` Tom de Vries

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).