[RFA/stabs] Fix for line table problems (was: Re: [RFC] Gdb line table implementation tweak)

Daniel Jacobowitz
2002-03-17T06:51:32Z

[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 <callee+6> but contains no code.
+Line 13 of "../../../src/gdb/testsuite/gdb.base/ending-run.c" starts
at address 0x8048490 <callee> and ends at 0x8048496 <callee+6>.

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 <drow@mvista.com>

* 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;