Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))

Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))

[Ulrich Weigand]

Pedro Alves wrote:

> Please also make sure that you're setting SYMBOL_CLASS
> before calling fixup_*symbol_section everywhere.  This will break
> dwarf2read.c, for example:

Ah, thanks for pointing that out!

> +  struct minimal_symbol *msym = NULL;
> +  if (addr != ~(CORE_ADDR) 0)
> +    /* If we have an address to lookup, use it.  */
> +    msym = lookup_minimal_symbol_by_pc (addr);
> +
> +  if (!msym
> +      || addr != SYMBOL_VALUE_ADDRESS (msym)
> +      || strcmp (DEPRECATED_SYMBOL_NAME (msym), ginfo->name) != 0)
> +    /* Try by looking up by name.  Not perfect, since it can match the
> +       wrong symbol.  */
> +    msym = lookup_minimal_symbol (ginfo->name, NULL, objfile);

Hmm, I guess there is the possibility that even though there is
a msymbol at ADDR with name NAME, both 
  lookup_minimal_symbol_by_pc (ADDR)
and
  lookup_minimal_symbol (NAME)

might fail to find it ...   E.g. if there are minimal symbols
  ADDR' NAME
  ADDR  NAME'
  ADDR  NAME
lookup by pc might find NAME', but lookup by name might find ADDR'.

Maybe we need a lookup_minimal_symbol_by_pc_name or so?

> +  /* We either have an OBJFILE, or we can get at it from the sym's
> +     symtab.  Anything else is a bug.  */
> +  gdb_assert (objfile || (sym->symtab && sym->symtab->objfile));
> +
> +  if (objfile == NULL)
> +    objfile = sym->symtab->objfile;

Huh.  If that's true, why does fixup_symbol_section even have an
OBJFILE argument?  Is there ever a situation where we cannot use
sym->symtab->objfile?

> +  switch (SYMBOL_CLASS (sym))
> +    {
> +    case LOC_UNRESOLVED:
> +      addr = ~(CORE_ADDR) 0;
> +      break;

Why do we need to fixup the section for an LOC_UNRESOLVED symbol?

I understand that every time we want to use the address of a
LOC_UNRESOLVED, the user needs to look up the msymbol anyway.
They should then use the section from the msymbol too, right?


> +  switch (SYMBOL_CLASS (psym))
> +    {
> +    case LOC_STATIC:
> +    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;
> +    }

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

Bye,
Ulrich

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

Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))

[Pedro Alves]

A Tuesday 13 May 2008 22:12:20, Ulrich Weigand wrote:
> Pedro Alves wrote:
> > +  struct minimal_symbol *msym = NULL;
> > +  if (addr != ~(CORE_ADDR) 0)
> > +    /* If we have an address to lookup, use it.  */
> > +    msym = lookup_minimal_symbol_by_pc (addr);
> > +
> > +  if (!msym
> > +      || addr != SYMBOL_VALUE_ADDRESS (msym)
> > +      || strcmp (DEPRECATED_SYMBOL_NAME (msym), ginfo->name) != 0)
> > +    /* Try by looking up by name.  Not perfect, since it can match the
> > +       wrong symbol.  */
> > +    msym = lookup_minimal_symbol (ginfo->name, NULL, objfile);
>
> Hmm, I guess there is the possibility that even though there is
> a msymbol at ADDR with name NAME, both
>   lookup_minimal_symbol_by_pc (ADDR)
> and
>   lookup_minimal_symbol (NAME)
>
> might fail to find it ...   E.g. if there are minimal symbols
>   ADDR' NAME
>   ADDR  NAME'
>   ADDR  NAME
> lookup by pc might find NAME', but lookup by name might find ADDR'.
>
> Maybe we need a lookup_minimal_symbol_by_pc_name or so?
>

Ah, good point.  Looks like that would be the best practical
approach, yes.

> > +  /* We either have an OBJFILE, or we can get at it from the sym's
> > +     symtab.  Anything else is a bug.  */
> > +  gdb_assert (objfile || (sym->symtab && sym->symtab->objfile));
> > +
> > +  if (objfile == NULL)
> > +    objfile = sym->symtab->objfile;
>
> Huh.  If that's true, why does fixup_symbol_section even have an
> OBJFILE argument?  Is there ever a situation where we cannot use
> sym->symtab->objfile?
>

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

(gdb) p sym->symtab
$2 = (struct symtab *) 0x0

(gdb) bt
#0  fixup_symbol_section (sym=0xb583e0, objfile=0xb336d0) 
at ../../src/gdb/symtab.c:1090
#1  0x0000000000552a1a in var_decode_location (attr=0xb51380, sym=0xb583e0, 
cu=0xb4c7a0)
    at ../../src/gdb/dwarf2read.c:7326
#2  0x00000000005530ce in new_symbol (die=0xb512f0, type=0x0, cu=0xb4c7a0) 
at ../../src/gdb/dwarf2read.c:7462
#3  0x00000000005491a7 in process_die (die=0xb512f0, cu=0xb4c7a0) 
at ../../src/gdb/dwarf2read.c:2777
#4  0x00000000005495a9 in read_file_scope (die=0xb4a630, cu=0xb4c7a0) 
at ../../src/gdb/dwarf2read.c:2895
#5  0x00000000005490c4 in process_die (die=0xb4a630, cu=0xb4c7a0) 
at ../../src/gdb/dwarf2read.c:2713
#6  0x0000000000548fcd in process_full_comp_unit (per_cu=0xb418d0) 
at ../../src/gdb/dwarf2read.c:2680
#7  0x0000000000548a3a in process_queue (objfile=0xb336d0) 
at ../../src/gdb/dwarf2read.c:2476
#8  0x0000000000548c56 in psymtab_to_symtab_1 (pst=0xb41cd0) 
at ../../src/gdb/dwarf2read.c:2556
...

That was reading the the symbol "main" while stopping at "main".

You can also get here with a NULL objfile, for example, by 
means of lookup_symbol_aux_block, when you pass
a NULL symtab output parameter around, fixup_symbol_section
is always called with a NULL objfile, but it's OK, since at
the time lookup_symbol* are called, the symbols should already
have a sym->symtab and a sym->symtab->objfile.

(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?)

> > +  switch (SYMBOL_CLASS (sym))
> > +    {
> > +    case LOC_UNRESOLVED:
> > +      addr = ~(CORE_ADDR) 0;
> > +      break;
>
> Why do we need to fixup the section for an LOC_UNRESOLVED symbol?
>
> I understand that every time we want to use the address of a
> LOC_UNRESOLVED, the user needs to look up the msymbol anyway.
> They should then use the section from the msymbol too, right?
>

Ah, you're right, from read_var_value:

    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;


> > +  switch (SYMBOL_CLASS (psym))
> > +    {
> > +    case LOC_STATIC:
> > +    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;
> > +    }
>
> 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.
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.

-- 
Pedro Alves

[patch] Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))

[Ulrich Weigand]

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

Re: [patch] Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))

[Pedro Alves]

A Thursday 15 May 2008 17:50:29, Ulrich Weigand wrote:

> I've now merged your patch with mine and added the above changes.
> Would it be OK with you if I commit that patch?

Certainly OK with me.  Thanks a lot!

-- 
Pedro Alves

Re: [patch] Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))

[Ulrich Weigand]

Pedro Alves wrote:
> A Thursday 15 May 2008 17:50:29, Ulrich Weigand wrote:
> 
> > I've now merged your patch with mine and added the above changes.
> > Would it be OK with you if I commit that patch?
> 
> Certainly OK with me.  Thanks a lot!

OK, I've checked this in now.

Thanks,
Ulrich

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

[patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap)

[Jan Kratochvil]

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

Hi,

fix + a testcase for DWARF/non-DWARF debuginfos with continuous sections for
childless CUs.

Going to post the results of some more verifications, posting as Doug Evans has
asked.

It is only a minor update against its previous version:
	http://sourceware.org/ml/gdb-patches/2007-12/msg00143.html


Regards,
Jan

[-- Attachment #2: gdb-cvs-psymtab-ranges-stabs.patch --]
[-- Type: text/plain, Size: 22662 bytes --]

2008-04-23  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* Makefile.in: Update dependencies.
	* dwarf2read.c: Include "addrmap.h"
	(struct dwarf2_cu): New fields RANGES_OFFSET and HAS_RANGES_OFFSET.
	(dwarf2_ranges_read): New prototype.
	(dwarf2_build_psymtabs_hard): Initialize and prepare PSYMTABS_ADDRMAP.
	Add discontiguous range to PSYMTABS_ADDRMAP by DWARF2_RANGES_READ on
	HAS_RANGES_OFFSET, otherwise add there the contiguous range.
	(dwarf2_ranges_read): New parameter RANGES_PST, update the function
	comment for it.  Add the found ranges to RANGES_PST.  New variable
	BASEADDR, initialize it the common way.
	(dwarf2_get_pc_bounds): Update the caller for the new parameter.
	(read_partial_die): `DW_AT_ranges' now only sets RANGES_OFFSET and
	HAS_RANGES_OFFSET for the later processing.
	* objfiles.h (struct objfile): New field PSYMTABS_ADDRMAP.
	* symtab.c: Include "addrmap.h"
	(find_pc_sect_psymtab): Support reading the field PSYMTABS_ADDRMAP.
	Move the psymtab locator into ...
	(find_pc_sect_psymtab_closer): ... a new function.

2008-04-23  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.dwarf2/dw2-ranges.S: Merge the secondary section with `.fini'.
	* gdb.dwarf2/dw2-ranges.exp: Compile also `dw2-ranges2.S' and
	`dw2-ranges3.S' and test also their MAIN2, FUNC2 and MAIN3 symbols.
	* gdb.dwarf2/dw2-ranges2.S, gdb.dwarf2/dw2-ranges3.S: New files.

--- ./gdb/Makefile.in	2008-04-20 16:37:51.000000000 +0200
+++ ./gdb/Makefile.in	2008-04-23 10:28:05.000000000 +0200
@@ -2085,7 +2085,8 @@ dwarf2-frame.o: dwarf2-frame.c $(defs_h)
 dwarf2loc.o: dwarf2loc.c $(defs_h) $(ui_out_h) $(value_h) $(frame_h) \
 	$(gdbcore_h) $(target_h) $(inferior_h) $(ax_h) $(ax_gdb_h) \
 	$(regcache_h) $(objfiles_h) $(exceptions_h) $(elf_dwarf2_h) \
-	$(dwarf2expr_h) $(dwarf2loc_h) $(gdb_string_h) $(gdb_assert_h)
+	$(dwarf2expr_h) $(dwarf2loc_h) $(gdb_string_h) $(gdb_assert_h) \
+	$(addrmap_h)
 dwarf2read.o: dwarf2read.c $(defs_h) $(bfd_h) $(symtab_h) $(gdbtypes_h) \
 	$(objfiles_h) $(elf_dwarf2_h) $(buildsym_h) $(demangle_h) \
 	$(expression_h) $(filenames_h) $(macrotab_h) $(language_h) \
@@ -2893,7 +2894,7 @@ symtab.o: symtab.c $(defs_h) $(symtab_h)
 	$(filenames_h) $(objc_lang_h) $(ada_lang_h) $(hashtab_h) \
 	$(gdb_obstack_h) $(block_h) $(dictionary_h) $(gdb_string_h) \
 	$(gdb_stat_h) $(cp_abi_h) $(observer_h) $(gdb_assert_h) \
-	$(solist_h) $(p_lang_h)
+	$(solist_h) $(p_lang_h) $(addrmap_h)
 target.o: target.c $(defs_h) $(gdb_string_h) $(target_h) $(gdbcmd_h) \
 	$(symtab_h) $(inferior_h) $(bfd_h) $(symfile_h) $(objfiles_h) \
 	$(gdb_wait_h) $(dcache_h) $(regcache_h) $(gdb_assert_h) $(gdbcore_h) \
--- ./gdb/dwarf2read.c	2008-04-23 00:16:47.000000000 +0200
+++ ./gdb/dwarf2read.c	2008-04-23 11:47:02.000000000 +0200
@@ -45,6 +45,7 @@
 #include "hashtab.h"
 #include "command.h"
 #include "gdbcmd.h"
+#include "addrmap.h"
 
 #include <fcntl.h>
 #include "gdb_string.h"
@@ -303,6 +304,9 @@ struct dwarf2_cu
   /* Hash table holding all the loaded partial DIEs.  */
   htab_t partial_dies;
 
+  /* `.debug_ranges' offset for this `DW_TAG_compile_unit' DIE.  */
+  unsigned long ranges_offset;
+
   /* Storage for things with the same lifetime as this read-in compilation
      unit, including partial DIEs.  */
   struct obstack comp_unit_obstack;
@@ -345,6 +349,9 @@ struct dwarf2_cu
      DIEs for namespaces, we don't need to try to infer them
      from mangled names.  */
   unsigned int has_namespace_info : 1;
+
+  /* Field `ranges_offset' is filled in; flag as the value may be zero.  */
+  unsigned int has_ranges_offset : 1;
 };
 
 /* Persistent data held for a compilation unit, even when not
@@ -894,6 +901,9 @@ static void read_func_scope (struct die_
 
 static void read_lexical_block_scope (struct die_info *, struct dwarf2_cu *);
 
+static int dwarf2_ranges_read (unsigned, CORE_ADDR *, CORE_ADDR *,
+			       struct dwarf2_cu *, struct partial_symtab *);
+
 static int dwarf2_get_pc_bounds (struct die_info *,
 				 CORE_ADDR *, CORE_ADDR *, struct dwarf2_cu *);
 
@@ -1472,6 +1482,9 @@ dwarf2_build_psymtabs_hard (struct objfi
 
   create_all_comp_units (objfile);
 
+  objfile->psymtabs_addrmap = addrmap_create_mutable
+						    (&objfile->objfile_obstack);
+
   /* Since the objects we're extracting from .debug_info vary in
      length, only the individual functions to extract them (like
      read_comp_unit_head and load_partial_die) can really know whether
@@ -1537,7 +1550,8 @@ dwarf2_build_psymtabs_hard (struct objfi
       /* Allocate a new partial symbol table structure */
       pst = start_psymtab_common (objfile, objfile->section_offsets,
 				  comp_unit_die.name ? comp_unit_die.name : "",
-				  comp_unit_die.lowpc,
+				  /* TEXTLOW and TEXTHIGH are set below.  */
+				  0,
 				  objfile->global_psymbols.next,
 				  objfile->static_psymbols.next);
 
@@ -1570,6 +1584,15 @@ dwarf2_build_psymtabs_hard (struct objfi
 
       this_cu->psymtab = pst;
 
+      /* Possibly set the default values of LOWPC and HIGHPC from
+         `DW_AT_ranges'.  */
+      if (cu.has_ranges_offset)
+	{
+	  if (dwarf2_ranges_read (cu.ranges_offset, &comp_unit_die.lowpc,
+				  &comp_unit_die.highpc, &cu, pst))
+	    comp_unit_die.has_pc_info = 1;
+	}
+
       /* Check if comp unit has_children.
          If so, read the rest of the partial symbols from this comp unit.
          If not, there's no more debug_info for this comp unit. */
@@ -1600,6 +1623,12 @@ dwarf2_build_psymtabs_hard (struct objfi
       pst->textlow = comp_unit_die.lowpc + baseaddr;
       pst->texthigh = comp_unit_die.highpc + baseaddr;
 
+      /* Store the contiguous range; `DW_AT_ranges' range is stored above.  The
+         range can be also empty for CUs with no code.  */
+      if (!cu.has_ranges_offset && pst->textlow < pst->texthigh)
+	addrmap_set_empty (objfile->psymtabs_addrmap, pst->textlow,
+			   pst->texthigh - 1, pst);
+
       pst->n_global_syms = objfile->global_psymbols.next -
 	(objfile->global_psymbols.list + pst->globals_offset);
       pst->n_static_syms = objfile->static_psymbols.next -
@@ -1623,6 +1652,10 @@ dwarf2_build_psymtabs_hard (struct objfi
 
       do_cleanups (back_to_inner);
     }
+
+  objfile->psymtabs_addrmap = addrmap_create_fixed (objfile->psymtabs_addrmap,
+						    &objfile->objfile_obstack);
+
   do_cleanups (back_to);
 }
 
@@ -3143,11 +3176,13 @@ read_lexical_block_scope (struct die_inf
 }
 
 /* Get low and high pc attributes from DW_AT_ranges attribute value OFFSET.
-   Return 1 if the attributes are present and valid, otherwise, return 0.  */
+   Return 1 if the attributes are present and valid, otherwise, return 0.
+   If RANGES_PST is not NULL we should setup `objfile->psymtabs_addrmap'.  */
 
 static int
 dwarf2_ranges_read (unsigned offset, CORE_ADDR *low_return,
-		    CORE_ADDR *high_return, struct dwarf2_cu *cu)
+		    CORE_ADDR *high_return, struct dwarf2_cu *cu,
+		    struct partial_symtab *ranges_pst)
 {
   struct objfile *objfile = cu->objfile;
   struct comp_unit_head *cu_header = &cu->header;
@@ -3163,6 +3198,7 @@ dwarf2_ranges_read (unsigned offset, COR
   int low_set;
   CORE_ADDR low = 0;
   CORE_ADDR high = 0;
+  CORE_ADDR baseaddr;
 
   found_base = cu_header->base_known;
   base = cu_header->base_address;
@@ -3190,6 +3226,9 @@ dwarf2_ranges_read (unsigned offset, COR
 
   low_set = 0;
 
+  if (ranges_pst != NULL)
+    baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
+
   while (1)
     {
       CORE_ADDR range_beginning, range_end;
@@ -3229,6 +3268,11 @@ dwarf2_ranges_read (unsigned offset, COR
       range_beginning += base;
       range_end += base;
 
+      if (ranges_pst != NULL && range_beginning < range_end)
+	addrmap_set_empty (objfile->psymtabs_addrmap,
+			   range_beginning + baseaddr, range_end - 1 + baseaddr,
+			   ranges_pst);
+
       /* FIXME: This is recording everything as a low-high
 	 segment of consecutive addresses.  We should have a
 	 data structure for discontiguous block ranges
@@ -3293,7 +3337,7 @@ dwarf2_get_pc_bounds (struct die_info *d
 	{
 	  /* Value of the DW_AT_ranges attribute is the offset in the
 	     .debug_ranges section.  */
-	  if (!dwarf2_ranges_read (DW_UNSND (attr), &low, &high, cu))
+	  if (!dwarf2_ranges_read (DW_UNSND (attr), &low, &high, cu, NULL))
 	    return 0;
 	  /* Found discontinuous range of addresses.  */
 	  ret = -1;
@@ -5857,9 +5901,11 @@ read_partial_die (struct partial_die_inf
 	  part_die->highpc = DW_ADDR (&attr);
 	  break;
 	case DW_AT_ranges:
-	  if (dwarf2_ranges_read (DW_UNSND (&attr), &part_die->lowpc,
-				  &part_die->highpc, cu))
-	    has_low_pc_attr = has_high_pc_attr = 1;
+	  if (part_die->tag == DW_TAG_compile_unit)
+	    {
+	      cu->ranges_offset = DW_UNSND (&attr);
+	      cu->has_ranges_offset = 1;
+	    }
 	  break;
 	case DW_AT_location:
           /* Support the .debug_loc offsets */
--- ./gdb/objfiles.h	2008-04-20 16:37:53.000000000 +0200
+++ ./gdb/objfiles.h	2008-04-23 10:28:05.000000000 +0200
@@ -220,6 +220,13 @@ struct objfile
 
     struct partial_symtab *psymtabs;
 
+    /* Map addresses to the entries of PSYMTABS.  It would be more efficient to
+       have a map per the whole process but ADDRMAP cannot selectively remove
+       its items during FREE_OBJFILE.  This mapping is already present even for
+       PARTIAL_SYMTABs which still have no corresponding full SYMTABs read.  */
+
+    struct addrmap *psymtabs_addrmap;
+
     /* List of freed partial symtabs, available for re-use */
 
     struct partial_symtab *free_psymtabs;
--- ./gdb/symtab.c	2008-04-20 16:37:53.000000000 +0200
+++ ./gdb/symtab.c	2008-04-23 10:28:05.000000000 +0200
@@ -41,6 +41,7 @@
 #include "objc-lang.h"
 #include "ada-lang.h"
 #include "p-lang.h"
+#include "addrmap.h"
 
 #include "hashtab.h"
 
@@ -801,6 +802,83 @@ matching_bfd_sections (asection *first, 
   return 0;
 }
 
+/* Find which partial symtab contains PC and SECTION starting at psymtab PST.
+   We may find a different psymtab than PST.  See FIND_PC_SECT_PSYMTAB.  */
+
+struct partial_symtab *
+find_pc_sect_psymtab_closer (CORE_ADDR pc, asection *section,
+			     struct partial_symtab *pst,
+			     struct minimal_symbol *msymbol)
+{
+  struct objfile *objfile = pst->objfile;
+  struct partial_symtab *tpst;
+  struct partial_symtab *best_pst = pst;
+  CORE_ADDR best_addr = pst->textlow;
+
+  /* An objfile that has its functions reordered might have
+     many partial symbol tables containing the PC, but
+     we want the partial symbol table that contains the
+     function containing the PC.  */
+  if (!(objfile->flags & OBJF_REORDERED) &&
+      section == 0)	/* can't validate section this way */
+    return pst;
+
+  if (msymbol == NULL)
+    return (pst);
+
+  /* The code range of partial symtabs sometimes overlap, so, in
+     the loop below, we need to check all partial symtabs and
+     find the one that fits better for the given PC address. We
+     select the partial symtab that contains a symbol whose
+     address is closest to the PC address.  By closest we mean
+     that find_pc_sect_symbol returns the symbol with address
+     that is closest and still less than the given PC.  */
+  for (tpst = pst; tpst != NULL; tpst = tpst->next)
+    {
+      if (pc >= tpst->textlow && pc < tpst->texthigh)
+	{
+	  struct partial_symbol *p;
+	  CORE_ADDR this_addr;
+
+	  /* 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 (tpst, pc, section);
+	  if (p != NULL
+	      && SYMBOL_VALUE_ADDRESS (p)
+	      == SYMBOL_VALUE_ADDRESS (msymbol))
+	    return tpst;
+
+	  /* Also accept the textlow value of a psymtab as a
+	     "symbol", to provide some support for partial
+	     symbol tables with line information but no debug
+	     symbols (e.g. those produced by an assembler).  */
+	  if (p != NULL)
+	    this_addr = SYMBOL_VALUE_ADDRESS (p);
+	  else
+	    this_addr = tpst->textlow;
+
+	  /* Check whether it is closer than our current
+	     BEST_ADDR.  Since this symbol address is
+	     necessarily lower or equal to PC, the symbol closer
+	     to PC is the symbol which address is the highest.
+	     This way we return the psymtab which contains such
+	     best match symbol. This can help in cases where the
+	     symbol information/debuginfo is not complete, like
+	     for instance on IRIX6 with gcc, where no debug info
+	     is emitted for statics. (See also the nodebug.exp
+	     testcase.) */
+	  if (this_addr > best_addr)
+	    {
+	      best_addr = this_addr;
+	      best_pst = tpst;
+	    }
+	}
+    }
+  return best_pst;
+}
+
 /* Find which partial symtab contains PC and SECTION.  Return 0 if
    none.  We return the psymtab that contains a symbol whose address
    exactly matches PC, or, if we cannot find an exact match, the
@@ -808,7 +886,6 @@ matching_bfd_sections (asection *first, 
 struct partial_symtab *
 find_pc_sect_psymtab (CORE_ADDR pc, asection *section)
 {
-  struct partial_symtab *pst;
   struct objfile *objfile;
   struct minimal_symbol *msymbol;
 
@@ -824,79 +901,53 @@ find_pc_sect_psymtab (CORE_ADDR pc, asec
 	  || msymbol->type == mst_file_bss))
     return NULL;
 
-  ALL_PSYMTABS (objfile, pst)
-  {
-    if (pc >= pst->textlow && pc < pst->texthigh)
+  /* Try just the PSYMTABS_ADDRMAP mapping first as it has better granularity
+     than the later used TEXTLOW/TEXTHIGH one.  */
+
+  ALL_OBJFILES (objfile)
+    if (objfile->psymtabs_addrmap != NULL)
       {
-	struct partial_symtab *tpst;
-	struct partial_symtab *best_pst = pst;
-	CORE_ADDR best_addr = pst->textlow;
-
-	/* An objfile that has its functions reordered might have
-	   many partial symbol tables containing the PC, but
-	   we want the partial symbol table that contains the
-	   function containing the PC.  */
-	if (!(objfile->flags & OBJF_REORDERED) &&
-	    section == 0)	/* can't validate section this way */
-	  return (pst);
-
-	if (msymbol == NULL)
-	  return (pst);
-
-	/* The code range of partial symtabs sometimes overlap, so, in
-	   the loop below, we need to check all partial symtabs and
-	   find the one that fits better for the given PC address. We
-	   select the partial symtab that contains a symbol whose
-	   address is closest to the PC address.  By closest we mean
-	   that find_pc_sect_symbol returns the symbol with address
-	   that is closest and still less than the given PC.  */
-	for (tpst = pst; tpst != NULL; tpst = tpst->next)
+	struct partial_symtab *pst;
+
+	pst = addrmap_find (objfile->psymtabs_addrmap, pc);
+	if (pst != NULL)
 	  {
-	    if (pc >= tpst->textlow && pc < tpst->texthigh)
-	      {
-		struct partial_symbol *p;
-		CORE_ADDR this_addr;
+	    /* 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
+	       a worse chosen section due to the TEXTLOW/TEXTHIGH ranges
+	       overlap.  */
 
-		/* 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 (tpst, pc, section);
-		if (p != NULL
-		    && SYMBOL_VALUE_ADDRESS (p)
-		    == SYMBOL_VALUE_ADDRESS (msymbol))
-		  return (tpst);
-
-		/* Also accept the textlow value of a psymtab as a
-		   "symbol", to provide some support for partial
-		   symbol tables with line information but no debug
-		   symbols (e.g. those produced by an assembler).  */
-		if (p != NULL)
-		  this_addr = SYMBOL_VALUE_ADDRESS (p);
-		else
-		  this_addr = tpst->textlow;
-
-		/* Check whether it is closer than our current
-		   BEST_ADDR.  Since this symbol address is
-		   necessarily lower or equal to PC, the symbol closer
-		   to PC is the symbol which address is the highest.
-		   This way we return the psymtab which contains such
-		   best match symbol. This can help in cases where the
-		   symbol information/debuginfo is not complete, like
-		   for instance on IRIX6 with gcc, where no debug info
-		   is emitted for statics. (See also the nodebug.exp
-		   testcase.) */
-		if (this_addr > best_addr)
-		  {
-		    best_addr = this_addr;
-		    best_pst = tpst;
-		  }
-	      }
+	    return pst;
 	  }
-	return (best_pst);
       }
-  }
-  return (NULL);
+
+  /* Existing PSYMTABS_ADDRMAP mapping is present even for PARTIAL_SYMTABs
+     which still have no corresponding full SYMTABs read.  But it is not
+     present for non-DWARF2 debug infos not supporting PSYMTABS_ADDRMAP in GDB
+     so far.  */
+
+  ALL_OBJFILES (objfile)
+    {
+      struct partial_symtab *pst;
+
+      /* Check even OBJFILE with non-zero PSYMTABS_ADDRMAP as only several of
+	 its CUs may be missing in PSYMTABS_ADDRMAP as they may be varying
+	 debug info type in single OBJFILE.  */
+
+      ALL_OBJFILE_PSYMTABS (objfile, pst)
+	if (pc >= pst->textlow && pc < pst->texthigh)
+	  {
+	    struct partial_symtab *best_pst;
+
+	    best_pst = find_pc_sect_psymtab_closer (pc, section, pst,
+						    msymbol);
+	    if (best_pst != NULL)
+	      return best_pst;
+	  }
+    }
+
+  return NULL;
 }
 
 /* Find which partial symtab contains PC.  Return 0 if none. 
--- ./gdb/testsuite/gdb.dwarf2/dw2-ranges.S	2008-04-20 16:38:19.000000000 +0200
+++ ./gdb/testsuite/gdb.dwarf2/dw2-ranges.S	2008-04-23 10:28:05.000000000 +0200
@@ -35,7 +35,10 @@ main:	.int	0
 	.endfunc
 	.size	main, . - main
 
-	.section	.text.func, "ax", @progbits
+	/* `.fini' section is here to make sure `dw2-ranges.S'
+	   vs. `dw2-ranges2.S' overlap their DW_AT_ranges with each other.  */
+	.section	.fini, "ax", @progbits
+
 	.globl	func
 	.func	func
 func:	.int	0
--- ./gdb/testsuite/gdb.dwarf2/dw2-ranges.exp	2008-04-20 16:38:19.000000000 +0200
+++ ./gdb/testsuite/gdb.dwarf2/dw2-ranges.exp	2008-04-23 10:28:05.000000000 +0200
@@ -30,9 +30,23 @@ if {![istarget *-*-linux*]
 
 set testfile "dw2-ranges"
 set srcfile ${testfile}.S
-set binfile ${objdir}/${subdir}/${testfile}.o
+set srcfile2 ${testfile}2.S
+set srcfile3 ${testfile}3.S
+set objfile ${objdir}/${subdir}/${testfile}.o
+set objfile2 ${objdir}/${subdir}/${testfile}2.o
+set objfile3 ${objdir}/${subdir}/${testfile}3.o
+set binfile ${objdir}/${subdir}/${testfile}
 
-if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" object debug] != "" } {
+if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${objfile}" object {additional_flags=-gdwarf-2}] != "" } {
+    return -1
+}
+if {[gdb_compile "${srcdir}/${subdir}/${srcfile2}" "${objfile2}" object {additional_flags=-gdwarf-2}] != "" } {
+    return -1
+}
+if {[gdb_compile "${srcdir}/${subdir}/${srcfile3}" "${objfile3}" object {additional_flags=-gstabs}] != "" } {
+    return -1
+}
+if {[gdb_compile "${objfile} ${objfile2} ${objfile3}" "${binfile}" executable {}] != "" } {
     return -1
 }
 
@@ -46,4 +60,8 @@ gdb_load ${binfile}
 # Wrong output:
 # 	No line number information available for address 0x4
 
+gdb_test "info line main" "Line \[0-9\]* of .* starts at address .* and ends at .*"
 gdb_test "info line func" "Line \[0-9\]* of .* starts at address .* and ends at .*"
+gdb_test "info line main2" "Line \[0-9\]* of .* starts at address .* and ends at .*"
+gdb_test "info line func2" "Line \[0-9\]* of .* starts at address .* and ends at .*"
+gdb_test "info line main3" "Line \[0-9\]* of .* starts at address .* and ends at .*"
--- ./gdb/testsuite/gdb.dwarf2/dw2-ranges2.S	1970-01-01 01:00:00.000000000 +0100
+++ ./gdb/testsuite/gdb.dwarf2/dw2-ranges2.S	2008-04-23 10:28:05.000000000 +0200
@@ -0,0 +1,46 @@
+/*
+   Copyright 2007 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/>.
+ */
+
+/* Despite the sections below will be adjacent the assembler has to produce
+   DW_AT_ranges as the linker could place both sections at arbitrary locations.
+   */
+
+	/* Such directive is required by GAS for builds without `-g'.  */
+	.file	1 "dw2-ranges2.S"
+
+	/* Without this directive GAS will not emit DWARF2 unless we provide an
+	   instruction to assemble.  We want to avoid any instructions to
+	   remain architecture independent.  */
+	.loc_mark_labels	1
+
+	.text
+
+	.globl	main2
+	.func	main2
+main2:	.int	0
+	.endfunc
+	.size	main2, . - main2
+
+	/* `.fini' section is here to make sure `dw2-ranges.S'
+	   vs. `dw2-ranges2.S' overlap their DW_AT_ranges with each other.  */
+	.section	.fini, "ax", @progbits
+
+	.globl	func2
+	.func	func2
+func2:	.int	0
+	.endfunc
+	.size	func2, . - func2
--- ./gdb/testsuite/gdb.dwarf2/dw2-ranges3.S	1970-01-01 01:00:00.000000000 +0100
+++ ./gdb/testsuite/gdb.dwarf2/dw2-ranges3.S	2008-04-23 10:28:05.000000000 +0200
@@ -0,0 +1,36 @@
+/*
+   Copyright 2007, 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/>.
+ */
+
+/* Despite the sections below will be adjacent the assembler has to produce
+   DW_AT_ranges as the linker could place both sections at arbitrary locations.
+   */
+
+	/* Such directive is required by GAS for builds without `-g'.  */
+	.file	1 "dw2-ranges3.S"
+
+	/* Without this directive GAS will not emit DWARF2 unless we provide an
+	   instruction to assemble.  We want to avoid any instructions to
+	   remain architecture independent.  */
+	.loc_mark_labels	1
+
+	.text
+
+	.globl	main3
+	.func	main3
+main3:	.int	0
+	.endfunc
+	.size	main3, . - main3

Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))

[Ulrich Weigand]

Jan Kratochvil wrote:

> 	* symtab.c: Include "addrmap.h"
> 	(find_pc_sect_psymtab): Support reading the field PSYMTABS_ADDRMAP.
> 	Move the psymtab locator into ...
> 	(find_pc_sect_psymtab_closer): ... a new function.

It appears this change broke overlay support.  In fact, it looks like the
main idea of this patch is fundamentally incompatible with overlays:

> +    /* Map addresses to the entries of PSYMTABS.  It would be more efficient to
> +       have a map per the whole process but ADDRMAP cannot selectively remove
> +       its items during FREE_OBJFILE.  This mapping is already present even for
> +       PARTIAL_SYMTABs which still have no corresponding full SYMTABs read.  */
> +
> +    struct addrmap *psymtabs_addrmap;

In the presence of overlays, a single address can map to *multiple* PSYMTABS
corresponding to multiple overlay segments optionally loaded at that address.

The old code chose between them using the "section" argument to 
find_pc_sect_psymtab; it seems the new code just ignores this argument:

  /* Try just the PSYMTABS_ADDRMAP mapping first as it has better granularity
     than the later used TEXTLOW/TEXTHIGH one.  */

  ALL_OBJFILES (objfile)
    if (objfile->psymtabs_addrmap != NULL)
      {
        struct partial_symtab *pst;

        pst = addrmap_find (objfile->psymtabs_addrmap, pc);
        if (pst != NULL)
          {
            /* 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
               a worse chosen section due to the TEXTLOW/TEXTHIGH ranges
               overlap.  */

            return pst;
          }
      }


Any suggestions how to fix this?

Bye,
Ulrich

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

Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))

[Michael Snyder]

On Mon, 2008-05-12 at 21:45 +0200, Ulrich Weigand wrote:
> Jan Kratochvil wrote:
> 
> > 	* symtab.c: Include "addrmap.h"
> > 	(find_pc_sect_psymtab): Support reading the field PSYMTABS_ADDRMAP.
> > 	Move the psymtab locator into ...
> > 	(find_pc_sect_psymtab_closer): ... a new function.
> 
> It appears this change broke overlay support.  

Thanks for noticing.   ;-)

I wrote that overlay code over 10 years ago.
I wonder whether anyone still uses it?

Michael

Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))

[Daniel Jacobowitz]

On Mon, May 12, 2008 at 01:06:44PM -0700, Michael Snyder wrote:
> I wrote that overlay code over 10 years ago.
> I wonder whether anyone still uses it?

On Mon, May 12, 2008 at 10:32:32PM +0200, Jan Kratochvil wrote:
> I generally considered the overlays to be no longer supported and just still
> not removed from the code.  While reading other GDB code I was feeling overlays
> are no longer supported on multiple places but sure I may be wrong.
> 
> Could you please name which current arch is dependent on overlaying?

As you may be able to guess from context, the answer is SPU :-)

Each SPE has only a 256K local store which it uses for code, stack,
and heap.  That's not very much space, so overlays are frequently
required.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))

[Jan Kratochvil]

On Mon, 12 May 2008 22:39:34 +0200, Daniel Jacobowitz wrote:
...
> As you may be able to guess from context, the answer is SPU :-)

OK.  Going to provide some patch to be somehow sections-aware.


Regards,
Jan

Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))

[Doug Evans]

On Mon, May 12, 2008 at 1:39 PM, Daniel Jacobowitz <drow@false.org> wrote:
> On Mon, May 12, 2008 at 01:06:44PM -0700, Michael Snyder wrote:
>  > I wrote that overlay code over 10 years ago.
>  > I wonder whether anyone still uses it?
>
>
> On Mon, May 12, 2008 at 10:32:32PM +0200, Jan Kratochvil wrote:
>  > I generally considered the overlays to be no longer supported and just still
>  > not removed from the code.  While reading other GDB code I was feeling overlays
>  > are no longer supported on multiple places but sure I may be wrong.
>  >
>  > Could you please name which current arch is dependent on overlaying?
>
>  As you may be able to guess from context, the answer is SPU :-)

Ah hah! :-)

>  Each SPE has only a 256K local store which it uses for code, stack,
>  and heap.  That's not very much space, so overlays are frequently
>  required.

Yep.

Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))

[Michael Snyder]

On Mon, 2008-05-12 at 16:39 -0400, Daniel Jacobowitz wrote:
> On Mon, May 12, 2008 at 01:06:44PM -0700, Michael Snyder wrote:
> > I wrote that overlay code over 10 years ago.
> > I wonder whether anyone still uses it?
> 
> On Mon, May 12, 2008 at 10:32:32PM +0200, Jan Kratochvil wrote:
> > I generally considered the overlays to be no longer supported and just still
> > not removed from the code.  While reading other GDB code I was feeling overlays
> > are no longer supported on multiple places but sure I may be wrong.
> > 
> > Could you please name which current arch is dependent on overlaying?
> 
> As you may be able to guess from context, the answer is SPU :-)
> 
> Each SPE has only a 256K local store which it uses for code, stack,
> and heap.  That's not very much space, so overlays are frequently
> required.

Oho, well, Stan Shebs should be gratified.  He assured me
at the time that overlays would get more use than I thought
they would.  ;-)

Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))

[Doug Evans]

On Mon, May 12, 2008 at 1:06 PM, Michael Snyder <msnyder@specifix.com> wrote:
>  I wrote that overlay code over 10 years ago.
>  I wonder whether anyone still uses it?

I've been wondering if it'd be useful in the spu context.  Is what how
you discovered the breakage Ulrich?

Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))

[Jan Kratochvil]

On Mon, 12 May 2008 21:45:39 +0200, Ulrich Weigand wrote:
...
> It appears this change broke overlay support.

I generally considered the overlays to be no longer supported and just still
not removed from the code.  While reading other GDB code I was feeling overlays
are no longer supported on multiple places but sure I may be wrong.

Could you please name which current arch is dependent on overlaying?


Sorry,
Jan

Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))

[Ulrich Weigand]

Jan Kratochvil wrote:
> I generally considered the overlays to be no longer supported and just still
> not removed from the code.  While reading other GDB code I was feeling overlays
> are no longer supported on multiple places but sure I may be wrong.
> 
> Could you please name which current arch is dependent on overlaying?

As Daniel mentioned, I noticed this on spu-elf, where we do make use of
overlays (at least code overlays -- data overlays are not currently used).

Which parts of GDB do you think do not support overlays?  I'd be interested
in fixing such problems ...

For now, I'm using the patch below that simply falls back to the non-addrmap
case when debugging overlays and the addrmap returned the wrong section.


In testing this, I noticed an independent overlay regression introduced by
a patch of mine:
http://sourceware.org/ml/gdb-patches/2008-05/msg00120.html
which added code to fixup_section to verify the msymbol address.

This patch exposed a pre-existing bug in fixup_section: it uses
  ginfo->value.address
to access the address of a symbol or psymbol.  This is mostly correct,
but in one case it is not: when handling a LOC_BLOCK symbol, in which
case ginfo->value.address is undefined, and the start address needs to
be retrieved from the block at ginfo->value.block.

The following patch fixes this regression as well by having the callers
of fixup_section pass in the proper address.


Tested on spu-elf, powerpc-linux and powerpc64-linux with no regressions;
fixes overlays.exp on spu-elf.

If there is no objection, I'd like to commit this to fix the present
regression until we have a proper implementation of addrmaps for the
overlay case.

Bye,
Ulrich


ChangeLog:

	* symtab.c (fixup_section): Remove prototype.  Add ADDR parameter;
	use it instead of ginfo->value.address.
	(fixup_symbol_section): Pass in correct symbol address.
	(fixup_psymbol_section): Pass in correct symbol address.

	(find_pc_sect_psymtab): Fall back to non-addrmap case when debugging
	overlays and the addrmap returned the wrong section.


diff -urNp gdb-orig/gdb/symtab.c gdb-head/gdb/symtab.c
--- gdb-orig/gdb/symtab.c	2008-05-11 23:41:56.000000000 +0200
+++ gdb-head/gdb/symtab.c	2008-05-13 15:48:55.626975367 +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,7 +1025,8 @@ 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);
@@ -1020,8 +1036,7 @@ fixup_section (struct general_symbol_inf
      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)
+  if (msym && SYMBOL_VALUE_ADDRESS (msym) == addr)
     {
       ginfo->bfd_section = SYMBOL_BFD_SECTION (msym);
       ginfo->section = SYMBOL_SECTION (msym);
@@ -1064,11 +1079,8 @@ 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;
@@ -1093,7 +1105,22 @@ fixup_symbol_section (struct symbol *sym
   if (SYMBOL_BFD_SECTION (sym))
     return sym;
 
-  fixup_section (&sym->ginfo, objfile);
+  switch (SYMBOL_CLASS (sym))
+    {
+    case LOC_LABEL:
+    case LOC_STATIC:
+    case LOC_INDIRECT:
+      fixup_section (&sym->ginfo, SYMBOL_VALUE_ADDRESS (sym), objfile);
+      break;
+
+    case LOC_BLOCK:
+      fixup_section (&sym->ginfo,
+		     BLOCK_START (SYMBOL_BLOCK_VALUE (sym)), objfile);
+      break;
+
+    default:
+      break;
+    }
 
   return sym;
 }
@@ -1107,7 +1134,18 @@ fixup_psymbol_section (struct partial_sy
   if (SYMBOL_BFD_SECTION (psym))
     return psym;
 
-  fixup_section (&psym->ginfo, objfile);
+  switch (SYMBOL_CLASS (psym))
+    {
+    case LOC_LABEL:
+    case LOC_STATIC:
+    case LOC_INDIRECT:
+    case LOC_BLOCK:
+      fixup_section (&psym->ginfo, SYMBOL_VALUE_ADDRESS (psym), objfile);
+      break;
+
+    default:
+      break;
+    }
 
   return psym;
 }

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

Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))

[Pedro Alves]

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

A Tuesday 13 May 2008 18:00:48, Ulrich Weigand wrote:
> Jan Kratochvil wrote:
> > I generally considered the overlays to be no longer supported and just
> > still not removed from the code.  While reading other GDB code I was
> > feeling overlays are no longer supported on multiple places but sure I
> > may be wrong.
> >
> > Could you please name which current arch is dependent on overlaying?
>
> As Daniel mentioned, I noticed this on spu-elf, where we do make use of
> overlays (at least code overlays -- data overlays are not currently used).
>
> Which parts of GDB do you think do not support overlays?  I'd be interested
> in fixing such problems ...
>
> For now, I'm using the patch below that simply falls back to the
> non-addrmap case when debugging overlays and the addrmap returned the wrong
> section.
>
>
> In testing this, I noticed an independent overlay regression introduced by
> a patch of mine:
> http://sourceware.org/ml/gdb-patches/2008-05/msg00120.html
> which added code to fixup_section to verify the msymbol address.
>
> This patch exposed a pre-existing bug in fixup_section: it uses
>   ginfo->value.address
> to access the address of a symbol or psymbol.  This is mostly correct,
> but in one case it is not: when handling a LOC_BLOCK symbol, in which
> case ginfo->value.address is undefined, and the start address needs to
> be retrieved from the block at ginfo->value.block.
>
> The following patch fixes this regression as well by having the callers
> of fixup_section pass in the proper address.
>

FWIW, we have been using something very similar in our trees that I
had never submitted because of not being able to test on an arch
that uses overlays.

>
> Tested on spu-elf, powerpc-linux and powerpc64-linux with no regressions;
> fixes overlays.exp on spu-elf.
>
> If there is no objection, I'd like to commit this to fix the present
> regression until we have a proper implementation of addrmaps for the
> overlay case.
>

Please also make sure that you're setting SYMBOL_CLASS
before calling fixup_*symbol_section everywhere.  This will break
dwarf2read.c, for example:

 if (attr_form_is_block (attr)
      && DW_BLOCK (attr)->size == 1 + cu_header->addr_size
      && DW_BLOCK (attr)->data[0] == DW_OP_addr)
    {
      unsigned int dummy;

      SYMBOL_VALUE_ADDRESS (sym) =
	read_address (objfile->obfd, DW_BLOCK (attr)->data + 1, cu, &dummy);
      fixup_symbol_section (sym, objfile);
      SYMBOL_VALUE_ADDRESS (sym) += ANOFFSET (objfile->section_offsets,
					      SYMBOL_SECTION (sym));
      SYMBOL_CLASS (sym) = LOC_STATIC;
      return;
    }

The breakage goes unnoticed on linux, as all loadable sections will
have the same ANOFFSET, but will break in systems loading .text and
.data with different offsets, as uclinux, for example.

Attached is what we're using, as an example.  The testcase no longer
fails on head, I suspect because of your other patch that checks
the address.  The fix was for GDB fixing up the wrong section, due
to the lookup by name finding the wrong minsym.

> Bye,
> Ulrich
>
>
> ChangeLog:
>
> 	* symtab.c (fixup_section): Remove prototype.  Add ADDR parameter;
> 	use it instead of ginfo->value.address.
> 	(fixup_symbol_section): Pass in correct symbol address.
> 	(fixup_psymbol_section): Pass in correct symbol address.
>
> 	(find_pc_sect_psymtab): Fall back to non-addrmap case when debugging
> 	overlays and the addrmap returned the wrong section.
>
>
> diff -urNp gdb-orig/gdb/symtab.c gdb-head/gdb/symtab.c
> --- gdb-orig/gdb/symtab.c	2008-05-11 23:41:56.000000000 +0200
> +++ gdb-head/gdb/symtab.c	2008-05-13 15:48:55.626975367 +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,7 +1025,8 @@ 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);
> @@ -1020,8 +1036,7 @@ fixup_section (struct general_symbol_inf
>       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)
> +  if (msym && SYMBOL_VALUE_ADDRESS (msym) == addr)
>      {
>        ginfo->bfd_section = SYMBOL_BFD_SECTION (msym);
>        ginfo->section = SYMBOL_SECTION (msym);
> @@ -1064,11 +1079,8 @@ 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;
> @@ -1093,7 +1105,22 @@ fixup_symbol_section (struct symbol *sym
>    if (SYMBOL_BFD_SECTION (sym))
>      return sym;
>
> -  fixup_section (&sym->ginfo, objfile);
> +  switch (SYMBOL_CLASS (sym))
> +    {
> +    case LOC_LABEL:
> +    case LOC_STATIC:
> +    case LOC_INDIRECT:
> +      fixup_section (&sym->ginfo, SYMBOL_VALUE_ADDRESS (sym), objfile);
> +      break;
> +
> +    case LOC_BLOCK:
> +      fixup_section (&sym->ginfo,
> +		     BLOCK_START (SYMBOL_BLOCK_VALUE (sym)), objfile);
> +      break;
> +
> +    default:
> +      break;
> +    }
>
>    return sym;
>  }
> @@ -1107,7 +1134,18 @@ fixup_psymbol_section (struct partial_sy
>    if (SYMBOL_BFD_SECTION (psym))
>      return psym;
>
> -  fixup_section (&psym->ginfo, objfile);
> +  switch (SYMBOL_CLASS (psym))
> +    {
> +    case LOC_LABEL:
> +    case LOC_STATIC:
> +    case LOC_INDIRECT:
> +    case LOC_BLOCK:
> +      fixup_section (&psym->ginfo, SYMBOL_VALUE_ADDRESS (psym), objfile);
> +      break;
> +
> +    default:
> +      break;
> +    }
>
>    return psym;
>  }



-- 
Pedro Alves

[-- Attachment #2: fixup_sections.diff --]
[-- Type: text/x-diff, Size: 8895 bytes --]

2008-01-23  Pedro Alves  <pedro@codesourcery.com>

	Match symbol/msymbol/bfd_section by address.

	gdb/
	* symtab.c (fixup_section): Add addr parameter.  If possible,
	lookup the minimal symbol by address.  If that fails, fallback to
	the old by-name method.
	(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.
	* dwarf2read.c (var_decode_location): Set SYMBOL_CLASS before
	calling fixup_symbol_section.

	gdb/testsuite/
	* gdb.base/fixsection.exp: New file.
	* gdb.base/fixsection0.c: New file.
	* gdb.base/fixsection1.c: New file.
---
 gdb/dwarf2read.c                      |    2 
 gdb/symtab.c                          |   70 ++++++++++++++++++++++++++++++---
 gdb/testsuite/gdb.base/fixsection.c   |   35 ++++++++++++++++
 gdb/testsuite/gdb.base/fixsection.exp |   71 ++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/fixsectshr.c   |   10 ++++
 5 files changed, 180 insertions(+), 8 deletions(-)

Index: src/gdb/symtab.c
===================================================================
--- src.orig/gdb/symtab.c	2008-05-13 19:04:29.000000000 +0100
+++ src/gdb/symtab.c	2008-05-13 19:14:31.000000000 +0100
@@ -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,
@@ -1010,10 +1008,20 @@ 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);
+  struct minimal_symbol *msym = NULL;
+  if (addr != ~(CORE_ADDR) 0)
+    /* If we have an address to lookup, use it.  */
+    msym = lookup_minimal_symbol_by_pc (addr);
+
+  if (!msym
+      || addr != SYMBOL_VALUE_ADDRESS (msym)
+      || strcmp (DEPRECATED_SYMBOL_NAME (msym), ginfo->name) != 0)
+    /* Try by looking up by name.  Not perfect, since it can match the
+       wrong symbol.  */
+    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
@@ -1087,13 +1095,45 @@ 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 || (sym->symtab && sym->symtab->objfile));
+
+  if (objfile == NULL)
+    objfile = sym->symtab->objfile;
+
+  /* We should have an objfile by now.  */
+  gdb_assert (objfile);
+
+  switch (SYMBOL_CLASS (sym))
+    {
+    case LOC_UNRESOLVED:
+      addr = ~(CORE_ADDR) 0;
+      break;
+    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,29 @@ 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_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;
 }
Index: src/gdb/dwarf2read.c
===================================================================
--- src.orig/gdb/dwarf2read.c	2008-05-13 19:04:29.000000000 +0100
+++ src/gdb/dwarf2read.c	2008-05-13 19:14:31.000000000 +0100
@@ -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;
     }
 
Index: src/gdb/testsuite/gdb.base/fixsectshr.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/testsuite/gdb.base/fixsectshr.c	2008-05-13 19:14:31.000000000 +0100
@@ -0,0 +1,10 @@
+#include <stdio.h>
+#include <stdlib.h>
+
+static FILE *static_fun = NULL;
+
+FILE *
+force_static_fun (void)
+{
+  return static_fun;
+}
Index: src/gdb/testsuite/gdb.base/fixsection.exp
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/testsuite/gdb.base/fixsection.exp	2008-05-13 19:14:31.000000000 +0100
@@ -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
Index: src/gdb/testsuite/gdb.base/fixsection.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/testsuite/gdb.base/fixsection.c	2008-05-13 19:14:31.000000000 +0100
@@ -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;
+}

Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))

[Pedro Alves]

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

A Tuesday 13 May 2008 18:00:48, Ulrich Weigand wrote:
> Jan Kratochvil wrote:
> > I generally considered the overlays to be no longer supported and just
> > still not removed from the code.  While reading other GDB code I was
> > feeling overlays are no longer supported on multiple places but sure I
> > may be wrong.
> >
> > Could you please name which current arch is dependent on overlaying?
>
> As Daniel mentioned, I noticed this on spu-elf, where we do make use of
> overlays (at least code overlays -- data overlays are not currently used).
>
> Which parts of GDB do you think do not support overlays?  I'd be interested
> in fixing such problems ...
>
> For now, I'm using the patch below that simply falls back to the
> non-addrmap case when debugging overlays and the addrmap returned the wrong
> section.
>
>
> In testing this, I noticed an independent overlay regression introduced by
> a patch of mine:
> http://sourceware.org/ml/gdb-patches/2008-05/msg00120.html
> which added code to fixup_section to verify the msymbol address.
>
> This patch exposed a pre-existing bug in fixup_section: it uses
>   ginfo->value.address
> to access the address of a symbol or psymbol.  This is mostly correct,
> but in one case it is not: when handling a LOC_BLOCK symbol, in which
> case ginfo->value.address is undefined, and the start address needs to
> be retrieved from the block at ginfo->value.block.
>
> The following patch fixes this regression as well by having the callers
> of fixup_section pass in the proper address.
>

FWIW, we have been using something very similar in our trees that I
had never submitted because of not being able to test on an arch
that uses overlays.

>
> Tested on spu-elf, powerpc-linux and powerpc64-linux with no regressions;
> fixes overlays.exp on spu-elf.
>
> If there is no objection, I'd like to commit this to fix the present
> regression until we have a proper implementation of addrmaps for the
> overlay case.
>

Please also make sure that you're setting SYMBOL_CLASS
before calling fixup_*symbol_section everywhere.  This will break
dwarf2read.c, for example:

 if (attr_form_is_block (attr)
      && DW_BLOCK (attr)->size == 1 + cu_header->addr_size
      && DW_BLOCK (attr)->data[0] == DW_OP_addr)
    {
      unsigned int dummy;

      SYMBOL_VALUE_ADDRESS (sym) =
	read_address (objfile->obfd, DW_BLOCK (attr)->data + 1, cu, &dummy);
      fixup_symbol_section (sym, objfile);
      SYMBOL_VALUE_ADDRESS (sym) += ANOFFSET (objfile->section_offsets,
					      SYMBOL_SECTION (sym));
      SYMBOL_CLASS (sym) = LOC_STATIC;
      return;
    }

The breakage goes unnoticed on linux, as all loadable sections will
have the same ANOFFSET, but will break in systems loading .text and
.data with different offsets, as uclinux, for example.

Attached is what we're using, as an example.  The testcase no longer
fails on head, I suspect because of your other patch that checks
the address.  The fix was for GDB fixing up the wrong section, due
to the lookup by name finding the wrong minsym.

> Bye,
> Ulrich
>
>
> ChangeLog:
>
> 	* symtab.c (fixup_section): Remove prototype.  Add ADDR parameter;
> 	use it instead of ginfo->value.address.
> 	(fixup_symbol_section): Pass in correct symbol address.
> 	(fixup_psymbol_section): Pass in correct symbol address.
>
> 	(find_pc_sect_psymtab): Fall back to non-addrmap case when debugging
> 	overlays and the addrmap returned the wrong section.
>
>
> diff -urNp gdb-orig/gdb/symtab.c gdb-head/gdb/symtab.c
> --- gdb-orig/gdb/symtab.c	2008-05-11 23:41:56.000000000 +0200
> +++ gdb-head/gdb/symtab.c	2008-05-13 15:48:55.626975367 +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,7 +1025,8 @@ 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);
> @@ -1020,8 +1036,7 @@ fixup_section (struct general_symbol_inf
>       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)
> +  if (msym && SYMBOL_VALUE_ADDRESS (msym) == addr)
>      {
>        ginfo->bfd_section = SYMBOL_BFD_SECTION (msym);
>        ginfo->section = SYMBOL_SECTION (msym);
> @@ -1064,11 +1079,8 @@ 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;
> @@ -1093,7 +1105,22 @@ fixup_symbol_section (struct symbol *sym
>    if (SYMBOL_BFD_SECTION (sym))
>      return sym;
>
> -  fixup_section (&sym->ginfo, objfile);
> +  switch (SYMBOL_CLASS (sym))
> +    {
> +    case LOC_LABEL:
> +    case LOC_STATIC:
> +    case LOC_INDIRECT:
> +      fixup_section (&sym->ginfo, SYMBOL_VALUE_ADDRESS (sym), objfile);
> +      break;
> +
> +    case LOC_BLOCK:
> +      fixup_section (&sym->ginfo,
> +		     BLOCK_START (SYMBOL_BLOCK_VALUE (sym)), objfile);
> +      break;
> +
> +    default:
> +      break;
> +    }
>
>    return sym;
>  }
> @@ -1107,7 +1134,18 @@ fixup_psymbol_section (struct partial_sy
>    if (SYMBOL_BFD_SECTION (psym))
>      return psym;
>
> -  fixup_section (&psym->ginfo, objfile);
> +  switch (SYMBOL_CLASS (psym))
> +    {
> +    case LOC_LABEL:
> +    case LOC_STATIC:
> +    case LOC_INDIRECT:
> +    case LOC_BLOCK:
> +      fixup_section (&psym->ginfo, SYMBOL_VALUE_ADDRESS (psym), objfile);
> +      break;
> +
> +    default:
> +      break;
> +    }
>
>    return psym;
>  }



-- 
Pedro Alves

[-- Attachment #2: fixup_sections.diff --]
[-- Type: text/x-diff, Size: 8895 bytes --]

2008-01-23  Pedro Alves  <pedro@codesourcery.com>

	Match symbol/msymbol/bfd_section by address.

	gdb/
	* symtab.c (fixup_section): Add addr parameter.  If possible,
	lookup the minimal symbol by address.  If that fails, fallback to
	the old by-name method.
	(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.
	* dwarf2read.c (var_decode_location): Set SYMBOL_CLASS before
	calling fixup_symbol_section.

	gdb/testsuite/
	* gdb.base/fixsection.exp: New file.
	* gdb.base/fixsection0.c: New file.
	* gdb.base/fixsection1.c: New file.
---
 gdb/dwarf2read.c                      |    2 
 gdb/symtab.c                          |   70 ++++++++++++++++++++++++++++++---
 gdb/testsuite/gdb.base/fixsection.c   |   35 ++++++++++++++++
 gdb/testsuite/gdb.base/fixsection.exp |   71 ++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/fixsectshr.c   |   10 ++++
 5 files changed, 180 insertions(+), 8 deletions(-)

Index: src/gdb/symtab.c
===================================================================
--- src.orig/gdb/symtab.c	2008-05-13 19:04:29.000000000 +0100
+++ src/gdb/symtab.c	2008-05-13 19:14:31.000000000 +0100
@@ -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,
@@ -1010,10 +1008,20 @@ 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);
+  struct minimal_symbol *msym = NULL;
+  if (addr != ~(CORE_ADDR) 0)
+    /* If we have an address to lookup, use it.  */
+    msym = lookup_minimal_symbol_by_pc (addr);
+
+  if (!msym
+      || addr != SYMBOL_VALUE_ADDRESS (msym)
+      || strcmp (DEPRECATED_SYMBOL_NAME (msym), ginfo->name) != 0)
+    /* Try by looking up by name.  Not perfect, since it can match the
+       wrong symbol.  */
+    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
@@ -1087,13 +1095,45 @@ 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 || (sym->symtab && sym->symtab->objfile));
+
+  if (objfile == NULL)
+    objfile = sym->symtab->objfile;
+
+  /* We should have an objfile by now.  */
+  gdb_assert (objfile);
+
+  switch (SYMBOL_CLASS (sym))
+    {
+    case LOC_UNRESOLVED:
+      addr = ~(CORE_ADDR) 0;
+      break;
+    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,29 @@ 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_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;
 }
Index: src/gdb/dwarf2read.c
===================================================================
--- src.orig/gdb/dwarf2read.c	2008-05-13 19:04:29.000000000 +0100
+++ src/gdb/dwarf2read.c	2008-05-13 19:14:31.000000000 +0100
@@ -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;
     }
 
Index: src/gdb/testsuite/gdb.base/fixsectshr.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/testsuite/gdb.base/fixsectshr.c	2008-05-13 19:14:31.000000000 +0100
@@ -0,0 +1,10 @@
+#include <stdio.h>
+#include <stdlib.h>
+
+static FILE *static_fun = NULL;
+
+FILE *
+force_static_fun (void)
+{
+  return static_fun;
+}
Index: src/gdb/testsuite/gdb.base/fixsection.exp
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/testsuite/gdb.base/fixsection.exp	2008-05-13 19:14:31.000000000 +0100
@@ -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
Index: src/gdb/testsuite/gdb.base/fixsection.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/testsuite/gdb.base/fixsection.c	2008-05-13 19:14:31.000000000 +0100
@@ -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;
+}

Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))

[Michael Snyder]

On Tue, 2008-05-13 at 19:20 +0100, Pedro Alves wrote:

> FWIW, we have been using something very similar in our trees that I
> had never submitted because of not being able to test on an arch
> that uses overlays.

Try building the m32r target and testing with the simulator.

Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))

[Jan Kratochvil]

On Tue, 13 May 2008 19:00:48 +0200, Ulrich Weigand wrote:
...
> Which parts of GDB do you think do not support overlays?  I'd be interested
> in fixing such problems ...

There is still a lot of stub functions X() just calling X_sect() using
find_pc_mapped_section(), these should get removed as otherwise one may find
a countercase where it fails for the overlayed sections.
[blockvector_for_pc, block_for_pc, find_pc_psymtab, find_pc_psymbol,
 find_pc_symtab, find_pc_section, find_pc_function]


> For now, I'm using the patch below that simply falls back to the non-addrmap
> case when debugging overlays and the addrmap returned the wrong section.

I started coding a similiar patch as IMO the overlayed sections have no use for
addrmap as they are not discontiguous, thanks for fixing it up this way.

...
> If there is no objection, I'd like to commit this to fix the present
> regression

> until we have a proper implementation of addrmaps for the overlay case.

It is not worth it as it would mean to either
* support multiple results from a single call of addrmap_find()
or
* to move `addrmap_find' from `struct objfile' to each `struct partial_symtab'
which is IMO not worth the performance in a general case as your patch only
stays at the same performance as before addrmap for the overlayed case.



Regards,
Jan

Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))

[Ulrich Weigand]

Jan Kratochvil wrote:
> On Tue, 13 May 2008 19:00:48 +0200, Ulrich Weigand wrote:
> ...
> > Which parts of GDB do you think do not support overlays?  I'd be interested
> > in fixing such problems ...
> 
> There is still a lot of stub functions X() just calling X_sect() using
> find_pc_mapped_section(), these should get removed as otherwise one may find
> a countercase where it fails for the overlayed sections.
> [blockvector_for_pc, block_for_pc, find_pc_psymtab, find_pc_psymbol,
>  find_pc_symtab, find_pc_section, find_pc_function]

I thought those remaining usages were generally OK -- but I guess you're
right that new uses may have crept in over time ...

> > For now, I'm using the patch below that simply falls back to the non-addrmap
> > case when debugging overlays and the addrmap returned the wrong section.
> 
> I started coding a similiar patch as IMO the overlayed sections have no use for
> addrmap as they are not discontiguous, thanks for fixing it up this way.

Hmm, OK.  However, even with overlay debugging, there might be some other
discontiguous sections, so I don't really like the
  if (overlay_debugging ...)
aspect of my patch.  But without that condition, one of your new test cases
would fail again.

Bye,
Ulrich

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

Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))

[Daniel Jacobowitz]

On Thu, May 15, 2008 at 06:38:48PM +0200, Ulrich Weigand wrote:
> > > For now, I'm using the patch below that simply falls back to the non-addrmap
> > > case when debugging overlays and the addrmap returned the wrong section.
> > 
> > I started coding a similiar patch as IMO the overlayed sections have no use for
> > addrmap as they are not discontiguous, thanks for fixing it up this way.
> 
> Hmm, OK.  However, even with overlay debugging, there might be some other
> discontiguous sections, so I don't really like the
>   if (overlay_debugging ...)
> aspect of my patch.  But without that condition, one of your new test cases
> would fail again.

Yes, I don't like that part either.  I wonder if the memory usage
would be too bad if we kept an addrmap for each section and one
combined one for the non-overlay case?

I don't know what overlay debug info looks like.  Can we detect
overlays in any practical way when reading in symbols?

-- 
Daniel Jacobowitz
CodeSourcery

Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))

[Ulrich Weigand]

Daniel Jacobowitz wrote:

> Yes, I don't like that part either.  I wonder if the memory usage
> would be too bad if we kept an addrmap for each section and one
> combined one for the non-overlay case?
> 
> I don't know what overlay debug info looks like.  Can we detect
> overlays in any practical way when reading in symbols?

Not really, I don't think there is any indication in the debug
info that we have an overlay.  The address ranges just happen
to overlap ...

Bye,
Ulrich

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

Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))

[Ulrich Weigand]

Daniel Jacobowitz wrote:

> Yes, I don't like that part either.  I wonder if the memory usage
> would be too bad if we kept an addrmap for each section and one
> combined one for the non-overlay case?

Hmm, maybe this would even save memory; a per-section addrmap would
not need to store any result except from the boolean "address in
section or not" flag ...

Bye,
Ulrich

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

Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))

[Michael Snyder]

On Thu, 2008-05-15 at 12:11 +0200, Jan Kratochvil wrote:
> On Tue, 13 May 2008 19:00:48 +0200, Ulrich Weigand wrote:
> ...
> > Which parts of GDB do you think do not support overlays?  I'd be interested
> > in fixing such problems ...
> 
> There is still a lot of stub functions X() just calling X_sect() using
> find_pc_mapped_section(), these should get removed as otherwise one may find
> a countercase where it fails for the overlayed sections.
> [blockvector_for_pc, block_for_pc, find_pc_psymtab, find_pc_psymbol,
>  find_pc_symtab, find_pc_section, find_pc_function]

?  It's been 10 years, but I think that's the way it is
supposed to work.

Old code calls X(), without supplying a section.
Then X forwards the call to X_sect() after attempting
to lookup the appropriate section.

In most cases this should not be critical for overlays, 
because code that depends on overlays should be calling
X_sect() directly.  It was just a shortcut so that we
wouldn't have to change all the calls of X into X_sect
(and do the section lookup in many places instead of one).