* [PATCH] testsuite: Extend TLS core file testing with an OS-generated dump @ 2018-05-23 4:59 Maciej W. Rozycki 2018-05-23 8:50 ` Djordje Todorovic 2018-05-23 13:35 ` Pedro Alves 0 siblings, 2 replies; 8+ messages in thread From: Maciej W. Rozycki @ 2018-05-23 4:59 UTC (permalink / raw) To: gdb-patches; +Cc: Djordje Todorovic 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 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 to native 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 to gdbserver testing. The latter is because we'd have to handle core file copying from the remote system and then cleaning up with a possibly variable name of the core file created. This code has been based on gdb.base/auxv.exp, which does not do it either. gdb/testsuite/ * gdb.threads/tls-core.c [USE_RLIMIT] (RLIM_INFINITY): Define macro if not defined. (thread_proc): Call `abort'. (main) [USE_RLIMIT]: Call `setrlimit' for RLIMIT_CORE. * gdb.threads/tls-core.exp: Verify an OS-generated core too. --- Hi, Dusted off and verified with native and native-gdbserver o32, n32 and n64 MIPS targets. OK to apply? Maciej --- gdb/testsuite/gdb.threads/tls-core.c | 17 ++++++ gdb/testsuite/gdb.threads/tls-core.exp | 90 +++++++++++++++++++++++++++------ 2 files changed, 91 insertions(+), 16 deletions(-) gdb-tls-core-test-update.diff Index: binutils/gdb/testsuite/gdb.threads/tls-core.c =================================================================== --- binutils.orig/gdb/testsuite/gdb.threads/tls-core.c 2017-11-06 21:58:53.369742438 +0000 +++ binutils/gdb/testsuite/gdb.threads/tls-core.c 2017-11-06 22:47:22.684061150 +0000 @@ -16,12 +16,21 @@ along with this program. If not, see <http://www.gnu.org/licenses/>. */ #include <pthread.h> +#include <stdlib.h> + +#ifdef USE_RLIMIT +# include <sys/resource.h> +# ifndef RLIM_INFINITY +# define RLIM_INFINITY -1 +# endif +#endif /* USE_RLIMIT */ int __thread foo = 0xdeadbeef; static void * thread_proc (void *arg) { + abort (); return arg; } @@ -30,6 +39,14 @@ main (void) { pthread_t thread; +#ifdef USE_RLIMIT + { + struct rlimit rlim = { RLIM_INFINITY, RLIM_INFINITY }; + + setrlimit (RLIMIT_CORE, &rlim); + } +#endif + pthread_create (&thread, NULL, thread_proc, NULL); pthread_join (thread, NULL); Index: binutils/gdb/testsuite/gdb.threads/tls-core.exp =================================================================== --- binutils.orig/gdb/testsuite/gdb.threads/tls-core.exp 2017-11-06 21:58:53.384913472 +0000 +++ binutils/gdb/testsuite/gdb.threads/tls-core.exp 2017-11-06 22:50:40.419947495 +0000 @@ -16,41 +16,99 @@ standard_testfile if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" \ + executable { debug additional_flags=-DUSE_RLIMIT }] != "" + && [gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" \ executable { debug }] != "" } { return -1 } - clean_restart ${binfile} +# +# Use a fresh directory to confine the native core dumps. +# Make it the working directory for the inferior. +# +set core_supported [expr ![use_gdb_stub]] +if {$core_supported} { + set coredir [standard_output_file coredir.[getpid]] + file mkdir $coredir + if {[gdb_test_no_output "set cwd $coredir" \ + "set cwd to temporary directory for core dumps"]} { + set core_supported 0 + } +} + runto thread_proc # # Generate corefile. # -set corefile [standard_output_file gcore.test] -set core_supported [gdb_gcore_cmd "$corefile" "save a corefile"] +set gcorefile [standard_output_file gcore.test] +set gcore_supported [gdb_gcore_cmd "$gcorefile" "save a corefile"] + +# +# Let the program die making the OS dump core if possible too. +# +set corefile [standard_output_file core.test] +set test "generate native core dump" +if {$core_supported} { + gdb_test continue ".*Thread .* received signal SIGABRT.*" \ + "continue to signal" + gdb_test continue ".*Program terminated with signal SIGABRT.*" \ + "continue to termination" + + # Find the core file produced. + set names [glob -nocomplain -directory $coredir *core*] + if {[llength $names] == 1} { + set file [file join $coredir [lindex $names 0]] + remote_exec build "mv $file $corefile" + pass $test + } else { + set core_supported 0 + warning "can't generate a core file - core tests suppressed -\ + check ulimit -c" + } + + remote_exec build "rm -rf $coredir" +} if {!$core_supported} { - return 0 + unsupported $test } # # Restart gdb and load generated corefile. # -clean_restart ${binfile} +proc tls_core_test {supported corefile test1 test2} { + upvar target_triplet target_triplet + upvar host_triplet host_triplet + upvar binfile binfile -set core_loaded [gdb_core_cmd "$corefile" "load generated corefile"] -if { $core_loaded != 1 } { - # No use proceeding from here. - return 0 -} + clean_restart ${binfile} -# This fails in cross-debugging due to the use of native `libthread_db'. -if {![string match $host_triplet $target_triplet]} { - setup_kfail "threads/22381" "*-*-*" + if {$supported} { + set core_loaded [gdb_core_cmd $corefile $test1] + } else { + set core_loaded 0 + unsupported $test1 + } + 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" $test2 + } else { + unsupported $test2 + } } -gdb_test "p/x foo" \ - "\\$\[0-9]+ = 0xdeadbeef" \ - "print thread-local storage variable" + +tls_core_test $gcore_supported $gcorefile "load generated corefile" \ + "print thread-local storage variable from generated corefile" + +tls_core_test $core_supported $corefile "load native corefile" \ + "print thread-local storage variable from native corefile" + +rename tls_core_test "" gdb_exit ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] testsuite: Extend TLS core file testing with an OS-generated dump 2018-05-23 4:59 [PATCH] testsuite: Extend TLS core file testing with an OS-generated dump Maciej W. Rozycki @ 2018-05-23 8:50 ` Djordje Todorovic 2018-05-23 13:35 ` Pedro Alves 1 sibling, 0 replies; 8+ messages in thread From: Djordje Todorovic @ 2018-05-23 8:50 UTC (permalink / raw) To: Maciej W. Rozycki, gdb-patches Hi Maciej, This looks good to me. Thanks for this! Best regards, Djordje On 23.05.2018. 01:10, Maciej W. Rozycki wrote: > 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 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 > > to native 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 > > to gdbserver testing. The latter is because we'd have to handle core > file copying from the remote system and then cleaning up with a possibly > variable name of the core file created. This code has been based on > gdb.base/auxv.exp, which does not do it either. > > gdb/testsuite/ > * gdb.threads/tls-core.c [USE_RLIMIT] (RLIM_INFINITY): Define > macro if not defined. > (thread_proc): Call `abort'. > (main) [USE_RLIMIT]: Call `setrlimit' for RLIMIT_CORE. > * gdb.threads/tls-core.exp: Verify an OS-generated core too. > --- > Hi, > > Dusted off and verified with native and native-gdbserver o32, n32 and n64 > MIPS targets. OK to apply? > > Maciej > --- > gdb/testsuite/gdb.threads/tls-core.c | 17 ++++++ > gdb/testsuite/gdb.threads/tls-core.exp | 90 +++++++++++++++++++++++++++------ > 2 files changed, 91 insertions(+), 16 deletions(-) > > gdb-tls-core-test-update.diff > Index: binutils/gdb/testsuite/gdb.threads/tls-core.c > =================================================================== > --- binutils.orig/gdb/testsuite/gdb.threads/tls-core.c 2017-11-06 21:58:53.369742438 +0000 > +++ binutils/gdb/testsuite/gdb.threads/tls-core.c 2017-11-06 22:47:22.684061150 +0000 > @@ -16,12 +16,21 @@ > along with this program. If not, see <http://www.gnu.org/licenses/>. */ > > #include <pthread.h> > +#include <stdlib.h> > + > +#ifdef USE_RLIMIT > +# include <sys/resource.h> > +# ifndef RLIM_INFINITY > +# define RLIM_INFINITY -1 > +# endif > +#endif /* USE_RLIMIT */ > > int __thread foo = 0xdeadbeef; > > static void * > thread_proc (void *arg) > { > + abort (); > return arg; > } > > @@ -30,6 +39,14 @@ main (void) > { > pthread_t thread; > > +#ifdef USE_RLIMIT > + { > + struct rlimit rlim = { RLIM_INFINITY, RLIM_INFINITY }; > + > + setrlimit (RLIMIT_CORE, &rlim); > + } > +#endif > + > pthread_create (&thread, NULL, thread_proc, NULL); > pthread_join (thread, NULL); > > Index: binutils/gdb/testsuite/gdb.threads/tls-core.exp > =================================================================== > --- binutils.orig/gdb/testsuite/gdb.threads/tls-core.exp 2017-11-06 21:58:53.384913472 +0000 > +++ binutils/gdb/testsuite/gdb.threads/tls-core.exp 2017-11-06 22:50:40.419947495 +0000 > @@ -16,41 +16,99 @@ > standard_testfile > > if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" \ > + executable { debug additional_flags=-DUSE_RLIMIT }] != "" > + && [gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" \ > executable { debug }] != "" } { > return -1 > } > > - > clean_restart ${binfile} > > +# > +# Use a fresh directory to confine the native core dumps. > +# Make it the working directory for the inferior. > +# > +set core_supported [expr ![use_gdb_stub]] > +if {$core_supported} { > + set coredir [standard_output_file coredir.[getpid]] > + file mkdir $coredir > + if {[gdb_test_no_output "set cwd $coredir" \ > + "set cwd to temporary directory for core dumps"]} { > + set core_supported 0 > + } > +} > + > runto thread_proc > > # > # Generate corefile. > # > -set corefile [standard_output_file gcore.test] > -set core_supported [gdb_gcore_cmd "$corefile" "save a corefile"] > +set gcorefile [standard_output_file gcore.test] > +set gcore_supported [gdb_gcore_cmd "$gcorefile" "save a corefile"] > + > +# > +# Let the program die making the OS dump core if possible too. > +# > +set corefile [standard_output_file core.test] > +set test "generate native core dump" > +if {$core_supported} { > + gdb_test continue ".*Thread .* received signal SIGABRT.*" \ > + "continue to signal" > + gdb_test continue ".*Program terminated with signal SIGABRT.*" \ > + "continue to termination" > + > + # Find the core file produced. > + set names [glob -nocomplain -directory $coredir *core*] > + if {[llength $names] == 1} { > + set file [file join $coredir [lindex $names 0]] > + remote_exec build "mv $file $corefile" > + pass $test > + } else { > + set core_supported 0 > + warning "can't generate a core file - core tests suppressed -\ > + check ulimit -c" > + } > + > + remote_exec build "rm -rf $coredir" > +} > if {!$core_supported} { > - return 0 > + unsupported $test > } > > # > # Restart gdb and load generated corefile. > # > -clean_restart ${binfile} > +proc tls_core_test {supported corefile test1 test2} { > + upvar target_triplet target_triplet > + upvar host_triplet host_triplet > + upvar binfile binfile > > -set core_loaded [gdb_core_cmd "$corefile" "load generated corefile"] > -if { $core_loaded != 1 } { > - # No use proceeding from here. > - return 0 > -} > + clean_restart ${binfile} > > -# This fails in cross-debugging due to the use of native `libthread_db'. > -if {![string match $host_triplet $target_triplet]} { > - setup_kfail "threads/22381" "*-*-*" > + if {$supported} { > + set core_loaded [gdb_core_cmd $corefile $test1] > + } else { > + set core_loaded 0 > + unsupported $test1 > + } > + 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" $test2 > + } else { > + unsupported $test2 > + } > } > -gdb_test "p/x foo" \ > - "\\$\[0-9]+ = 0xdeadbeef" \ > - "print thread-local storage variable" > + > +tls_core_test $gcore_supported $gcorefile "load generated corefile" \ > + "print thread-local storage variable from generated corefile" > + > +tls_core_test $core_supported $corefile "load native corefile" \ > + "print thread-local storage variable from native corefile" > + > +rename tls_core_test "" > > gdb_exit > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] testsuite: Extend TLS core file testing with an OS-generated dump 2018-05-23 4:59 [PATCH] testsuite: Extend TLS core file testing with an OS-generated dump Maciej W. Rozycki 2018-05-23 8:50 ` Djordje Todorovic @ 2018-05-23 13:35 ` Pedro Alves 2018-05-23 14:17 ` Maciej W. Rozycki 1 sibling, 1 reply; 8+ messages in thread From: Pedro Alves @ 2018-05-23 13:35 UTC (permalink / raw) To: Maciej W. Rozycki, gdb-patches; +Cc: Djordje Todorovic On 05/23/2018 12:10 AM, Maciej W. Rozycki wrote: > to gdbserver testing. The latter is because we'd have to handle core > file copying from the remote system and then cleaning up with a possibly > variable name of the core file created. This code has been based on > gdb.base/auxv.exp, which does not do it either. Hmm, that sounds like an outdated testcase to copy. See gdb.threads/corethreads.exp, for example, and the use of the core_find procedure. Can we use that procedure here too? I also don't see why (gdbserver && !is_remote) testing can't work, such as with --target=native-gdbserver and --target=native-extended-gdbserver boards. It works for gdb.threads/corethreads.exp, for example. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] testsuite: Extend TLS core file testing with an OS-generated dump 2018-05-23 13:35 ` Pedro Alves @ 2018-05-23 14:17 ` Maciej W. Rozycki 2018-05-23 17:33 ` Pedro Alves 0 siblings, 1 reply; 8+ messages in thread From: Maciej W. Rozycki @ 2018-05-23 14:17 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches, Djordje Todorovic On Wed, 23 May 2018, Pedro Alves wrote: > > to gdbserver testing. The latter is because we'd have to handle core > > file copying from the remote system and then cleaning up with a possibly > > variable name of the core file created. This code has been based on > > gdb.base/auxv.exp, which does not do it either. > > Hmm, that sounds like an outdated testcase to copy. See > gdb.threads/corethreads.exp, for example, and the use of the core_find > procedure. Can we use that procedure here too? Possibly. The test case is skipped with all my testing, native or native-gdbserver, due to the use of `![isnative]' (I have $build != $host; the native compiler I have on the host machine is too old to build GDB these days), so I didn't even notice it there. > I also don't see why (gdbserver && !is_remote) testing can't work, such as > with --target=native-gdbserver and --target=native-extended-gdbserver boards. > It works for gdb.threads/corethreads.exp, for example. Acknowledged. Given the current situation I cannot afford investing any further work into this change, so let's ditch it (and a change to gdb.base/auxv.exp to use `use_gdb_stub' I planned to submit next). If I find some time after all, then I can revisit the decision later on. Maciej ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] testsuite: Extend TLS core file testing with an OS-generated dump 2018-05-23 14:17 ` Maciej W. Rozycki @ 2018-05-23 17:33 ` Pedro Alves 2018-05-23 23:59 ` Maciej W. Rozycki 0 siblings, 1 reply; 8+ messages in thread From: Pedro Alves @ 2018-05-23 17:33 UTC (permalink / raw) To: Maciej W. Rozycki; +Cc: gdb-patches, Djordje Todorovic On 05/23/2018 01:42 PM, Maciej W. Rozycki wrote: > On Wed, 23 May 2018, Pedro Alves wrote: > >>> to gdbserver testing. The latter is because we'd have to handle core >>> file copying from the remote system and then cleaning up with a possibly >>> variable name of the core file created. This code has been based on >>> gdb.base/auxv.exp, which does not do it either. >> >> Hmm, that sounds like an outdated testcase to copy. See >> gdb.threads/corethreads.exp, for example, and the use of the core_find >> procedure. Can we use that procedure here too? > > Possibly. The test case is skipped with all my testing, native or > native-gdbserver, due to the use of `![isnative]' (I have $build != > $host; the native compiler I have on the host machine is too old to > build GDB these days), so I didn't even notice it there. > >> I also don't see why (gdbserver && !is_remote) testing can't work, such as >> with --target=native-gdbserver and --target=native-extended-gdbserver boards. >> It works for gdb.threads/corethreads.exp, for example. > > Acknowledged. Given the current situation I cannot afford investing > any further work into this change, so let's ditch it (and a change to > gdb.base/auxv.exp to use `use_gdb_stub' I planned to submit next). If I > find some time after all, then I can revisit the decision later on. I understand. Let me take it over then. Below's a version using core_find. I also simplified the patch a bit: - 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). - Removed the test message parameters from tls_core_test, and switched to using with_test_prefix instead - Extended tls_core_test's description a bit to mention the SUPPORTED parameter. This gives an all pass for me on native, native-gdbserver, and native-extended-gdbserver (x86-64 GNU/Linux). 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. Thanks, Pedro Alves From 154986d31cc124486e28076e0902065f054dff12 Mon Sep 17 00:00:00 2001 From: "Maciej W. Rozycki" <macro@mips.com> Date: Wed, 23 May 2018 15:33:55 +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 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 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 to remote testing, or for testing on ports that don't supports cores. gdb/testsuite/ChangeLog: 2018-05-23 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/gdb.threads/tls-core.c | 2 ++ gdb/testsuite/gdb.threads/tls-core.exp | 63 ++++++++++++++++++++++------------ 2 files changed, 44 insertions(+), 21 deletions(-) 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] testsuite: Extend TLS core file testing with an OS-generated dump 2018-05-23 17:33 ` Pedro Alves @ 2018-05-23 23:59 ` Maciej W. Rozycki 2018-05-24 15:05 ` Pedro Alves 0 siblings, 1 reply; 8+ messages in thread From: Maciej W. Rozycki @ 2018-05-23 23:59 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches, Djordje Todorovic Hi Pedro, > > Acknowledged. Given the current situation I cannot afford investing > > any further work into this change, so let's ditch it (and a change to > > gdb.base/auxv.exp to use `use_gdb_stub' I planned to submit next). If I > > find some time after all, then I can revisit the decision later on. > I understand. Let me take it over then. Below's a version using > core_find. > > I also simplified the patch a bit: > > - 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? > 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). 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? 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). Maciej ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] testsuite: Extend TLS core file testing with an OS-generated dump 2018-05-23 23:59 ` Maciej W. Rozycki @ 2018-05-24 15:05 ` Pedro Alves 2018-05-25 13:49 ` Maciej W. Rozycki 0 siblings, 1 reply; 8+ messages in thread From: Pedro Alves @ 2018-05-24 15:05 UTC (permalink / raw) To: Maciej W. Rozycki; +Cc: gdb-patches, Djordje Todorovic 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] testsuite: Extend TLS core file testing with an OS-generated dump 2018-05-24 15:05 ` Pedro Alves @ 2018-05-25 13:49 ` Maciej W. Rozycki 0 siblings, 0 replies; 8+ messages in thread From: Maciej W. Rozycki @ 2018-05-25 13:49 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches, Djordje Todorovic 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-05-25 11:54 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-05-23 4:59 [PATCH] testsuite: Extend TLS core file testing with an OS-generated dump Maciej W. Rozycki 2018-05-23 8:50 ` Djordje Todorovic 2018-05-23 13:35 ` Pedro Alves 2018-05-23 14:17 ` Maciej W. Rozycki 2018-05-23 17:33 ` Pedro Alves 2018-05-23 23:59 ` Maciej W. Rozycki 2018-05-24 15:05 ` Pedro Alves 2018-05-25 13:49 ` Maciej W. Rozycki
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).