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

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

[Kevin Buettner]

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)

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

[Kevin Buettner]

On Mar 26,  2:54pm, Elena Zannoni wrote:

> OK, approved. I tested this on solaris, w/ no regressions.
[...]
>  > 	* symtab.c (find_pc_sect_line): Revise method used for finding
>  > 	the ending pc.

Committed.

[MAINT] xcoffread.c

[Elena Zannoni]

Would this change be OK?  Seems like Kevin is the most likely
candidate for questions about xcoff. Both Jim and I are not extremely
familiar with the platform. Kevin can test changes much more easily
too.

Elena


Index: MAINTAINERS
===================================================================
RCS file: /cvs/src/src/gdb/MAINTAINERS,v
retrieving revision 1.90
diff -u -p -r1.90 MAINTAINERS
--- MAINTAINERS 2001/05/07 20:02:26     1.90
+++ MAINTAINERS 2001/05/08 21:36:33
@@ -231,6 +231,8 @@ generic symtabs             Jim Blandy              jimb@cygnus
   stabs reader         Jim Blandy              jimb@cygnus.com
                        Elena Zannoni           ezannoni@cygnus.com
   coff reader          Philippe De Muyter      phdm@macqel.be
+  xcoff reader         Any maintainer can modify this; please send tricky
+                       ones to Kevin Buettner <kevinb@cygnus.com>
   linespec             Jim Blandy              jimb@cygnus.com
                        Elena Zannoni           ezannoni@cygnus.com
                        Fernando Nasser         fnasser@cygnus.com

Re: [MAINT] xcoffread.c

[Kevin Buettner]

On May 8,  5:37pm, Elena Zannoni wrote:

> Would this change be OK?  Seems like Kevin is the most likely
> candidate for questions about xcoff. Both Jim and I are not extremely
> familiar with the platform. Kevin can test changes much more easily
> too.
[...]
> +  xcoff reader         Any maintainer can modify this; please send tricky
> +                       ones to Kevin Buettner <kevinb@cygnus.com>

It's okay with me.

FYI (to other interested parties):  I'm not an xcoff expert, but I do
have an interest in making sure that GDB xcoff support doesn't become
broken.  This is why I've asked that I not be listed as a maintainer,
yet have still requested that the "tricky" modifications be sent to me
for review.  If someone else out there *would like* to be the
maintainer for the xcoff reader, I'm sure something could be worked
out...

Kevin

Re: [MAINT] xcoffread.c

[Elena Zannoni]

I have checked this in.

Elena


Elena Zannoni writes:
 > 
 > Would this change be OK?  Seems like Kevin is the most likely
 > candidate for questions about xcoff. Both Jim and I are not extremely
 > familiar with the platform. Kevin can test changes much more easily
 > too.
 > 
 > Elena
 > 
 > 
 > Index: MAINTAINERS
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/MAINTAINERS,v
 > retrieving revision 1.90
 > diff -u -p -r1.90 MAINTAINERS
 > --- MAINTAINERS 2001/05/07 20:02:26     1.90
 > +++ MAINTAINERS 2001/05/08 21:36:33
 > @@ -231,6 +231,8 @@ generic symtabs             Jim Blandy              jimb@cygnus
 >    stabs reader         Jim Blandy              jimb@cygnus.com
 >                         Elena Zannoni           ezannoni@cygnus.com
 >    coff reader          Philippe De Muyter      phdm@macqel.be
 > +  xcoff reader         Any maintainer can modify this; please send tricky
 > +                       ones to Kevin Buettner <kevinb@cygnus.com>
 >    linespec             Jim Blandy              jimb@cygnus.com
 >                         Elena Zannoni           ezannoni@cygnus.com
 >                         Fernando Nasser         fnasser@cygnus.com
 >