public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] PR varobj/18564  regression in showing __thread so extern variable
@ 2015-08-30 11:34 Philippe Waroquiers
  2015-09-02 13:03 ` Yao Qi
  0 siblings, 1 reply; 9+ messages in thread
From: Philippe Waroquiers @ 2015-08-30 11:34 UTC (permalink / raw)
  To: gdb-patches

Hello,

The below patch fixes the regression
https://sourceware.org/bugzilla/show_bug.cgi?id=18564
introduced in
https://sourceware.org/ml/gdb-patches/2014-02/msg00811.html

Due to the regression, __thread variables in a shared lib
(such as errno) cannot be printed.

Thanks to Mark Wielaard that bisect the regression
origin, and thanks to Tom Tromey, that helped investigating
(after the end of the warranty of the 2014 patch :).

The regression is caused by relocating the TLS offset.
The fix (using MSYMBOL_VALUE_RAW_ADDRESS) is similar to what has
been done in
https://sourceware.org/ml/gdb-patches/2015-08/msg00803.html
([Cell/B.E.] Fix wrong relocation for TLS variable offset).

Tested on x86/Debian8.

Ok to commit ?

Thanks

Philippe

2015-09-??  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* symtab.h (address_class): Document that TLS variables
        are handled by LOC_UNRESOLVED.
	* findvar.c (default_read_var_value): Don't relocate TLS variables.
	* printcmd.c (address_info): Don't relocate TLS variables.

2015-09-??  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* gdb.threads/tls-so_extern.exp: New test file.
	* gdb.threads/tls-so_extern.c: New test file.
	* gdb.threads/tls-so_extern_main.c: New test file.

diff --git a/gdb/findvar.c b/gdb/findvar.c
index 1c077f7..fd1b9d7 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -739,14 +739,17 @@ default_read_var_value (struct symbol *var, const struct block *var_block,
 
 	if (msym == NULL)
 	  error (_("No global symbol \"%s\"."), SYMBOL_LINKAGE_NAME (var));
-	if (overlay_debugging)
-	  addr = symbol_overlayed_address (BMSYMBOL_VALUE_ADDRESS (lookup_data.result),
-					   MSYMBOL_OBJ_SECTION (lookup_data.result.objfile,
-								msym));
-	else
-	  addr = BMSYMBOL_VALUE_ADDRESS (lookup_data.result);
-
 	obj_section = MSYMBOL_OBJ_SECTION (lookup_data.result.objfile, msym);
+	/* Relocate address, unless there is no section or the variable is
+	   a TLS variable. */
+	if (obj_section == NULL
+	    || (obj_section->the_bfd_section->flags & SEC_THREAD_LOCAL) != 0)
+	   addr = MSYMBOL_VALUE_RAW_ADDRESS (msym);
+	else
+	   addr = BMSYMBOL_VALUE_ADDRESS (lookup_data.result);
+	if (overlay_debugging)
+	  addr = symbol_overlayed_address (addr, obj_section);
+	/* Determine address of TLS variable. */
 	if (obj_section
 	    && (obj_section->the_bfd_section->flags & SEC_THREAD_LOCAL) != 0)
 	  addr = target_translate_tls_address (obj_section->objfile, addr);
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 5729b24..823f27b 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -1376,16 +1376,19 @@ address_info (char *exp, int from_tty)
 	else
 	  {
 	    section = MSYMBOL_OBJ_SECTION (msym.objfile, msym.minsym);
-	    load_addr = BMSYMBOL_VALUE_ADDRESS (msym);
 
 	    if (section
 		&& (section->the_bfd_section->flags & SEC_THREAD_LOCAL) != 0)
-	      printf_filtered (_("a thread-local variable at offset %s "
-				 "in the thread-local storage for `%s'"),
-			       paddress (gdbarch, load_addr),
-			       objfile_name (section->objfile));
+	      {
+		load_addr = MSYMBOL_VALUE_RAW_ADDRESS (msym.minsym);
+		printf_filtered (_("a thread-local variable at offset %s "
+				   "in the thread-local storage for `%s'"),
+				 paddress (gdbarch, load_addr),
+				 objfile_name (section->objfile));
+	      }
 	    else
 	      {
+		load_addr = BMSYMBOL_VALUE_ADDRESS (msym);
 		printf_filtered (_("static storage at address "));
 		fputs_filtered (paddress (gdbarch, load_addr), gdb_stdout);
 		if (section_is_overlay (section))
diff --git a/gdb/symtab.h b/gdb/symtab.h
index c6f26e7..d4a8e5f 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -584,7 +584,13 @@ enum address_class
      not find it in the full symbol table.  But a reference to an external
      symbol in a local block shadowing other definition requires full symbol
      without possibly having its address available for LOC_STATIC.  Testcase
-     is provided as `gdb.dwarf2/dw2-unresolved.exp'.  */
+     is provided as `gdb.dwarf2/dw2-unresolved.exp'.
+
+     This is also used for thread local storage (TLS) variables.  In this case,
+     the address of the TLS variable must be determined when the variable is
+     referenced, from the MSYMBOL_VALUE_RAW_ADDRESS, which is the offset
+     of the TLS variable in the thread local storage of the shared
+     library/object.  */
 
   LOC_UNRESOLVED,
 
diff --git a/gdb/testsuite/gdb.threads/tls-so_extern.c b/gdb/testsuite/gdb.threads/tls-so_extern.c
new file mode 100644
index 0000000..1559de2
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/tls-so_extern.c
@@ -0,0 +1 @@
+__thread void *so_extern;
diff --git a/gdb/testsuite/gdb.threads/tls-so_extern.exp b/gdb/testsuite/gdb.threads/tls-so_extern.exp
new file mode 100644
index 0000000..c4acb21
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/tls-so_extern.exp
@@ -0,0 +1,70 @@
+# Copyright 2015-2015 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/>.  */
+
+# tls-so_extern.exp -- Expect script to test thread local storage in gdb, with
+# a variable defined in a shared library.
+
+standard_testfile tls-so_extern_main.c
+set libfile tls-so_extern
+set srcfile_lib ${libfile}.c
+set binfile_lib [standard_output_file ${libfile}.so]
+
+remote_exec build "rm -f ${binfile}"
+
+# get the value of gcc_compiled
+if [get_compiler_info] {
+    return -1
+}
+
+
+if { [gdb_compile_shlib_pthreads ${srcdir}/${subdir}/${srcfile_lib} ${binfile_lib} {debug}] != ""
+     || [gdb_compile_pthreads ${srcdir}/${subdir}/${srcfile} ${binfile} executable [list debug shlib=${binfile_lib}]] != ""} {
+    return -1
+}
+
+
+clean_restart ${binfile}
+gdb_load_shlibs ${binfile_lib}
+
+if ![runto_main] then {
+    fail "Can't run to main"
+    return 0
+}
+
+gdb_test "print so_extern" "0x0" "print thread local storage variable"
+
+gdb_test "ptype so_extern" "void \\*" "ptype of thread local storage variable"
+
+gdb_test "info address so_extern" \
+	"Symbol \\\"so_extern\\\" is a thread-local variable at offset 0x0 in the thread-local storage for .*tls-so_extern.*" \
+	"print storage info for thread local storage variable"
+
+set line_number [gdb_get_line_number "break here to check result"]
+
+gdb_test "break $line_number" \
+	"Breakpoint.*at.*file.*tls-so_extern_main.c.*line ${line_number}." \
+	"break in thread function"
+gdb_test "continue" \
+	"tls_ptr .* at .*:.*printf.*break here to check result.*" \
+        "continue to break in a thread"
+gdb_test "print so_extern == &so_extern" \
+         " = 1" \
+        "check so_extern address"
+gdb_test "continue" \
+	"tls_ptr .* at .*:.*printf.*break here to check result.*" \
+        "continue to break in the other thread"
+gdb_test "print so_extern == &so_extern" \
+         " = 1" \
+        "check so_extern address in other thread"
diff --git a/gdb/testsuite/gdb.threads/tls-so_extern_main.c b/gdb/testsuite/gdb.threads/tls-so_extern_main.c
new file mode 100644
index 0000000..5531fda
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/tls-so_extern_main.c
@@ -0,0 +1,21 @@
+#include <stdio.h>
+#include <pthread.h>
+
+extern __thread void *so_extern;
+
+static void *tls_ptr(void *p)
+{
+   so_extern = &so_extern;
+   printf("address is %p\n", &so_extern); /* break here to check result */
+}
+
+int main()
+{
+   pthread_t threads[2];
+   pthread_create (&threads[0], NULL, tls_ptr, NULL);
+   pthread_create (&threads[1], NULL, tls_ptr, NULL);
+
+   pthread_join(threads[0], NULL);
+   pthread_join(threads[1], NULL);
+}
+


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] PR varobj/18564  regression in showing __thread so extern variable
  2015-08-30 11:34 [PATCH] PR varobj/18564 regression in showing __thread so extern variable Philippe Waroquiers
@ 2015-09-02 13:03 ` Yao Qi
  2015-09-02 21:23   ` Philippe Waroquiers
  0 siblings, 1 reply; 9+ messages in thread
From: Yao Qi @ 2015-09-02 13:03 UTC (permalink / raw)
  To: Philippe Waroquiers; +Cc: gdb-patches

Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Hi Philippe,
I am not an expert on symbol stuff, but I can only review test case.
Some one else may review the rest of the patch later.

> diff --git a/gdb/testsuite/gdb.threads/tls-so_extern.c b/gdb/testsuite/gdb.threads/tls-so_extern.c
> new file mode 100644
> index 0000000..1559de2
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/tls-so_extern.c
> @@ -0,0 +1 @@
> +__thread void *so_extern;

We need a copy right header.

> diff --git a/gdb/testsuite/gdb.threads/tls-so_extern.exp b/gdb/testsuite/gdb.threads/tls-so_extern.exp
> new file mode 100644
> index 0000000..c4acb21
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/tls-so_extern.exp
> @@ -0,0 +1,70 @@
> +# Copyright 2015-2015 Free Software Foundation, Inc.

Either 2014-2015 or 2015.

> +
> +# 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/>.  */
> +
> +# tls-so_extern.exp -- Expect script to test thread local storage in gdb, with
> +# a variable defined in a shared library.
> +
> +standard_testfile tls-so_extern_main.c
> +set libfile tls-so_extern
> +set srcfile_lib ${libfile}.c
> +set binfile_lib [standard_output_file ${libfile}.so]
> +
> +remote_exec build "rm -f ${binfile}"
> +

Don't need to remove binfile.

> diff --git a/gdb/testsuite/gdb.threads/tls-so_extern_main.c b/gdb/testsuite/gdb.threads/tls-so_extern_main.c
> new file mode 100644
> index 0000000..5531fda
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/tls-so_extern_main.c
> @@ -0,0 +1,21 @@

Copy right header is needed.

> +#include <stdio.h>
> +#include <pthread.h>
> +
> +extern __thread void *so_extern;
> +
> +static void *tls_ptr(void *p)

The code should comply to GNU coding standard.

> +{
> +   so_extern = &so_extern;
> +   printf("address is %p\n", &so_extern); /* break here to check result */

Don't have to call printf and include stdio.h.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] PR varobj/18564  regression in showing __thread so extern variable
  2015-09-02 13:03 ` Yao Qi
@ 2015-09-02 21:23   ` Philippe Waroquiers
  2015-09-03  9:37     ` Yao Qi
  0 siblings, 1 reply; 9+ messages in thread
From: Philippe Waroquiers @ 2015-09-02 21:23 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On Wed, 2015-09-02 at 14:03 +0100, Yao Qi wrote:
> Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:
> 
> Hi Philippe,
> I am not an expert on symbol stuff, but I can only review test case.
> Some one else may review the rest of the patch later.
Thanks for the review comments, find feedback below.
I have handled all comments (except one for which I have a question).
I will repost a V2 patch after review of the rest of the patch (i.e.
the bug fixing part).

> > diff --git a/gdb/testsuite/gdb.threads/tls-so_extern.c b/gdb/testsuite/gdb.threads/tls-so_extern.c
> We need a copy right header.
copyright header added
  (note that several existing files in gdb.threads
   have no copyright header, e.g. tls.c, tls-main.c, ...)

> 
> > diff --git a/gdb/testsuite/gdb.threads/tls-so_extern.exp b/gdb/testsuite/gdb.threads/tls-so_extern.exp
> Either 2014-2015 or 2015.
Changed to 2003-2015, as I started from tls-shared.exp


> > +remote_exec build "rm -f ${binfile}"
> Don't need to remove binfile.
I removed the remove :).
Note that this line originates from tls-shared.exp.

> > diff --git a/gdb/testsuite/gdb.threads/tls-so_extern_main.c b/gdb/testsuite/gdb.threads/tls-so_extern_main.c
> Copy right header is needed.
Copyright added.

> 
> > +#include <stdio.h>
> > +#include <pthread.h>
> > +
> > +extern __thread void *so_extern;
> > +
> > +static void *tls_ptr(void *p)
> 
> The code should comply to GNU coding standard.
Reformatted the code (e.g. space before (, function name
starting at begin of line, ...)

> 
> > +{
> > +   so_extern = &so_extern;
> > +   printf("address is %p\n", &so_extern); /* break here to check result */
> 
> Don't have to call printf and include stdio.h.
Not clear by what to replace this printf (or why it harms).

I need to put a break after the assignment, as the .exp 
will compare so_extern value with address of so_extern.
(note: I changed the .exp, so as to also check so_extern value
in main thread).
The printf line allows to put a break after the assignment.
If it is really better to remove the printf/stdio.h, any suggestion
about what to replace it with ? (we need to avoid the compiler to
optimise away this code to be sure we can put a break).

Thanks for the review/detailed comments,


Philippe

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] PR varobj/18564  regression in showing __thread so extern variable
  2015-09-02 21:23   ` Philippe Waroquiers
@ 2015-09-03  9:37     ` Yao Qi
  2015-09-03 22:03       ` [PATCH V2] " Philippe Waroquiers
  0 siblings, 1 reply; 9+ messages in thread
From: Yao Qi @ 2015-09-03  9:37 UTC (permalink / raw)
  To: Philippe Waroquiers; +Cc: Yao Qi, gdb-patches

Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

>> > +{
>> > +   so_extern = &so_extern;
>> > +   printf("address is %p\n", &so_extern); /* break here to check result */
>> 
>> Don't have to call printf and include stdio.h.
> Not clear by what to replace this printf (or why it harms).
>

We don't have to get printf involved in the test, because some targets
may don't have printf at all.

> I need to put a break after the assignment, as the .exp 
> will compare so_extern value with address of so_extern.
> (note: I changed the .exp, so as to also check so_extern value
> in main thread).
> The printf line allows to put a break after the assignment.
> If it is really better to remove the printf/stdio.h, any suggestion
> about what to replace it with ? (we need to avoid the compiler to
> optimise away this code to be sure we can put a break).

We can replace it with "i++;" (i is a local variable, or a volatile
global variable).

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH V2] PR varobj/18564  regression in showing __thread so extern variable
  2015-09-03  9:37     ` Yao Qi
@ 2015-09-03 22:03       ` Philippe Waroquiers
  2015-09-10 14:34         ` Pedro Alves
  2015-09-10 20:14         ` Sergio Durigan Junior
  0 siblings, 2 replies; 9+ messages in thread
From: Philippe Waroquiers @ 2015-09-03 22:03 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On Thu, 2015-09-03 at 10:37 +0100, Yao Qi wrote:
> We don't have to get printf involved in the test, because some targets
> may don't have printf at all.
Ok, effectively, without printf, the test will then run on these
targets.

Find below the new version of the patch.
All your comments should be handled.

The fix itself still need to be reviewed.

Thanks for your review

Any comment on the fix ? Ok to commit ?

Philippe

diff --git a/gdb/findvar.c b/gdb/findvar.c
index 1c077f7..fd1b9d7 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -739,14 +739,17 @@ default_read_var_value (struct symbol *var, const struct block *var_block,
 
 	if (msym == NULL)
 	  error (_("No global symbol \"%s\"."), SYMBOL_LINKAGE_NAME (var));
-	if (overlay_debugging)
-	  addr = symbol_overlayed_address (BMSYMBOL_VALUE_ADDRESS (lookup_data.result),
-					   MSYMBOL_OBJ_SECTION (lookup_data.result.objfile,
-								msym));
-	else
-	  addr = BMSYMBOL_VALUE_ADDRESS (lookup_data.result);
-
 	obj_section = MSYMBOL_OBJ_SECTION (lookup_data.result.objfile, msym);
+	/* Relocate address, unless there is no section or the variable is
+	   a TLS variable. */
+	if (obj_section == NULL
+	    || (obj_section->the_bfd_section->flags & SEC_THREAD_LOCAL) != 0)
+	   addr = MSYMBOL_VALUE_RAW_ADDRESS (msym);
+	else
+	   addr = BMSYMBOL_VALUE_ADDRESS (lookup_data.result);
+	if (overlay_debugging)
+	  addr = symbol_overlayed_address (addr, obj_section);
+	/* Determine address of TLS variable. */
 	if (obj_section
 	    && (obj_section->the_bfd_section->flags & SEC_THREAD_LOCAL) != 0)
 	  addr = target_translate_tls_address (obj_section->objfile, addr);
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 5729b24..823f27b 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -1376,16 +1376,19 @@ address_info (char *exp, int from_tty)
 	else
 	  {
 	    section = MSYMBOL_OBJ_SECTION (msym.objfile, msym.minsym);
-	    load_addr = BMSYMBOL_VALUE_ADDRESS (msym);
 
 	    if (section
 		&& (section->the_bfd_section->flags & SEC_THREAD_LOCAL) != 0)
-	      printf_filtered (_("a thread-local variable at offset %s "
-				 "in the thread-local storage for `%s'"),
-			       paddress (gdbarch, load_addr),
-			       objfile_name (section->objfile));
+	      {
+		load_addr = MSYMBOL_VALUE_RAW_ADDRESS (msym.minsym);
+		printf_filtered (_("a thread-local variable at offset %s "
+				   "in the thread-local storage for `%s'"),
+				 paddress (gdbarch, load_addr),
+				 objfile_name (section->objfile));
+	      }
 	    else
 	      {
+		load_addr = BMSYMBOL_VALUE_ADDRESS (msym);
 		printf_filtered (_("static storage at address "));
 		fputs_filtered (paddress (gdbarch, load_addr), gdb_stdout);
 		if (section_is_overlay (section))
diff --git a/gdb/symtab.h b/gdb/symtab.h
index c6f26e7..d4a8e5f 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -584,7 +584,13 @@ enum address_class
      not find it in the full symbol table.  But a reference to an external
      symbol in a local block shadowing other definition requires full symbol
      without possibly having its address available for LOC_STATIC.  Testcase
-     is provided as `gdb.dwarf2/dw2-unresolved.exp'.  */
+     is provided as `gdb.dwarf2/dw2-unresolved.exp'.
+
+     This is also used for thread local storage (TLS) variables.  In this case,
+     the address of the TLS variable must be determined when the variable is
+     referenced, from the MSYMBOL_VALUE_RAW_ADDRESS, which is the offset
+     of the TLS variable in the thread local storage of the shared
+     library/object.  */
 
   LOC_UNRESOLVED,
 
diff --git a/gdb/testsuite/gdb.threads/tls-so_extern.c b/gdb/testsuite/gdb.threads/tls-so_extern.c
new file mode 100644
index 0000000..03febeb
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/tls-so_extern.c
@@ -0,0 +1,19 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2015 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/>.  */
+
+__thread void *so_extern;
+__thread void *so_extern2;
diff --git a/gdb/testsuite/gdb.threads/tls-so_extern.exp b/gdb/testsuite/gdb.threads/tls-so_extern.exp
new file mode 100644
index 0000000..35a55f0
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/tls-so_extern.exp
@@ -0,0 +1,81 @@
+# Copyright 2003-2015 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/>.  */
+
+# tls-so_extern.exp -- Expect script to test thread local storage in gdb, with
+# a variable defined in a shared library.
+
+standard_testfile tls-so_extern_main.c
+set libfile tls-so_extern
+set srcfile_lib ${libfile}.c
+set binfile_lib [standard_output_file ${libfile}.so]
+
+
+# get the value of gcc_compiled
+if [get_compiler_info] {
+    return -1
+}
+
+
+if { [gdb_compile_shlib_pthreads ${srcdir}/${subdir}/${srcfile_lib} ${binfile_lib} {debug}] != ""
+     || [gdb_compile_pthreads ${srcdir}/${subdir}/${srcfile} ${binfile} executable [list debug shlib=${binfile_lib}]] != ""} {
+    return -1
+}
+
+
+clean_restart ${binfile}
+gdb_load_shlibs ${binfile_lib}
+
+if ![runto_main] then {
+    fail "Can't run to main"
+    return 0
+}
+
+gdb_test "print so_extern" "0x0" "print thread local storage variable"
+
+gdb_test "ptype so_extern" "void \\*" "ptype of thread local storage variable"
+
+gdb_test "info address so_extern" \
+	"Symbol \\\"so_extern\\\" is a thread-local variable at offset 0x0 in the thread-local storage for .*tls-so_extern.*" \
+	"print storage info for thread local storage variable"
+
+set line_number [gdb_get_line_number "break here to check result"]
+
+gdb_test "break $line_number" \
+	"Breakpoint.*at.*file.*tls-so_extern_main.c.*line ${line_number}." \
+	"break in thread function"
+gdb_test "continue" \
+	"tls_ptr .* at .*:.*break here to check result.*" \
+        "continue to break in tls_ptr called by main"
+gdb_test "print so_extern == &so_extern" \
+         " = 1" \
+        "check so_extern address in main"
+gdb_test "continue" \
+	"tls_ptr .* at .*:.*break here to check result.*" \
+        "continue to break in a thread"
+gdb_test "print so_extern == &so_extern" \
+         " = 1" \
+        "check so_extern address"
+gdb_test "continue" \
+	"tls_ptr .* at .*:.*break here to check result.*" \
+        "continue to break in the other thread"
+gdb_test "print so_extern == &so_extern" \
+         " = 1" \
+        "check so_extern address in other thread"
+gdb_test "continue" \
+	"tls_ptr .* at .*:.*break here to check result.*" \
+        "continue to break in tls_ptr called at end of main"
+gdb_test "print so_extern == &so_extern" \
+         " = 1" \
+        "check so_extern address at end of main"
diff --git a/gdb/testsuite/gdb.threads/tls-so_extern_main.c b/gdb/testsuite/gdb.threads/tls-so_extern_main.c
new file mode 100644
index 0000000..ab70faf
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/tls-so_extern_main.c
@@ -0,0 +1,47 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2015 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 <pthread.h>
+
+extern __thread void *so_extern;
+extern __thread void *so_extern2;
+
+static void *
+tls_ptr (void *p)
+{
+   so_extern = &so_extern;
+   so_extern2 = &so_extern2; /* break here to check result */
+}
+
+int
+main (void)
+{
+   pthread_t threads[2];
+
+   tls_ptr (NULL);
+
+   pthread_create (&threads[0], NULL, tls_ptr, NULL);
+   pthread_create (&threads[1], NULL, tls_ptr, NULL);
+
+   pthread_join (threads[0], NULL);
+   pthread_join (threads[1], NULL);
+
+   tls_ptr (NULL);
+
+   return 0;
+}
+



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH V2] PR varobj/18564  regression in showing __thread so extern variable
  2015-09-03 22:03       ` [PATCH V2] " Philippe Waroquiers
@ 2015-09-10 14:34         ` Pedro Alves
  2015-09-15 19:15           ` Philippe Waroquiers
  2015-09-10 20:14         ` Sergio Durigan Junior
  1 sibling, 1 reply; 9+ messages in thread
From: Pedro Alves @ 2015-09-10 14:34 UTC (permalink / raw)
  To: Philippe Waroquiers, Yao Qi; +Cc: gdb-patches

On 09/03/2015 11:03 PM, Philippe Waroquiers wrote:
> On Thu, 2015-09-03 at 10:37 +0100, Yao Qi wrote:
>> We don't have to get printf involved in the test, because some targets
>> may don't have printf at all.
> Ok, effectively, without printf, the test will then run on these
> targets.
> 
> Find below the new version of the patch.
> All your comments should be handled.
> 
> The fix itself still need to be reviewed.
> 
> Thanks for your review
> 
> Any comment on the fix ? Ok to commit ?

This is OK.  Thanks for fixing this.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH V2] PR varobj/18564  regression in showing __thread so extern variable
  2015-09-03 22:03       ` [PATCH V2] " Philippe Waroquiers
  2015-09-10 14:34         ` Pedro Alves
@ 2015-09-10 20:14         ` Sergio Durigan Junior
  2015-09-10 20:17           ` Sergio Durigan Junior
  1 sibling, 1 reply; 9+ messages in thread
From: Sergio Durigan Junior @ 2015-09-10 20:14 UTC (permalink / raw)
  To: Philippe Waroquiers; +Cc: Yao Qi, gdb-patches

On Thursday, September 03 2015, Philippe Waroquiers wrote:

> On Thu, 2015-09-03 at 10:37 +0100, Yao Qi wrote:
>> We don't have to get printf involved in the test, because some targets
>> may don't have printf at all.
> Ok, effectively, without printf, the test will then run on these
> targets.
>
> Find below the new version of the patch.
> All your comments should be handled.
>
> The fix itself still need to be reviewed.
>
> Thanks for your review
>
> Any comment on the fix ? Ok to commit ?

Thanks, Philippe.

I wanted to reply to this earlier, sorry.  I looked at your proposed
patch, and it looks fine for me.  It makes sense, and is "simple" enough
that nothing seemed out of place.

I am not a maintainer for this part of the code, but I think it can be
safely approved.

Thanks again,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH V2] PR varobj/18564  regression in showing __thread so extern variable
  2015-09-10 20:14         ` Sergio Durigan Junior
@ 2015-09-10 20:17           ` Sergio Durigan Junior
  0 siblings, 0 replies; 9+ messages in thread
From: Sergio Durigan Junior @ 2015-09-10 20:17 UTC (permalink / raw)
  To: Philippe Waroquiers; +Cc: Yao Qi, gdb-patches

On Thursday, September 10 2015, I wrote:

> On Thursday, September 03 2015, Philippe Waroquiers wrote:
>
>> On Thu, 2015-09-03 at 10:37 +0100, Yao Qi wrote:
>>> We don't have to get printf involved in the test, because some targets
>>> may don't have printf at all.
>> Ok, effectively, without printf, the test will then run on these
>> targets.
>>
>> Find below the new version of the patch.
>> All your comments should be handled.
>>
>> The fix itself still need to be reviewed.
>>
>> Thanks for your review
>>
>> Any comment on the fix ? Ok to commit ?
>
> Thanks, Philippe.
>
> I wanted to reply to this earlier, sorry.  I looked at your proposed
> patch, and it looks fine for me.  It makes sense, and is "simple" enough
> that nothing seemed out of place.
>
> I am not a maintainer for this part of the code, but I think it can be
> safely approved.

And now I saw that Pedro already approved the patch.  Well, sorry about
the delay, again.

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH V2] PR varobj/18564  regression in showing __thread so extern variable
  2015-09-10 14:34         ` Pedro Alves
@ 2015-09-15 19:15           ` Philippe Waroquiers
  0 siblings, 0 replies; 9+ messages in thread
From: Philippe Waroquiers @ 2015-09-15 19:15 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Yao Qi, gdb-patches

On Thu, 2015-09-10 at 15:34 +0100, Pedro Alves wrote:
> On 09/03/2015 11:03 PM, Philippe Waroquiers wrote:
> > On Thu, 2015-09-03 at 10:37 +0100, Yao Qi wrote:
> >> We don't have to get printf involved in the test, because some targets
> >> may don't have printf at all.
> > Ok, effectively, without printf, the test will then run on these
> > targets.
> > 
> > Find below the new version of the patch.
> > All your comments should be handled.
> > 
> > The fix itself still need to be reviewed.
> > 
> > Thanks for your review
> > 
> > Any comment on the fix ? Ok to commit ?
> 
> This is OK.  Thanks for fixing this.
Thanks, pushed.

Philippe


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2015-09-15 19:15 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-30 11:34 [PATCH] PR varobj/18564 regression in showing __thread so extern variable Philippe Waroquiers
2015-09-02 13:03 ` Yao Qi
2015-09-02 21:23   ` Philippe Waroquiers
2015-09-03  9:37     ` Yao Qi
2015-09-03 22:03       ` [PATCH V2] " Philippe Waroquiers
2015-09-10 14:34         ` Pedro Alves
2015-09-15 19:15           ` Philippe Waroquiers
2015-09-10 20:14         ` Sergio Durigan Junior
2015-09-10 20:17           ` Sergio Durigan Junior

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).