Re: [PATCH 1/4] GDB: Add test for fetching TLS from core file

["Maciej W. Rozycki" <macro at imgtec dot com>]

On Fri, 6 Oct 2017, Djordje Todorovic wrote:

> GDB needs correct PID information in order to read value of TLS variable from
> core file, so the test for it looks as following:
> 
> From abfa8dcddc8f3c70ac141f223646a74c787c055a Mon Sep 17 00:00:00 2001
> From: Djordje Todorovic <djordje.todorovic@rt-rk.com>
> Date: Wed, 4 Oct 2017 14:27:54 +0200
> Subject: [PATCH 1/4] GDB: Add test for fetching TLS from core file

 As I noted in the introduction, please make it a proper description 
(fitting in 74 columns) with the ChangeLog entry at the end (as you 
already did), and place any discussion not to be committed after a `---' 
marker.  Likewise with the remaining patches.

 Also I take it this test case does not pass without one or more of the 
remaining patches applied, so please reorder it in the series such as not 
to cause a new failure in its repository revision.

> diff --git a/gdb/testsuite/gdb.threads/tls-core.c
> b/gdb/testsuite/gdb.threads/tls-core.c
> new file mode 100644
> index 0000000..186fdac
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/tls-core.c
> @@ -0,0 +1,48 @@
> +/* This test is part of GDB, the GNU debugger.
> +
> +   Copyright 2017 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <pthread.h>
> +
> +#define NUMBER_OF_THREADS 5
> +
> +int __thread foo = 0xdeadbeef;
> +
> +void *
> +thread (void *in)
> +{
> +  int *tmp = (int *) in;
> +  int value = *tmp;
> +  foo += *tmp;
> +  while (1)
> +  {
> +    sleep (10);
> +  }

 Please indent braces around blocks by 2 spaces.  Are they really needed 
here though?

> +}
> +
> +int
> +main (void)
> +{
> +  pthread_t threads[NUMBER_OF_THREADS];
> +  int i;
> +  for (i = 0; i < NUMBER_OF_THREADS; i++)
> +  {
> +    pthread_create (&threads[i], NULL, thread, &i);
> +  }

 Likewise.

> diff --git a/gdb/testsuite/gdb.threads/tls-core.exp
> b/gdb/testsuite/gdb.threads/tls-core.exp
> new file mode 100644
> index 0000000..c0c7469
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/tls-core.exp
> @@ -0,0 +1,60 @@
> +# Copyright 2017 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +standard_testfile
> +
> +if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
> +	 executable { debug }] != "" } {
> +    return -1
> +}
> +
> +
> +clean_restart ${binfile}
> +
> +#
> +# Set breakpoint at line 27.
> +#
> +gdb_breakpoint 27
> +gdb_test "run" "Starting program: .*"

 Using explicit line numbers in test cases is fragile; can you please try 
using `gdb_get_line_number' instead?

> +
> +#
> +# Generate corefile.
> +#
> +set corefile [standard_output_file gcore.test]
> +set core_supported [gdb_gcore_cmd "$corefile" "save a corefile"]
> +if {!$core_supported} {
> +  return -1

 Inconsistent indentation; please use 4 leading spaces as elsewhere.

  Maciej