public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 4/6] Fix unstable test names in gdb.python/py-objfile.exp
  2017-10-23 14:18 [PATCH 0/6] Fix several cases of unstable test names Pedro Alves
@ 2017-10-23 14:18 ` Pedro Alves
  2017-10-23 14:18 ` [PATCH 5/6] Fix unstable test names in gdb.threads/attach-into-signal.exp Pedro Alves
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Pedro Alves @ 2017-10-23 14:18 UTC (permalink / raw)
  To: gdb-patches

Currently, if you diff testsuite/gdb.sum of different builds you see
this spurious hunk:

  -PASS: gdb.python/py-objfile.exp: get python valueof "sep_objfile.build_id" (6a0bfcab663f9810ccff33c756afdebb940037d4)
  +PASS: gdb.python/py-objfile.exp: get python valueof "sep_objfile.build_id" (1f5531c657c57777b05fc95baa0025fd1d115c3b)

Fix this by syncing get_python_valueof with get_integer_valueof, which
stopped outputting the value in commit 2f20e312aad6
("get_integer_valueof: Don't output value in test name").

After this commit we'll show:

  PASS: gdb.python/py-objfile.exp: get python valueof "sep_objfile.build_id"

As the comment explicitly says get_python_valueof is modeled on
get_integer_valueof, I went ahead and also added the optional 'test'
parameter while at it.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* lib/gdb-python.exp (get_python_valueof): Add 'test' optional
	parameter and handle it.  Don't output read value in test name.
---
 gdb/testsuite/lib/gdb-python.exp | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/gdb/testsuite/lib/gdb-python.exp b/gdb/testsuite/lib/gdb-python.exp
index 1fef27e..c0c0d68 100644
--- a/gdb/testsuite/lib/gdb-python.exp
+++ b/gdb/testsuite/lib/gdb-python.exp
@@ -46,19 +46,23 @@ proc gdb_py_test_multiple { name args } {
     return 0
 }
 
-# Return the result of python expression EXPR.
-# DEFAULT is returned if there's an error.
-# This is modelled after get_integer_valueof.
+# Return the result of python expression EXPR.  DEFAULT is returned if
+# there's an error.  TEST is the test message to use.  It can be
+# omitted, in which case a test message is built from EXP.  This is
+# modeled after get_integer_valueof.
 
-proc get_python_valueof { exp default } {
+proc get_python_valueof { exp default {test ""} } {
     global gdb_prompt
 
-    set test "get python valueof \"${exp}\""
+    if {$test == ""} {
+	set test "get python valueof \"${exp}\""
+    }
+
     set val ${default}
     gdb_test_multiple "python print (\"valueof: %s\" % (${exp}))" "$test" {
 	-re "valueof: (\[^\r\n\]*)\[\r\n\]*$gdb_prompt $" {
 	    set val $expect_out(1,string)
-	    pass "$test ($val)"
+	    pass "$test"
 	}
 	timeout {
 	    fail "$test (timeout)"
-- 
2.5.5

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

* [PATCH 5/6] Fix unstable test names in gdb.threads/attach-into-signal.exp
  2017-10-23 14:18 [PATCH 0/6] Fix several cases of unstable test names Pedro Alves
  2017-10-23 14:18 ` [PATCH 4/6] Fix unstable test names in gdb.python/py-objfile.exp Pedro Alves
@ 2017-10-23 14:18 ` Pedro Alves
  2017-10-23 14:18 ` [PATCH 3/6] Fix unstable test names in gdb.gdb/unittest.exp Pedro Alves
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Pedro Alves @ 2017-10-23 14:18 UTC (permalink / raw)
  To: gdb-patches

Currently, if you diff testsuite/gdb.sum of two testsuite runs you'll
often see spurious hunks like these:

  -PASS: gdb.threads/attach-into-signal.exp: nonthreaded: attempt 2: attach (pass 2), pending signal catch
  +PASS: gdb.threads/attach-into-signal.exp: nonthreaded: attempt 1: attach (pass 2), pending signal catch
   PASS: gdb.threads/attach-into-signal.exp: successfully compiled posix threads test case
   PASS: gdb.threads/attach-into-signal.exp: threaded: handle SIGALRM stop print pass
  -PASS: gdb.threads/attach-into-signal.exp: threaded: attempt 1: attach (pass 1), pending signal catch
  -PASS: gdb.threads/attach-into-signal.exp: threaded: attempt 1: attach (pass 2), pending signal catch
  +PASS: gdb.threads/attach-into-signal.exp: threaded: attempt 2: attach (pass 1), pending signal catch
  +PASS: gdb.threads/attach-into-signal.exp: threaded: attempt 4: attach (pass 2), pending signal catch

Fix this by removing the "attempt $attempt" test prefix.  The attempt
number can be retrieved from gdb.log instead, since the testcase is
already using "verbose -log" to that effect.

(The 'with_test_prefix "stoppedtry $stoppedtry"' prefix is unnecessary
too, because inside that block there are no pass/fail calls.  In fact
the block includes a comment saying:

  # No PASS message as we may be looping in multiple
  # attempts.

but I'll drop that whole loop in the next patch instead.)

After this commit we'll show:

  PASS: gdb.threads/attach-into-signal.exp: nonthreaded: handle SIGALRM stop print pass
  PASS: gdb.threads/attach-into-signal.exp: nonthreaded: attach (pass 1), pending signal catch
  PASS: gdb.threads/attach-into-signal.exp: nonthreaded: attach (pass 2), pending signal catch
  PASS: gdb.threads/attach-into-signal.exp: successfully compiled posix threads test case
  PASS: gdb.threads/attach-into-signal.exp: threaded: handle SIGALRM stop print pass
  PASS: gdb.threads/attach-into-signal.exp: threaded: attach (pass 1), pending signal catch
  PASS: gdb.threads/attach-into-signal.exp: threaded: attach (pass 2), pending signal catch

(I've avoided reindenting to make the patch easier to maintain/read.
I'll reindent the blocks after this is in.)

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.threads/attach-into-signal.exp (corefunc): Remove "attach
	$attempt" and "stoppedtry $stoppedtry" test prefixes.
---
 gdb/testsuite/gdb.threads/attach-into-signal.exp | 2 --
 1 file changed, 2 deletions(-)

diff --git a/gdb/testsuite/gdb.threads/attach-into-signal.exp b/gdb/testsuite/gdb.threads/attach-into-signal.exp
index cfb06cf..7502479 100644
--- a/gdb/testsuite/gdb.threads/attach-into-signal.exp
+++ b/gdb/testsuite/gdb.threads/attach-into-signal.exp
@@ -56,7 +56,6 @@ proc corefunc { threadtype executable } {
 	set attempt 1
 	set passes 1
 	while { $passes < 3 && $attempt <= $attempts } {
-	    with_test_prefix "attempt $attempt" {
 		set stoppedtry 0
 		while { $stoppedtry < 10 } {
 		    with_test_prefix "stoppedtry $stoppedtry" {
@@ -128,7 +127,6 @@ proc corefunc { threadtype executable } {
 		}
 
 		gdb_test "detach" "Detaching from.*" ""
-	    }
 	}
 
 	if {$passes < 3} {
-- 
2.5.5

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

* [PATCH 1/6] Fix unstable test names in gdb.arch/arc-tdesc-cpu.exp
  2017-10-23 14:18 [PATCH 0/6] Fix several cases of unstable test names Pedro Alves
                   ` (2 preceding siblings ...)
  2017-10-23 14:18 ` [PATCH 3/6] Fix unstable test names in gdb.gdb/unittest.exp Pedro Alves
@ 2017-10-23 14:18 ` Pedro Alves
  2017-10-23 14:18 ` [PATCH 2/6] Fix unstable test names in gdb.base/startup-with-shell.exp Pedro Alves
  2017-10-23 14:28 ` [PATCH 6/6] Drop /proc/PID/status polling from gdb.threads/attach-into-signal.exp Pedro Alves
  5 siblings, 0 replies; 12+ messages in thread
From: Pedro Alves @ 2017-10-23 14:18 UTC (permalink / raw)
  To: gdb-patches

Currently if you diff testsuite/gdb.sum of two builds built from
different source trees you see this spurious hunk:

  -PASS: gdb.arch/arc-tdesc-cpu.exp: set tdesc filename /home/pedro/gdb1/src/gdb/testsuite/gdb.arch/arc-tdesc-cpu.xml
  +PASS: gdb.arch/arc-tdesc-cpu.exp: set tdesc filename /home/pedro/gdb2/src/gdb/testsuite/gdb.arch/arc-tdesc-cpu.xml

After this commit we'll show this instead in gdb.sum:
  PASS: gdb.arch/arc-tdesc-cpu.exp: set tdesc filename $srcdir/gdb.arch/arc-tdesc-cpu.xml

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.arch/arc-tdesc-cpu.exp ('set tdesc filename'): Use gdb_test
	with explicit test name.
---
 gdb/testsuite/gdb.arch/arc-tdesc-cpu.exp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gdb/testsuite/gdb.arch/arc-tdesc-cpu.exp b/gdb/testsuite/gdb.arch/arc-tdesc-cpu.exp
index f1c009d..13f677f 100644
--- a/gdb/testsuite/gdb.arch/arc-tdesc-cpu.exp
+++ b/gdb/testsuite/gdb.arch/arc-tdesc-cpu.exp
@@ -26,10 +26,10 @@ gdb_start
 # doesn't pass architecture from the target description directly to the
 # disassembler and instead uses one of the valid CPU names.
 
-set filename $srcdir/$subdir/arc-tdesc-cpu.xml
-
-set cmd "set tdesc filename $filename"
-gdb_test $cmd
+gdb_test \
+    "set tdesc filename $srcdir/$subdir/arc-tdesc-cpu.xml" \
+    ".*" \
+    "set tdesc filename \$srcdir/$subdir/arc-tdesc-cpu.xml"
 
 # An error message is emitted by the disassembler, therefore it is not shown
 # unless the disassembler is actually invoked.  Address "0" is not invalid,
-- 
2.5.5

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

* [PATCH 0/6] Fix several cases of unstable test names
@ 2017-10-23 14:18 Pedro Alves
  2017-10-23 14:18 ` [PATCH 4/6] Fix unstable test names in gdb.python/py-objfile.exp Pedro Alves
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Pedro Alves @ 2017-10-23 14:18 UTC (permalink / raw)
  To: gdb-patches

I frequently diff testresults between builds in different build and
source directories, and that reveals several spurious differences.

Most are caused by the fact the some tests include the source or build
directories in their names, like:

  -PASS: gdb.arch/arc-tdesc-cpu.exp: set tdesc filename /home/pedro/gdb1/src/gdb/testsuite/gdb.arch/arc-tdesc-cpu.xml 
  +PASS: gdb.arch/arc-tdesc-cpu.exp: set tdesc filename /home/pedro/gdb2/src/gdb/testsuite/gdb.arch/arc-tdesc-cpu.xml
                                                                   ^^^^

  -PASS: gdb.base/startup-with-shell.exp: touch /home/pedro/gdb1/build/gdb/testsuite/outputs/gdb.base/startup-with-shell/unique-file.unique-extension 
  +PASS: gdb.base/startup-with-shell.exp: touch /home/pedro/gdb2/build/gdb/testsuite/outputs/gdb.base/startup-with-shell/unique-file.unique-extension 
                                                            ^^^^

  (I've underlined the differences above.)

though there are other causes too.  See each individual patch.

This series fixes all the spurious differences I see when native
testing on GNU/Linux.

Pedro Alves (6):
  Fix unstable test names in gdb.arch/arc-tdesc-cpu.exp
  Fix unstable test names in gdb.base/startup-with-shell.exp
  Fix unstable test names in gdb.gdb/unittest.exp
  Fix unstable test names in gdb.python/py-objfile.exp
  Fix unstable test names in gdb.threads/attach-into-signal.exp
  Drop /proc/PID/status polling from gdb.threads/attach-into-signal.exp

 gdb/testsuite/gdb.arch/arc-tdesc-cpu.exp         |  8 +++----
 gdb/testsuite/gdb.base/startup-with-shell.exp    |  7 ++++--
 gdb/testsuite/gdb.gdb/unittest.exp               |  3 ++-
 gdb/testsuite/gdb.threads/attach-into-signal.exp | 28 ------------------------
 gdb/testsuite/lib/gdb-python.exp                 | 16 +++++++++-----
 5 files changed, 21 insertions(+), 41 deletions(-)

-- 
2.5.5

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

* [PATCH 3/6] Fix unstable test names in gdb.gdb/unittest.exp
  2017-10-23 14:18 [PATCH 0/6] Fix several cases of unstable test names Pedro Alves
  2017-10-23 14:18 ` [PATCH 4/6] Fix unstable test names in gdb.python/py-objfile.exp Pedro Alves
  2017-10-23 14:18 ` [PATCH 5/6] Fix unstable test names in gdb.threads/attach-into-signal.exp Pedro Alves
@ 2017-10-23 14:18 ` Pedro Alves
  2017-10-23 14:18 ` [PATCH 1/6] Fix unstable test names in gdb.arch/arc-tdesc-cpu.exp Pedro Alves
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Pedro Alves @ 2017-10-23 14:18 UTC (permalink / raw)
  To: gdb-patches

Currently, if you diff testsuite/gdb.sum of two builds built from different
source directories you see this spurious hunk:

  -PASS: gdb.gdb/unittest.exp: maintenance check xml-descriptions /home/pedro/gdb1/src/gdb/testsuite/../features
  +PASS: gdb.gdb/unittest.exp: maintenance check xml-descriptions /home/pedro/gdb2/src/gdb/testsuite/../features

After this commit we'll show instead:

  PASS: gdb.gdb/unittest.exp: maintenance check xml-descriptions ${srcdir}/../features

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.gdb/unittest.exp ('maintenance check xml-descriptions'): Use
	custom test name.
---
 gdb/testsuite/gdb.gdb/unittest.exp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.gdb/unittest.exp b/gdb/testsuite/gdb.gdb/unittest.exp
index c3e36e6..58494e1 100644
--- a/gdb/testsuite/gdb.gdb/unittest.exp
+++ b/gdb/testsuite/gdb.gdb/unittest.exp
@@ -18,5 +18,6 @@ gdb_test "maintenance selftest" "Ran $decimal unit tests, 0 failed"
 
 if { ![is_remote host] } {
     gdb_test "maintenance check xml-descriptions ${srcdir}/../features" \
-	"Tested $decimal XML files, 0 failed"
+	"Tested $decimal XML files, 0 failed" \
+	"maintenance check xml-descriptions \${srcdir}/../features"
 }
-- 
2.5.5

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

* [PATCH 2/6] Fix unstable test names in gdb.base/startup-with-shell.exp
  2017-10-23 14:18 [PATCH 0/6] Fix several cases of unstable test names Pedro Alves
                   ` (3 preceding siblings ...)
  2017-10-23 14:18 ` [PATCH 1/6] Fix unstable test names in gdb.arch/arc-tdesc-cpu.exp Pedro Alves
@ 2017-10-23 14:18 ` Pedro Alves
  2017-10-23 16:24   ` Yao Qi
  2017-10-23 14:28 ` [PATCH 6/6] Drop /proc/PID/status polling from gdb.threads/attach-into-signal.exp Pedro Alves
  5 siblings, 1 reply; 12+ messages in thread
From: Pedro Alves @ 2017-10-23 14:18 UTC (permalink / raw)
  To: gdb-patches

Currently, if you diff testsuite/gdb.sum of two builds in different
directories you see these spurious hunks:

  -PASS: gdb.base/startup-with-shell.exp: touch /home/pedro/gdb1/build/gdb/testsuite/outputs/gdb.base/startup-with-shell/unique-file.unique-extension
  +PASS: gdb.base/startup-with-shell.exp: touch /home/pedro/gdb2/build/gdb/testsuite/outputs/gdb.base/startup-with-shell/unique-file.unique-extension

  -PASS: gdb.base/startup-with-shell.exp: startup_with_shell = on; run_args = *.unique-extension: set args /home/pedro/gdb1/build/gdb/testsuite/outputs/gdb.base/startup-with-shell/*.unique-extension
  +PASS: gdb.base/startup-with-shell.exp: startup_with_shell = on; run_args = *.unique-extension: set args /home/pedro/gdb2/build/gdb/testsuite/outputs/gdb.base/startup-with-shell/*.unique-extension

  -PASS: gdb.base/startup-with-shell.exp: startup_with_shell = off; run_args = *.unique-extension: set args /home/pedro/gdb1/build/gdb/testsuite/outputs/gdb.base/startup-with-shell/*.unique-extension
  +PASS: gdb.base/startup-with-shell.exp: startup_with_shell = off; run_args = *.unique-extension: set args /home/pedro/gdb2/build/gdb/testsuite/outputs/gdb.base/startup-with-shell/*.unique-extension

Since the run_args arguments are already shown in the test prefix, we
can change the "set args" test name to literally "set args $run_args".
I.e., after this commit we'll show:

  PASS: gdb.base/startup-with-shell.exp: startup_with_shell = on; run_args = *.unique-extension: set args $run_args
  PASS: gdb.base/startup-with-shell.exp: startup_with_shell = off; run_args = *.unique-extension: set args $run_args
  PASS: gdb.base/startup-with-shell.exp: startup_with_shell = on; run_args = $TEST: set args $run_args
  PASS: gdb.base/startup-with-shell.exp: startup_with_shell = off; run_args = $TEST: set args $run_args

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.base/startup-with-shell.exp ('touch $unique_file): Don't
	include the unstable output directory name in the test's name.
	(initial_setup_simple) <'set args'>: Use custom test name.
---
 gdb/testsuite/gdb.base/startup-with-shell.exp | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.base/startup-with-shell.exp b/gdb/testsuite/gdb.base/startup-with-shell.exp
index af06c88..29a4ec5 100644
--- a/gdb/testsuite/gdb.base/startup-with-shell.exp
+++ b/gdb/testsuite/gdb.base/startup-with-shell.exp
@@ -37,7 +37,9 @@ if { [prepare_for_testing "failed to prepare" $testfile $srcfile debug] } {
 set unique_file [standard_output_file "unique-file.unique-extension"]
 set unique_file_dir [standard_output_file ""]
 
-run_on_host "touch $unique_file" "touch" "$unique_file"
+run_on_host \
+    "touch OUTPUT_DIR/unique-file.unique-extension" \
+    "touch" "$unique_file"
 
 # Initial setup for simple test (wildcard expansion, variable substitution).
 
@@ -48,7 +50,8 @@ proc initial_setup_simple { startup_with_shell run_args } {
 
     gdb_test_no_output "set startup-with-shell $startup_with_shell"
 
-    gdb_test_no_output "set args $run_args"
+    gdb_test_no_output "set args $run_args" \
+	"set args \$run_args"
 
     set test "inferior started"
     if { [runto_main] } {
-- 
2.5.5

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

* [PATCH 6/6] Drop /proc/PID/status polling from gdb.threads/attach-into-signal.exp
  2017-10-23 14:18 [PATCH 0/6] Fix several cases of unstable test names Pedro Alves
                   ` (4 preceding siblings ...)
  2017-10-23 14:18 ` [PATCH 2/6] Fix unstable test names in gdb.base/startup-with-shell.exp Pedro Alves
@ 2017-10-23 14:28 ` Pedro Alves
  2017-10-24  7:27   ` Yao Qi
  5 siblings, 1 reply; 12+ messages in thread
From: Pedro Alves @ 2017-10-23 14:28 UTC (permalink / raw)
  To: gdb-patches

I noticed that the 'with_test_prefix "stoppedtry $stoppedtry"' prefix
in this testcase is unnecessary, because inside that block there are
no pass/fail calls.  In fact the block includes a comment saying:

  # No PASS message as we may be looping in multiple
  # attempts.

but looking deeper at this I noticed a few odd things with this code
block:

1. This code is assuming that the second line in the /proc/PID/status
   files is the "State:" line, which may have been true when this was
   originally written, but is not true on my machine at least (Linux
   4.8.13).

     $ cat /proc/self/status
     Name:   cat
     Umask:  0002
     State:  R (running)

   So nowadays, that 'string match "*(stopped)*"' is running against
   the "Umask:" line and thus always returns false, meaning the loop
   always breaks on $stoppedtry == 0.

2. The loop seems to be waiting for the process to become "(stopped)",
   but if so then that 'if {![string match]}' check is reversed, it
   should be checking 'if {[string match]}' instead, because "string
   match" returns true if the string matches, not 0.

3. But if we fixed all that, we'd still run into the simple fact that
   nothing is actually stopping the test's inferior process before GDB
   attaches...  The top of the testcase says:

    # This test was created by modifying attach-stopped.exp.

   ... and attach-stopped.exp does have:

       # Stop the program
       remote_exec build "kill -s STOP ${testpid}"

   but then attach-stopped.exp doesn't have an equivalent
   /proc/PID/status poll loop...  (Maybe it could.)

So remove this whole loop as useless.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.threads/attach-into-signal.exp: Remove whole "stoppedtry"
	loop.
---
 gdb/testsuite/gdb.threads/attach-into-signal.exp | 26 ------------------------
 1 file changed, 26 deletions(-)

diff --git a/gdb/testsuite/gdb.threads/attach-into-signal.exp b/gdb/testsuite/gdb.threads/attach-into-signal.exp
index 7502479..e0ea213 100644
--- a/gdb/testsuite/gdb.threads/attach-into-signal.exp
+++ b/gdb/testsuite/gdb.threads/attach-into-signal.exp
@@ -56,32 +56,6 @@ proc corefunc { threadtype executable } {
 	set attempt 1
 	set passes 1
 	while { $passes < 3 && $attempt <= $attempts } {
-		set stoppedtry 0
-		while { $stoppedtry < 10 } {
-		    with_test_prefix "stoppedtry $stoppedtry" {
-			if [catch {open /proc/${testpid}/status r} fileid] {
-			    set stoppedtry 10
-			    break
-			}
-			gets $fileid line1
-			gets $fileid line2
-			close $fileid
-
-			if {![string match "*(stopped)*" $line2]} {
-			    # No PASS message as we may be looping in multiple
-			    # attempts.
-			    break
-			}
-			sleep 1
-			set stoppedtry [expr $stoppedtry + 1]
-		    }
-		}
-		if { $stoppedtry >= 10 } {
-		    verbose -log $line2
-		    set test "process is still running on the attempt # $attempt of $attempts"
-		    break
-		}
-
 		# Main test:
 		set test "attach (pass $passes), pending signal catch"
 		if {[gdb_test_multiple "attach $testpid" $test {
-- 
2.5.5

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

* Re: [PATCH 2/6] Fix unstable test names in gdb.base/startup-with-shell.exp
  2017-10-23 14:18 ` [PATCH 2/6] Fix unstable test names in gdb.base/startup-with-shell.exp Pedro Alves
@ 2017-10-23 16:24   ` Yao Qi
  2017-10-24  9:58     ` Pedro Alves
  0 siblings, 1 reply; 12+ messages in thread
From: Yao Qi @ 2017-10-23 16:24 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Pedro Alves <palves@redhat.com> writes:

> gdb/testsuite/ChangeLog:
> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
>
> 	* gdb.base/startup-with-shell.exp ('touch $unique_file): Don't

Missing closing ' or redundant ' at the beginning?

Patches 1-5 are good to me.  Have to stop here today before reviewing
patch 6.

-- 
Yao (齐尧)

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

* Re: [PATCH 6/6] Drop /proc/PID/status polling from gdb.threads/attach-into-signal.exp
  2017-10-23 14:28 ` [PATCH 6/6] Drop /proc/PID/status polling from gdb.threads/attach-into-signal.exp Pedro Alves
@ 2017-10-24  7:27   ` Yao Qi
  2017-10-24 10:00     ` [pushed] Reindent gdb.threads/attach-into-signal.exp (Re: [PATCH 6/6] Drop /proc/PID/status polling from gdb.threads/attach-into-signal.exp) Pedro Alves
  2017-10-24 10:09     ` [PATCH 7/6] More gdb.threads/attach-into-signal.exp stale stopped bits " Pedro Alves
  0 siblings, 2 replies; 12+ messages in thread
From: Yao Qi @ 2017-10-24  7:27 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Pedro Alves <palves@redhat.com> writes:

> gdb/testsuite/ChangeLog:
> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
>
> 	* gdb.threads/attach-into-signal.exp: Remove whole "stoppedtry"
> 	loop.

Patch is good to me.

-- 
Yao (齐尧)

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

* Re: [PATCH 2/6] Fix unstable test names in gdb.base/startup-with-shell.exp
  2017-10-23 16:24   ` Yao Qi
@ 2017-10-24  9:58     ` Pedro Alves
  0 siblings, 0 replies; 12+ messages in thread
From: Pedro Alves @ 2017-10-24  9:58 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches


On 10/23/2017 05:24 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> gdb/testsuite/ChangeLog:
>> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
>>
>> 	* gdb.base/startup-with-shell.exp ('touch $unique_file): Don't
> 
> Missing closing ' or redundant ' at the beginning?
> 
> Patches 1-5 are good to me.  

Thanks, pushed with that fixed.

> Have to stop here today before reviewing patch 6.

Thanks,
Pedro Alves

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

* [pushed] Reindent gdb.threads/attach-into-signal.exp (Re: [PATCH 6/6] Drop /proc/PID/status polling from gdb.threads/attach-into-signal.exp)
  2017-10-24  7:27   ` Yao Qi
@ 2017-10-24 10:00     ` Pedro Alves
  2017-10-24 10:09     ` [PATCH 7/6] More gdb.threads/attach-into-signal.exp stale stopped bits " Pedro Alves
  1 sibling, 0 replies; 12+ messages in thread
From: Pedro Alves @ 2017-10-24 10:00 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 10/24/2017 08:27 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> gdb/testsuite/ChangeLog:
>> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
>>
>> 	* gdb.threads/attach-into-signal.exp: Remove whole "stoppedtry"
>> 	loop.
> 
> Patch is good to me.

Thanks.  Pushed that one, and as promised, pushed the one below as well.

From eb2bfbadc159ff1463e58daf24c4ad5d1a23796d Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 24 Oct 2017 10:43:45 +0100
Subject: [PATCH] Reindent gdb.threads/attach-into-signal.exp

A previous patch removed one nesting level.

gdb/testsuite/ChangeLog:
2017-10-24  Pedro Alves  <palves@redhat.com>

	* gdb.threads/attach-into-signal.exp (corefunc): Reindent.
---
 gdb/testsuite/ChangeLog                          |  4 ++
 gdb/testsuite/gdb.threads/attach-into-signal.exp | 76 ++++++++++++------------
 2 files changed, 42 insertions(+), 38 deletions(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 25dcd06..d9ed687 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,9 @@
 2017-10-24  Pedro Alves  <palves@redhat.com>
 
+	* gdb.threads/attach-into-signal.exp (corefunc): Reindent.
+
+2017-10-24  Pedro Alves  <palves@redhat.com>
+
 	* gdb.threads/attach-into-signal.exp: Remove whole "stoppedtry"
 	loop.
 
diff --git a/gdb/testsuite/gdb.threads/attach-into-signal.exp b/gdb/testsuite/gdb.threads/attach-into-signal.exp
index ab80e82..3158b59 100644
--- a/gdb/testsuite/gdb.threads/attach-into-signal.exp
+++ b/gdb/testsuite/gdb.threads/attach-into-signal.exp
@@ -56,50 +56,50 @@ proc corefunc { threadtype executable } {
 	set attempt 1
 	set passes 1
 	while { $passes < 3 && $attempt <= $attempts } {
-		set test "attach (pass $passes), pending signal catch"
-		if {[gdb_test_multiple "attach $testpid" $test {
-		    -re "Attaching to program.*`?$escapedbinfile'?, process $testpid.* received signal SIGALRM.*$gdb_prompt $" {
-			# nonthreaded:
-			pass $test
-			verbose -log "$test succeeded on the attempt # $attempt of $attempts"
-			set passes [expr $passes + 1]
-		    }
-		    -re "Attaching to program.*`?$escapedbinfile'?, process $testpid.*$gdb_prompt $" {
-			set ok 0
-
-			if { $threadtype == "threaded" } {
-			    # In the threaded case, the signal is left
-			    # pending on the second thread.  Check for
-			    # that by peeking at the thread's siginfo.
-			    # SIGALRM is 14, SIGSTOP is 19.
-
-			    set test2 "thread apply 2 print \$_siginfo.si_signo"
-			    gdb_test_multiple $test2 $test2 {
-				-re " = 14\r\n$gdb_prompt $" {
-				    set ok 1
-				}
-				-re " = 19\r\n$gdb_prompt $" {
-				}
+	    set test "attach (pass $passes), pending signal catch"
+	    if {[gdb_test_multiple "attach $testpid" $test {
+		-re "Attaching to program.*`?$escapedbinfile'?, process $testpid.* received signal SIGALRM.*$gdb_prompt $" {
+		    # nonthreaded:
+		    pass $test
+		    verbose -log "$test succeeded on the attempt # $attempt of $attempts"
+		    set passes [expr $passes + 1]
+		}
+		-re "Attaching to program.*`?$escapedbinfile'?, process $testpid.*$gdb_prompt $" {
+		    set ok 0
+
+		    if { $threadtype == "threaded" } {
+			# In the threaded case, the signal is left
+			# pending on the second thread.  Check for
+			# that by peeking at the thread's siginfo.
+			# SIGALRM is 14, SIGSTOP is 19.
+
+			set test2 "thread apply 2 print \$_siginfo.si_signo"
+			gdb_test_multiple $test2 $test2 {
+			    -re " = 14\r\n$gdb_prompt $" {
+				set ok 1
+			    }
+			    -re " = 19\r\n$gdb_prompt $" {
 			    }
-			} else {
-			    # In the nonthreaded case, GDB should tell the
-			    # user about having seen a signal.
 			}
+		    } else {
+			# In the nonthreaded case, GDB should tell the
+			# user about having seen a signal.
+		    }
 
-			if { $ok == 0} {
-			    # We just lack the luck, we should try it again.
-			    set attempt [expr $attempt + 1]
-			} else {
-			    pass $test
-			    verbose -log "$test succeeded on the attempt # $attempt of $attempts"
-			    set passes [expr $passes + 1]
-			}
+		    if { $ok == 0} {
+			# We just lack the luck, we should try it again.
+			set attempt [expr $attempt + 1]
+		    } else {
+			pass $test
+			verbose -log "$test succeeded on the attempt # $attempt of $attempts"
+			set passes [expr $passes + 1]
 		    }
-		}] != 0 } {
-		    break
 		}
+	    }] != 0 } {
+		break
+	    }
 
-		gdb_test "detach" "Detaching from.*" ""
+	    gdb_test "detach" "Detaching from.*" ""
 	}
 
 	if {$passes < 3} {
-- 
2.5.5

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

* [PATCH 7/6] More gdb.threads/attach-into-signal.exp stale stopped bits (Re: [PATCH 6/6] Drop /proc/PID/status polling from gdb.threads/attach-into-signal.exp)
  2017-10-24  7:27   ` Yao Qi
  2017-10-24 10:00     ` [pushed] Reindent gdb.threads/attach-into-signal.exp (Re: [PATCH 6/6] Drop /proc/PID/status polling from gdb.threads/attach-into-signal.exp) Pedro Alves
@ 2017-10-24 10:09     ` Pedro Alves
  1 sibling, 0 replies; 12+ messages in thread
From: Pedro Alves @ 2017-10-24 10:09 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 10/24/2017 08:27 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> gdb/testsuite/ChangeLog:
>> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
>>
>> 	* gdb.threads/attach-into-signal.exp: Remove whole "stoppedtry"
>> 	loop.
> 
> Patch is good to me.
> 

Hmmm, noticed a related stale bit further below.  Here's a patch
that removes it.

From 8a645cfef64d4251f34f644792a756959ea440d4 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Mon, 23 Oct 2017 17:46:07 +0100
Subject: [PATCH] More gdb.threads/attach-into-signal.exp stale stopped bits

This looks as stale as the other bit that was removed in a previous
commit.  The test isn't attaching to a stopped process.  (And if this
were generaly necessary, then it'd be better done from within
kill_wait_spawned_process.)

gdb/testsuite/ChangeLog:
2017-10-24  Pedro Alves  <palves@redhat.com>

	* gdb.threads/attach-into-signal.exp (corefunc): Don't SIGCONT
	test program before killing it.
---
 gdb/testsuite/gdb.threads/attach-into-signal.exp | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/gdb/testsuite/gdb.threads/attach-into-signal.exp b/gdb/testsuite/gdb.threads/attach-into-signal.exp
index 3158b59..0af9ed5 100644
--- a/gdb/testsuite/gdb.threads/attach-into-signal.exp
+++ b/gdb/testsuite/gdb.threads/attach-into-signal.exp
@@ -113,10 +113,6 @@ proc corefunc { threadtype executable } {
 	# Exit and detach the process.
 	gdb_exit
 
-	# Continue the program - some Linux kernels need it before -9 if the
-	# process is stopped.
-	remote_exec build "kill -s CONT ${testpid}"
-
 	kill_wait_spawned_process $test_spawn_id
     }
 }
-- 
2.5.5

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

end of thread, other threads:[~2017-10-24 10:09 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-23 14:18 [PATCH 0/6] Fix several cases of unstable test names Pedro Alves
2017-10-23 14:18 ` [PATCH 4/6] Fix unstable test names in gdb.python/py-objfile.exp Pedro Alves
2017-10-23 14:18 ` [PATCH 5/6] Fix unstable test names in gdb.threads/attach-into-signal.exp Pedro Alves
2017-10-23 14:18 ` [PATCH 3/6] Fix unstable test names in gdb.gdb/unittest.exp Pedro Alves
2017-10-23 14:18 ` [PATCH 1/6] Fix unstable test names in gdb.arch/arc-tdesc-cpu.exp Pedro Alves
2017-10-23 14:18 ` [PATCH 2/6] Fix unstable test names in gdb.base/startup-with-shell.exp Pedro Alves
2017-10-23 16:24   ` Yao Qi
2017-10-24  9:58     ` Pedro Alves
2017-10-23 14:28 ` [PATCH 6/6] Drop /proc/PID/status polling from gdb.threads/attach-into-signal.exp Pedro Alves
2017-10-24  7:27   ` Yao Qi
2017-10-24 10:00     ` [pushed] Reindent gdb.threads/attach-into-signal.exp (Re: [PATCH 6/6] Drop /proc/PID/status polling from gdb.threads/attach-into-signal.exp) Pedro Alves
2017-10-24 10:09     ` [PATCH 7/6] More gdb.threads/attach-into-signal.exp stale stopped bits " Pedro Alves

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).