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

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

[Maciej W. Rozycki]

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

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

[Djordje Todorovic]

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
> 

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

[Pedro Alves]

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

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

[Maciej W. Rozycki]

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

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

[Pedro Alves]

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

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

[Maciej W. Rozycki]

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

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

[Pedro Alves]

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

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

[Maciej W. Rozycki]

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