[patch] Re: Accessing tls variables across files causes a bug
Jan Kratochvil
jan.kratochvil@redhat.com
Wed Aug 6 15:20:00 GMT 2008
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
-------------- next part --------------
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;
+}
More information about the Gdb-patches
mailing list