Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
To: Yao Qi <qiyaoltc@gmail.com>
Cc: gdb-patches@sourceware.org
Subject: [PATCH V2] PR varobj/18564  regression in showing __thread so extern variable
Date: Thu, 03 Sep 2015 22:03:00 -0000	[thread overview]
Message-ID: <1441317832.4429.4.camel@skynet.be> (raw)
In-Reply-To: <86h9nbyhb6.fsf@gmail.com>

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;
+}
+




  reply	other threads:[~2015-09-03 22:03 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-30 11:34 [PATCH] " 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       ` Philippe Waroquiers [this message]
2015-09-10 14:34         ` [PATCH V2] " 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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1441317832.4429.4.camel@skynet.be \
    --to=philippe.waroquiers@skynet.be \
    --cc=gdb-patches@sourceware.org \
    --cc=qiyaoltc@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox