From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14092 invoked by alias); 9 Nov 2003 00:13:00 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 14083 invoked from network); 9 Nov 2003 00:12:59 -0000 Received: from unknown (HELO nevyn.them.org) (66.93.172.17) by sources.redhat.com with SMTP; 9 Nov 2003 00:12:59 -0000 Received: from drow by nevyn.them.org with local (Exim 4.24 #1 (Debian)) id 1AIdCM-0000LK-Ci; Sat, 08 Nov 2003 19:12:58 -0500 Date: Sun, 09 Nov 2003 00:13:00 -0000 From: Daniel Jacobowitz To: Andrew Cagney Cc: gdb-patches@sources.redhat.com Subject: Re: [rfa:symtab] deprecate inside_entry_func Message-ID: <20031109001258.GA1138@nevyn.them.org> Mail-Followup-To: Andrew Cagney , gdb-patches@sources.redhat.com References: <3FA2F940.5040102@redhat.com> <20031101004233.GA11987@nevyn.them.org> <3FA31C64.9060306@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3FA31C64.9060306@gnu.org> User-Agent: Mutt/1.5.1i X-SW-Source: 2003-11/txt/msg00158.txt.bz2 On Fri, Oct 31, 2003 at 09:37:24PM -0500, Andrew Cagney wrote: > >On Fri, Oct 31, 2003 at 07:07:28PM -0500, Andrew Cagney wrote: > > > >>Hello, > >> > >>This patch deprecates the function inside_entry_func. Per the new > >>comments: > >> > >>+ /* NOTE: cagney/2003-10-31: A very simple test, such as > >>+ get_frame_func == entry_point should be sufficient for > >>+ identifying a pc in the entry function. Does anyone know why it > >>+ wasn't sufficient and hence, why the very convoluted > >>+ "deprecated_inside_entry_func" is needed. */ > >>+ /* NOTE: cagney/2003-10-31: An ABI and its crt0 code should define > >>+ and implement a clean frame termination. Not doing that is > >>+ really a bug in the ABI/crt0, and, hence, not a reason for > >>+ enabling the call to deprecated_inside_entry_func. */ > > > > > >If handling broken systems isn't the job of GDB, then I surely don't > >know what is. > >Do you have a reason for wanting to deprecate this function? > > Per my first new comment: > > /* NOTE: cagney/2003-10-31: A very simple test, such as > get_frame_func == entry_point should be sufficient for > identifying a pc in the entry function. Does anyone know why it > wasn't sufficient and hence, why the very convoluted > "deprecated_inside_entry_func" is needed. */ > > and the previous comment: > > /* NOTE: cagney/2003-02-25: Don't enable until someone has found > hard evidence that this is needed. */ > > There is _still_ no evidence that this disabled hack is needed - time to > deprecate it. If you think about it for a little while, and if you consider my objection to the comment about broken ABIs above, constructing a testcase is really quite easy. Here's one, though it's contrived. Bear with me. Take gdb.asm/asm-source (or anything else, really), for an i386 target, and this script: b _start r bt set $ebp = 2 bt Here's what GDB produces: Using host libthread_db library "/lib/tls/i686/cmov/libthread_db.so.1". [Hrm, is there a missing from_tty somewhere? This seems superfluous.] Breakpoint 1 at 0x8048138: file /opt/src/gdb/src/gdb/testsuite/gdb.asm/asmsrc1.s, line 14. Breakpoint 1, _start () at /opt/src/gdb/src/gdb/testsuite/gdb.asm/asmsrc1.s:14 14 gdbasm_startup Current language: auto; currently asm #0 _start () at /opt/src/gdb/src/gdb/testsuite/gdb.asm/asmsrc1.s:14 #0 _start () at /opt/src/gdb/src/gdb/testsuite/gdb.asm/asmsrc1.s:14 #1 0x00000001 in ?? () #2 0xbffff680 in ?? () Apply this patch: Index: frame.c =================================================================== RCS file: /cvs/src/src/gdb/frame.c,v retrieving revision 1.147 diff -u -p -r1.147 frame.c --- frame.c 24 Oct 2003 17:37:03 -0000 1.147 +++ frame.c 8 Nov 2003 23:58:31 -0000 @@ -1831,7 +1831,7 @@ get_prev_frame (struct frame_info *this_ /* NOTE: cagney/2003-07-15: Need to add a "set backtrace beyond-entry-func" command so that this can be selectively disabled. */ - if (0 + if (1 #if 0 && backtrace_beyond_entry_func #endif and GDB's output isn't changed. That's because of the hokeyness of figuring out the bounds of the entry function. Apply this patch instead, though (proof of concept only; it needs to be cleaned up and inside_entry_func's signature changed, but it illustrates the point): Index: blockframe.c =================================================================== RCS file: /cvs/src/src/gdb/blockframe.c,v retrieving revision 1.82 diff -u -p -r1.82 blockframe.c --- blockframe.c 20 Oct 2003 14:38:42 -0000 1.82 +++ blockframe.c 8 Nov 2003 23:58:30 -0000 @@ -186,6 +186,9 @@ inside_entry_func (CORE_ADDR pc) if (DEPRECATED_PC_IN_CALL_DUMMY (pc, 0, 0)) return 0; } + if (symfile_objfile->ei.entry_point == pc) + return 1; + return (symfile_objfile->ei.entry_func_lowpc <= pc && symfile_objfile->ei.entry_func_highpc > pc); } @@ -615,7 +618,7 @@ legacy_frame_chain_valid (CORE_ADDR fp, /* If we're already inside the entry function for the main objfile, then it isn't valid. */ - if (inside_entry_func (get_frame_pc (fi))) + if (inside_entry_func (get_frame_func (fi))) return 0; /* If we're inside the entry file, it isn't valid. */ Index: frame.c =================================================================== RCS file: /cvs/src/src/gdb/frame.c,v retrieving revision 1.147 diff -u -p -r1.147 frame.c --- frame.c 24 Oct 2003 17:37:03 -0000 1.147 +++ frame.c 8 Nov 2003 23:58:31 -0000 @@ -1831,12 +1831,12 @@ get_prev_frame (struct frame_info *this_ /* NOTE: cagney/2003-07-15: Need to add a "set backtrace beyond-entry-func" command so that this can be selectively disabled. */ - if (0 + if (1 #if 0 && backtrace_beyond_entry_func #endif && this_frame->type != DUMMY_FRAME && this_frame->level >= 0 - && inside_entry_func (get_frame_pc (this_frame))) + && inside_entry_func (get_frame_func (this_frame))) { if (frame_debug) { and we get this output: Using host libthread_db library "/lib/tls/i686/cmov/libthread_db.so.1". Breakpoint 1 at 0x8048138: file /opt/src/gdb/src/gdb/testsuite/gdb.asm/asmsrc1.s, line 14. Breakpoint 1, _start () at /opt/src/gdb/src/gdb/testsuite/gdb.asm/asmsrc1.s:14 14 gdbasm_startup Current language: auto; currently asm #0 _start () at /opt/src/gdb/src/gdb/testsuite/gdb.asm/asmsrc1.s:14 #0 _start () at /opt/src/gdb/src/gdb/testsuite/gdb.asm/asmsrc1.s:14 Which is, I think, an obvious improvement. If the object file has an entry point, by default we should not try to backtrace past it. The extra "set backtrace" option mentioned in your comment above is probably called for. Think my testcase was too contrived? It's possible to rewrite asm-source in a number of ways to acheive similar effect. I believe I've also produced this result in the ARM simulator using newlib, but I don't have that setup around to recreate it at the moment. You could probably do it using static linking and -fomit-frame-pointer, except that GDB on my host system gets lost in __libc_start_main in that case and never gets to _start. I believe the function should be fixed and re-enabled, not deleted. -- Daniel Jacobowitz MontaVista Software Debian GNU/Linux Developer