* [patch] Re: Accessing tls variables across files causes a bug
[not found] <1217480020.4755.1.camel@vinaysridhar.in.ibm.com>
@ 2008-08-02 17:18 ` Jan Kratochvil
2008-08-04 19:36 ` Luis Machado
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Jan Kratochvil @ 2008-08-02 17:18 UTC (permalink / raw)
To: Vinay Sridhar; +Cc: gdb-patches
[-- 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;
+}
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] Re: Accessing tls variables across files causes a bug
2008-08-02 17:18 ` [patch] Re: Accessing tls variables across files causes a bug Jan Kratochvil
@ 2008-08-04 19:36 ` Luis Machado
2008-08-05 13:26 ` [patch] Finish removing <bug-gdb@prep.ai.mit.edu> (PR gdb/1543) Jan Kratochvil
2008-08-04 19:40 ` [patch] Re: Accessing tls variables across files causes a bug Ulrich Weigand
2008-08-05 8:39 ` Vinay Sridhar
2 siblings, 1 reply; 13+ messages in thread
From: Luis Machado @ 2008-08-04 19:36 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: Vinay Sridhar, gdb-patches
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] Re: Accessing tls variables across files causes a bug
2008-08-02 17:18 ` [patch] Re: Accessing tls variables across files causes a bug Jan Kratochvil
2008-08-04 19:36 ` Luis Machado
@ 2008-08-04 19:40 ` Ulrich Weigand
2008-08-05 8:39 ` Vinay Sridhar
2 siblings, 0 replies; 13+ messages in thread
From: Ulrich Weigand @ 2008-08-04 19:40 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: Vinay Sridhar, gdb-patches
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] Re: Accessing tls variables across files causes a bug
2008-08-02 17:18 ` [patch] Re: Accessing tls variables across files causes a bug Jan Kratochvil
2008-08-04 19:36 ` Luis Machado
2008-08-04 19:40 ` [patch] Re: Accessing tls variables across files causes a bug Ulrich Weigand
@ 2008-08-05 8:39 ` Vinay Sridhar
2008-08-05 12:22 ` Daniel Jacobowitz
2 siblings, 1 reply; 13+ messages in thread
From: Vinay Sridhar @ 2008-08-05 8:39 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches, luisgpm, uweigand
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] Re: Accessing tls variables across files causes a bug
2008-08-05 8:39 ` Vinay Sridhar
@ 2008-08-05 12:22 ` Daniel Jacobowitz
2008-08-06 11:16 ` Vinay Sridhar
0 siblings, 1 reply; 13+ messages in thread
From: Daniel Jacobowitz @ 2008-08-05 12:22 UTC (permalink / raw)
To: Vinay Sridhar; +Cc: Jan Kratochvil, gdb-patches, luisgpm, uweigand
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [patch] Finish removing <bug-gdb@prep.ai.mit.edu> (PR gdb/1543)
2008-08-04 19:36 ` Luis Machado
@ 2008-08-05 13:26 ` Jan Kratochvil
2008-08-06 12:13 ` Daniel Jacobowitz
0 siblings, 1 reply; 13+ messages in thread
From: Jan Kratochvil @ 2008-08-05 13:26 UTC (permalink / raw)
To: Luis Machado; +Cc: gdb-patches
[-- 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] Re: Accessing tls variables across files causes a bug
2008-08-05 12:22 ` Daniel Jacobowitz
@ 2008-08-06 11:16 ` Vinay Sridhar
2008-08-06 11:25 ` Daniel Jacobowitz
2008-08-06 11:43 ` Jan Kratochvil
0 siblings, 2 replies; 13+ messages in thread
From: Vinay Sridhar @ 2008-08-06 11:16 UTC (permalink / raw)
To: Daniel Jacobowitz; +Cc: Jan Kratochvil, gdb-patches, luisgpm, uweigand
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..
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] Re: Accessing tls variables across files causes a bug
2008-08-06 11:16 ` Vinay Sridhar
@ 2008-08-06 11:25 ` Daniel Jacobowitz
2008-08-06 11:43 ` Jan Kratochvil
1 sibling, 0 replies; 13+ messages in thread
From: Daniel Jacobowitz @ 2008-08-06 11:25 UTC (permalink / raw)
To: Vinay Sridhar; +Cc: Jan Kratochvil, gdb-patches, luisgpm, uweigand
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] Re: Accessing tls variables across files causes a bug
2008-08-06 11:16 ` Vinay Sridhar
2008-08-06 11:25 ` Daniel Jacobowitz
@ 2008-08-06 11:43 ` Jan Kratochvil
2008-08-06 15:20 ` Jan Kratochvil
1 sibling, 1 reply; 13+ messages in thread
From: Jan Kratochvil @ 2008-08-06 11:43 UTC (permalink / raw)
To: Vinay Sridhar; +Cc: Daniel Jacobowitz, gdb-patches, luisgpm, uweigand
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] Finish removing <bug-gdb@prep.ai.mit.edu> (PR gdb/1543)
2008-08-05 13:26 ` [patch] Finish removing <bug-gdb@prep.ai.mit.edu> (PR gdb/1543) Jan Kratochvil
@ 2008-08-06 12:13 ` Daniel Jacobowitz
0 siblings, 0 replies; 13+ messages in thread
From: Daniel Jacobowitz @ 2008-08-06 12:13 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: Luis Machado, gdb-patches
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] Re: Accessing tls variables across files causes a bug
2008-08-06 11:43 ` Jan Kratochvil
@ 2008-08-06 15:20 ` Jan Kratochvil
2008-08-27 4:37 ` Vinay Sridhar
2009-06-12 17:06 ` [resolved] " Jan Kratochvil
0 siblings, 2 replies; 13+ messages in thread
From: Jan Kratochvil @ 2008-08-06 15:20 UTC (permalink / raw)
To: gdb-patches; +Cc: Vinay Sridhar, Daniel Jacobowitz, luisgpm, uweigand
[-- 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;
+}
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] Re: Accessing tls variables across files causes a bug
2008-08-06 15:20 ` Jan Kratochvil
@ 2008-08-27 4:37 ` Vinay Sridhar
2009-06-12 17:06 ` [resolved] " Jan Kratochvil
1 sibling, 0 replies; 13+ messages in thread
From: Vinay Sridhar @ 2008-08-27 4:37 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches, Daniel Jacobowitz, luisgpm, uweigand
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;
> +}
^ permalink raw reply [flat|nested] 13+ messages in thread
* [resolved] Re: [patch] Re: Accessing tls variables across files causes a bug
2008-08-06 15:20 ` Jan Kratochvil
2008-08-27 4:37 ` Vinay Sridhar
@ 2009-06-12 17:06 ` Jan Kratochvil
1 sibling, 0 replies; 13+ messages in thread
From: Jan Kratochvil @ 2009-06-12 17:06 UTC (permalink / raw)
To: gdb-patches
Cc: Vinay Sridhar, Daniel Jacobowitz, luisgpm, uweigand, Tom Tromey
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2009-06-12 17:06 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1217480020.4755.1.camel@vinaysridhar.in.ibm.com>
2008-08-02 17:18 ` [patch] Re: Accessing tls variables across files causes a bug Jan Kratochvil
2008-08-04 19:36 ` Luis Machado
2008-08-05 13:26 ` [patch] Finish removing <bug-gdb@prep.ai.mit.edu> (PR gdb/1543) Jan Kratochvil
2008-08-06 12:13 ` Daniel Jacobowitz
2008-08-04 19:40 ` [patch] Re: Accessing tls variables across files causes a bug Ulrich Weigand
2008-08-05 8:39 ` Vinay Sridhar
2008-08-05 12:22 ` Daniel Jacobowitz
2008-08-06 11:16 ` Vinay Sridhar
2008-08-06 11:25 ` Daniel Jacobowitz
2008-08-06 11:43 ` Jan Kratochvil
2008-08-06 15:20 ` Jan Kratochvil
2008-08-27 4:37 ` Vinay Sridhar
2009-06-12 17:06 ` [resolved] " Jan Kratochvil
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox