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