testsuite: Extend TLS core file testing with an OS-generated dump
Commit Message
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(-)
Comments
On Thu, 24 May 2018, Pedro Alves wrote:
> > 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".
Given your observations I've thought about namespaces too, especially as
they can be deleted once they're done with. So why wouldn't they work
with `--target_board=foo,bar', how is the test suite executed in such an
arrangement?
> 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.
Are globals available to those separate processes though? What if one is
updated as a side-effect of a library procedure called from a test case,
(like to achieve a caching effect)?
> > 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.
Great, thanks!
> > 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.
Pushed as commit d8dab6c3bbe6 ("MIPS/Linux: Correct o32 core file FGR
interpretation") now. I hope I didn't make the test case outdated from
the beginning.
Maciej
@@ -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.
@@ -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;
}
@@ -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