From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14037 invoked by alias); 21 Mar 2002 19:03:48 -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 13952 invoked from network); 21 Mar 2002 19:03:46 -0000 Received: from unknown (HELO zwingli.cygnus.com) (208.245.165.35) by sources.redhat.com with SMTP; 21 Mar 2002 19:03:46 -0000 Received: by zwingli.cygnus.com (Postfix, from userid 442) id B96675EA12; Thu, 21 Mar 2002 14:03:44 -0500 (EST) To: Daniel Jacobowitz Cc: fnf@redhat.com, Mark Kettenis , gdb-patches@sources.redhat.com Subject: Re: [RFA/stabs] Fix for line table problems (was: Re: [RFC] Gdb line table implementation tweak) References: <200202272320.g1RNK5e14347@fishpond.ninemoons.com> <20020317014816.B1589@nevyn.them.org> From: Jim Blandy Date: Thu, 21 Mar 2002 11:03:00 -0000 In-Reply-To: <20020317014816.B1589@nevyn.them.org> Message-ID: User-Agent: Gnus/5.09 (Gnus v5.9.0) Emacs/21.1 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-SW-Source: 2002-03/txt/msg00409.txt.bz2 This looks sound to me. Let's put it in and see what happens. Daniel Jacobowitz writes: > [This is long. Feel free to skip to the patch at the very end, and > stop to take a gander at the testsuite numbers in the middle :)] > > On Wed, Feb 27, 2002 at 04:20:05PM -0700, Fred Fish wrote: > > > Fred, could you look into these failures? > > > > Yes, but I would need access to a host system that exhibits > > the failures. Given a pointer and access to such a system, > > I can try and see what is going on. > > I've got it. It's almost exactly like the bug I fixed in binutils > addr2line a few hours ago. We do not have an N_SLINE at the beginning > of the function. Our starting lines for functions have, as a result, > always been a little odd... > > So how do we figure out where we are? There's two ways. GCC gives us > the line number of the start of the function in the N_FUN, but other > compilers don't. Also, every function should contain at least one > line number note. We could assume we have the GCC-style line numbering > information if we had an ending marker for the function, but that's an > assumption. Or we could do it this way: If we are inside a function, > and we found our first line note, use the start address of the function > instead of the address on the line note. > > If we do it the first way, we also fix various problems with inlining > (reported to this list a couple of weeks ago). But there's an > assumption in there I really dislike making. Perhaps we could trigger > that off processing_gcc_compilation, hackish as it is, and gain a few > more improvements in addition to this patch. For now, I went with the > second way instead. > > Can anyone think of a possible problem? I suppose this might cause > more badness with the section-jumping creativity that the Linux kernel > is so fond of, if it happened to be the very first thing in the > function, but I believe we should always have a line note before any of > that happens. > > The results from this patch are very good. In fact, I was astonished. > I'm considering adapting a similar method for DWARF-2; it's less often > necessary but definitely applicable, because the line table does not > indicate function boundaries. > > I got the lowest number of unexpected failures I've ever seen with this > patch. I've some way to go with C++/Java, and there's the heap of MI > tests that have been failing as long as I've been running regular > testsuites, and we still haven't resolved the MAYBE_PROTOTYPED and > related changes for the eight or ten failures that causes across my > testsuite runs; but hitting 0 is in sight again now. > > Testsuite numbers (across {gcc 2.95.3, gcc 3.0.4} {-gstabs+, -gdwarf-2}): > > Current CVS, with Fred's original patch reverted: > === gdb Summary === > > # of expected passes 31991 > # of unexpected failures 358 > # of unexpected successes 108 > # of expected failures 399 > # of unresolved testcases 200 > # of untested testcases 24 > # of unsupported tests 8 > > Current CVS, which includes Fred's original patch: > === gdb Summary === > > # of expected passes 31939 > # of unexpected failures 408 > # of unexpected successes 108 > # of expected failures 392 > # of unresolved testcases 212 > # of untested testcases 24 > # of unsupported tests 8 > > With the attached patch: > === gdb Summary === > > # of expected passes 32120 > # of unexpected failures 270 > # of unexpected successes 108 > # of expected failures 399 > # of unresolved testcases 100 > # of untested testcases 24 > # of unsupported tests 8 > > > The only worrysome results were (2.95.3,stabs): > +FAIL: gdb.base/reread.exp: second pass: breakpoint foo in first file > > (2.95.3, dwarf-2) > +FAIL: gdb.base/scope.exp: running to foo in runto > +FAIL: gdb.base/scope.exp: running to bar in runto > +FAIL: gdb.base/scope.exp: setting breakpoint at localscopes > > The reread failure is present with Fred's patch or with Fred's and > mine; it looks like: > Reading symbols from /opt/src/binutils/x86-as/gdb/testsuite/gdb.base/reread...done. > -Breakpoint 1 at 0x80483df: file ../../../src/gdb/testsuite/gdb.base/reread1.c, line 14. > +Breakpoint 1 at 0x80483dc > (gdb) break foo > -Note: breakpoint 1 also set at pc 0x80483df. > -Breakpoint 2 at 0x80483df: file ../../../src/gdb/testsuite/gdb.base/reread1.c, line 14. > -(gdb) PASS: gdb.base/reread.exp: second pass: breakpoint foo in first file > +Note: breakpoint 1 also set at pc 0x80483dc. > +Breakpoint 2 at 0x80483dc > +(gdb) FAIL: gdb.base/reread.exp: second pass: breakpoint foo in first file > run > The program being debugged has been started already. > Start it from the beginning? (y or n) y > Starting program: /opt/src/binutils/x86-as/gdb/testsuite/gdb.base/reread > +Breakpoint 1 at 0x80483df: file ../../../src/gdb/testsuite/gdb.base/reread1.c, line 14. > +Breakpoint 2 at 0x80483df: file ../../../src/gdb/testsuite/gdb.base/reread1.c, line 14. > > > That is, after rereading, we think that 0x80483dc is out of function - > and we break there rather than at 0x80483df, where we used to. > > > scope.exp replaces hitting breakpoint 2 with: > +Program terminated with signal SIGKILL, Killed. > +The program no longer exists. > +You can't do that without a process to debug. > +(gdb) FAIL: gdb.base/scope.exp: running to foo in runto > > Which appears to have been temporary, since I can't reproduce it any > more with the exact same binaries. > > We seem to be much better at stopping after the prologue with this > patch. And some other breakpoints get earlier. This is because of > changes like: > -Line 13 of "../../../src/gdb/testsuite/gdb.base/ending-run.c" is at > address 0x8048496 but contains no code. > +Line 13 of "../../../src/gdb/testsuite/gdb.base/ending-run.c" starts > at address 0x8048490 and ends at 0x8048496 . > > i.e. the first N_SLINE was placed too late and we now correctly move > it. No breakpoints changed PC in the entire testsuite with GCC 3.x, so > I'm satisfied as to the correctness of the change. I'd appreciate > testsuite results with a non-GCC compiler. > > > > And after all that - OK to commit? > > > -- > Daniel Jacobowitz Carnegie Mellon University > MontaVista Software Debian GNU/Linux Developer > > 2002-03-17 Daniel Jacobowitz > > * dbxread.c (process_one_symbol): Extend the first N_SLINE > in a function to cover the entire beginning of the function > as well if it does not already. > > Index: dbxread.c > =================================================================== > RCS file: /cvs/src/src/gdb/dbxread.c,v > retrieving revision 1.30 > diff -u -p -r1.30 dbxread.c > --- dbxread.c 2002/02/22 00:17:13 1.30 > +++ dbxread.c 2002/03/17 06:41:21 > @@ -2707,6 +2707,15 @@ process_one_symbol (int type, int desc, > used to relocate these symbol types rather than SECTION_OFFSETS. */ > static CORE_ADDR function_start_offset; > > + /* This holds the address of the start of a function, without the system > + peculiarities of function_start_offset. */ > + static CORE_ADDR last_function_start; > + > + /* If this is nonzero, we've seen an N_SLINE since the start of the current > + function. Initialized to nonzero to assure that last_function_start > + is never used uninitialized. */ > + static int sline_found_in_function = 1; > + > /* If this is nonzero, we've seen a non-gcc N_OPT symbol for this source > file. Used to detect the SunPRO solaris compiler. */ > static int n_opt_found; > @@ -2758,9 +2767,13 @@ process_one_symbol (int type, int desc, > break; > } > > + sline_found_in_function = 0; > + > /* Relocate for dynamic loading */ > valu += ANOFFSET (section_offsets, SECT_OFF_TEXT (objfile)); > valu = SMASH_TEXT_ADDRESS (valu); > + last_function_start = valu; > + > goto define_a_symbol; > > case N_LBRAC: > @@ -2953,7 +2966,15 @@ process_one_symbol (int type, int desc, > #ifdef SUN_FIXED_LBRAC_BUG > last_pc_address = valu; /* Save for SunOS bug circumcision */ > #endif > - record_line (current_subfile, desc, valu); > + /* If this is the first SLINE note in the function, record it at > + the start of the function instead of at the listed location. */ > + if (within_function && sline_found_in_function == 0) > + { > + record_line (current_subfile, desc, last_function_start); > + sline_found_in_function = 1; > + } > + else > + record_line (current_subfile, desc, valu); > break; > > case N_BCOMM: