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

["Ulrich Weigand" <uweigand@de.ibm.com>]

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