[1/2] PowerPC: fix for gdb.reverse/finish-precsave.exp and gdb.reverse/finish-reverse.exp

Message ID e98fc0fa97c73e1da67414ae5a6f59a9019f4443.camel@us.ibm.com
State New
Headers
Series [1/2] PowerPC: fix for gdb.reverse/finish-precsave.exp and gdb.reverse/finish-reverse.exp |

Commit Message

Carl Love March 1, 2023, 8:59 p.m. UTC
  Tom, Ulrich, Bruno, Pedro, GDB maintainers:

This patch moves the step_until procedure from gdb.reverse/step-
indirect-call-thunk.exp to gdb/testsuite/lib/gdb.exp and renames it
repeat_cmd_until.  There are no functional changes as a result of this
change.

Patch tested on PowerPC and 5th generation X86 with no regression
failures.

                  Carl 

--------------------------------------------------------
Move step_until procedure

Procedure proc_step_until from test gdb.reverse/step-indirect-call-thunk.exp
is moved to lib/gdb.exp and renamed repeat_cmd_until.  The existing proceedure
gdb_step_until in lib/gdb.exp is simpler variant of the new repeat_cmd_until
proceedure.  The existing proceedure gdb_step_until is changed to just call
the new repeat_cmd_until proceedure with the command set to "step" and an
optional CURRENT string.  The default CURRENT string is set to "\}" to work
with the existing uses of proceedure gdb_step_until.
---
 .../gdb.reverse/step-indirect-call-thunk.exp  | 49 +++----------------
 gdb/testsuite/lib/gdb.exp                     | 49 ++++++++++++++-----
 2 files changed, 46 insertions(+), 52 deletions(-)
  

Comments

Guinevere Larsen March 3, 2023, 11:56 a.m. UTC | #1
On 01/03/2023 21:59, Carl Love wrote:
> Tom, Ulrich, Bruno, Pedro, GDB maintainers:
>
> This patch moves the step_until procedure from gdb.reverse/step-
> indirect-call-thunk.exp to gdb/testsuite/lib/gdb.exp and renames it
> repeat_cmd_until.  There are no functional changes as a result of this
> change.
>
> Patch tested on PowerPC and 5th generation X86 with no regression
> failures.
>
>                    Carl
>
> --------------------------------------------------------
The changes seem simple enough and dont change any outputs neither for 
gcc nor clang testing, so:
Reviewed-By: Bruno Larsen <blarsen@redhat.com>
  
Carl Love March 8, 2023, 4:19 p.m. UTC | #2
GCC developers:

Ping.  Just wondering if anyone else has had a chance to review this
patch.  Thanks.

                Carl 

On Fri, 2023-03-03 at 12:56 +0100, Bruno Larsen wrote:
> On 01/03/2023 21:59, Carl Love wrote:
> > Tom, Ulrich, Bruno, Pedro, GDB maintainers:
> > 
> > This patch moves the step_until procedure from gdb.reverse/step-
> > indirect-call-thunk.exp to gdb/testsuite/lib/gdb.exp and renames it
> > repeat_cmd_until.  There are no functional changes as a result of
> > this
> > change.
> > 
> > Patch tested on PowerPC and 5th generation X86 with no regression
> > failures.
> > 
> >                    Carl
> > 
> > --------------------------------------------------------
> The changes seem simple enough and dont change any outputs neither
> for 
> gcc nor clang testing, so:
> Reviewed-By: Bruno Larsen <blarsen@redhat.com>
>
  
Carl Love March 9, 2023, 4:09 p.m. UTC | #3
Oops, typo that should read GDB developers.  Sorry about that.

On Wed, 2023-03-08 at 08:19 -0800, Carl Love wrote:
> GCC developers:
> 
> Ping.  Just wondering if anyone else has had a chance to review this
> patch.  Thanks.
> 
>                 Carl 
> 
> On Fri, 2023-03-03 at 12:56 +0100, Bruno Larsen wrote:
> > On 01/03/2023 21:59, Carl Love wrote:
> > > Tom, Ulrich, Bruno, Pedro, GDB maintainers:
> > > 
> > > This patch moves the step_until procedure from gdb.reverse/step-
> > > indirect-call-thunk.exp to gdb/testsuite/lib/gdb.exp and renames
> > > it
> > > repeat_cmd_until.  There are no functional changes as a result of
> > > this
> > > change.
> > > 
> > > Patch tested on PowerPC and 5th generation X86 with no regression
> > > failures.
> > > 
> > >                    Carl
> > > 
> > > --------------------------------------------------------
> > The changes seem simple enough and dont change any outputs neither
> > for 
> > gcc nor clang testing, so:
> > Reviewed-By: Bruno Larsen <blarsen@redhat.com>
> > 
> 
>
  
Tom Tromey March 9, 2023, 7:03 p.m. UTC | #4
>>>>> "Carl" == Carl Love via Gdb-patches <gdb-patches@sourceware.org> writes:

Carl> Procedure proc_step_until from test gdb.reverse/step-indirect-call-thunk.exp

The "proc_" in there seems incorrect.

Carl>  # Step until the pattern REGEXP is found.  Step at most
Carl>  # MAX_STEPS times, but stop stepping once REGEXP is found.
Carl> -#
Carl> +# CURRENT matches current location
Carl>  # If REGEXP is found then a single pass is emitted, otherwise, after
Carl>  # MAX_STEPS steps, a single fail is emitted.
Carl>  #
Carl>  # TEST_NAME is the name used in the pass/fail calls.
 
Carl> -proc gdb_step_until { regexp {test_name ""} {max_steps 10} } {
Carl> +proc gdb_step_until { regexp {test_name ""} {current ""} \
Carl> +			  { max_steps 10 } } {
Carl> +    if { $current == "" } {
Carl> +	set current "\}"
Carl> +    }
Carl> +    if { $test_name == "" } {
Carl> +	set test_name "stepping until regexp"
Carl> +    }

I think you can just supply these as defaults directly in the proc
definition.

Carl> +# Do repeated stepping COMMANDs in order to reach TARGET from CURRENT
Carl> +#
Carl> +#  COMMAND is a stepping command
Carl> +#  CURRENT is a string matching the current location
Carl> +#  TARGET  is a string matching the target location
Carl> +#  TEST    is the test name
Carl> +#  MAX_STEPS is number of steps attempted before fail is emitted
Carl> +#
Carl> +# The function issues repeated COMMANDs as long as the location matches
Carl> +# CURRENT up to a maximum of 100 steps.

s/100/MAX_STEPS

Carl> +#
Carl> +# TEST passes if the resulting location matches TARGET and fails
Carl> +# otherwise.
Carl> +#
Carl> +proc repeat_cmd_until { command current target test_name {max_steps 100} } {

The docs refer to "TEST" but should use "TEST_NAME".

thanks,
Tom
  
Carl Love March 9, 2023, 9:42 p.m. UTC | #5
On Thu, 2023-03-09 at 12:03 -0700, Tom Tromey wrote:
> > > > > > "Carl" == Carl Love via Gdb-patches <
> > > > > > gdb-patches@sourceware.org> writes:
> 
> Carl> Procedure proc_step_until from test gdb.reverse/step-indirect-
> call-thunk.exp
> 
> The "proc_" in there seems incorrect.

Ah, yea.  Good catch.  Fixed that.

> 
> Carl>  # Step until the pattern REGEXP is found.  Step at most
> Carl>  # MAX_STEPS times, but stop stepping once REGEXP is found.
> Carl> -#
> Carl> +# CURRENT matches current location
> Carl>  # If REGEXP is found then a single pass is emitted, otherwise,
> after
> Carl>  # MAX_STEPS steps, a single fail is emitted.
> Carl>  #
> Carl>  # TEST_NAME is the name used in the pass/fail calls.
> 
> Carl> -proc gdb_step_until { regexp {test_name ""} {max_steps 10} } {
> Carl> +proc gdb_step_until { regexp {test_name ""} {current ""} \
> Carl> +			  { max_steps 10 } } {
> Carl> +    if { $current == "" } {
> Carl> +	set current "\}"
> Carl> +    }
> Carl> +    if { $test_name == "" } {
> Carl> +	set test_name "stepping until regexp"
> Carl> +    }
> 
> I think you can just supply these as defaults directly in the proc
> definition.

True, I guess I was just keeping with the original "style".  But yea,
better to do these as defaults.  Changed to use defaults in this
function and similarly in the the new proceedure gdb_step_until.
> 
> Carl> +# Do repeated stepping COMMANDs in order to reach TARGET from
> CURRENT
> Carl> +#
> Carl> +#  COMMAND is a stepping command
> Carl> +#  CURRENT is a string matching the current location
> Carl> +#  TARGET  is a string matching the target location
> Carl> +#  TEST    is the test name
> Carl> +#  MAX_STEPS is number of steps attempted before fail is
> emitted
> Carl> +#
> Carl> +# The function issues repeated COMMANDs as long as the
> location matches
> Carl> +# CURRENT up to a maximum of 100 steps.
> 
> s/100/MAX_STEPS
OK, changed.
> 
> Carl> +#
> Carl> +# TEST passes if the resulting location matches TARGET and
> fails
> Carl> +# otherwise.
> Carl> +#
> Carl> +proc repeat_cmd_until { command current target test_name
> {max_steps 100} } {
> 
> The docs refer to "TEST" but should use "TEST_NAME".

yup, thanks.  Changed.

Will post version 2.  Thanks for the review.

                            Carl
  

Patch

diff --git a/gdb/testsuite/gdb.reverse/step-indirect-call-thunk.exp b/gdb/testsuite/gdb.reverse/step-indirect-call-thunk.exp
index f433efb11c2..e6c81b80a7b 100644
--- a/gdb/testsuite/gdb.reverse/step-indirect-call-thunk.exp
+++ b/gdb/testsuite/gdb.reverse/step-indirect-call-thunk.exp
@@ -38,39 +38,6 @@  if { ![runto_main] } {
     return -1
 }
 
-# Do repeated stepping COMMANDs in order to reach TARGET from CURRENT
-#
-#  COMMAND is a stepping command
-#  CURRENT is a string matching the current location
-#  TARGET  is a string matching the target location
-#  TEST    is the test name
-#
-# The function issues repeated COMMANDs as long as the location matches
-# CURRENT up to a maximum of 100 steps.
-#
-# TEST passes if the resulting location matches TARGET and fails
-# otherwise.
-#
-proc step_until { command current target test } {
-    global gdb_prompt
-
-    set count 0
-    gdb_test_multiple "$command" "$test" {
-        -re "$current.*$gdb_prompt $" {
-            incr count
-            if { $count < 100 } {
-                send_gdb "$command\n"
-                exp_continue
-            } else {
-                fail "$test"
-            }
-        }
-        -re "$target.*$gdb_prompt $" {
-            pass "$test"
-        }
-    }
-}
-
 gdb_test_no_output "record"
 gdb_test "next" ".*" "record trace"
 
@@ -90,20 +57,20 @@  gdb_test "reverse-next" "apply\.2.*" \
     "reverse-step through thunks and over inc"
 
 # We can use instruction stepping to step into thunks.
-step_until "stepi" "apply\.2" "indirect_thunk" "stepi into call thunk"
-step_until "stepi" "indirect_thunk" "inc" \
+repeat_cmd_until "stepi" "apply\.2" "indirect_thunk" "stepi into call thunk"
+repeat_cmd_until "stepi" "indirect_thunk" "inc" \
     "stepi out of call thunk into inc"
 set alphanum_re "\[a-zA-Z0-9\]"
 set pic_thunk_re  "__$alphanum_re*\\.get_pc_thunk\\.$alphanum_re* \\(\\)"
-step_until "stepi" "(inc|$pic_thunk_re)" "return_thunk" "stepi into return thunk"
-step_until "stepi" "return_thunk" "apply" \
+repeat_cmd_until "stepi" "(inc|$pic_thunk_re)" "return_thunk" "stepi into return thunk"
+repeat_cmd_until "stepi" "return_thunk" "apply" \
     "stepi out of return thunk back into apply"
 
-step_until "reverse-stepi" "apply" "return_thunk" \
+repeat_cmd_until "reverse-stepi" "apply" "return_thunk" \
     "reverse-stepi into return thunk"
-step_until "reverse-stepi" "return_thunk" "inc" \
+repeat_cmd_until "reverse-stepi" "return_thunk" "inc" \
     "reverse-stepi out of return thunk into inc"
-step_until "reverse-stepi" "(inc|$pic_thunk_re)" "indirect_thunk" \
+repeat_cmd_until "reverse-stepi" "(inc|$pic_thunk_re)" "indirect_thunk" \
     "reverse-stepi into call thunk"
-step_until "reverse-stepi" "indirect_thunk" "apply" \
+repeat_cmd_until "reverse-stepi" "indirect_thunk" "apply" \
     "reverse-stepi out of call thunk into apply"
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 19c782bea46..2b2a554f4c7 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -9270,31 +9270,58 @@  gdb_caching_proc arm_cc_for_target {
 
 # Step until the pattern REGEXP is found.  Step at most
 # MAX_STEPS times, but stop stepping once REGEXP is found.
-#
+# CURRENT matches current location
 # If REGEXP is found then a single pass is emitted, otherwise, after
 # MAX_STEPS steps, a single fail is emitted.
 #
 # TEST_NAME is the name used in the pass/fail calls.
 
-proc gdb_step_until { regexp {test_name ""} {max_steps 10} } {
+proc gdb_step_until { regexp {test_name ""} {current ""} \
+			  { max_steps 10 } } {
+    if { $current == "" } {
+	set current "\}"
+    }
+    if { $test_name == "" } {
+	set test_name "stepping until regexp"
+    }
+
+    repeat_cmd_until "step" $current  $regexp  $test_name "10"
+}
+
+# Do repeated stepping COMMANDs in order to reach TARGET from CURRENT
+#
+#  COMMAND is a stepping command
+#  CURRENT is a string matching the current location
+#  TARGET  is a string matching the target location
+#  TEST    is the test name
+#  MAX_STEPS is number of steps attempted before fail is emitted
+#
+# The function issues repeated COMMANDs as long as the location matches
+# CURRENT up to a maximum of 100 steps.
+#
+# TEST passes if the resulting location matches TARGET and fails
+# otherwise.
+#
+proc repeat_cmd_until { command current target test_name {max_steps 100} } {
+    global gdb_prompt
     if { $test_name == "" } {
 	set test_name "stepping until regexp"
     }
 
     set count 0
-    gdb_test_multiple "step" "$test_name" {
-	-re "$regexp\r\n$::gdb_prompt $" {
-	    pass $test_name
-	}
-	-re ".*$::gdb_prompt $" {
-	    if {$count < $max_steps} {
-		incr count
-		send_gdb "step\n"
+    gdb_test_multiple "$command" "$test_name" {
+	-re "$current.*$gdb_prompt $" {
+	    incr count
+	    if { $count < $max_steps } {
+		send_gdb "$command\n"
 		exp_continue
 	    } else {
-		fail $test_name
+		fail "$test_name"
 	    }
 	}
+	-re "$target.*$gdb_prompt $" {
+	    pass "$test_name"
+	}
     }
 }