Re: [PATCH RFA] symtab.c: Handle functions with multiple #line directives

[Elena Zannoni <ezannoni at cygnus dot com>]

OK, approved. I tested this on solaris, w/ no regressions.

Elena



Kevin Buettner writes:
 > The patch (way) below fixes a bug in find_pc_sect_line() which is most
 > evident when debugging a function which switches between two (or more)
 > source files multiple times via #line directives in the source.  In
 > order to trigger the bug, it is critical that there be at least two
 > such transitions in a given function.
 > 
 > Now for some background... 
 > 
 > Perl's xsubpp script is used to transform a source (.xs) file
 > containing a mix of C code and other statements into pure C code.  In
 > the resulting C source code, #line directives are used to indicate
 > where the original C code from the .xs file was.  It is common for
 > there to be several transitions back and forth between
 > xsubpp-generated code and the code that appeared in the .xs file. 
 > Here is a small snippet of code taken from the generated "perl.c" from
 > another project that I occasionally work on:
 > 
 >     XS(XS_Vile__Window_window_count)
 >     {
 > 	dXSARGS;
 > 	dXSI32;
 > 	if (items != 0)
 > 	   Perl_croak(aTHX_ "Usage: %s()", GvNAME(CvGV(cv)));
 > 	{
 >     #line 4370 "perl.xs"
 > 	    int count;
 > 	    WINDOW *wp;
 > 
 >     #line 3616 "perl.c"
 > 	    int	RETVAL;
 > 	    dXSTARG;
 >     #line 4374 "perl.xs"
 > 	    count = 0;
 > 	    for_each_visible_window(wp)
 > 		count++;
 > 	    RETVAL = count;
 > 
 >     #line 3625 "perl.c"
 > 	    XSprePUSH; PUSHi((IV)RETVAL);
 > 	}
 > 	XSRETURN(1);
 >     }
 > 
 > Again, the thing to notice here is that the xsubpp tool intermixes code
 > that it has generated with code taken from the .xs file.  It marks each
 > transition with a #line directive.  When debugging this code, the
 > programmer should be able to step from one line to the next.  However,
 > at the moment, GDB is not very accomodating:
 > 
 >     Breakpoint 1, XS_Vile__Window_window_count (cv=0x82014f8) at perl.c:3606
 >     3606        dXSARGS;
 >     (gdb) n
 >     3607        dXSI32;
 >     (gdb) n
 >     3608        if (items != 0)
 >     (gdb) n
 >     3617            dXSTARG;
 >     (gdb) n
 >     3625            XSprePUSH; PUSHi((IV)RETVAL);
 >     (gdb) n
 >     3627        XSRETURN(1);
 > 
 > Note that GDB simply skipped over all the following statements:
 > 
 > 	    count = 0;
 > 	    for_each_visible_window(wp)
 > 		count++;
 > 	    RETVAL = count;
 > 
 > When we debug the same function with the above mentioned symtab.c
 > patch, we see the following (correct) behavior:
 > 
 >     Breakpoint 1, XS_Vile__Window_window_count (cv=0x82014f8) at perl.c:3606
 >     3606        dXSARGS;
 >     (gdb) n
 >     3607        dXSI32;
 >     (gdb) 
 >     3608        if (items != 0)
 >     (gdb) 
 >     3617            dXSTARG;
 >     (gdb) 
 >     4374            count = 0;
 >     (gdb) 
 >     4375            for_each_visible_window(wp)
 >     (gdb) 
 >     4376                count++;
 >     (gdb) 
 >     4375            for_each_visible_window(wp)
 >     (gdb) 
 >     4376                count++;
 >     (gdb) 
 >     4375            for_each_visible_window(wp)
 >     (gdb) 
 >     4377            RETVAL = count;
 >     (gdb) 
 >     3625            XSprePUSH; PUSHi((IV)RETVAL);
 >     (gdb) 
 >     3627        XSRETURN(1);
 > 
 > I have constructed a new test which I am proposing be added to the
 > GDB testsuite to test for this bug.  The proposal is at:
 > 
 >     http://sources.redhat.com/ml/gdb-patches/2001-03/msg00185.html
 > 
 > This test causes 12 new FAILs when run against a current GDB and
 > no fails when patched with the patch below.
 > 
 > I have tested the patch below on i386-unknown-freebsd4.2 and
 > i686-pc-linux-gnu and see no regressions.  Also, as shown above,
 > it certainly produces better results when debugging real code.
 > 
 > Okay to commit?
 > 
 > 	* symtab.c (find_pc_sect_line): Revise method used for finding
 > 	the ending pc.
 > 
 > Index: symtab.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/symtab.c,v
 > retrieving revision 1.32
 > diff -u -p -r1.32 symtab.c
 > --- symtab.c	2001/03/06 08:21:17	1.32
 > +++ symtab.c	2001/03/10 08:22:11
 > @@ -1759,11 +1759,18 @@ find_pc_sect_line (CORE_ADDR pc, struct 
 >  	{
 >  	  best = prev;
 >  	  best_symtab = s;
 > -	  /* If another line is in the linetable, and its PC is closer
 > -	     than the best_end we currently have, take it as best_end.  */
 > -	  if (i < len && (best_end == 0 || best_end > item->pc))
 > -	    best_end = item->pc;
 > +
 > +	  /* Discard BEST_END if it's before the PC of the current BEST.  */
 > +	  if (best_end <= best->pc)
 > +	    best_end = 0;
 >  	}
 > +
 > +      /* If another line (denoted by ITEM) is in the linetable and its
 > +         PC is after BEST's PC, but before the current BEST_END, then
 > +	 use ITEM's PC as the new best_end.  */
 > +      if (best && i < len && item->pc > best->pc
 > +          && (best_end == 0 || best_end > item->pc))
 > +	best_end = item->pc;
 >      }
 >  
 >    if (!best_symtab)
 >