Re: [PATCH][gdb/testsuite] Fix gdb.opt/solib-intra-step.exp with -m32 and gcc-10

[Tom de Vries <tdevries@suse.de>]

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

On 1/28/21 4:03 PM, Simon Marchi wrote:
> 
> 
> On 2021-01-26 1:03 p.m., Tom de Vries wrote:
>> Hi,
>>
>> When running test-case gdb.opt/solib-intra-step.exp with target board
>> unix/-m32 and gcc-10, I run into:
>> ...
>> (gdb) step^M
>> __x86.get_pc_thunk.bx () at ../sysdeps/i386/crti.S:68^M
>> 68      ../sysdeps/i386/crti.S: No such file or directory.^M
>> (gdb) step^M
>> shlib_second (dummy=0) at solib-intra-step-lib.c:23^M
>> 23        abort (); /* second-hit */^M
>> (gdb) FAIL: gdb.opt/solib-intra-step.exp: second-hit
>> ...
>>
>> The problem is that the test-case expects to step past the retry line,
>> which is optional.
>>
>> Fix this by make the retry line step optional, both for shlib_first and
>> shlib_second.
>>
>> Any comments?
>>
>> Thanks,
>> - Tom
>>
>> [gdb/testsuite] Fix gdb.opt/solib-intra-step.exp with -m32 and gcc-10
>>
>> gdb/testsuite/ChangeLog:
>>
>> 2021-01-26  Tom de Vries  <tdevries@suse.de>
>>
>> 	* gdb.opt/solib-intra-step.exp: Make step into retry line optional.
>>
>> ---
>>  gdb/testsuite/gdb.opt/solib-intra-step.exp | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/gdb/testsuite/gdb.opt/solib-intra-step.exp b/gdb/testsuite/gdb.opt/solib-intra-step.exp
>> index ad19895d890..13e0cf04c7a 100644
>> --- a/gdb/testsuite/gdb.opt/solib-intra-step.exp
>> +++ b/gdb/testsuite/gdb.opt/solib-intra-step.exp
>> @@ -63,7 +63,7 @@ gdb_test_multiple "step" $test {
>>  	exp_continue
>>      }
>>      -re -wrap "get_pc_thunk.*" {
>> -	if  { $state != 1 } {
>> +	if  { $state != 0 && $state != 1 } {
>>  	    set state -1
>>  	} else {
>>  	    set state 2
>> @@ -89,7 +89,7 @@ gdb_test_multiple "step" $test {
>>  	exp_continue
>>      }
>>      -re -wrap "get_pc_thunk.*" {
>> -	if  { $state != 1 } {
>> +	if  { $state != 0 && $state != 1 } {
>>  	    set state -1
>>  	} else {
>>  	    set state 2
>>
> 
> I don't really understand what happens here, what state value means what.
> 
> A bit of commenting would help.

I tried to add comments but didn't manage to come up with something
sensible.

Instead, I simplified gdb_test_multiple to just track the order of
events, and then added a few asserts about order of events.

I hope this clarifies what the test is trying to do.  WDYT?

Thanks,
- Tom

[-- Attachment #2: 0005-gdb-testsuite-Fix-gdb.opt-solib-intra-step.exp-with-m32-and-gcc-10.patch --]
[-- Type: text/x-patch, Size: 3285 bytes --]

[gdb/testsuite] Fix gdb.opt/solib-intra-step.exp with -m32 and gcc-10

When running test-case gdb.opt/solib-intra-step.exp with target board
unix/-m32 and gcc-10, I run into:
...
(gdb) step^M
__x86.get_pc_thunk.bx () at ../sysdeps/i386/crti.S:68^M
68      ../sysdeps/i386/crti.S: No such file or directory.^M
(gdb) step^M
shlib_second (dummy=0) at solib-intra-step-lib.c:23^M
23        abort (); /* second-hit */^M
(gdb) FAIL: gdb.opt/solib-intra-step.exp: second-hit
...

The problem is that the test-case expects to step past the retry line,
which is optional.

Fix this by make the retry line step optional, both for shlib_first and
shlib_second.

gdb/testsuite/ChangeLog:

2021-01-26  Tom de Vries  <tdevries@suse.de>

	* gdb.opt/solib-intra-step.exp: Make step into retry line optional.
	Rewrite state machine to make it clear what order conditions need
	checking.

---
 gdb/testsuite/gdb.opt/solib-intra-step.exp | 63 +++++++++++++++++++-----------
 1 file changed, 41 insertions(+), 22 deletions(-)

diff --git a/gdb/testsuite/gdb.opt/solib-intra-step.exp b/gdb/testsuite/gdb.opt/solib-intra-step.exp
index ad19895d890..5c78e07772e 100644
--- a/gdb/testsuite/gdb.opt/solib-intra-step.exp
+++ b/gdb/testsuite/gdb.opt/solib-intra-step.exp
@@ -47,58 +47,77 @@ if ![runto_main] then {
   return 0
 }
 
+proc check_before { a b } {
+    eval global $a $b
+    eval set val_a $$a
+    eval set val_b $$b
+    if { $val_a == -1 || $val_b == -1 } {
+	return
+    }
+    gdb_assert { $val_a < $val_b } "$a before $b"
+}
+
 set test "first-hit"
+set hit_state -1
+set retry_state -1
+set thunk_state -1
 set state 0
 gdb_test_multiple "step" $test {
     -re -wrap " first-hit .*" {
-	gdb_assert { $state != -1 } $test
+	set hit_state $state
+	pass $gdb_test_name
     }
     -re -wrap " first-retry .*" {
-	if  { $state != 0 } {
-	    set state -1
-	} else {
-	    set state 1
-	}
+	set retry_state $state
+	incr state
 	send_gdb "step\n"
 	exp_continue
     }
     -re -wrap "get_pc_thunk.*" {
-	if  { $state != 1 } {
-	    set state -1
-	} else {
-	    set state 2
-	}
+	set thunk_state $state
+	incr state
 	send_gdb "step\n"
 	exp_continue
     }
 }
 
+with_test_prefix $test {
+    check_before retry_state hit_state
+    check_before thunk_state hit_state
+    check_before retry_state thunk_state
+}
+
 set test "second-hit"
+set hit_state -1
+set retry_state -1
+set thunk_state -1
+set state 0
 set state 0
 gdb_test_multiple "step" $test {
     -re -wrap " second-hit .*" {
-	gdb_assert { $state != -1 } $test
+	set hit_state $state
+	pass $gdb_test_name
     }
     -re -wrap " second-retry .*" {
-	if { $state != 0 } {
-	    set state -1
-	} else {
-	    set state 1
-	}
+	set retry_state $state
+	incr state
 	send_gdb "step\n"
 	exp_continue
     }
     -re -wrap "get_pc_thunk.*" {
-	if  { $state != 1 } {
-	    set state -1
-	} else {
-	    set state 2
-	}
+	set thunk_state $state
+	incr state
 	send_gdb "step\n"
 	exp_continue
     }
 }
 
+with_test_prefix $test {
+    check_before retry_state hit_state
+    check_before thunk_state hit_state
+    check_before retry_state thunk_state
+}
+
 if ![runto_main] then {
   return 0
 }