diff mbox

[01/16] Eliminate literal line numbers in so-impl-ld.exp

Message ID 87h9ymag24.fsf@br87z6lw.de.ibm.com
State New
Headers show

Commit Message

Andreas Arnez Oct. 29, 2014, 6:25 p.m. UTC
Hi Sergio,

Thanks for reviewing!

On Wed, Oct 29 2014, Sergio Durigan Junior wrote:

> On Wednesday, October 29 2014, Andreas Arnez wrote:
>
> [...]
>> diff --git a/gdb/testsuite/gdb.base/so-impl-ld.exp b/gdb/testsuite/gdb.base/so-impl-ld.exp
>> index 073c3df..03d10a5 100644
>> --- a/gdb/testsuite/gdb.base/so-impl-ld.exp
>> +++ b/gdb/testsuite/gdb.base/so-impl-ld.exp
>> @@ -61,12 +61,12 @@ gdb_test "next" "21\[ \t\]*result = solib_main .result.;" \
>>  
>>  # Verify that we can step into the second shlib call.
>>  #
>> -gdb_test "step" "solib_main .arg=10000. at.*${libfile}.c:17.*" \
>> +gdb_test "step" "solib_main .arg=10000. at.*${libfile}.c:.* HERE .*" \
>>      "step into solib call"
>
> Can't you use ${decimal} here, instead of expecting anything?

Certainly.

>
>>  # Verify that we can step within the shlib call.
>>  #
>> -gdb_test "next" "18\[ \t\]*\}" "step in solib call"
>> +gdb_test "next" "\[0-9\]+\[ \t\]*\}.* STEP .*" "step in solib call"
>
> Same here: ${decimal} is made to replace this \[0-9\]+.

Good point.  Changed both as suggested.  Also replaced \[0-9\]+ by
${decimal} in the patches for dbx.exp, call-ar-st.exp, call-rt-st.exp,
jump.exp, and shlib-call.exp.  While reviewing again I also noticed that
I introduced an excess ".*" in front of a regexp in foll-exec.exp and
removed it.

Note that some regexps are currently enclosed in a brace-quoted group,
where variables are not substituted -- particularly in the various uses
of gdb_test_sequence in call-ar-st.exp.  There I still use \[0-9\]+, to
avoid larger modifications to the existing regexps.

Here are the changes I performed based on your suggestion:

Comments

Sergio Durigan Junior Oct. 29, 2014, 10:11 p.m. UTC | #1
On Wednesday, October 29 2014, Andreas Arnez wrote:

> Hi Sergio,
>
> Thanks for reviewing!

Thank you, Andreas!

> On Wed, Oct 29 2014, Sergio Durigan Junior wrote:
>
>> On Wednesday, October 29 2014, Andreas Arnez wrote:
>>
>> [...]
>>> diff --git a/gdb/testsuite/gdb.base/so-impl-ld.exp b/gdb/testsuite/gdb.base/so-impl-ld.exp
>>> index 073c3df..03d10a5 100644
>>> --- a/gdb/testsuite/gdb.base/so-impl-ld.exp
>>> +++ b/gdb/testsuite/gdb.base/so-impl-ld.exp
>>> @@ -61,12 +61,12 @@ gdb_test "next" "21\[ \t\]*result = solib_main .result.;" \
>>>  
>>>  # Verify that we can step into the second shlib call.
>>>  #
>>> -gdb_test "step" "solib_main .arg=10000. at.*${libfile}.c:17.*" \
>>> +gdb_test "step" "solib_main .arg=10000. at.*${libfile}.c:.* HERE .*" \
>>>      "step into solib call"
>>
>> Can't you use ${decimal} here, instead of expecting anything?
>
> Certainly.
>
>>
>>>  # Verify that we can step within the shlib call.
>>>  #
>>> -gdb_test "next" "18\[ \t\]*\}" "step in solib call"
>>> +gdb_test "next" "\[0-9\]+\[ \t\]*\}.* STEP .*" "step in solib call"
>>
>> Same here: ${decimal} is made to replace this \[0-9\]+.
>
> Good point.  Changed both as suggested.  Also replaced \[0-9\]+ by
> ${decimal} in the patches for dbx.exp, call-ar-st.exp, call-rt-st.exp,
> jump.exp, and shlib-call.exp.  While reviewing again I also noticed that
> I introduced an excess ".*" in front of a regexp in foll-exec.exp and
> removed it.
>
> Note that some regexps are currently enclosed in a brace-quoted group,
> where variables are not substituted -- particularly in the various uses
> of gdb_test_sequence in call-ar-st.exp.  There I still use \[0-9\]+, to
> avoid larger modifications to the existing regexps.

Yeah, that's good.  You patch is a fix needed to run the testcase using
a new GCC, so it's better to leave general cleanups for other patches.

> Here are the changes I performed based on your suggestion:

They seem great now, thanks a lot.

I will review the other patches in the series later.

>
> --- a/gdb/testsuite/gdb.base/call-ar-st.exp
> +++ b/gdb/testsuite/gdb.base/call-ar-st.exp
> @@ -62,7 +62,7 @@
>  
>  set stop_line [gdb_get_line_number "-tbreak1-"]
>  gdb_test "tbreak $stop_line" \
> -    "Temporary breakpoint \[0-9\]+.*file.*$srcfile, line $stop_line.*" \
> +    "Temporary breakpoint ${decimal}.*file.*$srcfile, line $stop_line.*" \
>      "tbreakpoint at tbreak1"
>  
>  gdb_test continue \
>
> --- a/gdb/testsuite/gdb.base/call-rt-st.exp
> +++ b/gdb/testsuite/gdb.base/call-rt-st.exp
> @@ -74,10 +74,10 @@
>      "continue to loop_count"
>  
>  gdb_test_multiple "finish" "finish out from loop count" {
> -    -re "Run till exit from .0  loop_count \\(\\) at.*call-rt-st.c:$stop_line\[ \t\r\n\]+main \\(\\) at.*call-rt-st.c:\[0-9\]+\[ \t\r\n\]+\[0-9\]+\[\t \]+return 0;.*-finish1-.*$gdb_prompt $" {
> +    -re "Run till exit from .0  loop_count \\(\\) at.*call-rt-st.c:$stop_line\[ \t\r\n\]+main \\(\\) at.*call-rt-st.c:${decimal}\[ \t\r\n\]+${decimal}\[\t \]+return 0;.*-finish1-.*$gdb_prompt $" {
>  	pass "finish out from loop_count (finish1)"
>      }
> -    -re  "Run till exit from .0  loop_count \\(\\) at.*call-rt-st.c:$stop_line\[ \t\r\n\]+main \\(\\) at.*call-rt-st.c:\[0-9\]+\[ \t\r\n\]+\[0-9\]+\[\t \]+loop_count.*-finish2-.*$gdb_prompt $" {
> +    -re  "Run till exit from .0  loop_count \\(\\) at.*call-rt-st.c:$stop_line\[ \t\r\n\]+main \\(\\) at.*call-rt-st.c:${decimal}\[ \t\r\n\]+${decimal}\[\t \]+loop_count.*-finish2-.*$gdb_prompt $" {
>  	pass "finish out from loop_count (line 775)"
>      }
>  }
>
> --- a/gdb/testsuite/gdb.base/dbx.exp
> +++ b/gdb/testsuite/gdb.base/dbx.exp
> @@ -289,6 +289,7 @@
>  #test_func
>  #
>  proc test_func { } {
> +    global decimal
>      global srcfile2
>      gdb_test "cont" ".*" "cont 1"
>      gdb_test "step" ".*"
> @@ -300,7 +301,7 @@
>      gdb_test "cont" ".*" "cont 2"
>      # This always fails, but it's not clear why. -sts 1999-08-17
>      setup_xfail "*-*-*"
> -    gdb_test "func print_average" ".*in print_average.*\\(list=.*, low=0, high=6\\).*at.*average\.c:\[0-9\]+\r\n\[0-9\]+\[ \t\]+total = sum\\(list, low, high\\);"
> +    gdb_test "func print_average" ".*in print_average.*\\(list=.*, low=0, high=6\\).*at.*average\.c:${decimal}\r\n\${decimal}\[ \t\]+total = sum\\(list, low, high\\);"
>  }
>  
>  # Start with a fresh gdb.
>
> --- a/gdb/testsuite/gdb.base/foll-exec.exp
> +++ b/gdb/testsuite/gdb.base/foll-exec.exp
> @@ -119,7 +119,7 @@
>     #
>     send_gdb "next 3\n"
>     gdb_expect {
> -     -re ".*execlp \\(.*$gdb_prompt $"\
> +     -re "execlp \\(.*$gdb_prompt $"\
>                       {pass "step to exec call"}
>       -re "$gdb_prompt $" {fail "step to exec call"}
>       timeout         {fail "(timeout) step to exec call"}
>
> --- a/gdb/testsuite/gdb.base/jump.exp
> +++ b/gdb/testsuite/gdb.base/jump.exp
> @@ -40,7 +40,7 @@
>  set bp_on_non_call 0
>  set non_call_line [gdb_get_line_number "bp-on-non-call"]
>  gdb_test_multiple "break $non_call_line" "break before jump to non-call" {
> -    -re "\[Bb\]reakpoint (\[0-9\]*) at 0x\[0-9a-fA-F\]*: file .*${srcfile}, line $non_call_line.*$gdb_prompt $" {
> +    -re "\[Bb\]reakpoint (${decimal}) at ${hex}: file .*${srcfile}, line $non_call_line.*$gdb_prompt $" {
>  	set bp_on_non_call $expect_out(1,string)
>  	pass "break before jump to non-call"
>      }
> @@ -48,7 +48,7 @@
>  
>  # Can we jump to the statement?  Do we stop there?
>  #
> -gdb_test "jump $non_call_line" "Breakpoint \[0-9\]*, .*${srcfile}:$non_call_line.*" \
> +gdb_test "jump $non_call_line" "Breakpoint ${decimal}, .*${srcfile}:$non_call_line.*" \
>      "jump to non-call"
>  
>  # Set a breakpoint on the statement that we're about to jump to.
> @@ -57,7 +57,7 @@
>  set bp_on_call 0
>  set call_line [gdb_get_line_number "bp-on-call"]
>  gdb_test_multiple "break $call_line" "break before jump to call" {
> -    -re "\[Bb\]reakpoint (\[0-9\]*) at 0x\[0-9a-fA-F\]*: file .*${srcfile}, line $call_line.*$gdb_prompt $" {
> +    -re "\[Bb\]reakpoint (${decimal}) at ${hex}: file .*${srcfile}, line $call_line.*$gdb_prompt $" {
>  	set bp_on_call $expect_out(1,string)
>  	pass "break before jump to call"
>      }
> @@ -66,7 +66,7 @@
>  # Can we jump to the statement?  Do we stop there?
>  #
>  gdb_test "jump $call_line" \
> -    "Breakpoint \[0-9\]*, .*${srcfile}:$call_line.*" \
> +    "Breakpoint ${decimal}, .*${srcfile}:$call_line.*" \
>      "jump to call"
>  
>  # If we disable the breakpoint at the function call, and then
> @@ -75,7 +75,7 @@
>  #
>  gdb_test_no_output "disable $bp_on_call" "disable breakpoint on call"
>  
> -gdb_test "jump $call_line" "Breakpoint \[0-9\]*, .*${srcfile}:$non_call_line.*" \
> +gdb_test "jump $call_line" "Breakpoint ${decimal}, .*${srcfile}:$non_call_line.*" \
>      "jump to call with disabled breakpoint"
>  
>  # Verify that GDB responds gracefully to the "jump" command without
>
> --- a/gdb/testsuite/gdb.base/shlib-call.exp
> +++ b/gdb/testsuite/gdb.base/shlib-call.exp
> @@ -111,7 +111,7 @@
>      "breakpoint function shr2"
>  
>  gdb_test "continue" \
> -	"Continuing\\..*Breakpoint \[0-9\]+, shr2 \\(.*\\) at.*shr2\\.c:.*shr2-return \\*\\/" \
> +	"Continuing\\..*Breakpoint ${decimal}, shr2 \\(.*\\) at.*shr2\\.c:${decimal}.*shr2-return \\*\\/" \
>  	"run until breakpoint set at a function"
>  
>  
> --- a/gdb/testsuite/gdb.base/so-impl-ld.exp
> +++ b/gdb/testsuite/gdb.base/so-impl-ld.exp
> @@ -61,12 +61,12 @@
>  
>  # Verify that we can step into the second shlib call.
>  #
> -gdb_test "step" "solib_main .arg=10000. at.*${libfile}.c:.* HERE .*" \
> +gdb_test "step" "solib_main .arg=10000. at.*${libfile}.c:${decimal}.* HERE .*" \
>      "step into solib call"
>  
>  # Verify that we can step within the shlib call.
>  #
> -gdb_test "next" "\[0-9\]+\[ \t\]*\}.* STEP .*" "step in solib call"
> +gdb_test "next" "${decimal}\[ \t\]*\}.* STEP .*" "step in solib call"
>  
>  # Verify that we can step out of the shlib call, and back out into
>  # the caller.
Sergio Durigan Junior Oct. 31, 2014, 8:59 p.m. UTC | #2
On Wednesday, October 29 2014, I wrote:

>> Here are the changes I performed based on your suggestion:
>
> They seem great now, thanks a lot.
>
> I will review the other patches in the series later.

FWIW, I have skimmed through the other patches, and have not seen
anything wrong.  IMO this patch series should go in.

Thanks,
diff mbox

Patch

--- a/gdb/testsuite/gdb.base/call-ar-st.exp
+++ b/gdb/testsuite/gdb.base/call-ar-st.exp
@@ -62,7 +62,7 @@ 
 
 set stop_line [gdb_get_line_number "-tbreak1-"]
 gdb_test "tbreak $stop_line" \
-    "Temporary breakpoint \[0-9\]+.*file.*$srcfile, line $stop_line.*" \
+    "Temporary breakpoint ${decimal}.*file.*$srcfile, line $stop_line.*" \
     "tbreakpoint at tbreak1"
 
 gdb_test continue \

--- a/gdb/testsuite/gdb.base/call-rt-st.exp
+++ b/gdb/testsuite/gdb.base/call-rt-st.exp
@@ -74,10 +74,10 @@ 
     "continue to loop_count"
 
 gdb_test_multiple "finish" "finish out from loop count" {
-    -re "Run till exit from .0  loop_count \\(\\) at.*call-rt-st.c:$stop_line\[ \t\r\n\]+main \\(\\) at.*call-rt-st.c:\[0-9\]+\[ \t\r\n\]+\[0-9\]+\[\t \]+return 0;.*-finish1-.*$gdb_prompt $" {
+    -re "Run till exit from .0  loop_count \\(\\) at.*call-rt-st.c:$stop_line\[ \t\r\n\]+main \\(\\) at.*call-rt-st.c:${decimal}\[ \t\r\n\]+${decimal}\[\t \]+return 0;.*-finish1-.*$gdb_prompt $" {
 	pass "finish out from loop_count (finish1)"
     }
-    -re  "Run till exit from .0  loop_count \\(\\) at.*call-rt-st.c:$stop_line\[ \t\r\n\]+main \\(\\) at.*call-rt-st.c:\[0-9\]+\[ \t\r\n\]+\[0-9\]+\[\t \]+loop_count.*-finish2-.*$gdb_prompt $" {
+    -re  "Run till exit from .0  loop_count \\(\\) at.*call-rt-st.c:$stop_line\[ \t\r\n\]+main \\(\\) at.*call-rt-st.c:${decimal}\[ \t\r\n\]+${decimal}\[\t \]+loop_count.*-finish2-.*$gdb_prompt $" {
 	pass "finish out from loop_count (line 775)"
     }
 }

--- a/gdb/testsuite/gdb.base/dbx.exp
+++ b/gdb/testsuite/gdb.base/dbx.exp
@@ -289,6 +289,7 @@ 
 #test_func
 #
 proc test_func { } {
+    global decimal
     global srcfile2
     gdb_test "cont" ".*" "cont 1"
     gdb_test "step" ".*"
@@ -300,7 +301,7 @@ 
     gdb_test "cont" ".*" "cont 2"
     # This always fails, but it's not clear why. -sts 1999-08-17
     setup_xfail "*-*-*"
-    gdb_test "func print_average" ".*in print_average.*\\(list=.*, low=0, high=6\\).*at.*average\.c:\[0-9\]+\r\n\[0-9\]+\[ \t\]+total = sum\\(list, low, high\\);"
+    gdb_test "func print_average" ".*in print_average.*\\(list=.*, low=0, high=6\\).*at.*average\.c:${decimal}\r\n\${decimal}\[ \t\]+total = sum\\(list, low, high\\);"
 }
 
 # Start with a fresh gdb.

--- a/gdb/testsuite/gdb.base/foll-exec.exp
+++ b/gdb/testsuite/gdb.base/foll-exec.exp
@@ -119,7 +119,7 @@ 
    #
    send_gdb "next 3\n"
    gdb_expect {
-     -re ".*execlp \\(.*$gdb_prompt $"\
+     -re "execlp \\(.*$gdb_prompt $"\
                      {pass "step to exec call"}
      -re "$gdb_prompt $" {fail "step to exec call"}
      timeout         {fail "(timeout) step to exec call"}

--- a/gdb/testsuite/gdb.base/jump.exp
+++ b/gdb/testsuite/gdb.base/jump.exp
@@ -40,7 +40,7 @@ 
 set bp_on_non_call 0
 set non_call_line [gdb_get_line_number "bp-on-non-call"]
 gdb_test_multiple "break $non_call_line" "break before jump to non-call" {
-    -re "\[Bb\]reakpoint (\[0-9\]*) at 0x\[0-9a-fA-F\]*: file .*${srcfile}, line $non_call_line.*$gdb_prompt $" {
+    -re "\[Bb\]reakpoint (${decimal}) at ${hex}: file .*${srcfile}, line $non_call_line.*$gdb_prompt $" {
 	set bp_on_non_call $expect_out(1,string)
 	pass "break before jump to non-call"
     }
@@ -48,7 +48,7 @@ 
 
 # Can we jump to the statement?  Do we stop there?
 #
-gdb_test "jump $non_call_line" "Breakpoint \[0-9\]*, .*${srcfile}:$non_call_line.*" \
+gdb_test "jump $non_call_line" "Breakpoint ${decimal}, .*${srcfile}:$non_call_line.*" \
     "jump to non-call"
 
 # Set a breakpoint on the statement that we're about to jump to.
@@ -57,7 +57,7 @@ 
 set bp_on_call 0
 set call_line [gdb_get_line_number "bp-on-call"]
 gdb_test_multiple "break $call_line" "break before jump to call" {
-    -re "\[Bb\]reakpoint (\[0-9\]*) at 0x\[0-9a-fA-F\]*: file .*${srcfile}, line $call_line.*$gdb_prompt $" {
+    -re "\[Bb\]reakpoint (${decimal}) at ${hex}: file .*${srcfile}, line $call_line.*$gdb_prompt $" {
 	set bp_on_call $expect_out(1,string)
 	pass "break before jump to call"
     }
@@ -66,7 +66,7 @@ 
 # Can we jump to the statement?  Do we stop there?
 #
 gdb_test "jump $call_line" \
-    "Breakpoint \[0-9\]*, .*${srcfile}:$call_line.*" \
+    "Breakpoint ${decimal}, .*${srcfile}:$call_line.*" \
     "jump to call"
 
 # If we disable the breakpoint at the function call, and then
@@ -75,7 +75,7 @@ 
 #
 gdb_test_no_output "disable $bp_on_call" "disable breakpoint on call"
 
-gdb_test "jump $call_line" "Breakpoint \[0-9\]*, .*${srcfile}:$non_call_line.*" \
+gdb_test "jump $call_line" "Breakpoint ${decimal}, .*${srcfile}:$non_call_line.*" \
     "jump to call with disabled breakpoint"
 
 # Verify that GDB responds gracefully to the "jump" command without

--- a/gdb/testsuite/gdb.base/shlib-call.exp
+++ b/gdb/testsuite/gdb.base/shlib-call.exp
@@ -111,7 +111,7 @@ 
     "breakpoint function shr2"
 
 gdb_test "continue" \
-	"Continuing\\..*Breakpoint \[0-9\]+, shr2 \\(.*\\) at.*shr2\\.c:.*shr2-return \\*\\/" \
+	"Continuing\\..*Breakpoint ${decimal}, shr2 \\(.*\\) at.*shr2\\.c:${decimal}.*shr2-return \\*\\/" \
 	"run until breakpoint set at a function"
 
 
--- a/gdb/testsuite/gdb.base/so-impl-ld.exp
+++ b/gdb/testsuite/gdb.base/so-impl-ld.exp
@@ -61,12 +61,12 @@ 
 
 # Verify that we can step into the second shlib call.
 #
-gdb_test "step" "solib_main .arg=10000. at.*${libfile}.c:.* HERE .*" \
+gdb_test "step" "solib_main .arg=10000. at.*${libfile}.c:${decimal}.* HERE .*" \
     "step into solib call"
 
 # Verify that we can step within the shlib call.
 #
-gdb_test "next" "\[0-9\]+\[ \t\]*\}.* STEP .*" "step in solib call"
+gdb_test "next" "${decimal}\[ \t\]*\}.* STEP .*" "step in solib call"
 
 # Verify that we can step out of the shlib call, and back out into
 # the caller.