This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[patch] Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))
- From: "Ulrich Weigand" <uweigand at de dot ibm dot com>
- To: pedro at codesourcery dot com (Pedro Alves)
- Cc: gdb-patches at sourceware dot org, jan dot kratochvil at redhat dot com, drow at false dot org
- Date: Thu, 15 May 2008 18:50:29 +0200 (CEST)
- Subject: [patch] Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))
Pedro Alves wrote:
> Yes, while still reading the debug info, you can get here with
> a sym->symtab == NULL, but you'll have a valid objfile to pass
> down. The symtab is then set at the end of end_symtab. E.g.:
I see, the var_decode_location case is indeed special.
> (I actually have no idea why that output *symtab arg is needed
> in the lookup_symbol* functions, if a symbol has a symtab
> pointer. Legacy?)
This seems quite broken, in particular because of this code in
lookup_symbol_in_language:
/* Override the returned symtab with the symbol's specific one. */
if (returnval != NULL && symtab != NULL)
*symtab = SYMBOL_SYMTAB (returnval);
All the dozen functions below lookup_symbol_in_language in the
call tree pass around a symtab output pointer and try to fill
in a value -- but everything is completely ignored anyway ...
This is certainly an opportunity for some cleanup.
> > Any reason for not supporting LOC_LABEL or LOC_INDIRECT for psymbols?
> >
> > (Well, except from the fact that apparently none of the symbol readers
> > left in GDB will ever generate LOC_INDIRECT ... But at least mdebugread.c
> > will generate LOC_LABEL psymbols, it seems.)
> >
>
> But that comes from debug info. I didn't think LOC_LABEL's would ever
> be listed in the minsyms. Can they? There's certainly no harm in
> adding it to the switch, though.
I guess labels could still be in the symbol table, depending on how you
link -- in any case, it certainly cannot hurt to add the case to the
switch here (also to be consistent between fixup_psymbol_section and
fixup_symbol_section).
> As for LOC_INDIRECT, I remember finding out no reader records it, and
> meaning to garbage collect it instead, but defered that to
> submission time. :-/ If we want to keep it, it looks like, yes,
> we should be fixing up its section too.
I've left it in for now; this can be cleaned up in another patch.
I've now merged your patch with mine and added the above changes.
Would it be OK with you if I commit that patch?
Tested on spu-elf (fixes all overlays.exp FAILs), powerpc-linux, and
powerpc64-linux with no regressions.
Bye,
Ulrich
gdb/
2008-05-15 Pedro Alves <pedro@codesourcery.com>
Ulrich Weigand <uweigand@de.ibm.com>
* minsyms.c (lookup_minimal_symbol_by_pc_name): New function.
* symtab.h (lookup_minimal_symbol_by_pc_name): Add prototype.
* symtab.c (fixup_section): Remove prototype. Add ADDR parameter;
use it instead of ginfo->value.address. Look up minimal symbol by
address and name. Assume OBJFILE is non-NULL.
(fixup_symbol_section): Ensure we always have an objfile to look
into. Extract and pass to fixup_section the symbol's address that
will match the minimal symbol's address.
(fixup_psymbol_section): Likewise.
(find_pc_sect_psymtab): Fall back to non-addrmap case when debugging
overlays and the addrmap returned the wrong section.
* dwarf2read.c (var_decode_location): Set SYMBOL_CLASS before
calling fixup_symbol_section.
gdb/testsuite/
2008-05-15 Pedro Alves <pedro@codesourcery.com>
* gdb.base/fixsection.exp: New file.
* gdb.base/fixsection0.c: New file.
* gdb.base/fixsection1.c: New file.
diff -urNp gdb-orig/gdb/dwarf2read.c gdb-head/gdb/dwarf2read.c
--- gdb-orig/gdb/dwarf2read.c 2008-05-11 23:41:46.000000000 +0200
+++ gdb-head/gdb/dwarf2read.c 2008-05-15 17:13:27.000000000 +0200
@@ -7323,10 +7323,10 @@ var_decode_location (struct attribute *a
SYMBOL_VALUE_ADDRESS (sym) =
read_address (objfile->obfd, DW_BLOCK (attr)->data + 1, cu, &dummy);
+ SYMBOL_CLASS (sym) = LOC_STATIC;
fixup_symbol_section (sym, objfile);
SYMBOL_VALUE_ADDRESS (sym) += ANOFFSET (objfile->section_offsets,
SYMBOL_SECTION (sym));
- SYMBOL_CLASS (sym) = LOC_STATIC;
return;
}
diff -urNp gdb-orig/gdb/minsyms.c gdb-head/gdb/minsyms.c
--- gdb-orig/gdb/minsyms.c 2008-05-14 20:26:07.000000000 +0200
+++ gdb-head/gdb/minsyms.c 2008-05-15 18:05:10.448995498 +0200
@@ -331,6 +331,41 @@ lookup_minimal_symbol_text (const char *
}
/* Look through all the current minimal symbol tables and find the
+ first minimal symbol that matches NAME and PC. If OBJF is non-NULL,
+ limit the search to that objfile. Returns a pointer to the minimal
+ symbol that matches, or NULL if no match is found. */
+
+struct minimal_symbol *
+lookup_minimal_symbol_by_pc_name (CORE_ADDR pc, const char *name,
+ struct objfile *objf)
+{
+ struct objfile *objfile;
+ struct minimal_symbol *msymbol;
+
+ unsigned int hash = msymbol_hash (name) % MINIMAL_SYMBOL_HASH_SIZE;
+
+ for (objfile = object_files;
+ objfile != NULL;
+ objfile = objfile->next)
+ {
+ if (objf == NULL || objf == objfile
+ || objf->separate_debug_objfile == objfile)
+ {
+ for (msymbol = objfile->msymbol_hash[hash];
+ msymbol != NULL;
+ msymbol = msymbol->hash_next)
+ {
+ if (SYMBOL_VALUE_ADDRESS (msymbol) == pc
+ && strcmp (SYMBOL_LINKAGE_NAME (msymbol), name) == 0)
+ return msymbol;
+ }
+ }
+ }
+
+ return NULL;
+}
+
+/* Look through all the current minimal symbol tables and find the
first minimal symbol that matches NAME and is a solib trampoline.
If OBJF is non-NULL, limit the search to that objfile. Returns a
pointer to the minimal symbol that matches, or NULL if no match is
diff -urNp gdb-orig/gdb/symtab.c gdb-head/gdb/symtab.c
--- gdb-orig/gdb/symtab.c 2008-05-14 15:57:46.000000000 +0200
+++ gdb-head/gdb/symtab.c 2008-05-15 18:02:17.398508575 +0200
@@ -110,8 +110,6 @@ struct symbol *lookup_symbol_aux_psymtab
const domain_enum domain,
struct symtab **symtab);
-static void fixup_section (struct general_symbol_info *, struct objfile *);
-
static int file_matches (char *, char **, int);
static void print_symbol_info (domain_enum,
@@ -878,6 +876,23 @@ find_pc_sect_psymtab (CORE_ADDR pc, asec
pst = addrmap_find (objfile->psymtabs_addrmap, pc);
if (pst != NULL)
{
+ /* FIXME: addrmaps currently do not handle overlayed sections,
+ so fall back to the non-addrmap case if we're debugging
+ overlays and the addrmap returned the wrong section. */
+ if (overlay_debugging && msymbol && section)
+ {
+ struct partial_symbol *p;
+ /* NOTE: This assumes that every psymbol has a
+ corresponding msymbol, which is not necessarily
+ true; the debug info might be much richer than the
+ object's symbol table. */
+ p = find_pc_sect_psymbol (pst, pc, section);
+ if (!p
+ || SYMBOL_VALUE_ADDRESS (p)
+ != SYMBOL_VALUE_ADDRESS (msymbol))
+ continue;
+ }
+
/* We do not try to call FIND_PC_SECT_PSYMTAB_CLOSER as
PSYMTABS_ADDRMAP we used has already the best 1-byte
granularity and FIND_PC_SECT_PSYMTAB_CLOSER may mislead us into
@@ -1010,23 +1025,23 @@ find_pc_psymbol (struct partial_symtab *
out of the minimal symbols and stash that in the debug symbol. */
static void
-fixup_section (struct general_symbol_info *ginfo, struct objfile *objfile)
+fixup_section (struct general_symbol_info *ginfo,
+ CORE_ADDR addr, struct objfile *objfile)
{
struct minimal_symbol *msym;
- msym = lookup_minimal_symbol (ginfo->name, NULL, objfile);
/* First, check whether a minimal symbol with the same name exists
and points to the same address. The address check is required
e.g. on PowerPC64, where the minimal symbol for a function will
point to the function descriptor, while the debug symbol will
point to the actual function code. */
- if (msym
- && SYMBOL_VALUE_ADDRESS (msym) == ginfo->value.address)
+ msym = lookup_minimal_symbol_by_pc_name (addr, ginfo->name, objfile);
+ if (msym)
{
ginfo->bfd_section = SYMBOL_BFD_SECTION (msym);
ginfo->section = SYMBOL_SECTION (msym);
}
- else if (objfile)
+ else
{
/* Static, function-local variables do appear in the linker
(minimal) symbols, but are frequently given names that won't
@@ -1064,11 +1079,7 @@ fixup_section (struct general_symbol_inf
this reason, we still attempt a lookup by name prior to doing
a search of the section table. */
- CORE_ADDR addr;
struct obj_section *s;
-
- addr = ginfo->value.address;
-
ALL_OBJFILE_OSECTIONS (objfile, s)
{
int idx = s->the_bfd_section->index;
@@ -1087,13 +1098,42 @@ fixup_section (struct general_symbol_inf
struct symbol *
fixup_symbol_section (struct symbol *sym, struct objfile *objfile)
{
+ CORE_ADDR addr;
+
if (!sym)
return NULL;
if (SYMBOL_BFD_SECTION (sym))
return sym;
- fixup_section (&sym->ginfo, objfile);
+ /* We either have an OBJFILE, or we can get at it from the sym's
+ symtab. Anything else is a bug. */
+ gdb_assert (objfile || SYMBOL_SYMTAB (sym));
+
+ if (objfile == NULL)
+ objfile = SYMBOL_SYMTAB (sym)->objfile;
+
+ /* We should have an objfile by now. */
+ gdb_assert (objfile);
+
+ switch (SYMBOL_CLASS (sym))
+ {
+ case LOC_STATIC:
+ case LOC_LABEL:
+ case LOC_INDIRECT:
+ addr = SYMBOL_VALUE_ADDRESS (sym);
+ break;
+ case LOC_BLOCK:
+ addr = BLOCK_START (SYMBOL_BLOCK_VALUE (sym));
+ break;
+
+ default:
+ /* Nothing else will be listed in the minsyms -- no use looking
+ it up. */
+ return sym;
+ }
+
+ fixup_section (&sym->ginfo, addr, objfile);
return sym;
}
@@ -1101,13 +1141,31 @@ fixup_symbol_section (struct symbol *sym
struct partial_symbol *
fixup_psymbol_section (struct partial_symbol *psym, struct objfile *objfile)
{
+ CORE_ADDR addr;
+
if (!psym)
return NULL;
if (SYMBOL_BFD_SECTION (psym))
return psym;
- fixup_section (&psym->ginfo, objfile);
+ gdb_assert (objfile);
+
+ switch (SYMBOL_CLASS (psym))
+ {
+ case LOC_STATIC:
+ case LOC_LABEL:
+ case LOC_INDIRECT:
+ case LOC_BLOCK:
+ addr = SYMBOL_VALUE_ADDRESS (psym);
+ break;
+ default:
+ /* Nothing else will be listed in the minsyms -- no use looking
+ it up. */
+ return psym;
+ }
+
+ fixup_section (&psym->ginfo, addr, objfile);
return psym;
}
diff -urNp gdb-orig/gdb/symtab.h gdb-head/gdb/symtab.h
--- gdb-orig/gdb/symtab.h 2008-05-11 23:41:56.000000000 +0200
+++ gdb-head/gdb/symtab.h 2008-05-15 17:52:33.454647964 +0200
@@ -1183,6 +1183,9 @@ struct minimal_symbol *lookup_minimal_sy
struct objfile
*);
+extern struct minimal_symbol *lookup_minimal_symbol_by_pc_name
+ (CORE_ADDR, const char *, struct objfile *);
+
extern struct minimal_symbol *lookup_minimal_symbol_by_pc (CORE_ADDR);
extern struct minimal_symbol *lookup_minimal_symbol_by_pc_section (CORE_ADDR,
diff -urNp gdb-orig/gdb/testsuite/gdb.base/fixsection.c gdb-head/gdb/testsuite/gdb.base/fixsection.c
--- gdb-orig/gdb/testsuite/gdb.base/fixsection.c 1970-01-01 01:00:00.000000000 +0100
+++ gdb-head/gdb/testsuite/gdb.base/fixsection.c 2008-05-15 17:13:27.000000000 +0200
@@ -0,0 +1,35 @@
+/* Copyright (C) 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/>.
+
+ This file was written by Pedro Alves (pedro@codesourcery.com). */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+extern FILE *force_static_fun (void);
+
+static void *
+static_fun (void *arg)
+{
+ return NULL;
+}
+
+int
+main (int argc, char **argv)
+{
+ static_fun (NULL);
+ force_static_fun ();
+ return 0;
+}
diff -urNp gdb-orig/gdb/testsuite/gdb.base/fixsection.exp gdb-head/gdb/testsuite/gdb.base/fixsection.exp
--- gdb-orig/gdb/testsuite/gdb.base/fixsection.exp 1970-01-01 01:00:00.000000000 +0100
+++ gdb-head/gdb/testsuite/gdb.base/fixsection.exp 2008-05-15 17:13:27.000000000 +0200
@@ -0,0 +1,71 @@
+# Copyright (C) 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/>.
+
+if $tracelevel then {
+ strace $tracelevel
+}
+
+set prms_id 0
+set bug_id 0
+
+if {[skip_shlib_tests]} {
+ return 0
+}
+
+set testfile "fixsection"
+set srcfile ${srcdir}/${subdir}/${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+
+set libfile "fixsectshr"
+set libsrc ${srcdir}/${subdir}/${libfile}.c
+set lib_sl ${objdir}/${subdir}/${libfile}.sl
+
+set lib_opts [list debug nowarnings]
+set exec_opts [list debug nowarnings shlib=$lib_sl]
+
+if [get_compiler_info ${binfile}] {
+ return -1
+}
+
+if { [gdb_compile_shlib $libsrc $lib_sl $lib_opts] != ""
+ || [gdb_compile $srcfile $binfile executable $exec_opts] != ""} {
+ untested "Could not compile either $libsrc or $srcfile."
+ return -1
+}
+
+# Start with a fresh gdb.
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+gdb_load_shlibs ${lib_sl}
+
+if ![runto_main] then {
+ fail "Can't run to main"
+ return 1;
+}
+
+#
+# set breakpoint at static function static_fun
+#
+gdb_test "break static_fun" \
+ "Breakpoint.*at.* file .*${srcfile}, line.*" \
+ "breakpoint at static_fun"
+
+#
+# exit gdb
+#
+gdb_exit
diff -urNp gdb-orig/gdb/testsuite/gdb.base/fixsectshr.c gdb-head/gdb/testsuite/gdb.base/fixsectshr.c
--- gdb-orig/gdb/testsuite/gdb.base/fixsectshr.c 1970-01-01 01:00:00.000000000 +0100
+++ gdb-head/gdb/testsuite/gdb.base/fixsectshr.c 2008-05-15 17:13:27.000000000 +0200
@@ -0,0 +1,10 @@
+#include <stdio.h>
+#include <stdlib.h>
+
+static FILE *static_fun = NULL;
+
+FILE *
+force_static_fun (void)
+{
+ return static_fun;
+}
--
Dr. Ulrich Weigand
GNU Toolchain for Linux on System z and Cell BE
Ulrich.Weigand@de.ibm.com