From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5449 invoked by alias); 17 Apr 2012 12:16:21 -0000 Received: (qmail 5361 invoked by uid 22791); 17 Apr 2012 12:16:17 -0000 X-SWARE-Spam-Status: No, hits=-6.5 required=5.0 tests=AWL,BAYES_00,KAM_STOCKGEN,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,SPF_HELO_PASS,TW_BJ,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 17 Apr 2012 12:15:59 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q3HCFgJK028975 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 17 Apr 2012 08:15:42 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q3HCFePX007558; Tue, 17 Apr 2012 08:15:41 -0400 Message-ID: <4F8D5EEC.30908@redhat.com> Date: Tue, 17 Apr 2012 12:45:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20120329 Thunderbird/11.0.1 MIME-Version: 1.0 To: Joel Brobecker CC: gdb-patches@sourceware.org Subject: Re: RFC: bfd_section should not be NULL in call to prim_record_minimal_* References: <1334610821-10974-1-git-send-email-brobecker@adacore.com> In-Reply-To: <1334610821-10974-1-git-send-email-brobecker@adacore.com> Content-Type: text/plain; charset=ISO-8859-1 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: 2012-04/txt/msg00460.txt.bz2 On 04/16/2012 10:13 PM, Joel Brobecker wrote: > It seems that creating breakpoints no longer works on ppc-aix: > > % gdb foo > (gdb) b main > /[...]/progspace.c:216: internal-error: set_current_program_space: Assertion `pspace != NULL' failed. > A problem internal to GDB has been detected, > further debugging may prove unreliable. > Quit this debugging session? (y or n) > > Patch #2 explains what is going on, but basically, xcoffread.c > creates minimal symbols where the obj_section is not set. At first > sight, minsyms.h seems to indicate that it is OK, if you look at > prim_record_minimal_symbol_full's documentation: > > BFD_SECTION - the symbol's BFD section; used to find the > appropriate obj_section for the minimal symbol. This can be NULL. > ^^^^^^^^^^^^^^^^ > > But I think it is wrong, because I think a lot of places in the GDB code > make the assumption that a minimal symbol's obj_section is not NULL, and > the only way to set it, I think, is to have the BFD section. When I added the pspace stuff, I remember trying to be careful to not follow SYMBOL_OBJ_SECTION (msymbol)->objfile to get at objfile->pspace. [In my mind, symbols ideally wouldn't need back pointers to thei "containers" (symtabs, object files, etc), because that wastes space; the lookup routines instead should bubble up the "container" the symbol was found in if necessary. But that may be very hard to do.] At first sight, I can't find any other place that does that; this was added recently with the linespec rewrite only, it seems. The "section" and "obj_section" are one of those sources of difference and confusion that it'd be nice to get rid of: /* Which section is this symbol in? This is an index into section_offsets for this objfile. Negative means that the symbol does not get relocated relative to a section. Disclaimer: currently this is just used for xcoff, so don't expect all symbol-reading code to set it correctly (the ELF code also tries to set it correctly). */ short section; /* The section associated with this symbol. It can be NULL. */ struct obj_section *obj_section; OTOH, this stuff is space sensitive, and in the long term, it could prove better to only hold a "short" instead of a pointer (8 bytes on 64-bit hosts). Anyway, all that to say that I think the new linespec code should be using the proper API to get at a msymbol's objfile -- msymbol_objfile. Like in the patch below. When testing it, I ran linespec.exp first, and that revealed that msymbol_objfile had a bug in that it didn't look at all the pspaces, only the current (the test adds a second inferior, therefore a second pspace). Not good when you want to look up the objfile because you want to know the msyms' pspace to begin with. I think we should put this change in anyway, and, if we really want to assume msymbol->obj_section->objfile will always be possible, then msymbol_objfile could/should be changed to do that directly link too instead of the hash based lookup. > In the meantime, patch #2 fixes the problem by making sure that we > always pass a BFD section. I haven't tested it against the official > testsuite, I will do so now, but I also wanted to start this discussion > before I forget. I looked over that patch, and nothing jumped out as wrong to me. The patch below was tested on x86_64 Fedora 16. I'm guessing it fixes AIX too. 2012-04-17 Pedro Alves Avoid SYMBOL_OBJ_SECTION (elem->minsym)->objfile. * linespec.c (convert_linespec_to_sals, compare_msymbols): Use msymbol_objfile to get at the msymbol's objfile. * minsyms.c (msymbol_objfile): Iterate over program spaces. --- gdb/linespec.c | 6 +++--- gdb/minsyms.c | 10 ++++++---- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/gdb/linespec.c b/gdb/linespec.c index 228214b..cf073ae 100644 --- a/gdb/linespec.c +++ b/gdb/linespec.c @@ -1899,7 +1899,7 @@ convert_linespec_to_sals (struct linespec_state *state, linespec_p ls) VEC_iterate (minsym_and_objfile_d, ls->minimal_symbols, i, elem); ++i) { - pspace = SYMBOL_OBJ_SECTION (elem->minsym)->objfile->pspace; + pspace = msymbol_objfile (elem->minsym)->pspace; set_current_program_space (pspace); minsym_found (state, elem->objfile, elem->minsym, &sals); } @@ -2588,8 +2588,8 @@ compare_msymbols (const void *a, const void *b) struct minimal_symbol * const *sb = b; uintptr_t uia, uib; - uia = (uintptr_t) SYMBOL_OBJ_SECTION (*sa)->objfile->pspace; - uib = (uintptr_t) SYMBOL_OBJ_SECTION (*sb)->objfile->pspace; + uia = (uintptr_t) msymbol_objfile (*sa)->pspace; + uib = (uintptr_t) msymbol_objfile (*sb)->pspace; if (uia < uib) return -1; diff --git a/gdb/minsyms.c b/gdb/minsyms.c index d762b2d..dfc2a55 100644 --- a/gdb/minsyms.c +++ b/gdb/minsyms.c @@ -144,16 +144,18 @@ add_minsym_to_demangled_hash_table (struct minimal_symbol *sym, struct objfile * msymbol_objfile (struct minimal_symbol *sym) { + struct program_space *pspace; struct objfile *objf; struct minimal_symbol *tsym; unsigned int hash = msymbol_hash (SYMBOL_LINKAGE_NAME (sym)) % MINIMAL_SYMBOL_HASH_SIZE; - for (objf = object_files; objf; objf = objf->next) - for (tsym = objf->msymbol_hash[hash]; tsym; tsym = tsym->hash_next) - if (tsym == sym) - return objf; + ALL_PSPACES (pspace) + for (objf = pspace->objfiles; objf; objf = objf->next) + for (tsym = objf->msymbol_hash[hash]; tsym; tsym = tsym->hash_next) + if (tsym == sym) + return objf; /* We should always be able to find the objfile ... */ internal_error (__FILE__, __LINE__, _("failed internal consistency check"));