This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[patch] Re: Accessing tls variables across files causes a bug
- From: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- To: Vinay Sridhar <vinay at linux dot vnet dot ibm dot com>
- Cc: gdb-patches at sources dot redhat dot com
- Date: Sat, 2 Aug 2008 19:18:07 +0200
- Subject: [patch] Re: Accessing tls variables across files causes a bug
- References: <1217480020.4755.1.camel@vinaysridhar.in.ibm.com>
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 <executable>
> 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
2008-08-02 Jan Kratochvil <jan.kratochvil@redhat.com>
* 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 <vinay@linux.vnet.ibm.com>.
2008-08-02 Jan Kratochvil <jan.kratochvil@redhat.com>
* 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 <http://www.gnu.org/licenses/>.
+
+ 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;
+}