[patch] Re: Accessing tls variables across files causes a bug

[patch] Re: Accessing tls variables across files causes a bug

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 2215 bytes --]

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


[-- Attachment #2: gdb-extern-loc_unresolved.patch --]
[-- Type: text/plain, Size: 3896 bytes --]

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

Re: [patch] Re: Accessing tls variables across files causes a bug

[Luis Machado]

Thanks for working on this. Just a few cosmetic comments.

On Sat, 2008-08-02 at 19:18 +0200, Jan Kratochvil wrote:
>      block = BLOCKVECTOR_BLOCK (bv, block_index);
>      sym = lookup_block_symbol (block, name, linkage_name, domain);
> -    if (sym)
> +    if (sym != NULL)

Shouldn't we just leave it as "if (sym)"?

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

> --- /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  */

This header is probably very old. Newer testcases have a more up-to-date
header. For example, gdb.opt/clobbered-registers-O2.c.

The patch works for me, getting rid of the memory access error when
trying to inspect the extern tls variable.

Regards,
Luis

[patch] Finish removing <bug-gdb@prep.ai.mit.edu> (PR gdb/1543)

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 1305 bytes --]

On Mon, 04 Aug 2008 21:17:14 +0200, Luis Machado wrote:
> On Sat, 2008-08-02 at 19:18 +0200, Jan Kratochvil wrote:
...
> > +   Please email any bugs, comments, and/or additions to this file to:
> > +   bug-gdb@prep.ai.mit.edu  */
> 
> This header is probably very old. Newer testcases have a more up-to-date
> header. For example, gdb.opt/clobbered-registers-O2.c.

I already faced this problem before, I was not aware of the differences.
This time I did a copy from file with recent enough modification timestamp.

Patch finally closes PR gdb/1543, its full contens is 115KB so I left it at:
	http://people.redhat.com/jkratoch/gdb-testsuite-mail-remove.patch

I found no other files (besides fixed `sep.exp') referencing absolute numbers
in the changed .c files.  Also no other testcases had regressions on x86_64.
$ grep '\(bigcore\|break\|break1\|callfuncs\|cvexpr\|del\|nofield\|pending\|pendshr\|randomize\|sep-proc\|sep\|sepdebug\|sigaltstack\|start\|step-bt\|structs\|value-double-free\|mi-pending\|mi-pendshr\)[.]c:[0-9]' */*.exp
gdb.base/sep.exp:gdb_test "list sep-proc.c:23" \
gdb.base/sep.exp:gdb_test_multiple "break sep-proc.c:25" "$test" {

(Testcases `gdb.base/bigcore.exp' and `gdb.base/condbreak.exp' changed the
reported PASS messages content containing the line number.)


Regards,
Jan

[-- Attachment #2: gdb-testsuite-mail-remove-reduced.patch --]
[-- Type: text/plain, Size: 7165 bytes --]

2008-08-05  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix for PR gdb/1543.
	* gdb.base/sep.exp: `sep-proc.c' absolute line numbers replaced with
	$LOCATION.
	(location): New variable.
	* config/cfdbug.exp, config/d10v.exp, config/dve.exp, config/i960.exp,
	config/m32r.exp, config/mn10300-eval.exp, config/proelf.exp,
	config/rom68k.exp, config/sh.exp, config/unix.exp, config/vr4300.exp,
	config/vr5000.exp, config/vxworks.exp, gdb.arch/altivec-regs.exp,
	gdb.arch/e500-abi.exp, gdb.arch/e500-regs.exp, gdb.asm/asm-source.exp,
	gdb.base/a2-run.exp, gdb.base/advance.exp, gdb.base/all-bin.exp,
	gdb.base/args.exp, gdb.base/arithmet.exp, gdb.base/assign.exp,
	gdb.base/async.exp, gdb.base/auxv.exp, gdb.base/bigcore.c,
	gdb.base/bigcore.exp, gdb.base/bitfields.exp, gdb.base/bitops.exp,
	gdb.base/break.c, gdb.base/break.exp, gdb.base/break1.c,
	gdb.base/call-ar-st.exp, gdb.base/call-rt-st.exp,
	gdb.base/call-strs.exp, gdb.base/callfuncs.c, gdb.base/callfuncs.exp,
	gdb.base/checkpoint.exp, gdb.base/chng-syms.exp,
	gdb.base/code-expr.exp, gdb.base/commands.exp, gdb.base/completion.exp,
	gdb.base/complex.exp, gdb.base/cond-expr.exp, gdb.base/condbreak.exp,
	gdb.base/consecutive.exp, gdb.base/corefile.exp, gdb.base/cvexpr.c,
	gdb.base/cvexpr.exp, gdb.base/dbx.exp, gdb.base/default.exp,
	gdb.base/define.exp, gdb.base/del.c, gdb.base/detach.exp,
	gdb.base/display.exp, gdb.base/dump.exp, gdb.base/echo.exp,
	gdb.base/environ.exp, gdb.base/eval-skip.exp, gdb.base/exprs.exp,
	gdb.base/fileio.exp, gdb.base/find.exp, gdb.base/finish.exp,
	gdb.base/funcargs.exp, gdb.base/gcore-buffer-overflow.exp,
	gdb.base/gcore.exp, gdb.base/gdb1555.exp, gdb.base/gdbvars.exp,
	gdb.base/help.exp, gdb.base/huge.exp, gdb.base/info-proc.exp,
	gdb.base/interrupt.exp, gdb.base/jump.exp, gdb.base/langs.exp,
	gdb.base/lineinc.exp, gdb.base/list.exp, gdb.base/macscp.exp,
	gdb.base/maint.exp, gdb.base/mips_pro.exp, gdb.base/miscexprs.exp,
	gdb.base/nodebug.exp, gdb.base/nofield.c, gdb.base/opaque.exp,
	gdb.base/overlays.exp, gdb.base/page.exp, gdb.base/pc-fp.exp,
	gdb.base/pending.c, gdb.base/pendshr.c, gdb.base/pointers.exp,
	gdb.base/psymtab.exp, gdb.base/ptype.exp, gdb.base/randomize.c,
	gdb.base/readline.exp, gdb.base/recurse.exp, gdb.base/regs.exp,
	gdb.base/relational.exp, gdb.base/relocate.exp, gdb.base/remote.exp,
	gdb.base/reread.exp, gdb.base/return.exp, gdb.base/return2.exp,
	gdb.base/scope.exp, gdb.base/sect-cmd.exp, gdb.base/sep-proc.c,
	gdb.base/sep.c, gdb.base/sep.exp, gdb.base/sepdebug.c,
	gdb.base/sepdebug.exp, gdb.base/setshow.exp, gdb.base/shlib-call.exp,
	gdb.base/sigaltstack.c, gdb.base/so-indr-cl.exp, gdb.base/solib.exp,
	gdb.base/source.exp, gdb.base/start.c, gdb.base/step-bt.c,
	gdb.base/step-line.exp, gdb.base/structs.c, gdb.base/structs.exp,
	gdb.base/structs2.exp, gdb.base/term.exp, gdb.base/twice.exp,
	gdb.base/type-opaque.exp, gdb.base/until.exp,
	gdb.base/value-double-free.c, gdb.base/varargs.exp,
	gdb.base/watchpoint.exp, gdb.base/whatis-exp.exp, gdb.disasm/am33.exp,
	gdb.disasm/h8300s.exp, gdb.disasm/hppa.exp, gdb.disasm/mn10300.exp,
	gdb.disasm/sh3.exp, gdb.disasm/t01_mov.exp, gdb.disasm/t02_mova.exp,
	gdb.disasm/t03_add.exp, gdb.disasm/t04_sub.exp, gdb.disasm/t05_cmp.exp,
	gdb.disasm/t06_ari2.exp, gdb.disasm/t07_ari3.exp,
	gdb.disasm/t08_or.exp, gdb.disasm/t09_xor.exp, gdb.disasm/t10_and.exp,
	gdb.disasm/t11_logs.exp, gdb.disasm/t12_bit.exp,
	gdb.disasm/t13_otr.exp, gdb.fortran/exprs.exp, gdb.fortran/types.exp,
	gdb.hp/gdb.aCC/exception.exp, gdb.hp/gdb.aCC/optimize.exp,
	gdb.hp/gdb.aCC/watch-cmd.exp, gdb.hp/gdb.base-hp/callfwmall.exp,
	gdb.hp/gdb.base-hp/dollar.exp, gdb.hp/gdb.base-hp/hwwatchbus.exp,
	gdb.hp/gdb.base-hp/pxdb.exp, gdb.hp/gdb.base-hp/reg-pa64.exp,
	gdb.hp/gdb.base-hp/reg.exp, gdb.hp/gdb.base-hp/sized-enum.exp,
	gdb.hp/gdb.base-hp/so-thresh.exp, gdb.hp/gdb.compat/xdb1.exp,
	gdb.hp/gdb.compat/xdb2.exp, gdb.hp/gdb.compat/xdb3.exp,
	gdb.java/jmisc.exp, gdb.java/jv-exp.exp, gdb.java/jv-print.exp,
	gdb.mi/gdb669.exp, gdb.mi/gdb680.exp, gdb.mi/gdb701.exp,
	gdb.mi/gdb792.exp, gdb.mi/mi-basics.exp, gdb.mi/mi-console.exp,
	gdb.mi/mi-hack-cli.exp, gdb.mi/mi-pending.c, gdb.mi/mi-pendshr.c,
	gdb.mi/mi-pthreads.exp, gdb.mi/mi-read-memory.exp, gdb.mi/mi-regs.exp,
	gdb.mi/mi-syn-frame.exp, gdb.mi/mi-until.exp, gdb.mi/mi2-basics.exp,
	gdb.mi/mi2-console.exp, gdb.mi/mi2-hack-cli.exp,
	gdb.mi/mi2-pthreads.exp, gdb.mi/mi2-read-memory.exp,
	gdb.mi/mi2-regs.exp, gdb.mi/mi2-syn-frame.exp, gdb.mi/mi2-until.exp,
	gdb.pascal/types.exp, gdb.stabs/weird.exp,
	gdb.threads/gcore-thread.exp, gdb.threads/manythreads.exp,
	gdb.threads/print-threads.exp, gdb.threads/pthreads.exp,
	gdb.threads/schedlock.exp, gdb.threads/step.exp, gdb.threads/step2.exp,
	gdb.threads/switch-threads.exp, gdb.threads/thread-specific.exp,
	gdb.threads/thread_check.exp, gdb.threads/thread_events.exp,
	gdb.threads/tls-nodebug.exp, gdb.threads/tls-shared.exp,
	gdb.threads/tls.exp, gdb.trace/actions.exp, gdb.trace/backtrace.exp,
	gdb.trace/circ.exp, gdb.trace/collection.exp, gdb.trace/deltrace.exp,
	gdb.trace/infotrace.exp, gdb.trace/limits.exp, gdb.trace/packetlen.exp,
	gdb.trace/passc-dyn.exp, gdb.trace/passcount.exp, gdb.trace/report.exp,
	gdb.trace/save-trace.exp, gdb.trace/tfind.exp, gdb.trace/tracecmd.exp,
	gdb.trace/while-dyn.exp, gdb.trace/while-stepping.exp,
	lib/mi-support.exp, lib/trace-support.exp: Remove reference
	to bug-gdb@prep.ai.mit.edu .

--- gdb/testsuite/gdb.base/sep.exp	1 Jan 2008 22:53:19 -0000	1.8
+++ gdb/testsuite/gdb.base/sep.exp	5 Aug 2008 13:05:17 -0000
@@ -12,9 +12,6 @@
 #
 # 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
 
 if $tracelevel then {
     strace $tracelevel
@@ -29,6 +26,8 @@ if  { [gdb_compile "${srcdir}/${subdir}/
     return -1
 }
 
+set location [gdb_get_line_number "say_hello" "sep-proc.c"]
+
 gdb_exit
 gdb_start
 gdb_reinitialize_dir $srcdir/$subdir
@@ -44,7 +43,7 @@ gdb_load ${binfile}
 # say_hello is good enough, and avoid unnecessary failures is someone
 # decides later to reformat sep-proc.c.
 
-gdb_test "list sep-proc.c:23" \
+gdb_test "list sep-proc.c:$location" \
          "void.*say_hello.*" \
          "list using location inside included file"
 
@@ -57,7 +56,7 @@ gdb_reinitialize_dir $srcdir/$subdir
 gdb_load ${binfile}
 
 set test "breakpoint inside included file"
-gdb_test_multiple "break sep-proc.c:25" "$test" {
+gdb_test_multiple "break sep-proc.c:$location" "$test" {
     -re "Breakpoint.*at.* file .*sep-proc.c, line .*" {
         pass "$test"
     }
--- gdb/testsuite/config/cfdbug.exp	1 Jan 2008 22:53:15 -0000	1.5
+++ gdb/testsuite/config/cfdbug.exp	5 Aug 2008 13:04:56 -0000
@@ -13,7 +13,4 @@
 # 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
-
 load_lib "../config/monitor.exp"

etc.
http://people.redhat.com/jkratoch/gdb-testsuite-mail-remove.patch

Re: [patch] Finish removing <bug-gdb@prep.ai.mit.edu> (PR gdb/1543)

[Daniel Jacobowitz]

On Tue, Aug 05, 2008 at 03:25:20PM +0200, Jan Kratochvil wrote:
> 2008-08-05  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	Fix for PR gdb/1543.
> 	* gdb.base/sep.exp: `sep-proc.c' absolute line numbers replaced with
> 	$LOCATION.
> 	(location): New variable.

[etc.]

Thanks for the patch - this is OK!

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] Re: Accessing tls variables across files causes a bug

[Ulrich Weigand]

Jan Kratochvil wrote:

> +	if (SYMBOL_CLASS (sym) != LOC_UNRESOLVED
> +	    && SYMBOL_CLASS (sym) != LOC_OPTIMIZED_OUT)
> +	  break;

I can see why this might be correct for LOC_UNRESOLVED,
but why for LOC_OPTIMIZED_OUT?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [patch] Re: Accessing tls variables across files causes a bug

[Vinay Sridhar]

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 <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
> 
> plain text document attachment (gdb-extern-loc_unresolved.patch)
> 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;
> +}


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  <jan.kratochvil@redhat.com>
Signed-off by : Vinay Sridhar  <vinay@linux.vnet.ibm.com>

--------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

Re: [patch] Re: Accessing tls variables across files causes a bug

[Daniel Jacobowitz]

On Tue, Aug 05, 2008 at 02:08:47PM +0530, Vinay Sridhar wrote:
> 2. obtain symbol info from msymtab and check for the section value.
> AFAIK, elf has section=17 for tls. 

This number doesn't mean anything.  You want the STT_TLS section type.
But if you're doing the right thing without a minimal symbol why rely
on a minimal symbol at all?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] Re: Accessing tls variables across files causes a bug

[Vinay Sridhar]

On Tue, 2008-08-05 at 08:21 -0400, Daniel Jacobowitz wrote:
> On Tue, Aug 05, 2008 at 02:08:47PM +0530, Vinay Sridhar wrote:
> > 2. obtain symbol info from msymtab and check for the section value.
> > AFAIK, elf has section=17 for tls. 
> 
> This number doesn't mean anything.  You want the STT_TLS section type.
> But if you're doing the right thing without a minimal symbol why rely
> on a minimal symbol at all?
> 

We need to determine that the variable is a "tls" variable. When this is
extern, the symbol is LOC_UNRESOLVED. So the section type from "sym" is
not filled. Thats why I tried to determine this from the minimal symbol.
If there is any other method of determining a variable is "tls" before
its owning symtab is linked in, could you please inform? I used this
section number as I could not find another way of determining this..

Re: [patch] Re: Accessing tls variables across files causes a bug

[Daniel Jacobowitz]

On Wed, Aug 06, 2008 at 04:46:50PM +0530, Vinay Sridhar wrote:
> We need to determine that the variable is a "tls" variable. When this is
> extern, the symbol is LOC_UNRESOLVED. So the section type from "sym" is
> not filled. Thats why I tried to determine this from the minimal symbol.
> If there is any other method of determining a variable is "tls" before
> its owning symtab is linked in, could you please inform? I used this
> section number as I could not find another way of determining this..

I imagine that Jan's right and we need to treat all unresolved symbols
this way - or be less eager about creating unresolved symbols.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] Re: Accessing tls variables across files causes a bug

[Jan Kratochvil]

On Wed, 06 Aug 2008 13:16:50 +0200, Vinay Sridhar wrote:
> On Tue, 2008-08-05 at 08:21 -0400, Daniel Jacobowitz wrote:
> > On Tue, Aug 05, 2008 at 02:08:47PM +0530, Vinay Sridhar wrote:
> > > 2. obtain symbol info from msymtab and check for the section value.
> > > AFAIK, elf has section=17 for tls. 
> > 
> > This number doesn't mean anything.  You want the STT_TLS section type.
> > But if you're doing the right thing without a minimal symbol why rely
> > on a minimal symbol at all?
> > 
> 
> We need to determine that the variable is a "tls" variable. When this is
> extern, the symbol is LOC_UNRESOLVED. So the section type from "sym" is
> not filled. Thats why I tried to determine this from the minimal symbol.

The real problem is that read_var_value() for LOC_UNRESOLVED should return the
address through target_translate_tls_address() while currently it returns only
SYMBOL_VALUE_ADDRESS().  In fact SYMBOL_VALUE_ADDRESS() should call
target_translate_tls_address() but that does not work as it is now being used
both as a getter and setter.  If SYMBOL_VALUE_ADDRESS() should represent only
the raw symbol value (and not the TLS-translated real address) most other GDB
places should use the translated variant instead of SYMBOL_VALUE_ADDRESS().

Another possibility is that LOC_UNRESOLVED may no longer be needed for recent
gcc debuginfos always(?) containting `DW_AT_location's, therefore we would not
have to deal with `minimal_symbol's in this case at all.

> If there is any other method of determining a variable is "tls" before
> its owning symtab is linked in, could you please inform?

You cannot as in the debuginfo the only TLS indication is the presence of
DW_OP_GNU_push_tls_address in the DW_AT_location tag.

For minimal symbols one can query for `minimal_symbol's
	ginfo.bfd_section->flags & SEC_THREAD_LOCAL
, for specific symbols one would have to store the BSF_THREAD_LOCAL flag from
BFD into the GDB structures.  Unaware if STT_TLS symbols are always just in
the SHF_TLS sections.

> On Tue, 2008-08-05 at 08:21 -0400, Daniel Jacobowitz wrote:
> > On Tue, Aug 05, 2008 at 02:08:47PM +0530, Vinay Sridhar wrote:
> > > AFAIK, elf has section=17 for tls. 
> > 
> > This number doesn't mean anything.  You want the STT_TLS section type.

To explain more the Daniel J.'s answer - the order of the sections is very
random and while in this compilation case the STT_TLS section was 17th in the
sections list in other compilations it will get a different number.

Section Headers:
  [Nr] Name              Type             Address           Offset
       Size              EntSize          Flags  Link  Info  Align
  [18] .tbss             NOBITS           0000000000600754  00000754
       0000000000000005  0000000000000000 WAT       0     0     4
(Flags=T)



Regards,
Jan

Re: [patch] Re: Accessing tls variables across files causes a bug

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 746 bytes --]

On Wed, 06 Aug 2008 13:42:41 +0200, Jan Kratochvil wrote:
> Another possibility is that LOC_UNRESOLVED may no longer be needed for recent
> gcc debuginfos always(?) containting `DW_AT_location's, therefore we would not
> have to deal with `minimal_symbol's in this case at all.

Attached.

It has no regressions on x86_64, Fedora gcc-4.3.1-6.x86_64 (but gcc-4.3 has
GDB regressions against gcc-4.1).  Also tried there are no regressions by
check//unix/-gstabs+ (although the three new TLS testcases FAIL there).

I do not fully grok why psymtabs were created for DW_AT_type DIEs with no
DW_AT_location.  IMO (DW_AT_location || DW_AT_const_value) is the right
condition (DW_AT_const_value requirement was found by a testsuite run).


Regards,
Jan

[-- Attachment #2: gdb-remove-LOC_UNRESOLVED.patch --]
[-- Type: text/plain, Size: 18518 bytes --]

2008-08-06  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix resolving external references to TLS variables.
	* ada-lang.c (ada_add_block_symbols): Remove LOC_UNRESOLVED.
	* ax-gdb.c (gen_var_ref): Likewise.
	* gen_var_ref (symbol_read_needs_frame, read_var_value): Likewise.
	* m2-exp.y (yylex): Likewise.
	* printcmd.c (address_info): Likewise.
	* symmisc.c (print_symbol, print_partial_symbols): Likewise.
	* symtab.h (enum address_class): Likewise.
	* tracepoint.c (collect_symbol, scope_info): Likewise.
	* mi/mi-cmd-stack.c (list_args_or_locals): Likewise.
	* stabsread.c (scan_file_globals): Complain even on LOC_STATIC symbols.
	Set such symbols to LOC_UNDEF instead of LOC_UNRESOLVED.
	* dwarf2read.c (struct partial_die_info): Remove the field HAS_TYPE.
	New field HAS_CONST_VALUE.
	(add_partial_symbol) <DW_TAG_variable>: HAS_TYPE condition removed.
	HAS_CONST_VALUE condition added.  Comment updated.
	(read_partial_die) <DW_AT_type>: Case removed.
	(read_partial_die) <DW_AT_const_value>: New case.
	(new_symbol): Remove setting LOC_UNRESOLVED.  Comment updated.

2008-08-06  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Update for removed LOC_UNRESOLVED, test resolving external references
	to TLS variables.
	* gdb.dwarf2/dw2-noloc.S: New variable "optloc".
	* gdb.dwarf2/dw2-noloc.exp: Test the new variable "optloc".  Update the
	current message for the variable "noloc".
	* gdb.threads/tls.exp: New tests to print 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/ada-lang.c	21 Jul 2008 16:47:10 -0000	1.151
+++ gdb/ada-lang.c	6 Aug 2008 15:13:51 -0000
@@ -5211,9 +5211,7 @@ ada_add_block_symbols (struct obstack *o
                                    SYMBOL_DOMAIN (sym), domain)
             && wild_match (name, name_len, SYMBOL_LINKAGE_NAME (sym)))
           {
-	    if (SYMBOL_CLASS (sym) == LOC_UNRESOLVED)
-	      continue;
-	    else if (SYMBOL_IS_ARGUMENT (sym))
+	    if (SYMBOL_IS_ARGUMENT (sym))
 	      arg_sym = sym;
 	    else
 	      {
@@ -5236,17 +5234,14 @@ ada_add_block_symbols (struct obstack *o
             if (cmp == 0
                 && is_name_suffix (SYMBOL_LINKAGE_NAME (sym) + name_len))
               {
-		if (SYMBOL_CLASS (sym) != LOC_UNRESOLVED)
+		if (SYMBOL_IS_ARGUMENT (sym))
+		  arg_sym = sym;
+		else
 		  {
-		    if (SYMBOL_IS_ARGUMENT (sym))
-		      arg_sym = sym;
-		    else
-		      {
-			found_sym = 1;
-			add_defn_to_vec (obstackp,
-					 fixup_symbol_section (sym, objfile),
-					 block);
-		      }
+		    found_sym = 1;
+		    add_defn_to_vec (obstackp,
+				     fixup_symbol_section (sym, objfile),
+				     block);
 		  }
               }
           }
@@ -5284,17 +5279,14 @@ ada_add_block_symbols (struct obstack *o
             if (cmp == 0
                 && is_name_suffix (SYMBOL_LINKAGE_NAME (sym) + name_len + 5))
               {
-		if (SYMBOL_CLASS (sym) != LOC_UNRESOLVED)
+		if (SYMBOL_IS_ARGUMENT (sym))
+		  arg_sym = sym;
+		else
 		  {
-		    if (SYMBOL_IS_ARGUMENT (sym))
-		      arg_sym = sym;
-		    else
-		      {
-			found_sym = 1;
-			add_defn_to_vec (obstackp,
-					 fixup_symbol_section (sym, objfile),
-					 block);
-		      }
+		    found_sym = 1;
+		    add_defn_to_vec (obstackp,
+				     fixup_symbol_section (sym, objfile),
+				     block);
 		  }
               }
           }
--- gdb/ax-gdb.c	27 May 2008 19:29:51 -0000	1.44
+++ gdb/ax-gdb.c	6 Aug 2008 15:13:53 -0000
@@ -596,19 +596,6 @@ gen_var_ref (struct agent_expr *ax, stru
       value->kind = axs_lvalue_memory;
       break;
 
-    case LOC_UNRESOLVED:
-      {
-	struct minimal_symbol *msym
-	= lookup_minimal_symbol (DEPRECATED_SYMBOL_NAME (var), NULL, NULL);
-	if (!msym)
-	  error (_("Couldn't resolve symbol `%s'."), SYMBOL_PRINT_NAME (var));
-
-	/* Push the address of the variable.  */
-	ax_const_l (ax, SYMBOL_VALUE_ADDRESS (msym));
-	value->kind = axs_lvalue_memory;
-      }
-      break;
-
     case LOC_COMPUTED:
       /* FIXME: cagney/2004-01-26: It should be possible to
 	 unconditionally call the SYMBOL_OPS method when available.
--- gdb/dwarf2read.c	27 Jun 2008 17:56:47 -0000	1.267
+++ gdb/dwarf2read.c	6 Aug 2008 15:14:01 -0000
@@ -462,7 +462,7 @@ struct partial_die_info
     unsigned int has_children : 1;
     unsigned int is_external : 1;
     unsigned int is_declaration : 1;
-    unsigned int has_type : 1;
+    unsigned int has_const_value : 1;
     unsigned int has_specification : 1;
     unsigned int has_stmt_list : 1;
     unsigned int has_pc_info : 1;
@@ -1988,18 +1988,17 @@ add_partial_symbol (struct partial_die_i
 	     Don't enter into the minimal symbol tables as there is
 	     a minimal symbol table entry from the ELF symbols already.
 	     Enter into partial symbol table if it has a location
-	     descriptor or a type.
-	     If the location descriptor is missing, new_symbol will create
-	     a LOC_UNRESOLVED symbol, the address of the variable will then
-	     be determined from the minimal symbol table whenever the variable
-	     is referenced.
+	     descriptor.  It may have a type but still if it has neither
+	     location descriptor nor it is a constant value it must be just an
+	     `extern' declaration we are not interested in.  Optimized out
+	     variables will have a 0-length location descriptor.
 	     The address for the partial symbol table entry is not
 	     used by GDB, but it comes in handy for debugging partial symbol
 	     table building.  */
 
 	  if (pdi->locdesc)
 	    addr = decode_locdesc (pdi->locdesc, cu);
-	  if (pdi->locdesc || pdi->has_type)
+	  if (pdi->locdesc || pdi->has_const_value)
 	    psym = add_psymbol_to_list (actual_name, strlen (actual_name),
 					VAR_DOMAIN, LOC_STATIC,
 					&objfile->global_psymbols,
@@ -5837,8 +5836,8 @@ read_partial_die (struct partial_die_inf
 	case DW_AT_declaration:
 	  part_die->is_declaration = DW_UNSND (&attr);
 	  break;
-	case DW_AT_type:
-	  part_die->has_type = 1;
+	case DW_AT_const_value:
+	  part_die->has_const_value = 1;
 	  break;
 	case DW_AT_abstract_origin:
 	case DW_AT_specification:
@@ -7486,19 +7485,15 @@ new_symbol (struct die_info *die, struct
 	    }
 	  else
 	    {
-	      /* We do not know the address of this symbol.
-	         If it is an external symbol and we have type information
-	         for it, enter the symbol as a LOC_UNRESOLVED symbol.
-	         The address of the variable will then be determined from
-	         the minimal symbol table whenever the variable is
-	         referenced.  */
-	      attr2 = dwarf2_attr (die, DW_AT_external, cu);
-	      if (attr2 && (DW_UNSND (attr2) != 0)
-		  && dwarf2_attr (die, DW_AT_type, cu) != NULL)
-		{
-		  SYMBOL_CLASS (sym) = LOC_UNRESOLVED;
-		  add_symbol_to_list (sym, &global_symbols);
-		}
+	      /* We do not know the address of this symbol.  If it is an
+		 external symbol and we have type information
+		 for it, we could enter the symbol as a LOC_UNRESOLVED symbol.
+		 The address of the variable could then be determined from the
+		 minimal symbol table whenever the variable is referenced.
+		 Just it needs an exception in the code whenever we get
+		 LOC_UNRESOLVED and some places miss it, fortunately recent GCC
+		 puts DW_AT_location everywhere so this workaround is no longer
+		 needed.  */
 	    }
 	  break;
 	case DW_TAG_formal_parameter:
--- gdb/findvar.c	15 Jul 2008 17:53:11 -0000	1.115
+++ gdb/findvar.c	6 Aug 2008 15:14:02 -0000
@@ -371,7 +371,6 @@ symbol_read_needs_frame (struct symbol *
 
     case LOC_BLOCK:
     case LOC_CONST_BYTES:
-    case LOC_UNRESOLVED:
     case LOC_OPTIMIZED_OUT:
       return 0;
     }
@@ -533,21 +532,6 @@ read_var_value (struct symbol *var, stru
 	return 0;
       return SYMBOL_OPS (var)->read_variable (var, frame);
 
-    case LOC_UNRESOLVED:
-      {
-	struct minimal_symbol *msym;
-
-	msym = lookup_minimal_symbol (DEPRECATED_SYMBOL_NAME (var), NULL, NULL);
-	if (msym == NULL)
-	  return 0;
-	if (overlay_debugging)
-	  addr = symbol_overlayed_address (SYMBOL_VALUE_ADDRESS (msym),
-					   SYMBOL_BFD_SECTION (msym));
-	else
-	  addr = SYMBOL_VALUE_ADDRESS (msym);
-      }
-      break;
-
     case LOC_OPTIMIZED_OUT:
       VALUE_LVAL (v) = not_lval;
       set_value_optimized_out (v, 1);
--- gdb/m2-exp.y	27 May 2008 19:29:51 -0000	1.21
+++ gdb/m2-exp.y	6 Aug 2008 15:14:02 -0000
@@ -1059,7 +1059,6 @@ yylex ()
 	  error("internal:  Undefined class in m2lex()");
 
        case LOC_LABEL:
-       case LOC_UNRESOLVED:
 	  error("internal:  Unforseen case in m2lex()");
 
        default:
--- gdb/printcmd.c	6 Jun 2008 20:58:08 -0000	1.127
+++ gdb/printcmd.c	6 Aug 2008 15:14:05 -0000
@@ -1169,30 +1169,6 @@ address_info (char *exp, int from_tty)
 	}
       break;
 
-    case LOC_UNRESOLVED:
-      {
-	struct minimal_symbol *msym;
-
-	msym = lookup_minimal_symbol (DEPRECATED_SYMBOL_NAME (sym), NULL, NULL);
-	if (msym == NULL)
-	  printf_filtered ("unresolved");
-	else
-	  {
-	    section = SYMBOL_BFD_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))
-	      {
-		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->name);
-	      }
-	  }
-      }
-      break;
-
     case LOC_OPTIMIZED_OUT:
       printf_filtered (_("optimized out"));
       break;
--- gdb/stabsread.c	27 May 2008 19:29:51 -0000	1.108
+++ gdb/stabsread.c	6 Aug 2008 15:14:08 -0000
@@ -4501,7 +4501,7 @@ scan_file_globals (struct objfile *objfi
     }
 
   /* Change the storage class of any remaining unresolved globals to
-     LOC_UNRESOLVED and remove them from the chain.  */
+     LOC_UNDEF and remove them from the chain.  */
   for (hash = 0; hash < HASHSIZE; hash++)
     {
       sym = global_sym_chain[hash];
@@ -4514,13 +4514,11 @@ scan_file_globals (struct objfile *objfi
 	     to address zero.  */
 	  SYMBOL_VALUE_ADDRESS (prev) = 0;
 
+	  SYMBOL_CLASS (prev) = LOC_UNDEF;
 	  /* Complain about unresolved common block symbols.  */
-	  if (SYMBOL_CLASS (prev) == LOC_STATIC)
-	    SYMBOL_CLASS (prev) = LOC_UNRESOLVED;
-	  else
-	    complaint (&symfile_complaints,
-		       _("%s: common block `%s' from global_sym_chain unresolved"),
-		       objfile->name, DEPRECATED_SYMBOL_NAME (prev));
+	  complaint (&symfile_complaints,
+		     _("%s: common block `%s' from global_sym_chain unresolved"),
+		     objfile->name, DEPRECATED_SYMBOL_NAME (prev));
 	}
     }
   memset (global_sym_chain, 0, sizeof (global_sym_chain));
--- gdb/symmisc.c	27 May 2008 19:29:51 -0000	1.53
+++ gdb/symmisc.c	6 Aug 2008 15:14:09 -0000
@@ -701,10 +701,6 @@ print_symbol (void *args)
 	  fprintf_filtered (outfile, "computed at runtime");
 	  break;
 
-	case LOC_UNRESOLVED:
-	  fprintf_filtered (outfile, "unresolved");
-	  break;
-
 	case LOC_OPTIMIZED_OUT:
 	  fprintf_filtered (outfile, "optimized out");
 	  break;
@@ -837,9 +833,6 @@ print_partial_symbols (struct partial_sy
 	case LOC_CONST_BYTES:
 	  fputs_filtered ("constant bytes", outfile);
 	  break;
-	case LOC_UNRESOLVED:
-	  fputs_filtered ("unresolved", outfile);
-	  break;
 	case LOC_OPTIMIZED_OUT:
 	  fputs_filtered ("optimized out", outfile);
 	  break;
--- gdb/symtab.h	27 May 2008 19:29:51 -0000	1.128
+++ gdb/symtab.h	6 Aug 2008 15:14:12 -0000
@@ -476,18 +476,6 @@ enum address_class
 
   LOC_CONST_BYTES,
 
-  /* Value is at fixed address, but the address of the variable has
-     to be determined from the minimal symbol table whenever the
-     variable is referenced.
-     This happens if debugging information for a global symbol is
-     emitted and the corresponding minimal symbol is defined
-     in another object file or runtime common storage.
-     The linker might even remove the minimal symbol if the global
-     symbol is never referenced, in which case the symbol remains
-     unresolved.  */
-
-  LOC_UNRESOLVED,
-
   /* The variable does not actually exist in the program.
      The value is ignored.  */
 
--- gdb/tracepoint.c	25 Jul 2008 16:12:03 -0000	1.107
+++ gdb/tracepoint.c	6 Aug 2008 15:14:14 -0000
@@ -1289,10 +1289,6 @@ collect_symbol (struct collection_list *
 	}
       add_memrange (collect, reg, offset, len);
       break;
-    case LOC_UNRESOLVED:
-      printf_filtered ("Don't know LOC_UNRESOLVED %s\n", 
-		       DEPRECATED_SYMBOL_NAME (sym));
-      break;
     case LOC_OPTIMIZED_OUT:
       printf_filtered ("%s has been optimized out of existence.\n",
 		       DEPRECATED_SYMBOL_NAME (sym));
@@ -2457,17 +2453,6 @@ scope_info (char *args, int from_tty)
 	      printf_filtered ("a function at address ");
 	      printf_filtered ("%s", paddress (BLOCK_START (SYMBOL_BLOCK_VALUE (sym))));
 	      break;
-	    case LOC_UNRESOLVED:
-	      msym = lookup_minimal_symbol (DEPRECATED_SYMBOL_NAME (sym), 
-					    NULL, NULL);
-	      if (msym == NULL)
-		printf_filtered ("Unresolved Static");
-	      else
-		{
-		  printf_filtered ("static storage at address ");
-		  printf_filtered ("%s", paddress (SYMBOL_VALUE_ADDRESS (msym)));
-		}
-	      break;
 	    case LOC_OPTIMIZED_OUT:
 	      printf_filtered ("optimized out.\n");
 	      continue;
--- gdb/mi/mi-cmd-stack.c	25 Jun 2008 15:15:42 -0000	1.41
+++ gdb/mi/mi-cmd-stack.c	6 Aug 2008 15:14:15 -0000
@@ -238,7 +238,6 @@ list_args_or_locals (int locals, int val
 	    case LOC_LABEL:	/* local label           */
 	    case LOC_BLOCK:	/* local function        */
 	    case LOC_CONST_BYTES:	/* loc. byte seq.        */
-	    case LOC_UNRESOLVED:	/* unresolved static     */
 	    case LOC_OPTIMIZED_OUT:	/* optimized out         */
 	      print_me = 0;
 	      break;
--- gdb/testsuite/gdb.dwarf2/dw2-noloc.S	1 Jan 2008 22:53:19 -0000	1.3
+++ gdb/testsuite/gdb.dwarf2/dw2-noloc.S	6 Aug 2008 15:14:15 -0000
@@ -69,6 +69,12 @@ func_cu1:
 	.4byte		.Ltype_int-.Lcu1_begin	/* DW_AT_type */
 	.byte		1			/* DW_AT_external */
 
+	.uleb128	5			/* Abbrev: DW_TAG_variable */
+	.ascii		"optloc\0"		/* DW_AT_name */
+	.4byte		.Ltype_int-.Lcu1_begin	/* DW_AT_type */
+	.byte		1			/* DW_AT_external */
+	.byte		0			/* DW_AT_location: length */
+
 	.byte		0			/* End of children of CU */
 
 .Lcu1_end:
@@ -140,6 +146,20 @@ func_cu1:
 	.byte		0x0			/* Terminator */
 	.byte		0x0			/* Terminator */
 
+	.uleb128	5			/* Abbrev code */
+	.uleb128	0x34			/* DW_TAG_variable */
+	.byte		0			/* has_children */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x49			/* DW_AT_type */
+	.uleb128	0x13			/* DW_FORM_ref4 */
+	.uleb128	0x3f			/* DW_AT_external */
+	.uleb128	0xc			/* DW_FORM_flag */
+	.uleb128	0x2			/* DW_AT_location */
+	.uleb128	0xa			/* DW_FORM_block1 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
 	.byte		0x0			/* Terminator */
 	.byte		0x0			/* Terminator */
 
--- gdb/testsuite/gdb.dwarf2/dw2-noloc.exp	1 Jan 2008 22:53:19 -0000	1.3
+++ gdb/testsuite/gdb.dwarf2/dw2-noloc.exp	6 Aug 2008 15:14:15 -0000
@@ -45,4 +45,5 @@ gdb_start
 gdb_reinitialize_dir $srcdir/$subdir
 gdb_load ${binfile}
 
-gdb_test "print noloc" "Address of symbol \"noloc\" is unknown." "print noloc"
+gdb_test "print noloc" "No symbol \"noloc\" in current context." "print noloc"
+gdb_test "print optloc" " = <value optimized out>" "print optloc"
--- gdb/testsuite/gdb.threads/tls.c	29 Jul 2003 21:51:25 -0000	1.2
+++ gdb/testsuite/gdb.threads/tls.c	6 Aug 2008 15:14:17 -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	6 Aug 2008 15:14:17 -0000
@@ -15,7 +15,9 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 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,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\]+"
+# Here it could crash with: Cannot access memory at address 0x0
+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	6 Aug 2008 15:14:17 -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;
+}

Re: [patch] Re: Accessing tls variables across files causes a bug

[Vinay Sridhar]

When can we expect these patches to go in?

Thanks,
Vinay


On Wed, 2008-08-06 at 17:19 +0200, Jan Kratochvil wrote:
> On Wed, 06 Aug 2008 13:42:41 +0200, Jan Kratochvil wrote:
> > Another possibility is that LOC_UNRESOLVED may no longer be needed for recent
> > gcc debuginfos always(?) containting `DW_AT_location's, therefore we would not
> > have to deal with `minimal_symbol's in this case at all.
> 
> Attached.
> 
> It has no regressions on x86_64, Fedora gcc-4.3.1-6.x86_64 (but gcc-4.3 has
> GDB regressions against gcc-4.1).  Also tried there are no regressions by
> check//unix/-gstabs+ (although the three new TLS testcases FAIL there).
> 
> I do not fully grok why psymtabs were created for DW_AT_type DIEs with no
> DW_AT_location.  IMO (DW_AT_location || DW_AT_const_value) is the right
> condition (DW_AT_const_value requirement was found by a testsuite run).
> 
> 
> Regards,
> Jan
> plain text document attachment (gdb-remove-LOC_UNRESOLVED.patch)
> 2008-08-06  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	Fix resolving external references to TLS variables.
> 	* ada-lang.c (ada_add_block_symbols): Remove LOC_UNRESOLVED.
> 	* ax-gdb.c (gen_var_ref): Likewise.
> 	* gen_var_ref (symbol_read_needs_frame, read_var_value): Likewise.
> 	* m2-exp.y (yylex): Likewise.
> 	* printcmd.c (address_info): Likewise.
> 	* symmisc.c (print_symbol, print_partial_symbols): Likewise.
> 	* symtab.h (enum address_class): Likewise.
> 	* tracepoint.c (collect_symbol, scope_info): Likewise.
> 	* mi/mi-cmd-stack.c (list_args_or_locals): Likewise.
> 	* stabsread.c (scan_file_globals): Complain even on LOC_STATIC symbols.
> 	Set such symbols to LOC_UNDEF instead of LOC_UNRESOLVED.
> 	* dwarf2read.c (struct partial_die_info): Remove the field HAS_TYPE.
> 	New field HAS_CONST_VALUE.
> 	(add_partial_symbol) <DW_TAG_variable>: HAS_TYPE condition removed.
> 	HAS_CONST_VALUE condition added.  Comment updated.
> 	(read_partial_die) <DW_AT_type>: Case removed.
> 	(read_partial_die) <DW_AT_const_value>: New case.
> 	(new_symbol): Remove setting LOC_UNRESOLVED.  Comment updated.
> 
> 2008-08-06  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	Update for removed LOC_UNRESOLVED, test resolving external references
> 	to TLS variables.
> 	* gdb.dwarf2/dw2-noloc.S: New variable "optloc".
> 	* gdb.dwarf2/dw2-noloc.exp: Test the new variable "optloc".  Update the
> 	current message for the variable "noloc".
> 	* gdb.threads/tls.exp: New tests to print 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/ada-lang.c	21 Jul 2008 16:47:10 -0000	1.151
> +++ gdb/ada-lang.c	6 Aug 2008 15:13:51 -0000
> @@ -5211,9 +5211,7 @@ ada_add_block_symbols (struct obstack *o
>                                     SYMBOL_DOMAIN (sym), domain)
>              && wild_match (name, name_len, SYMBOL_LINKAGE_NAME (sym)))
>            {
> -	    if (SYMBOL_CLASS (sym) == LOC_UNRESOLVED)
> -	      continue;
> -	    else if (SYMBOL_IS_ARGUMENT (sym))
> +	    if (SYMBOL_IS_ARGUMENT (sym))
>  	      arg_sym = sym;
>  	    else
>  	      {
> @@ -5236,17 +5234,14 @@ ada_add_block_symbols (struct obstack *o
>              if (cmp == 0
>                  && is_name_suffix (SYMBOL_LINKAGE_NAME (sym) + name_len))
>                {
> -		if (SYMBOL_CLASS (sym) != LOC_UNRESOLVED)
> +		if (SYMBOL_IS_ARGUMENT (sym))
> +		  arg_sym = sym;
> +		else
>  		  {
> -		    if (SYMBOL_IS_ARGUMENT (sym))
> -		      arg_sym = sym;
> -		    else
> -		      {
> -			found_sym = 1;
> -			add_defn_to_vec (obstackp,
> -					 fixup_symbol_section (sym, objfile),
> -					 block);
> -		      }
> +		    found_sym = 1;
> +		    add_defn_to_vec (obstackp,
> +				     fixup_symbol_section (sym, objfile),
> +				     block);
>  		  }
>                }
>            }
> @@ -5284,17 +5279,14 @@ ada_add_block_symbols (struct obstack *o
>              if (cmp == 0
>                  && is_name_suffix (SYMBOL_LINKAGE_NAME (sym) + name_len + 5))
>                {
> -		if (SYMBOL_CLASS (sym) != LOC_UNRESOLVED)
> +		if (SYMBOL_IS_ARGUMENT (sym))
> +		  arg_sym = sym;
> +		else
>  		  {
> -		    if (SYMBOL_IS_ARGUMENT (sym))
> -		      arg_sym = sym;
> -		    else
> -		      {
> -			found_sym = 1;
> -			add_defn_to_vec (obstackp,
> -					 fixup_symbol_section (sym, objfile),
> -					 block);
> -		      }
> +		    found_sym = 1;
> +		    add_defn_to_vec (obstackp,
> +				     fixup_symbol_section (sym, objfile),
> +				     block);
>  		  }
>                }
>            }
> --- gdb/ax-gdb.c	27 May 2008 19:29:51 -0000	1.44
> +++ gdb/ax-gdb.c	6 Aug 2008 15:13:53 -0000
> @@ -596,19 +596,6 @@ gen_var_ref (struct agent_expr *ax, stru
>        value->kind = axs_lvalue_memory;
>        break;
> 
> -    case LOC_UNRESOLVED:
> -      {
> -	struct minimal_symbol *msym
> -	= lookup_minimal_symbol (DEPRECATED_SYMBOL_NAME (var), NULL, NULL);
> -	if (!msym)
> -	  error (_("Couldn't resolve symbol `%s'."), SYMBOL_PRINT_NAME (var));
> -
> -	/* Push the address of the variable.  */
> -	ax_const_l (ax, SYMBOL_VALUE_ADDRESS (msym));
> -	value->kind = axs_lvalue_memory;
> -      }
> -      break;
> -
>      case LOC_COMPUTED:
>        /* FIXME: cagney/2004-01-26: It should be possible to
>  	 unconditionally call the SYMBOL_OPS method when available.
> --- gdb/dwarf2read.c	27 Jun 2008 17:56:47 -0000	1.267
> +++ gdb/dwarf2read.c	6 Aug 2008 15:14:01 -0000
> @@ -462,7 +462,7 @@ struct partial_die_info
>      unsigned int has_children : 1;
>      unsigned int is_external : 1;
>      unsigned int is_declaration : 1;
> -    unsigned int has_type : 1;
> +    unsigned int has_const_value : 1;
>      unsigned int has_specification : 1;
>      unsigned int has_stmt_list : 1;
>      unsigned int has_pc_info : 1;
> @@ -1988,18 +1988,17 @@ add_partial_symbol (struct partial_die_i
>  	     Don't enter into the minimal symbol tables as there is
>  	     a minimal symbol table entry from the ELF symbols already.
>  	     Enter into partial symbol table if it has a location
> -	     descriptor or a type.
> -	     If the location descriptor is missing, new_symbol will create
> -	     a LOC_UNRESOLVED symbol, the address of the variable will then
> -	     be determined from the minimal symbol table whenever the variable
> -	     is referenced.
> +	     descriptor.  It may have a type but still if it has neither
> +	     location descriptor nor it is a constant value it must be just an
> +	     `extern' declaration we are not interested in.  Optimized out
> +	     variables will have a 0-length location descriptor.
>  	     The address for the partial symbol table entry is not
>  	     used by GDB, but it comes in handy for debugging partial symbol
>  	     table building.  */
> 
>  	  if (pdi->locdesc)
>  	    addr = decode_locdesc (pdi->locdesc, cu);
> -	  if (pdi->locdesc || pdi->has_type)
> +	  if (pdi->locdesc || pdi->has_const_value)
>  	    psym = add_psymbol_to_list (actual_name, strlen (actual_name),
>  					VAR_DOMAIN, LOC_STATIC,
>  					&objfile->global_psymbols,
> @@ -5837,8 +5836,8 @@ read_partial_die (struct partial_die_inf
>  	case DW_AT_declaration:
>  	  part_die->is_declaration = DW_UNSND (&attr);
>  	  break;
> -	case DW_AT_type:
> -	  part_die->has_type = 1;
> +	case DW_AT_const_value:
> +	  part_die->has_const_value = 1;
>  	  break;
>  	case DW_AT_abstract_origin:
>  	case DW_AT_specification:
> @@ -7486,19 +7485,15 @@ new_symbol (struct die_info *die, struct
>  	    }
>  	  else
>  	    {
> -	      /* We do not know the address of this symbol.
> -	         If it is an external symbol and we have type information
> -	         for it, enter the symbol as a LOC_UNRESOLVED symbol.
> -	         The address of the variable will then be determined from
> -	         the minimal symbol table whenever the variable is
> -	         referenced.  */
> -	      attr2 = dwarf2_attr (die, DW_AT_external, cu);
> -	      if (attr2 && (DW_UNSND (attr2) != 0)
> -		  && dwarf2_attr (die, DW_AT_type, cu) != NULL)
> -		{
> -		  SYMBOL_CLASS (sym) = LOC_UNRESOLVED;
> -		  add_symbol_to_list (sym, &global_symbols);
> -		}
> +	      /* We do not know the address of this symbol.  If it is an
> +		 external symbol and we have type information
> +		 for it, we could enter the symbol as a LOC_UNRESOLVED symbol.
> +		 The address of the variable could then be determined from the
> +		 minimal symbol table whenever the variable is referenced.
> +		 Just it needs an exception in the code whenever we get
> +		 LOC_UNRESOLVED and some places miss it, fortunately recent GCC
> +		 puts DW_AT_location everywhere so this workaround is no longer
> +		 needed.  */
>  	    }
>  	  break;
>  	case DW_TAG_formal_parameter:
> --- gdb/findvar.c	15 Jul 2008 17:53:11 -0000	1.115
> +++ gdb/findvar.c	6 Aug 2008 15:14:02 -0000
> @@ -371,7 +371,6 @@ symbol_read_needs_frame (struct symbol *
> 
>      case LOC_BLOCK:
>      case LOC_CONST_BYTES:
> -    case LOC_UNRESOLVED:
>      case LOC_OPTIMIZED_OUT:
>        return 0;
>      }
> @@ -533,21 +532,6 @@ read_var_value (struct symbol *var, stru
>  	return 0;
>        return SYMBOL_OPS (var)->read_variable (var, frame);
> 
> -    case LOC_UNRESOLVED:
> -      {
> -	struct minimal_symbol *msym;
> -
> -	msym = lookup_minimal_symbol (DEPRECATED_SYMBOL_NAME (var), NULL, NULL);
> -	if (msym == NULL)
> -	  return 0;
> -	if (overlay_debugging)
> -	  addr = symbol_overlayed_address (SYMBOL_VALUE_ADDRESS (msym),
> -					   SYMBOL_BFD_SECTION (msym));
> -	else
> -	  addr = SYMBOL_VALUE_ADDRESS (msym);
> -      }
> -      break;
> -
>      case LOC_OPTIMIZED_OUT:
>        VALUE_LVAL (v) = not_lval;
>        set_value_optimized_out (v, 1);
> --- gdb/m2-exp.y	27 May 2008 19:29:51 -0000	1.21
> +++ gdb/m2-exp.y	6 Aug 2008 15:14:02 -0000
> @@ -1059,7 +1059,6 @@ yylex ()
>  	  error("internal:  Undefined class in m2lex()");
> 
>         case LOC_LABEL:
> -       case LOC_UNRESOLVED:
>  	  error("internal:  Unforseen case in m2lex()");
> 
>         default:
> --- gdb/printcmd.c	6 Jun 2008 20:58:08 -0000	1.127
> +++ gdb/printcmd.c	6 Aug 2008 15:14:05 -0000
> @@ -1169,30 +1169,6 @@ address_info (char *exp, int from_tty)
>  	}
>        break;
> 
> -    case LOC_UNRESOLVED:
> -      {
> -	struct minimal_symbol *msym;
> -
> -	msym = lookup_minimal_symbol (DEPRECATED_SYMBOL_NAME (sym), NULL, NULL);
> -	if (msym == NULL)
> -	  printf_filtered ("unresolved");
> -	else
> -	  {
> -	    section = SYMBOL_BFD_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))
> -	      {
> -		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->name);
> -	      }
> -	  }
> -      }
> -      break;
> -
>      case LOC_OPTIMIZED_OUT:
>        printf_filtered (_("optimized out"));
>        break;
> --- gdb/stabsread.c	27 May 2008 19:29:51 -0000	1.108
> +++ gdb/stabsread.c	6 Aug 2008 15:14:08 -0000
> @@ -4501,7 +4501,7 @@ scan_file_globals (struct objfile *objfi
>      }
> 
>    /* Change the storage class of any remaining unresolved globals to
> -     LOC_UNRESOLVED and remove them from the chain.  */
> +     LOC_UNDEF and remove them from the chain.  */
>    for (hash = 0; hash < HASHSIZE; hash++)
>      {
>        sym = global_sym_chain[hash];
> @@ -4514,13 +4514,11 @@ scan_file_globals (struct objfile *objfi
>  	     to address zero.  */
>  	  SYMBOL_VALUE_ADDRESS (prev) = 0;
> 
> +	  SYMBOL_CLASS (prev) = LOC_UNDEF;
>  	  /* Complain about unresolved common block symbols.  */
> -	  if (SYMBOL_CLASS (prev) == LOC_STATIC)
> -	    SYMBOL_CLASS (prev) = LOC_UNRESOLVED;
> -	  else
> -	    complaint (&symfile_complaints,
> -		       _("%s: common block `%s' from global_sym_chain unresolved"),
> -		       objfile->name, DEPRECATED_SYMBOL_NAME (prev));
> +	  complaint (&symfile_complaints,
> +		     _("%s: common block `%s' from global_sym_chain unresolved"),
> +		     objfile->name, DEPRECATED_SYMBOL_NAME (prev));
>  	}
>      }
>    memset (global_sym_chain, 0, sizeof (global_sym_chain));
> --- gdb/symmisc.c	27 May 2008 19:29:51 -0000	1.53
> +++ gdb/symmisc.c	6 Aug 2008 15:14:09 -0000
> @@ -701,10 +701,6 @@ print_symbol (void *args)
>  	  fprintf_filtered (outfile, "computed at runtime");
>  	  break;
> 
> -	case LOC_UNRESOLVED:
> -	  fprintf_filtered (outfile, "unresolved");
> -	  break;
> -
>  	case LOC_OPTIMIZED_OUT:
>  	  fprintf_filtered (outfile, "optimized out");
>  	  break;
> @@ -837,9 +833,6 @@ print_partial_symbols (struct partial_sy
>  	case LOC_CONST_BYTES:
>  	  fputs_filtered ("constant bytes", outfile);
>  	  break;
> -	case LOC_UNRESOLVED:
> -	  fputs_filtered ("unresolved", outfile);
> -	  break;
>  	case LOC_OPTIMIZED_OUT:
>  	  fputs_filtered ("optimized out", outfile);
>  	  break;
> --- gdb/symtab.h	27 May 2008 19:29:51 -0000	1.128
> +++ gdb/symtab.h	6 Aug 2008 15:14:12 -0000
> @@ -476,18 +476,6 @@ enum address_class
> 
>    LOC_CONST_BYTES,
> 
> -  /* Value is at fixed address, but the address of the variable has
> -     to be determined from the minimal symbol table whenever the
> -     variable is referenced.
> -     This happens if debugging information for a global symbol is
> -     emitted and the corresponding minimal symbol is defined
> -     in another object file or runtime common storage.
> -     The linker might even remove the minimal symbol if the global
> -     symbol is never referenced, in which case the symbol remains
> -     unresolved.  */
> -
> -  LOC_UNRESOLVED,
> -
>    /* The variable does not actually exist in the program.
>       The value is ignored.  */
> 
> --- gdb/tracepoint.c	25 Jul 2008 16:12:03 -0000	1.107
> +++ gdb/tracepoint.c	6 Aug 2008 15:14:14 -0000
> @@ -1289,10 +1289,6 @@ collect_symbol (struct collection_list *
>  	}
>        add_memrange (collect, reg, offset, len);
>        break;
> -    case LOC_UNRESOLVED:
> -      printf_filtered ("Don't know LOC_UNRESOLVED %s\n", 
> -		       DEPRECATED_SYMBOL_NAME (sym));
> -      break;
>      case LOC_OPTIMIZED_OUT:
>        printf_filtered ("%s has been optimized out of existence.\n",
>  		       DEPRECATED_SYMBOL_NAME (sym));
> @@ -2457,17 +2453,6 @@ scope_info (char *args, int from_tty)
>  	      printf_filtered ("a function at address ");
>  	      printf_filtered ("%s", paddress (BLOCK_START (SYMBOL_BLOCK_VALUE (sym))));
>  	      break;
> -	    case LOC_UNRESOLVED:
> -	      msym = lookup_minimal_symbol (DEPRECATED_SYMBOL_NAME (sym), 
> -					    NULL, NULL);
> -	      if (msym == NULL)
> -		printf_filtered ("Unresolved Static");
> -	      else
> -		{
> -		  printf_filtered ("static storage at address ");
> -		  printf_filtered ("%s", paddress (SYMBOL_VALUE_ADDRESS (msym)));
> -		}
> -	      break;
>  	    case LOC_OPTIMIZED_OUT:
>  	      printf_filtered ("optimized out.\n");
>  	      continue;
> --- gdb/mi/mi-cmd-stack.c	25 Jun 2008 15:15:42 -0000	1.41
> +++ gdb/mi/mi-cmd-stack.c	6 Aug 2008 15:14:15 -0000
> @@ -238,7 +238,6 @@ list_args_or_locals (int locals, int val
>  	    case LOC_LABEL:	/* local label           */
>  	    case LOC_BLOCK:	/* local function        */
>  	    case LOC_CONST_BYTES:	/* loc. byte seq.        */
> -	    case LOC_UNRESOLVED:	/* unresolved static     */
>  	    case LOC_OPTIMIZED_OUT:	/* optimized out         */
>  	      print_me = 0;
>  	      break;
> --- gdb/testsuite/gdb.dwarf2/dw2-noloc.S	1 Jan 2008 22:53:19 -0000	1.3
> +++ gdb/testsuite/gdb.dwarf2/dw2-noloc.S	6 Aug 2008 15:14:15 -0000
> @@ -69,6 +69,12 @@ func_cu1:
>  	.4byte		.Ltype_int-.Lcu1_begin	/* DW_AT_type */
>  	.byte		1			/* DW_AT_external */
> 
> +	.uleb128	5			/* Abbrev: DW_TAG_variable */
> +	.ascii		"optloc\0"		/* DW_AT_name */
> +	.4byte		.Ltype_int-.Lcu1_begin	/* DW_AT_type */
> +	.byte		1			/* DW_AT_external */
> +	.byte		0			/* DW_AT_location: length */
> +
>  	.byte		0			/* End of children of CU */
> 
>  .Lcu1_end:
> @@ -140,6 +146,20 @@ func_cu1:
>  	.byte		0x0			/* Terminator */
>  	.byte		0x0			/* Terminator */
> 
> +	.uleb128	5			/* Abbrev code */
> +	.uleb128	0x34			/* DW_TAG_variable */
> +	.byte		0			/* has_children */
> +	.uleb128	0x3			/* DW_AT_name */
> +	.uleb128	0x8			/* DW_FORM_string */
> +	.uleb128	0x49			/* DW_AT_type */
> +	.uleb128	0x13			/* DW_FORM_ref4 */
> +	.uleb128	0x3f			/* DW_AT_external */
> +	.uleb128	0xc			/* DW_FORM_flag */
> +	.uleb128	0x2			/* DW_AT_location */
> +	.uleb128	0xa			/* DW_FORM_block1 */
> +	.byte		0x0			/* Terminator */
> +	.byte		0x0			/* Terminator */
> +
>  	.byte		0x0			/* Terminator */
>  	.byte		0x0			/* Terminator */
> 
> --- gdb/testsuite/gdb.dwarf2/dw2-noloc.exp	1 Jan 2008 22:53:19 -0000	1.3
> +++ gdb/testsuite/gdb.dwarf2/dw2-noloc.exp	6 Aug 2008 15:14:15 -0000
> @@ -45,4 +45,5 @@ gdb_start
>  gdb_reinitialize_dir $srcdir/$subdir
>  gdb_load ${binfile}
> 
> -gdb_test "print noloc" "Address of symbol \"noloc\" is unknown." "print noloc"
> +gdb_test "print noloc" "No symbol \"noloc\" in current context." "print noloc"
> +gdb_test "print optloc" " = <value optimized out>" "print optloc"
> --- gdb/testsuite/gdb.threads/tls.c	29 Jul 2003 21:51:25 -0000	1.2
> +++ gdb/testsuite/gdb.threads/tls.c	6 Aug 2008 15:14:17 -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	6 Aug 2008 15:14:17 -0000
> @@ -15,7 +15,9 @@
>  # along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> 
>  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,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\]+"
> +# Here it could crash with: Cannot access memory at address 0x0
> +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	6 Aug 2008 15:14:17 -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;
> +}

[resolved] Re: [patch] Re: Accessing tls variables across files causes a bug

[Jan Kratochvil]

Hi,

just marking this mail thread as resolved by later mail threads:

The TLS TLS cross-file references fix got checked-in by:
Re: [patch] Accessing tls variables across files causes a bug
http://sourceware.org/ml/gdb-patches/2008-12/msg00016.html
http://sourceware.org/ml/gdb-cvs/2008-12/msg00009.html

Why LOC_UNRESOLVED existence makes sense for GDB (testcase on it):
Re: [patch] Fix C `extern' shadowing in a local block
http://sourceware.org/ml/gdb-patches/2009-04/msg00051.html
http://sourceware.org/ml/gdb-cvs/2009-04/msg00021.html


Thanks,
Jan


On Wed, 06 Aug 2008 17:19:33 +0200, Jan Kratochvil wrote:
> On Wed, 06 Aug 2008 13:42:41 +0200, Jan Kratochvil wrote:
> > Another possibility is that LOC_UNRESOLVED may no longer be needed for recent
> > gcc debuginfos always(?) containting `DW_AT_location's, therefore we would not
> > have to deal with `minimal_symbol's in this case at all.
> 
> Attached.
> 
> It has no regressions on x86_64, Fedora gcc-4.3.1-6.x86_64 (but gcc-4.3 has
> GDB regressions against gcc-4.1).  Also tried there are no regressions by
> check//unix/-gstabs+ (although the three new TLS testcases FAIL there).
> 
> I do not fully grok why psymtabs were created for DW_AT_type DIEs with no
> DW_AT_location.  IMO (DW_AT_location || DW_AT_const_value) is the right
> condition (DW_AT_const_value requirement was found by a testsuite run).
> 
> 
> Regards,
> Jan

> 2008-08-06  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	Fix resolving external references to TLS variables.
> 	* ada-lang.c (ada_add_block_symbols): Remove LOC_UNRESOLVED.
> 	* ax-gdb.c (gen_var_ref): Likewise.
> 	* gen_var_ref (symbol_read_needs_frame, read_var_value): Likewise.
> 	* m2-exp.y (yylex): Likewise.
> 	* printcmd.c (address_info): Likewise.
> 	* symmisc.c (print_symbol, print_partial_symbols): Likewise.
> 	* symtab.h (enum address_class): Likewise.
> 	* tracepoint.c (collect_symbol, scope_info): Likewise.
> 	* mi/mi-cmd-stack.c (list_args_or_locals): Likewise.
> 	* stabsread.c (scan_file_globals): Complain even on LOC_STATIC symbols.
> 	Set such symbols to LOC_UNDEF instead of LOC_UNRESOLVED.
> 	* dwarf2read.c (struct partial_die_info): Remove the field HAS_TYPE.
> 	New field HAS_CONST_VALUE.
> 	(add_partial_symbol) <DW_TAG_variable>: HAS_TYPE condition removed.
> 	HAS_CONST_VALUE condition added.  Comment updated.
> 	(read_partial_die) <DW_AT_type>: Case removed.
> 	(read_partial_die) <DW_AT_const_value>: New case.
> 	(new_symbol): Remove setting LOC_UNRESOLVED.  Comment updated.
> 
> 2008-08-06  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	Update for removed LOC_UNRESOLVED, test resolving external references
> 	to TLS variables.
> 	* gdb.dwarf2/dw2-noloc.S: New variable "optloc".
> 	* gdb.dwarf2/dw2-noloc.exp: Test the new variable "optloc".  Update the
> 	current message for the variable "noloc".
> 	* gdb.threads/tls.exp: New tests to print 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.