Re: [PATCH 2/5] powerpc64-aix processing xlC generated line table

[Raunaq 12 <raunaq12@in.ibm.com>]

[-- Attachment #1: Type: text/plain, Size: 4844 bytes --]

"Ulrich Weigand" <uweigand@de.ibm.com> wrote on 08/09/2013 03:31:21 AM:
> Raunaq 12 wrote:
> > "Ulrich Weigand" <uweigand@de.ibm.com> wrote on 08/07/2013 08:53:42 PM:
> > > Did you confirm  -via test suite runs-  that:
> > > - this patch improves results when using XLC, and
> > > - this patch does not regress when using GCC?
> >
> > Yes, I did test it. Running the entire test bucket when test cases
> > are compiled with xlc is a pain as many compiler options used to
> > compile the test programs are not compatible with xlc.
>
> Well, the test suite is supposed to handle different compilers,
> but since only very few people test with anything but GCC,
> support for other compilers tends to bit-rot.  As a suggestion
> to help with future maintainability of GDB on AIX, it would be
> nice if you could go through the test suite at some point and
> clean those problems up ...

Agreed, that would greatly help with future maintainablity of GDB on AIX.
Will plan to take that task up once I am done fixing some other
issues faced by GDB on AIX like stabstring support for XLC++ compiled
binaries.

> > So this must be taken care of before we call arrange_linetable to take
> > care of the "line number 0" function markers.
>
> Well, I guess what I'm concerned about is that your patch eliminates
> the possibility for arrange_linetable to see separate functions in
> the line table  (since the line number 0 markers have actually been
> erased).  Since arrange_linetable wants to sort the line table by
> function, I'd have expected this to cause issues.  (Unless there is
> no real reason any more to perform this sorting?)
>
> > So I cannot make these changes in arrange_linetable().
>
> It's probably easier to show what I had been thinking of in the form
> of a patch.   Note that this is completely untested and just intended
> to show the idea:
>
>
> Index: gdb/xcoffread.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/xcoffread.c,v
> retrieving revision 1.114
> diff -u -p -r1.114 xcoffread.c
> --- gdb/xcoffread.c   6 May 2013 19:38:04 -0000   1.114
> +++ gdb/xcoffread.c   8 Aug 2013 21:52:42 -0000
> @@ -438,6 +438,7 @@ arrange_linetable (struct linetable *old
>    struct linetable_entry *fentry;   /* function entry vector */
>    int fentry_size;      /* # of function entries */
>    struct linetable *newLineTb;   /* new line table */
> +  int extra_lines = 0;
>
>  #define NUM_OF_FUNCTIONS 20
>
> @@ -459,6 +460,12 @@ arrange_linetable (struct linetable *old
>       fentry[function_count].line = ii;
>       fentry[function_count].pc = oldLineTb->item[ii].pc;
>       ++function_count;
> +
> +     /* If the function was compiled with XLC, we may have to add an
> +             extra line entry later.  Reserve space for that.  */
> +     if (ii + 1 < oldLineTb->nitems
> +         && oldLineTb->item[ii].pc != oldLineTb->item[ii + 1].pc)
> +       extra_lines++;
>     }
>      }
>
> @@ -475,7 +482,8 @@ arrange_linetable (struct linetable *old
>    newLineTb = (struct linetable *)
>      xmalloc
>      (sizeof (struct linetable) +
> -    (oldLineTb->nitems - function_count) * sizeof (struct
linetable_entry));
> +      ((oldLineTb->nitems - function_count + extra_lines)
> +       * sizeof (struct linetable_entry)));
>
>    /* If line table does not start with a function beginning, copy up
until
>       a function begin.  */
> @@ -490,6 +498,17 @@ arrange_linetable (struct linetable *old
>
>    for (ii = 0; ii < function_count; ++ii)
>      {
> +      /* If the function was compiled with XLC, we may have to add an
> +         extra line to cover the function prologue.  */
> +      jj = fentry[ii].line;
> +      if (jj + 1 < oldLineTb->nitems
> +     && oldLineTb->item[jj].pc != oldLineTb->item[jj + 1].pc)
> +   {
> +     newLineTb->item[newline] = oldLineTb->item[jj];
> +     newLineTb->item[newline].line = oldLineTb->item[jj + 1].line;
> +     newline++;
> +   }
> +
>        for (jj = fentry[ii].line + 1;
>        jj < oldLineTb->nitems && oldLineTb->item[jj].line != 0;
>        ++jj, ++newline)
>
> Does this also solve the problem you were trying to address?

Thanks for taking time to write this Ulrich. This does solve the problem
but one
extra line had to be added to update the number of items in the line table
as
seen below,

-  newLineTb->nitems = oldLineTb->nitems - function_count;
+  newLineTb->nitems = oldLineTb->nitems - function_count + extra_lines;

Added this line and ran the related testcases. Results same as before.

Can we go ahead and check this in?
If so, please find the changelog entry and attached patch below.
---
ChangeLog :-
* xcoffread.c (arrange_linetable): Add fix to correctly handle
line tables generated by XLC compiled binaries.

(See attached file: gdb-7.6-xcoff_xlc_lines.patch)

Thanks,
Raunaq M. Bathija

[-- Attachment #2: gdb-7.6-xcoff_xlc_lines.patch --]
[-- Type: application/octet-stream, Size: 2272 bytes --]

Index: ./gdb/xcoffread.c
===================================================================
--- ./gdb.orig/xcoffread.c	2013-08-12 10:47:58.000000000 +0600
+++ ./gdb/xcoffread.c		2013-08-12 12:46:54.000000000 +0600
@@ -438,6 +438,7 @@
   struct linetable_entry *fentry;	/* function entry vector */
   int fentry_size;		/* # of function entries */
   struct linetable *newLineTb;	/* new line table */
+  int extra_lines = 0;
 
 #define NUM_OF_FUNCTIONS 20
 
@@ -459,6 +460,12 @@
 	  fentry[function_count].line = ii;
 	  fentry[function_count].pc = oldLineTb->item[ii].pc;
 	  ++function_count;
+
+	  /* If the function was compiled with XLC, we may have to add an
+             extra line entry later.  Reserve space for that.  */
+	  if (ii + 1 < oldLineTb->nitems
+	      && oldLineTb->item[ii].pc != oldLineTb->item[ii + 1].pc)
+	    extra_lines++;
 	}
     }
 
@@ -475,7 +482,7 @@
   newLineTb = (struct linetable *)
     xmalloc
     (sizeof (struct linetable) +
-    (oldLineTb->nitems - function_count) * sizeof (struct linetable_entry));
+    (oldLineTb->nitems - function_count + extra_lines) * sizeof (struct linetable_entry));
 
   /* If line table does not start with a function beginning, copy up until
      a function begin.  */
@@ -490,13 +497,26 @@
 
   for (ii = 0; ii < function_count; ++ii)
     {
+      /* If the function was compiled with XLC, we may have to add an
+         extra line to cover the function prologue.  */
+      jj = fentry[ii].line;
+      if (jj + 1 < oldLineTb->nitems
+	  && oldLineTb->item[jj].pc != oldLineTb->item[jj + 1].pc)
+	{
+	  newLineTb->item[newline] = oldLineTb->item[jj];
+	  newLineTb->item[newline].line = oldLineTb->item[jj + 1].line;
+	  newline++;
+	}
+
       for (jj = fentry[ii].line + 1;
 	   jj < oldLineTb->nitems && oldLineTb->item[jj].line != 0;
 	   ++jj, ++newline)
 	newLineTb->item[newline] = oldLineTb->item[jj];
     }
   xfree (fentry);
-  newLineTb->nitems = oldLineTb->nitems - function_count;
+  /* The number of items in the line table must include these
+     extra lines which were added in case of XLC compiled functions.  */
+  newLineTb->nitems = oldLineTb->nitems - function_count + extra_lines;
   return newLineTb;
 }