From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26371 invoked by alias); 13 May 2008 23:16:06 -0000 Received: (qmail 26362 invoked by uid 22791); 13 May 2008 23:16:05 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 13 May 2008 23:15:47 +0000 Received: (qmail 2708 invoked from network); 13 May 2008 23:15:45 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 13 May 2008 23:15:45 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap)) Date: Wed, 14 May 2008 08:07:00 -0000 User-Agent: KMail/1.9.9 Cc: "Ulrich Weigand" , jan.kratochvil@redhat.com, drow@false.org References: <200805132112.m4DLCKfr018490@d12av02.megacenter.de.ibm.com> In-Reply-To: <200805132112.m4DLCKfr018490@d12av02.megacenter.de.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200805140015.48740.pedro@codesourcery.com> X-IsSubscribed: yes 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/msg00421.txt.bz2 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