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

["Ulrich Weigand" <uweigand@de.ibm.com>]

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 ...
 
> on Running -
> gmake check RUNTESTFLAGS='"gdb.base/*break*.exp"'
> 
> with xlc:-
> Without patch applied- 131 PASS | 43 FAIL
> With patch applied-    169 PASS | 5 FAIL
> 
> This is because GDB wrongly interprets function entry points
> in case of xlc compiled binaries as the line table is different.
> 
> No regressions with gcc also confirmed.
> With/Without patch applied-  172 PASS | 5 FAIL
> 
> Tested the same way for list.exp as well. Plus ran the overall
> gdb.base bucket with and without patch applied, results were
> identical.

OK, excellent!

> arrange_linetable basically takes a line table, removes the entries
> marked as line number 0 as they are to indicate function entry points
> and returns the new table without these "line number 0" entries.
> 
> The function I have added- modify_xlc_linenos is for adding additional
> entries into the line table which will have the PC for start of
> a function (this is already there for gcc) and the line number as equal
> to the immediate next line table entry.
> 
> 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?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com