From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21342 invoked by alias); 26 Jun 2006 23:55:42 -0000 Received: (qmail 21331 invoked by uid 22791); 26 Jun 2006 23:55:41 -0000 X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 26 Jun 2006 23:55:38 +0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.12.11.20060308/8.12.11) with ESMTP id k5QNtZ2L021924 for ; Mon, 26 Jun 2006 19:55:36 -0400 Received: from pobox.corp.redhat.com (pobox.corp.redhat.com [172.16.52.156]) by int-mx1.corp.redhat.com (8.12.11.20060308/8.12.11) with ESMTP id k5QNtUw6018231 for ; Mon, 26 Jun 2006 19:55:30 -0400 Received: from localhost.localdomain (vpn50-22.rdu.redhat.com [172.16.50.22]) by pobox.corp.redhat.com (8.12.8/8.12.8) with ESMTP id k5QNtULp018844 for ; Mon, 26 Jun 2006 19:55:30 -0400 Received: from ironwood.lan (ironwood.lan [192.168.64.8]) by localhost.localdomain (8.12.11/8.12.10) with ESMTP id k5QNtUl3016516 for ; Mon, 26 Jun 2006 16:55:30 -0700 Date: Mon, 26 Jun 2006 23:55:00 -0000 From: Kevin Buettner To: gdb-patches@sources.redhat.com Subject: Re: [patch] Fixes problem setting breakpoint in dynamic loader Message-ID: <20060626165529.38ae2903@ironwood.lan> In-Reply-To: <1151357021.7608.100.camel@dufur.beaverton.ibm.com> References: <1148513171.315.104.camel@dufur.beaverton.ibm.com> <20060525022635.GA15026@nevyn.them.org> <1148593970.315.131.camel@dufur.beaverton.ibm.com> <20060525225827.GA13498@nevyn.them.org> <1148594987.315.137.camel@dufur.beaverton.ibm.com> <1151094553.7608.89.camel@dufur.beaverton.ibm.com> <200606232148.k5NLm0Wx009049@elgar.sibelius.xs4all.nl> <1151357021.7608.100.camel@dufur.beaverton.ibm.com> X-Mailer: Sylpheed-Claws 2.0.0 (GTK+ 2.6.10; i386-redhat-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2006-06/txt/msg00382.txt.bz2 On Mon, 26 Jun 2006 14:23:41 -0700 PAUL GILLIAM wrote: > Here is the new patch, with out deleting the 'dot' symbol. I included a > copy of the rs6000 patch as well, just for completeness. I didn't see the rs6000 patch. > OK to commit? Not quite yet... > Index: solib-svr4.c > =================================================================== > RCS file: /cvs/src/src/gdb/solib-svr4.c,v > retrieving revision 1.58 > diff -a -u -r1.58 solib-svr4.c > --- solib-svr4.c 18 May 2006 20:38:56 -0000 1.58 > +++ solib-svr4.c 26 Jun 2006 22:08:43 -0000 > @@ -1043,20 +1043,45 @@ > /* Now try to set a breakpoint in the dynamic linker. */ > for (bkpt_namep = solib_break_names; *bkpt_namep != NULL; bkpt_namep++) > { > - /* On ABI's that use function descriptors, there are usually > - two linker symbols associated with each C function: one > - pointing at the actual entry point of the machine code, > - and one pointing at the function's descriptor. The > - latter symbol has the same name as the C function. > - > - What we're looking for here is the machine code entry > - point, so we are only interested in symbols in code > - sections. */ I have a few suggestions regarding your rewrite of the comment: > + /* What we're looking for here is the machine code entry point, > + so we are only interested in symbols in code sections. > + > + On ABI's that use function descriptors, the linker symbol with ^^^^^ ABIs > + the same name as a C funtion points to that functions descriptor. ^^^^^^^ ^^^^^^^^^ function function's > + When those function descriptors are in the code section, they > + contain executable code and we can set a breakpoint there. */ Also, I don't mind that the comment was rearranged, but I would like to see information regarding the two linker symbols retained in some fashion. > sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep, SEC_CODE); > if (sym_addr != 0) > break; > } > > + if (sym_addr == 0) > + { > + CORE_ADDR sect_offset; > + > + /* No symbol was found in a code section, so look in the data > + sections. This will only happen when the linker symbol points > + to a function descriptor that is in a data section. */ > + for (bkpt_namep = solib_break_names; *bkpt_namep!=NULL; bkpt_namep++) > + { > + /* On ABI's that use function descriptors that are in the data > + section, */ > + sym_addr = bfd_lookup_symbol (tmp_bfd, *bkpt_namep, SEC_DATA); > + if (sym_addr != 0) > + break; > + } Starting from the line immediately below... > + if (sym_addr == 0) > + { > + target_close (tmp_bfd_target, 0); > + goto bkpt_at_symbol; > + } ...through the line immediately above, could we delete those lines and instead just say: if (sym_addr != 0) before the assignment (sym_addr = gdbarch_convert...) below? (This gets rid of the goto and the extra call to target_close().) > + > + /* Convert 'sym_addr' from a function pointer to an address. */ > + sym_addr = gdbarch_convert_from_func_ptr_addr (current_gdbarch, > + sym_addr, > + tmp_bfd_target); > + } > + > /* We're done with both the temporary bfd and target. Remember, > closing the target closes the underlying bfd. */ > target_close (tmp_bfd_target, 0); With my suggested changes above, I think this is okay. I'd like to see another patch posted to this list though prior to committing... Thanks, Kevin