From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1189 invoked by alias); 1 Dec 2008 21:54:01 -0000 Received: (qmail 1041 invoked by uid 22791); 1 Dec 2008 21:53:59 -0000 X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 01 Dec 2008 21:53:04 +0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id mB1Lqn0Y017247; Mon, 1 Dec 2008 16:52:49 -0500 Received: from pobox.stuttgart.redhat.com (pobox.stuttgart.redhat.com [172.16.2.10]) by int-mx1.corp.redhat.com (8.13.1/8.13.1) with ESMTP id mB1Lqm10013791; Mon, 1 Dec 2008 16:52:49 -0500 Received: from host0.dyn.jankratochvil.net (sebastian-int.corp.redhat.com [172.16.52.221]) by pobox.stuttgart.redhat.com (8.13.1/8.13.1) with ESMTP id mB1Lqegm024102; Mon, 1 Dec 2008 16:52:42 -0500 Received: from host0.dyn.jankratochvil.net (localhost [127.0.0.1]) by host0.dyn.jankratochvil.net (8.14.3/8.14.2) with ESMTP id mB1Lqcv7003105; Mon, 1 Dec 2008 22:52:39 +0100 Received: (from jkratoch@localhost) by host0.dyn.jankratochvil.net (8.14.3/8.14.2/Submit) id mB1Lqaph003102; Mon, 1 Dec 2008 22:52:36 +0100 Date: Mon, 01 Dec 2008 21:54:00 -0000 From: Jan Kratochvil To: Ulrich Weigand Cc: Vinay Sridhar , gdb-patches@sources.redhat.com, Daniel Jacobowitz , luisgpm@linux.vnet.ibm.com, Gary Funck Subject: Re: [patch] Accessing tls variables across files causes a bug Message-ID: <20081201215236.GA17136@host0.dyn.jankratochvil.net> References: <20081201141408.GA10447@host0.dyn.jankratochvil.net> <200812011829.mB1ITtMU028584@d12av02.megacenter.de.ibm.com> <1221019993.4923.3.camel@localhost.localdomain> <20081126172332.GE19317@intrepid.com> <20081126204316.GA11366@host0.dyn.jankratochvil.net> <20081201141408.GA10447@host0.dyn.jankratochvil.net> <20081201182931.GA18248@intrepid.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="tThc/1wpZn/ma/RB" Content-Disposition: inline In-Reply-To: <200812011829.mB1ITtMU028584@d12av02.megacenter.de.ibm.com> <20081201182931.GA18248@intrepid.com> User-Agent: Mutt/1.5.18 (2008-05-17) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-12/txt/msg00016.txt.bz2 --tThc/1wpZn/ma/RB Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-length: 3113 On Mon, 01 Dec 2008 19:29:55 +0100, Ulrich Weigand wrote: Followed your advices, updated the patch. Thanks, Jan ------------------------------------------------------------------------------ > Jan Kratochvil wrote: > > > + /* SYMBOL_VALUE_ADDRESS should return the translated address. */ > > I'm not sure I understand this comment -- the translated address of > a thread-local variable obviously depends on the thread in which it > is evaluated, so how should a single location like SYMBOL_VALUE_ADDRESS > be able to hold that value for all threads? SYMBOL_VALUE_ADDRESS is used many times across GDB code and I assumed in most of such cases it would be more appropriate to return the absolute address valid for current inferior_ptid. But I now briefly checked all these points and found out in fact all of them work with SYMBOL_VALUE_ADDRESS referring to function symbols - PC values (just except value_static_field() but that is also not in use with TLS symbols). Sure PC values are never TLS-translated. > > + /* SYMBOL_VALUE_ADDRESS should return the translated address. */ > > + if (section > > + && (section->the_bfd_section->flags & SEC_THREAD_LOCAL) != 0) > > + { > > + printf_filtered (_("a thread-local variable at offset %s " > > + "at final address "), paddr_nz (load_addr)); > > + load_addr = target_translate_tls_address (section->objfile, > > + load_addr); > > + } > > Again, I think "at final address" may be misleading; if we give an absolute > address, it should explicitly mention it is relative to the current thread. `final' was put there as it printed before: Symbol "X" is a thread-local variable at offset 0x1234 at address 0xabcd0000. and I found it ambiguous whether the bytes of the variable X are in fact placed at (a) address 0xabcd0000 or (b) address 0xabcd1234. > In the LOC_COMPUTED case, we'd output something like "at offset ... > in the thread-local storage for ...". For consistency reasons, I'd > prefer to have the same output in the LOC_UNDEFINED case as well. While I copied the message from the LOC_COMPUTED case I agree it should be consistent and if I wish the absolute address there it should be printed even in the LOC_COMPUTED case which would be more a scope of a different patch. > On the other hand, I'm wondering if we should perform the resolution here > at all; isn't "info address" also allowed when the target is not actually > running, so we don't even have a current thread? While I forgot about this case I find the current behavior appropriate: (gdb) info address a_thread_local Symbol "a_thread_local" is a thread-local variable at offset 0 at final address Cannot find thread-local variables on this target But it is no longer printed due to the consistency advice above. On Mon, 01 Dec 2008 19:29:31 +0100, Gary Funck wrote: > --- gdb/printcmd.c 20 Nov 2008 16:13:11 -0000 1.138 > +++ gdb/printcmd.c 30 Nov 2008 21:10:06 -0000 > @@ -1234,6 +1234,7 @@ address_info (char *exp, int from_tty) > + struct obj_section *obj_section; > Is obj_section referenced? Removed. --tThc/1wpZn/ma/RB Content-Type: text/plain; charset=us-ascii Content-Disposition: inline; filename="gdb-extern-tls2.patch" Content-length: 6509 2008-12-01 Jan Kratochvil Fix resolving external references to TLS variables. * findvar.c: Include `objfiles.h'. (read_var_value ): New variable `obj_section'. Handle SEC_THREAD_LOCAL variables. * printcmd.c (address_info ): Handle SEC_THREAD_LOCAL variables. 2008-12-01 Jan Kratochvil Test resolving external references to TLS variables. * gdb.threads/tls.exp: New tests to examine A_THREAD_LOCAL and FILE2_THREAD_LOCAL. (testfile2, srcfile2): New variables. * gdb.threads/tls.c (file2_thread_local) (function_referencing_file2_thread_local): New. * gdb.threads/tls2.c: New file. --- gdb/findvar.c 5 Sep 2008 11:37:17 -0000 1.118 +++ gdb/findvar.c 1 Dec 2008 21:50:25 -0000 @@ -34,6 +34,7 @@ #include "regcache.h" #include "user-regs.h" #include "block.h" +#include "objfiles.h" /* Basic byte-swapping routines. GDB has needed these for a long time... All extract a target-format integer at ADDR which is LEN bytes long. */ @@ -536,6 +537,7 @@ read_var_value (struct symbol *var, stru case LOC_UNRESOLVED: { struct minimal_symbol *msym; + struct obj_section *obj_section; msym = lookup_minimal_symbol (SYMBOL_LINKAGE_NAME (var), NULL, NULL); if (msym == NULL) @@ -545,6 +547,11 @@ read_var_value (struct symbol *var, stru SYMBOL_OBJ_SECTION (msym)); else addr = SYMBOL_VALUE_ADDRESS (msym); + + obj_section = SYMBOL_OBJ_SECTION (msym); + if (obj_section + && (obj_section->the_bfd_section->flags & SEC_THREAD_LOCAL) != 0) + addr = target_translate_tls_address (obj_section->objfile, addr); } break; --- gdb/printcmd.c 20 Nov 2008 16:13:11 -0000 1.138 +++ gdb/printcmd.c 1 Dec 2008 21:50:28 -0000 @@ -1241,16 +1241,25 @@ address_info (char *exp, int from_tty) else { section = SYMBOL_OBJ_SECTION (msym); - printf_filtered (_("static storage at address ")); load_addr = SYMBOL_VALUE_ADDRESS (msym); - fputs_filtered (paddress (load_addr), gdb_stdout); - if (section_is_overlay (section)) + + 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'"), + paddr_nz (load_addr), section->objfile->name); + else { - load_addr = overlay_unmapped_address (load_addr, section); - printf_filtered (_(",\n -- loaded at ")); + printf_filtered (_("static storage at address ")); fputs_filtered (paddress (load_addr), gdb_stdout); - printf_filtered (_(" in overlay section %s"), - section->the_bfd_section->name); + if (section_is_overlay (section)) + { + load_addr = overlay_unmapped_address (load_addr, section); + printf_filtered (_(",\n -- loaded at ")); + fputs_filtered (paddress (load_addr), gdb_stdout); + printf_filtered (_(" in overlay section %s"), + section->the_bfd_section->name); + } } } } --- gdb/testsuite/gdb.threads/tls.c 29 Jul 2003 21:51:25 -0000 1.2 +++ gdb/testsuite/gdb.threads/tls.c 1 Dec 2008 21:50:30 -0000 @@ -20,6 +20,9 @@ __thread int a_thread_local; __thread int another_thread_local; +/* psymtabs->symtabs resolving check. */ +extern __thread int file2_thread_local; + /* Global variable just for info addr in gdb. */ int a_global; @@ -119,6 +122,12 @@ void *spin( vp ) } void +function_referencing_file2_thread_local (void) +{ + file2_thread_local = file2_thread_local; +} + +void do_pass() { int i; --- gdb/testsuite/gdb.threads/tls.exp 6 Aug 2008 12:52:08 -0000 1.9 +++ gdb/testsuite/gdb.threads/tls.exp 1 Dec 2008 21:50:30 -0000 @@ -15,7 +15,9 @@ # along with this program. If not, see . */ set testfile tls +set testfile2 tls2 set srcfile ${testfile}.c +set srcfile2 ${testfile2}.c set binfile ${objdir}/${subdir}/${testfile} if [istarget "*-*-linux"] then { @@ -24,7 +26,7 @@ if [istarget "*-*-linux"] then { set target_cflags "" } -if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug "incdir=${objdir}"]] != "" } { +if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile} ${srcdir}/${subdir}/${srcfile2}" "${binfile}" executable [list debug "incdir=${objdir}"]] != "" } { return -1 } @@ -284,6 +286,20 @@ gdb_test "info address a_global" \ setup_kfail "gdb/1294" "*-*-*" gdb_test "info address me" ".*me.*is a variable at offset.*" "info address me" + +# Test LOC_UNRESOLVED references resolving for `extern' TLS variables. + +gdb_test "p a_thread_local" " = \[0-9\]+" +# Here it could crash with: Cannot access memory at address 0x0 +gdb_test "p file2_thread_local" " = \[0-9\]+" +# Depending on the current lookup scope we get LOC_UNRESOLVED or LOC_COMPUTED +# both printing: +# Symbol "file2_thread_local" is a thread-local variable at offset 8 in the thread-local storage for `.../gdb.threads/tls'. +gdb_test "info address file2_thread_local" "Symbol \"file2_thread_local\" is a thread-local variable.*" +# Here it could also crash with: Cannot access memory at address 0x0 +gdb_test "p a_thread_local" " = \[0-9\]+" "p a_thread_local second time" +gdb_test "info address a_thread_local" "Symbol \"a_thread_local\" is a thread-local variable.*" + # Done! # gdb_exit --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ gdb/testsuite/gdb.threads/tls2.c 1 Dec 2008 21:50:30 -0000 @@ -0,0 +1,28 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2008 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 . + + Please email any bugs, comments, and/or additions to this file to: + bug-gdb@prep.ai.mit.edu */ + +extern __thread int a_thread_local; +__thread int file2_thread_local; + +void +function_referencing_a_thread_local (void) +{ + a_thread_local = a_thread_local; +} --tThc/1wpZn/ma/RB--