From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29971 invoked by alias); 5 Aug 2008 08:39:13 -0000 Received: (qmail 29911 invoked by uid 22791); 5 Aug 2008 08:38:59 -0000 X-Spam-Check-By: sourceware.org Received: from e3.ny.us.ibm.com (HELO e3.ny.us.ibm.com) (32.97.182.143) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 05 Aug 2008 08:38:04 +0000 Received: from d01relay04.pok.ibm.com (d01relay04.pok.ibm.com [9.56.227.236]) by e3.ny.us.ibm.com (8.13.8/8.13.8) with ESMTP id m758brpG006134 for ; Tue, 5 Aug 2008 04:37:53 -0400 Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by d01relay04.pok.ibm.com (8.13.8/8.13.8/NCO v9.0) with ESMTP id m758bdiX241740 for ; Tue, 5 Aug 2008 04:37:39 -0400 Received: from d01av02.pok.ibm.com (loopback [127.0.0.1]) by d01av02.pok.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id m758bceM014320 for ; Tue, 5 Aug 2008 04:37:39 -0400 Received: from [9.124.31.22] (vinaysridhar.in.ibm.com [9.124.31.22]) by d01av02.pok.ibm.com (8.12.11.20060308/8.12.11) with ESMTP id m758bWBe014170; Tue, 5 Aug 2008 04:37:34 -0400 Subject: Re: [patch] Re: Accessing tls variables across files causes a bug From: Vinay Sridhar To: Jan Kratochvil Cc: gdb-patches@sources.redhat.com, luisgpm@linux.vnet.ibm.com, uweigand@de.ibm.com In-Reply-To: <20080802171807.GA2755@host0.dyn.jankratochvil.net> References: <1217480020.4755.1.camel@vinaysridhar.in.ibm.com> <20080802171807.GA2755@host0.dyn.jankratochvil.net> Content-Type: text/plain; charset=UTF-8 Date: Tue, 05 Aug 2008 08:39:00 -0000 Message-Id: <1217925527.3658.41.camel@vinaysridhar.in.ibm.com> Mime-Version: 1.0 X-Mailer: Evolution 2.22.1 (2.22.1-2.fc9) Content-Transfer-Encoding: 8bit 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-08/txt/msg00073.txt.bz2 On Sat, 2008-08-02 at 19:18 +0200, Jan Kratochvil wrote: > Hi, > > just coded it as Vinay Sridhar suggested, thanks. lookup_block_symbol() > behaves right as it has only the scope of a single BLOCK. > > While there should be some more general approach for multiple instances of > a single symbol name in different files this patch has no known regressions. > > > Regards, > Jan > > > On Thu, 31 Jul 2008 06:53:40 +0200, Vinay Sridhar wrote: > [...] > > We came across this bug when debugging tls variables. > > > > ------------------------ > > Consider file1.c : > > ------------------------ > > #include<...> > > ... > > __thread int thr; > > int stopHere; > > ... > > void foo() > > { > > stopHere=8; > > } > > > > main() > > { > > thr = 0; > > foo(); > > } > > > > ------------------------- > > Now consider file2.c : > > ------------------------- > > > > __thread char myChar; > > extern __thread int thr; > > > > void foo1() > > { > > myChar = 'a' + thr; > > } > > > > > > On compiling these 2 and creating the executable, the bug is produced on > > running the following commands : > > > > 1. gdb > > 2. b 8 //at the stopHere part of file1 > > 3. run > > 4. print thr //prints value of thr > > 5. print myChar > > 6. print thr > > > > Now on the final print command, you get a "Cannot access memory at > > address 0x4" > > > > The reason for this is "thr", being a tls variable, is stored as an > > offset in the minsymtab. So for the "extern thr", gdb sets it to > > LOC_UNRESOLVED and by convention, looks up minsymtab to find its > > address. Here it justs picks up the offset and tries to dereference it. > > This works fine for other global variables that are extern but fails for > > "tls" variables that are extern. > > > > What I propose is that in "lookup_symbol_aux_symtabs()" @symtab.c, when > > the tls variable is to be looked up, once we find it in the current > > file's symtab and is at LOC_UNRESOLVED, we ignore it and look further in > > other symtabs of the object file as well and find one that has > > LOC_COMPUTED, i.e, we look into the symtab of the file which has defined > > this and retrieve the symbol information from this block of the objfile. > > This will be restricted to tls variables only. > > > > Is this fix acceptable? > > > > Request for Comments.. > > > > Regards, > > Vinay > > > > Vinay Sridhar, > > IBM-Linux Technology Centre > > vinay@linux.vnet.ibm.com > > plain text document attachment (gdb-extern-loc_unresolved.patch) > 2008-08-02 Jan Kratochvil > > * symtab.c (lookup_symbol_aux_symtabs): Continue the symtabs search if > only LOC_UNRESOLVED or LOC_OPTIMIZED_OUT symbol was found so far. > Fix suggested by Vinay Sridhar . > > 2008-08-02 Jan Kratochvil > > * gdb.threads/tls.exp: New tests to print `a_thread_local' after > `file2_thread_local'. > (testfile2, srcfile2): New variables. > * gdb.threads/tls2.c: New file. > > --- ./gdb/symtab.c 21 Jul 2008 16:47:10 -0000 1.192 > +++ ./gdb/symtab.c 2 Aug 2008 17:05:36 -0000 > @@ -1475,7 +1475,7 @@ lookup_symbol_aux_symtabs (int block_ind > const char *name, const char *linkage_name, > const domain_enum domain) > { > - struct symbol *sym; > + struct symbol *sym_best = NULL; > struct objfile *objfile; > struct blockvector *bv; > const struct block *block; > @@ -1483,16 +1483,26 @@ lookup_symbol_aux_symtabs (int block_ind > > ALL_PRIMARY_SYMTABS (objfile, s) > { > + struct symbol *sym; > + > bv = BLOCKVECTOR (s); > block = BLOCKVECTOR_BLOCK (bv, block_index); > sym = lookup_block_symbol (block, name, linkage_name, domain); > - if (sym) > + if (sym != NULL) > { > - block_found = block; > - return fixup_symbol_section (sym, objfile); > + sym_best = sym; > + if (SYMBOL_CLASS (sym) != LOC_UNRESOLVED > + && SYMBOL_CLASS (sym) != LOC_OPTIMIZED_OUT) > + break; > } > } > > + if (sym_best) > + { > + block_found = block; > + return fixup_symbol_section (sym_best, objfile); > + } > + > return NULL; > } > > --- ./gdb/testsuite/gdb.threads/tls.exp 1 Jan 2008 22:53:22 -0000 1.8 > +++ ./gdb/testsuite/gdb.threads/tls.exp 2 Aug 2008 17:05:40 -0000 > @@ -18,7 +18,9 @@ > # bug-gdb@prep.ai.mit.edu > > set testfile tls > +set testfile2 tls2 > set srcfile ${testfile}.c > +set srcfile2 ${testfile2}.c > set binfile ${objdir}/${subdir}/${testfile} > > if [istarget "*-*-linux"] then { > @@ -27,7 +29,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 > } > > @@ -287,6 +289,15 @@ 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 for the `extern' variables. > + > +gdb_test "p a_thread_local" " = \[0-9\]+" > +gdb_test "p file2_thread_local" " = \[0-9\]+" > +# Here it could crash with: Cannot access memory at address 0x0 > +gdb_test "p a_thread_local" " = \[0-9\]+" > + > + > # Done! > # > gdb_exit > --- /dev/null 1 Jan 1970 00:00:00 -0000 > +++ ./gdb/testsuite/gdb.threads/tls2.c 2 Aug 2008 17:05:40 -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; > +} Jan, I have a few additions to suggest: 1. The patch does not ensure that this is performed for "tls" variables only. Here the behaviour is modified for all variables that are declared extern. This will be modifying existing gdb behaviour. Is this acceptable? 2. If we access an extern tls variable in the file containing main() before the other symtabs are linked in, we again get a "cannot access memory at address ..." error, i.e if I declare a thread var as tls in file2 and it is declared extern in file1, then access of this variable results in the error. GDB first links in the block containing main(). Each time a variable in a different file is accessed, the symtab of that file is linked in. Gdb then anchors itself there and looks for symbols relative to this symtab. Hence, we need to link them in for this scenario. What happens currently is that "sym_best" is retained and the usual msymtab lookup is happening. So we "force link-in" the symtab by going the psymtab route. For this we need to find out if a particular variable is tls or not. I can think of 2 ways : 1. access msymtab and try dereferencing. If failure, assume its tls. 2. obtain symbol info from msymtab and check for the section value. AFAIK, elf has section=17 for tls. I prefer (2) if we can obtain section info for all exe types. For now, I've stuck to the section=17 method. I agree with Ulrich. Why does LOC_OPTIMIZED_OUT have to be included? Re-spin of Jan's earlier patch. Signed-off by : Jan Kratochvil Signed-off by : Vinay Sridhar --------Patch------------ --- symtab.c.orig 2008-08-05 13:29:26.000000000 +0530 +++ symtab.c 2008-08-05 13:57:54.000000000 +0530 @@ -1394,31 +1394,51 @@ lookup_global_symbol_from_objfile (const depending on whether or not we want to search global symbols or static symbols. */ +#define IS_TLS(val) (val==17?1:0) + static struct symbol * lookup_symbol_aux_symtabs (int block_index, const char *name, const char *linkage_name, const domain_enum domain, struct symtab **symtab) { - struct symbol *sym; + struct symbol *sym_best = NULL; struct objfile *objfile; struct blockvector *bv; const struct block *block; struct symtab *s; + struct minimal_symbol *msym; ALL_PRIMARY_SYMTABS (objfile, s) { + struct symbol *sym; + bv = BLOCKVECTOR (s); block = BLOCKVECTOR_BLOCK (bv, block_index); sym = lookup_block_symbol (block, name, linkage_name, domain); + + if (sym != NULL && SYMBOL_CLASS (sym) == LOC_UNRESOLVED) + { + msym = lookup_minimal_symbol (name, NULL, NULL); + if (IS_TLS (msym->ginfo.section) && s->next == NULL) + return NULL; + } + if (sym) { - block_found = block; - if (symtab != NULL) - *symtab = s; - return fixup_symbol_section (sym, objfile); + sym_best = sym; + if (SYMBOL_CLASS (sym) != LOC_UNRESOLVED) + break; } } + + if (sym_best) + { + block_found = block; + if(symtab != NULL) + *symtab = s; + return fixup_symbol_section (sym_best, objfile); + } return NULL; } Regards, Vinay Vinay Sridhar, IBM-Linux Technology Centre, vinay@linux.vnet.ibm.com