From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 124441 invoked by alias); 24 May 2018 14:35:03 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 124424 invoked by uid 89); 24 May 2018 14:35:02 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=losing, H*f:sk:5b76142, core_find, promising X-HELO: mx1.redhat.com Received: from mx3-rdu2.redhat.com (HELO mx1.redhat.com) (66.187.233.73) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 24 May 2018 14:34:59 +0000 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 406737C6A9; Thu, 24 May 2018 14:34:58 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6879E2166BB2; Thu, 24 May 2018 14:34:56 +0000 (UTC) Subject: Re: [PATCH] testsuite: Extend TLS core file testing with an OS-generated dump To: "Maciej W. Rozycki" References: <5b76142d-479d-3c34-9294-9f4510d0fc04@redhat.com> Cc: gdb-patches@sourceware.org, Djordje Todorovic From: Pedro Alves Message-ID: <8f45ebc8-8b8e-57b7-be5b-15e2f509d5f6@redhat.com> Date: Thu, 24 May 2018 15:05:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-05/txt/msg00648.txt.bz2 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" 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 Pedro Alves * gdb.threads/tls-core.c: Include (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 + Pedro Alves + + * gdb.threads/tls-core.c: Include + (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 * 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 . */ #include +#include 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