This is the mail archive of the gdb@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RFA: Line number fix for prologues


This is one of two patches to "fix" GCC-generated debug line information.
In a perfect world neither of these are really necessary; but with
var-tracking having missed 3.4, they will continue to be useful.

The two patches together have been bootstrapped and tested on i686-linux,
and tested vs. GDB on arm-elf and i686-linux.  Short version: they're an
improvement.  This is a regression from 3.3.

This patch:
  - Makes final_scan_insn static
  - Removes the initialization of a possibly large array that is no longer
    used
  - Forces us to emit a line marker after we have seen the beginning of a
    second basic block (if we haven't yet; this is just my paranoia talking)
    or after we have seen both NOTE_INSN_FUNCTION_BEG and
    NOTE_INSN_PROLOGUE_END.

The patch is necessary because of Honza's changes for locators in June.  It
used to be that we always emitted line notes at such notes, but now we don't
any more - we only emit them when the line numbers change.  Sometimes the
line numbers don't change, for instance for all-on-one-line functions. 
Other times the change is in the wrong place.

I was going to just use NOTE_INSN_FUNCTION_BEG, but because notes are not
part of basic blocks in the CFG, sometimes it floats to before the prologue;
i.e. even with a non-empty prologue, the NOTE_INSN_FUNCTION_BEG comes before
the first prologue insn.  Sometimes even the NOTE_INSN_FUNCTION_END does :)
So I wait until both have been emitted and then force a line change.

OK for 3.4?

===
Longer version of the test results, with a question about emit_nop:

For i686-linux the changes were:
  11 FAIL -> PASS
  6 KFAIL -> PASS
  5 XFAIL -> PASS
  4 PASS -> FAIL
  + a bunch of new tests were able to run, and PASSed

For arm-elf the numbers were more extreme and there were no regressions. 
For i686-linux the regressions were:

-PASS: gdb.base/break.exp: breakpoint at start of multi line while conditional
-PASS: gdb.base/break.exp: breakpoint info
+FAIL: gdb.base/break.exp: breakpoint at start of multi line while conditional
+FAIL: gdb.base/break.exp: breakpoint info
-PASS: gdb.base/sepdebug.exp: breakpoint at start of multi line while conditional
-PASS: gdb.base/sepdebug.exp: breakpoint info
+FAIL: gdb.base/sepdebug.exp: breakpoint at start of multi line while conditional
+FAIL: gdb.base/sepdebug.exp: breakpoint info

The problem is that the function looks like this with the unpatched compiler:

0804851c <multi_line_while_conditional>:
 804851c:       55                      push   %ebp
 804851d:       89 e5                   mov    %esp,%ebp
 804851f:       90                      nop    
 8048520:       83 7d 08 00             cmpl   $0x0,0x8(%ebp)
 8048524:       74 1b                   je     8048541 <multi_line_while_conditional+0x25>

  Special opcode 83: advance Address by 5 to 0x804851c and Line by 8 to 138
  Special opcode 62: advance Address by 4 to 0x8048520 and Line by 1 to 139
  Advance PC by constant 17 to 0x8048531
  Special opcode 23: advance Address by 1 to 0x8048532 and Line by 4 to 143

With the patched compiler:

  Special opcode 83: advance Address by 5 to 0x804851c and Line by 8 to 138
  Special opcode 47: advance Address by 3 to 0x804851f and Line by 0 to 138
  Special opcode 20: advance Address by 1 to 0x8048520 and Line by 1 to 139
  Advance PC by constant 17 to 0x8048531
  Special opcode 23: advance Address by 1 to 0x8048532 and Line by 4 to 143

i.e we now emit a line note at the end of the prologue, which puts us on the
padding NOP.  The padding nop has no location associated with it so it
points at the open brace, just like the prologue.  GDB expects us to point
at the while loop.

The NOP comes from:

void
genrtl_while_stmt (tree t)
{ 
  tree cond = WHILE_COND (t);

  emit_nop ();				<-----
  emit_line_note (input_location);
  expand_start_loop (1);
  genrtl_do_pushlevel ();

My silly question is, does that emit_nop () even serve any purpose?  I
imagine it was there to make the loop optimizer happy once upon a time but
the emit hasn't been touched in ten years.  I haven't bootstrapped without it,
but removing it fixes the new FAILs and reduces the size of unoptimized
code.

===


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2004-01-15  Daniel Jacobowitz  <drow@mvista.com>

	* final.c (SEEN_BB, SEEN_NOTE, SEEN_EMITTED): Define.
	(final_scan_insn): Update to take an additional SEEN argument.  Emit
	a line note after the prologue.  Make static.
	(line_note_exists): Remove.
	(final): Don't initialize line_note_exists.  Update call to
	final_scan_insn.
	* output.h (final_scan_insn): Remove prologue.
	* function.c (set_insn_locators): Update comment.
	(thread_prologue_and_epilogue_insns): Add a comment.

Index: final.c
===================================================================
RCS file: /big/fsf/rsync/gcc-cvs/gcc/gcc/final.c,v
retrieving revision 1.293.4.1
diff -u -p -r1.293.4.1 final.c
--- final.c	23 Dec 2003 22:07:54 -0000	1.293.4.1
+++ final.c	15 Jan 2004 20:15:09 -0000
@@ -111,6 +111,11 @@ Software Foundation, 59 Temple Place - S
 #define HAVE_READONLY_DATA_SECTION 0
 #endif
 
+/* Bitflags used by final_scan_insn.  */
+#define SEEN_BB		1
+#define SEEN_NOTE	2
+#define SEEN_EMITTED	4
+
 /* Last insn processed by final_scan_insn.  */
 static rtx debug_insn;
 rtx current_output_insn;
@@ -201,10 +206,6 @@ rtx final_sequence;
 static int dialect_number;
 #endif
 
-/* Indexed by line number, nonzero if there is a note for that line.  */
-
-static char *line_note_exists;
-
 #ifdef HAVE_conditional_execution
 /* Nonnull if the insn currently being emitted was a COND_EXEC pattern.  */
 rtx current_insn_predicate;
@@ -234,6 +235,7 @@ static int final_addr_vec_align (rtx);
 #ifdef HAVE_ATTR_length
 static int align_fuzz (rtx, rtx, int, unsigned);
 #endif
+static rtx final_scan_insn (rtx, FILE *, int, int, int, int *);
 
 /* Initialize data in final at the beginning of a compilation.  */
 
@@ -1506,16 +1508,15 @@ void
 final (rtx first, FILE *file, int optimize, int prescan)
 {
   rtx insn;
-  int max_line = 0;
   int max_uid = 0;
+  int seen = 0;
 
   last_ignored_compare = 0;
 
-  /* Make a map indicating which line numbers appear in this function.
-     When producing SDB debugging info, delete troublesome line number
+#ifdef SDB_DEBUGGING_INFO
+  /* When producing SDB debugging info, delete troublesome line number
      notes from inlined functions in other files as well as duplicate
      line number notes.  */
-#ifdef SDB_DEBUGGING_INFO
   if (write_symbols == SDB_DEBUG)
     {
       rtx last = 0;
@@ -1524,34 +1525,22 @@ final (rtx first, FILE *file, int optimi
 	  {
 	    if ((RTX_INTEGRATED_P (insn)
 		 && strcmp (NOTE_SOURCE_FILE (insn), main_input_filename) != 0)
-		 || (last != 0
-		     && NOTE_LINE_NUMBER (insn) == NOTE_LINE_NUMBER (last)
-		     && NOTE_SOURCE_FILE (insn) == NOTE_SOURCE_FILE (last)))
+		|| (last != 0
+		    && NOTE_LINE_NUMBER (insn) == NOTE_LINE_NUMBER (last)
+		    && NOTE_SOURCE_FILE (insn) == NOTE_SOURCE_FILE (last)))
 	      {
 		delete_insn (insn);	/* Use delete_note.  */
 		continue;
 	      }
 	    last = insn;
-	    if (NOTE_LINE_NUMBER (insn) > max_line)
-	      max_line = NOTE_LINE_NUMBER (insn);
 	  }
     }
-  else
 #endif
-    {
-      for (insn = first; insn; insn = NEXT_INSN (insn))
-	if (GET_CODE (insn) == NOTE && NOTE_LINE_NUMBER (insn) > max_line)
-	  max_line = NOTE_LINE_NUMBER (insn);
-    }
-
-  line_note_exists = xcalloc (max_line + 1, sizeof (char));
 
   for (insn = first; insn; insn = NEXT_INSN (insn))
     {
       if (INSN_UID (insn) > max_uid)       /* Find largest UID.  */
 	max_uid = INSN_UID (insn);
-      if (GET_CODE (insn) == NOTE && NOTE_LINE_NUMBER (insn) > 0)
-	line_note_exists[NOTE_LINE_NUMBER (insn)] = 1;
 #ifdef HAVE_cc0
       /* If CC tracking across branches is enabled, record the insn which
 	 jumps to each branch only reached from one place.  */
@@ -1587,11 +1576,8 @@ final (rtx first, FILE *file, int optimi
 	insn_current_address = INSN_ADDRESSES (INSN_UID (insn));
 #endif /* HAVE_ATTR_length */
 
-      insn = final_scan_insn (insn, file, optimize, prescan, 0);
+      insn = final_scan_insn (insn, file, optimize, prescan, 0, &seen);
     }
-
-  free (line_note_exists);
-  line_note_exists = NULL;
 }
 
 const char *
@@ -1651,11 +1637,18 @@ output_alternate_entry_point (FILE *file
    Value returned is the next insn to be scanned.
 
    NOPEEPHOLES is the flag to disallow peephole processing (currently
-   used for within delayed branch sequence output).  */
+   used for within delayed branch sequence output).
 
-rtx
+   SEEN is used to track the end of the prologue, for emitting
+   debug information.  We force the emission of a line note after
+   both NOTE_INSN_PROLOGUE_END and NOTE_INSN_FUNCTION_BEG, or
+   at the beginning of the second basic block, whichever comes
+   first.  */
+
+static rtx
 final_scan_insn (rtx insn, FILE *file, int optimize ATTRIBUTE_UNUSED,
-		 int prescan, int nopeepholes ATTRIBUTE_UNUSED)
+		 int prescan, int nopeepholes ATTRIBUTE_UNUSED,
+		 int *seen)
 {
 #ifdef HAVE_cc0
   rtx set;
@@ -1694,6 +1687,15 @@ final_scan_insn (rtx insn, FILE *file, i
 	  if (flag_debug_asm)
 	    fprintf (asm_out_file, "\t%s basic block %d\n",
 		     ASM_COMMENT_START, NOTE_BASIC_BLOCK (insn)->index);
+
+	  if ((*seen & (SEEN_EMITTED | SEEN_BB)) == SEEN_BB)
+	    {
+	      *seen |= SEEN_EMITTED;
+	      last_filename = NULL;
+	    }
+	  else
+	    *seen |= SEEN_BB;
+
 	  break;
 
 	case NOTE_INSN_EH_REGION_BEG:
@@ -1709,6 +1711,15 @@ final_scan_insn (rtx insn, FILE *file, i
 	case NOTE_INSN_PROLOGUE_END:
 	  (*targetm.asm_out.function_end_prologue) (file);
 	  profile_after_prologue (file);
+
+	  if ((*seen & (SEEN_EMITTED | SEEN_NOTE)) == SEEN_NOTE)
+	    {
+	      *seen |= SEEN_EMITTED;
+	      last_filename = NULL;
+	    }
+	  else
+	    *seen |= SEEN_NOTE;
+
 	  break;
 
 	case NOTE_INSN_EPILOGUE_BEG:
@@ -1718,6 +1729,15 @@ final_scan_insn (rtx insn, FILE *file, i
 	case NOTE_INSN_FUNCTION_BEG:
 	  app_disable ();
 	  (*debug_hooks->end_prologue) (last_linenum, last_filename);
+
+	  if ((*seen & (SEEN_EMITTED | SEEN_NOTE)) == SEEN_NOTE)
+	    {
+	      *seen |= SEEN_EMITTED;
+	      last_filename = NULL;
+	    }
+	  else
+	    *seen |= SEEN_NOTE;
+
 	  break;
 
 	case NOTE_INSN_BLOCK_BEG:
@@ -2091,7 +2111,7 @@ final_scan_insn (rtx insn, FILE *file, i
 	       thought unnecessary.  If that happens, cancel this sequence
 	       and cause that insn to be restored.  */
 
-	    next = final_scan_insn (XVECEXP (body, 0, 0), file, 0, prescan, 1);
+	    next = final_scan_insn (XVECEXP (body, 0, 0), file, 0, prescan, 1, seen);
 	    if (next != XVECEXP (body, 0, 1))
 	      {
 		final_sequence = 0;
@@ -2105,7 +2125,7 @@ final_scan_insn (rtx insn, FILE *file, i
 		/* We loop in case any instruction in a delay slot gets
 		   split.  */
 		do
-		  insn = final_scan_insn (insn, file, 0, prescan, 1);
+		  insn = final_scan_insn (insn, file, 0, prescan, 1, seen);
 		while (insn != next);
 	      }
 #ifdef DBR_OUTPUT_SEQEND
@@ -2309,7 +2329,7 @@ final_scan_insn (rtx insn, FILE *file, i
 
 		for (note = NEXT_INSN (insn); note != next;
 		     note = NEXT_INSN (note))
-		  final_scan_insn (note, file, optimize, prescan, nopeepholes);
+		  final_scan_insn (note, file, optimize, prescan, nopeepholes, seen);
 
 		/* In case this is prescan, put the notes
 		   in proper position for later rescan.  */
Index: function.c
===================================================================
RCS file: /big/fsf/rsync/gcc-cvs/gcc/gcc/function.c,v
retrieving revision 1.463.4.2
diff -u -p -r1.463.4.2 function.c
--- function.c	31 Dec 2003 00:44:09 -0000	1.463.4.2
+++ function.c	15 Jan 2004 20:05:02 -0000
@@ -7254,7 +7254,7 @@ record_insns (rtx insns, varray_type *ve
     }
 }
 
-/* Set the specified locator to the insn chain.  */
+/* Set the locator of the insn chain starting at INSN to LOC.  */
 static void
 set_insn_locators (rtx insn, int loc)
 {
@@ -7895,6 +7895,7 @@ epilogue_done:
 #endif
 
 #ifdef HAVE_prologue
+  /* This is probably all useless now that we use locators.  */
   if (prologue_end)
     {
       rtx insn, prev;
Index: output.h
===================================================================
RCS file: /big/fsf/rsync/gcc-cvs/gcc/gcc/output.h,v
retrieving revision 1.133
diff -u -p -r1.133 output.h
--- output.h	1 Oct 2003 22:57:57 -0000	1.133
+++ output.h	15 Jan 2004 20:00:56 -0000
@@ -68,11 +68,6 @@ extern void final_end_function (void);
 /* Output assembler code for some insns: all or part of a function.  */
 extern void final (rtx, FILE *, int, int);
 
-/* The final scan for one insn, INSN.  Args are same as in `final', except
-   that INSN is the insn being scanned.  Value returned is the next insn to
-   be scanned.  */
-extern rtx final_scan_insn (rtx, FILE *, int, int, int);
-
 /* Replace a SUBREG with a REG or a MEM, based on the thing it is a
    subreg of.  */
 extern rtx alter_subreg (rtx *);


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]