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

[Djordje Todorovic <djordje dot todorovic at rt-rk dot com>]

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
>