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

Message ID alpine.DEB.2.00.1805221919470.10896@tp.orcam.me.uk
State Superseded
Headers

Commit Message

Maciej W. Rozycki May 22, 2018, 11:10 p.m. UTC
  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
  

Comments

Djordje Todorovic May 23, 2018, 7:31 a.m. UTC | #1
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
>
  
Pedro Alves May 23, 2018, 12:04 p.m. UTC | #2
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
  
Maciej W. Rozycki May 23, 2018, 12:42 p.m. UTC | #3
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
  

Patch

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