[PUSHED] gdb/testsuite: Fix race condition in gdb.base/skip.exp

[PUSHED] gdb/testsuite: Fix race condition in gdb.base/skip.exp

[Andrew Burgess]

In this commit:

  commit 5024637fac653914d471808288dc3221bc7ec089
  Date:   Sun Dec 15 11:05:47 2019 +0100

      Fix skip.exp test failure observed with gcc-9.2.0

A race condition was introduced into the gdb.base/skip.exp test when
this line:

    gdb_test "step" "foo \\(\\) at.*" "step 3"

Was changed to this:

    gdb_test "step" "foo \\(\\) at.*" "step 3" "main \\(\\) at .*" "step"

Before the above change we expected GDB to behave like this:

  (gdb) step
  foo () at /path/to/gdb/testsuite/gdb.base/skip.c:42
  42        return 0;
  (gdb)

However, when the test is compiled with GCC 9.2.0 we get a different
behaviour, and so we need a second 'step', like this:

  (gdb) step
  main () at /path/to/gdb.base/skip.c:32
  32        x = baz ((bar (), foo ()));
  (gdb) step
  foo () at /path/to/gdb/testsuite/gdb.base/skip.c:42
  42        return 0;
  (gdb)

Now the change to the test matches against 'main () at .*', however if
GDB or expect is being slow then we might only get to see output like
this:

  (gdb) step
  main () at /path/to/g

This will happily match the question pattern, so we send 'step' to GDB
again.  Now GDB continues to produce output which expect accepts, we
now see this:

  b.base/skip.c:32
  32        x = baz ((bar (), foo ()));
  (gdb)

This has carried on from where the previous block of output left off.
This doesn't match the final pattern 'foo \\(\\) at.*', but it does
match the prompt pattern that gdb_test_multiple adds, and so we report
the test as failing.

The solution is to simply ensure that the question consumes everything
up to, and including the prompt.  This ensures that the prompt can't
then match the failure case.  The new test line becomes:

    gdb_test "step" "foo \\(\\) at.*" "step 3" \
       "main \\(\\) at .*\r\n$gdb_prompt " "step"

gdb/testsuite/ChangeLog:

	* gdb.base/skip.exp: Fix race condition in test.

Change-Id: I9f0b0b52ef1b4f980bfaa8fe405ff06d520f3482
---
 gdb/testsuite/ChangeLog         | 4 ++++
 gdb/testsuite/gdb.base/skip.exp | 9 ++++++---
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/gdb/testsuite/gdb.base/skip.exp b/gdb/testsuite/gdb.base/skip.exp
index d7dd3cedbec..513c9fcc82e 100644
--- a/gdb/testsuite/gdb.base/skip.exp
+++ b/gdb/testsuite/gdb.base/skip.exp
@@ -144,7 +144,8 @@ with_test_prefix "step after disabling 3" {
     gdb_test "step" ".*" "step 2"; # Return from bar()
     # With gcc 9.2.0 we jump once back to main before entering foo here.
     # If that happens try to step a second time.
-    gdb_test "step" "foo \\(\\) at.*" "step 3" "main \\(\\) at .*" "step"
+    gdb_test "step" "foo \\(\\) at.*" "step 3" \
+	"main \\(\\) at .*\r\n$gdb_prompt " "step"
     gdb_test "step" ".*" "step 4"; # Return from foo()
     gdb_test "step" "main \\(\\) at.*" "step 5"
 }
@@ -265,7 +266,8 @@ with_test_prefix "step using -fu for baz" {
     gdb_test "step" ".*" "step 2"; # Return from bar()
     # With gcc 9.2.0 we jump once back to main before entering foo here.
     # If that happens try to step a second time.
-    gdb_test "step" "foo \\(\\) at.*" "step 3" "main \\(\\) at.*" "step"
+    gdb_test "step" "foo \\(\\) at.*" "step 3" \
+	"main \\(\\) at .*\r\n$gdb_prompt " "step"
     gdb_test "step" ".*" "step 4"; # Return from foo()
     gdb_test "step" "main \\(\\) at.*" "step 5"
 }
@@ -282,7 +284,8 @@ with_test_prefix "step using -rfu for baz" {
     gdb_test "step" ".*" "step 2"; # Return from bar()
     # With gcc 9.2.0 we jump once back to main before entering foo here.
     # If that happens try to step a second time.
-    gdb_test "step" "foo \\(\\) at.*" "step 3" "main \\(\\) at.*" "step"
+    gdb_test "step" "foo \\(\\) at.*" "step 3" \
+	"main \\(\\) at .*\r\n$gdb_prompt " "step"
     gdb_test "step" ".*" "step 4"; # Return from foo()
     gdb_test "step" "main \\(\\) at.*" "step 5"
 }
-- 
2.14.5

Re: [PUSHED] gdb/testsuite: Fix race condition in gdb.base/skip.exp

[Simon Marchi]

On 2020-01-09 6:04 p.m., Andrew Burgess wrote:
> In this commit:
> 
>   commit 5024637fac653914d471808288dc3221bc7ec089
>   Date:   Sun Dec 15 11:05:47 2019 +0100
> 
>       Fix skip.exp test failure observed with gcc-9.2.0
> 
> A race condition was introduced into the gdb.base/skip.exp test when
> this line:
> 
>     gdb_test "step" "foo \\(\\) at.*" "step 3"
> 
> Was changed to this:
> 
>     gdb_test "step" "foo \\(\\) at.*" "step 3" "main \\(\\) at .*" "step"
> 
> Before the above change we expected GDB to behave like this:
> 
>   (gdb) step
>   foo () at /path/to/gdb/testsuite/gdb.base/skip.c:42
>   42        return 0;
>   (gdb)
> 
> However, when the test is compiled with GCC 9.2.0 we get a different
> behaviour, and so we need a second 'step', like this:
> 
>   (gdb) step
>   main () at /path/to/gdb.base/skip.c:32
>   32        x = baz ((bar (), foo ()));
>   (gdb) step
>   foo () at /path/to/gdb/testsuite/gdb.base/skip.c:42
>   42        return 0;
>   (gdb)
> 
> Now the change to the test matches against 'main () at .*', however if
> GDB or expect is being slow then we might only get to see output like
> this:
> 
>   (gdb) step
>   main () at /path/to/g
> 
> This will happily match the question pattern, so we send 'step' to GDB
> again.  Now GDB continues to produce output which expect accepts, we
> now see this:
> 
>   b.base/skip.c:32
>   32        x = baz ((bar (), foo ()));
>   (gdb)
> 
> This has carried on from where the previous block of output left off.
> This doesn't match the final pattern 'foo \\(\\) at.*', but it does
> match the prompt pattern that gdb_test_multiple adds, and so we report
> the test as failing.
> 
> The solution is to simply ensure that the question consumes everything
> up to, and including the prompt.  This ensures that the prompt can't
> then match the failure case.  The new test line becomes:
> 
>     gdb_test "step" "foo \\(\\) at.*" "step 3" \
>        "main \\(\\) at .*\r\n$gdb_prompt " "step"
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.base/skip.exp: Fix race condition in test.
> 
> Change-Id: I9f0b0b52ef1b4f980bfaa8fe405ff06d520f3482

Ah, thanks for fixing this, I missed it when looking at the patch.  I don't know
if you used it, but this kind of issue is also reproducible using "make check-read1".

Simon