This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] testsuite: Extend TLS core file testing with an OS-generated dump


On 05/23/2018 10:46 PM, Maciej W. Rozycki wrote:

>> - Removed the rlimit bits, since it seems that no other core-related test
>>   does that (so it seems to me that if needed, it would better be done
>>   separately and to several testcases at once).
> 
>  Well, I took them from gdb.base/auxv.c, so clearly there's at least one 
> test that has them.  Perhaps you could modernise gdb.base/auxv.exp too?

I'll see about that.

>> As a local hack, I flipped the logic in:
>>  set core_supported [expr {$corefile != ""}]
>> to make sure that the expected UNSUPPORTED messages come out.
>>
>> Let me know what you think.
> 
>  I'd keep that:
> 
> rename tls_core_test ""
> 
> command that I added at the end though, so as not to clutter the procedure 
> space.  Otherwise it'll stay there thoughout the rest of a test suite run 
> (we have some leftover clutter in the testsuite already, which sometimes 
> makes different .exp scripts interact with each other).

Ah, forgot to comment that.  I think that would be a losing battle.
We already have too many tests defining procedures and global variables
and we never took that care.  And frankly, it looks a bit too onerous
to me.  Particularly since to do it throughout would imply undefining
procedures at every early-bail-out return path too.

I think a better approach is to isolate the testcases somehow.

One way is to put each testcase in its own tcl namespace.  It still
wouldn't isolate properly if you test with more than one board at the
same time, like "--target_board=foo,bar".

Another way is to make sure that we run each testcase in its own
separate 'expect' process.  We actually can already do the latter,
if you run the testsuite with "make check-parallel", like e.g.,:

 $ make check-parallel TESTS="gdb.*/tls-core.exp */break.exp"

so we'd need to make "make check" do that by default.  I think
this might be the most promising approach.

> 
>  I ran it native and gdbserver-native, and remote, with correct results.  
> Messages in the commit description need to be updated though, as below:
> 
>> This adds:
>>
>>  PASS: gdb.threads/tls-core.exp: set cwd to temporary directory for core dumps
>>  PASS: gdb.threads/tls-core.exp: continue to signal
>>  PASS: gdb.threads/tls-core.exp: continue to termination
>>  PASS: gdb.threads/tls-core.exp: generate native core dump
>>  PASS: gdb.threads/tls-core.exp: load native corefile
>>  PASS: gdb.threads/tls-core.exp: print thread-local storage variable from native corefile
> 
> PASS: gdb.threads/tls-core.exp: native: load core file
> PASS: gdb.threads/tls-core.exp: native: print thread-local storage variable
> 
> here, and:
> 
>> to local testing and:
>>
>>  UNSUPPORTED: gdb.threads/tls-core.exp: generate native core dump
>>  UNSUPPORTED: gdb.threads/tls-core.exp: load native corefile
>>  UNSUPPORTED: gdb.threads/tls-core.exp: print thread-local storage variable from native corefile
> 
> WARNING: can't generate a core file - core tests suppressed - check ulimit -c
> UNSUPPORTED: gdb.threads/tls-core.exp: native: load core file
> UNSUPPORTED: gdb.threads/tls-core.exp: native: print thread-local storage variable
> 
> here.  Will you handle all this or shall I?

I've handled this now, and pushed the patch, as below.

> 
>  Many thanks for taking care of this.  Your updated test script actually 
> helped me greatly with a test case for the next change I am going to push 
> (another MIPS/Linux core file mishandling -- we live in a reality separate 
> from the kernel's as it turns out).
Nice, great to hear that.

>From d9f6d7f8b636a2b32004273143d72a77d82b40de Mon Sep 17 00:00:00 2001
From: "Maciej W. Rozycki" <macro@mips.com>
Date: Thu, 24 May 2018 15:31:32 +0100
Subject: [PATCH] testsuite: Extend TLS core file testing with an OS-generated
 dump

Complementing commit 280ca31f4d60 ("Add test for fetching TLS from
core file") extend gdb.threads/tls-core.exp with an OS-generated dump
where supported.

This verifies not only that our core dump interpreter is consistent
with our producer, but that it matches the OS verified as well,
avoiding a possible case where our interpreter would be bug-compatible
with our producer but not the OS and it would go unnoticed in testing.

This results in:

 PASS: gdb.threads/tls-core.exp: native: load core file
 PASS: gdb.threads/tls-core.exp: native: print thread-local storage variable
 PASS: gdb.threads/tls-core.exp: gcore: load core file
 PASS: gdb.threads/tls-core.exp: gcore: print thread-local storage variable

with local testing and:

 UNSUPPORTED: gdb.threads/tls-core.exp: native: load core file
 UNSUPPORTED: gdb.threads/tls-core.exp: native: print thread-local storage variable
 PASS: gdb.threads/tls-core.exp: gcore: load core file
 PASS: gdb.threads/tls-core.exp: gcore: print thread-local storage variable

with remote testing, or for testing on ports that don't supports
cores.

gdb/testsuite/ChangeLog:
2018-05-24  Maciej W. Rozycki  <macro@mips.com>
	    Pedro Alves  <palves@redhat.com>

	* gdb.threads/tls-core.c: Include <stdlib.h>
	(thread_proc): Call `abort'.
	* gdb.threads/tls-core.exp: Generate a core with core_find too.
	(tls_core_test): New procedure, bits factored out from ...
	(top level): ... here.  Test both native cores and gcore cores.
---
 gdb/testsuite/ChangeLog                |  9 +++++
 gdb/testsuite/gdb.threads/tls-core.c   |  2 ++
 gdb/testsuite/gdb.threads/tls-core.exp | 63 ++++++++++++++++++++++------------
 3 files changed, 53 insertions(+), 21 deletions(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 4b27640ccb7..426b43823b7 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,12 @@
+2018-05-24  Maciej W. Rozycki  <macro@mips.com>
+	    Pedro Alves  <palves@redhat.com>
+
+	* gdb.threads/tls-core.c: Include <stdlib.h>
+	(thread_proc): Call `abort'.
+	* gdb.threads/tls-core.exp: Generate a core with core_find too.
+	(tls_core_test): New procedure, bits factored out from ...
+	(top level): ... here.  Test both native cores and gcore cores.
+
 2018-05-23  Tom Tromey  <tom@tromey.com>
 
 	* gdb.gdb/complaints.exp (test_initial_complaints): Simplify.
diff --git a/gdb/testsuite/gdb.threads/tls-core.c b/gdb/testsuite/gdb.threads/tls-core.c
index 6089ba116ea..33dd43fb49a 100644
--- a/gdb/testsuite/gdb.threads/tls-core.c
+++ b/gdb/testsuite/gdb.threads/tls-core.c
@@ -16,12 +16,14 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include <pthread.h>
+#include <stdlib.h>
 
 int __thread foo = 0xdeadbeef;
 
 static void *
 thread_proc (void *arg)
 {
+  abort ();
   return arg;
 }
 
diff --git a/gdb/testsuite/gdb.threads/tls-core.exp b/gdb/testsuite/gdb.threads/tls-core.exp
index 730016d3bcf..4d2aaeb2989 100644
--- a/gdb/testsuite/gdb.threads/tls-core.exp
+++ b/gdb/testsuite/gdb.threads/tls-core.exp
@@ -20,37 +20,58 @@ if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
     return -1
 }
 
+# Generate a native core file.
+
+set corefile [core_find $binfile]
+set core_supported [expr {$corefile != ""}]
+
+# Generate a core file with "gcore".
 
 clean_restart ${binfile}
 
 runto thread_proc
 
-#
-# Generate corefile.
-#
-set corefile [standard_output_file gcore.test]
-set core_supported [gdb_gcore_cmd "$corefile" "save a corefile"]
-if {!$core_supported} {
-    return 0
-}
+set gcorefile [standard_output_file gcore.test]
+set gcore_supported [gdb_gcore_cmd "$gcorefile" "gcore"]
 
+# Restart gdb and load COREFILE as a core file.  SUPPORTED is true iff
+# the core was generated successfully; otherwise, the tests are marked
+# unsupported.
 #
-# Restart gdb and load generated corefile.
-#
-clean_restart ${binfile}
+proc tls_core_test {supported corefile} {
+    upvar target_triplet target_triplet
+    upvar host_triplet host_triplet
+    upvar binfile binfile
+
+    clean_restart ${binfile}
+
+    set test "load core file"
+    if {$supported} {
+	set core_loaded [gdb_core_cmd $corefile $test]
+    } else {
+	set core_loaded 0
+	unsupported $test
+    }
+
+    set test "print thread-local storage variable"
+    if { $core_loaded == 1 } {
+	# This fails in cross-debugging due to the use of native
+	# `libthread_db'.
+	if {![string match $host_triplet $target_triplet]} {
+	    setup_kfail "threads/22381" "*-*-*"
+	}
+	gdb_test "p/x foo" "\\$\[0-9]+ = 0xdeadbeef" $test
+    } else {
+	unsupported $test
+    }
+}
 
-set core_loaded [gdb_core_cmd "$corefile" "load generated corefile"]
-if { $core_loaded != 1 } {
-    # No use proceeding from here.
-    return 0
+with_test_prefix "native" {
+    tls_core_test $core_supported $corefile
 }
 
-# This fails in cross-debugging due to the use of native `libthread_db'.
-if {![string match $host_triplet $target_triplet]} {
-    setup_kfail "threads/22381" "*-*-*"
+with_test_prefix "gcore" {
+    tls_core_test $gcore_supported $gcorefile
 }
-gdb_test "p/x foo" \
-	"\\$\[0-9]+ = 0xdeadbeef" \
-	"print thread-local storage variable"
 
 gdb_exit
-- 
2.14.3


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]