From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12301 invoked by alias); 15 May 2008 16:51:46 -0000 Received: (qmail 12243 invoked by uid 22791); 15 May 2008 16:51:28 -0000 X-Spam-Check-By: sourceware.org Received: from mtagate6.de.ibm.com (HELO mtagate6.de.ibm.com) (195.212.29.155) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 15 May 2008 16:50:54 +0000 Received: from d12nrmr1607.megacenter.de.ibm.com (d12nrmr1607.megacenter.de.ibm.com [9.149.167.49]) by mtagate6.de.ibm.com (8.13.8/8.13.8) with ESMTP id m4FGoULS478572 for ; Thu, 15 May 2008 16:50:30 GMT Received: from d12av02.megacenter.de.ibm.com (d12av02.megacenter.de.ibm.com [9.149.165.228]) by d12nrmr1607.megacenter.de.ibm.com (8.13.8/8.13.8/NCO v8.7) with ESMTP id m4FGoUsw2605196 for ; Thu, 15 May 2008 18:50:30 +0200 Received: from d12av02.megacenter.de.ibm.com (loopback [127.0.0.1]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id m4FGoT6B004139 for ; Thu, 15 May 2008 18:50:30 +0200 Received: from tuxmaker.boeblingen.de.ibm.com (tuxmaker.boeblingen.de.ibm.com [9.152.85.9]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.12.11) with SMTP id m4FGoTod004135; Thu, 15 May 2008 18:50:29 +0200 Message-Id: <200805151650.m4FGoTod004135@d12av02.megacenter.de.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Thu, 15 May 2008 18:50:29 +0200 Subject: [patch] Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap)) To: pedro@codesourcery.com (Pedro Alves) Date: Thu, 15 May 2008 18:19:00 -0000 From: "Ulrich Weigand" Cc: gdb-patches@sourceware.org, jan.kratochvil@redhat.com, drow@false.org In-Reply-To: <200805140015.48740.pedro@codesourcery.com> from "Pedro Alves" at May 14, 2008 12:15:48 AM X-Mailer: ELM [version 2.5 PL2] MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-05/txt/msg00469.txt.bz2 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 Ulrich Weigand * 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 * 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 . + + This file was written by Pedro Alves (pedro@codesourcery.com). */ + +#include +#include + +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 . + +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 +#include + +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