diff mbox

PR debug/47471 (set prologue_end in .debug_line)

Message ID m3pqp8ii7n.fsf@redhat.com
State New
Headers show

Commit Message

Dodji Seketeli March 30, 2011, 6:19 p.m. UTC 
Hello,

This is about the line program emitted by the compiler into the
.debug_line section, without optimization.  In the example
accompanying the patch below, at the beginning of the function f, the
compiler emits two .loc asm directives that are identical.  The first
one is right before the .cfi_startproc that starts the prologue.  The
second one is before the instructions that copy the variable length
parameters into f's frame.  Both directives do locate instructions
that are in the prologue.

Unfortunately, GDB uses an heuristic that basically considers that the
first opcode of the line program that increments the line register
(even with an increment of zero) marks the end of the prologue.

Effectively, setting a breakpoint to the beginning of f (e.g, "break
f") won't work anymore when we emit this because GDB would then try to
set the breakpoint inside the prologue.

This patch does two things.

First, it avoids emitting two consecutive .loc that are identical.
Strictly speaking that should fix this issue in this particular case.

Second, it emits a '.loc <filenum> <linenum> 0 prologue_end' directive
on the first instruction that doesn't belong to the prologue (i.e,
when the end_prologue debug hook is called).  This sets the
prologue_end register in the .debug_line state machine to true.  After
discussing this with Jan (in CC), it appears that setting the
prologue_end register to true will help GDB to drop the somewhat
non-reliable heuristic it uses to detect the end of the prologue.

I have noticed that the end_prologue debug hook was being called, but
the dwarf back-end hasn't implemented it (on non-vms platforms).  Is
there a particular reason for not implementing this?  Also, I have
noticed that the patch causes the prologue_end directive to be emitted
even when we compile with optimizations.  I am not sure how right that
would be.

Tested on x86_64-unknown-linux-gnu against trunk.

Comments

Richard Henderson March 31, 2011, 2:59 a.m. UTC | #1 
On 03/30/2011 11:19 AM, Dodji Seketeli wrote:
> First, it avoids emitting two consecutive .loc that are identical.
> Strictly speaking that should fix this issue in this particular case.

What's the compatibility strategy?  I.e. how does gdb tell that we're
not using the double-loc mechanism?  Does it scan the line ops and if
you see any prologue_end marks assume that double-loc is not in effect?

Are you actually going to be able to delete any code within gdb, due
to the need to interpret older debug info?

> I have noticed that the end_prologue debug hook was being called, but
> the dwarf back-end hasn't implemented it (on non-vms platforms).  Is
> there a particular reason for not implementing this?  Also, I have
> noticed that the patch causes the prologue_end directive to be emitted
> even when we compile with optimizations.  I am not sure how right that
> would be.

Well, the main problem with the existing end_prologue hook is the
definition of the "end of the prologue".

The hook you see is called *after* the last insn that was emitted
by gen_prologue.

I believe that the hook you want would be called *before* the first
insn that was originally emitted after NOTE_INSN_FUNCTION_BEG.
(Note that under certain conditions, the parameters need to be spilled
to their canonical locations, and other similar "prologue-like" stuff
happens after gen_prologue, and before the "real" function begins.)

Stop me now if I'm mistaken in this understanding of what gdb needs.

Without instruction scheduling, and without the need for parameter
spills, etc, these two definitions are (nearly) the same.  I.e. the
existing hook would likely work for -O0 -g, for many parameter lists.

On the other end, the definition the compiler uses for "begin the
epilogue" is also *before* the first insn that was emitted by
gen_epilogue.  And again I believe that what you want is *after*
the last insn that had been part of the main function body.

One possibility for noticing the debugger-definition of end of
prologue is simply to mark the first line number of the function.
None of the gen_prologue or parameter spill instructions will have
location information attached (I believe).  Thus the first insn 
that does generate a line number ought to be the first "active"
insn of the function.

Presumably one could do something similar at the ends of the
function; though that scan may be a tad more complicated.

Another problem is that we're not supporting these other features
without assembler support.  Which is silly.  I'm going to go ahead
and re-vamp how we're recording line number tables for direct output.
That should allow us to generate more-or-less equivalently capable
.debug_line output by hand.


r~
Jan Kratochvil March 31, 2011, 9:10 a.m. UTC | #2 
On Thu, 31 Mar 2011 04:59:18 +0200, Richard Henderson wrote:
> On 03/30/2011 11:19 AM, Dodji Seketeli wrote:
> > First, it avoids emitting two consecutive .loc that are identical.
> > Strictly speaking that should fix this issue in this particular case.
> 
> What's the compatibility strategy?  I.e. how does gdb tell that we're
> not using the double-loc mechanism?  Does it scan the line ops and if
> you see any prologue_end marks assume that double-loc is not in effect?

Currently GDB ignores / has no support for DW_LNS_set_prologue_end.


> Are you actually going to be able to delete any code within gdb, due
> to the need to interpret older debug info?

No code is going to be deleted from GDB.


For -O2 -g code the prologue does not (will not) need to be known to GDB:
	[patch] Do not skip prologue for -O2 -g with GCC VTA Re: [patch] Fix for PR gdb/12573
	http://sourceware.org/ml/gdb-patches/2011-03/msg01129.html

For -O0 -g code the line number information may be good enough.  For
compatibility with older GDBs the line number tricks should remain in place
and in such case DW_LNS_set_prologue_end is redundant for -O0 -g.

I thought DW_LNS_set_prologue_end would be cleaner for -O0 -g but that may not
be required.


Thanks,
Jan
diff mbox

Patch

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index efd30ea..6f8285c 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -94,6 +94,8 @@  along with GCC; see the file COPYING3.  If not see
 #include "tree-flow.h"
 #include "cfglayout.h"
 
+static void output_source_line_asm_info (unsigned int, const char *,
+					 int, bool, bool);
 static void dwarf2out_source_line (unsigned int, const char *, int, bool);
 static rtx last_var_location_insn;
 
@@ -465,6 +467,7 @@  static void output_call_frame_info (int);
 static void dwarf2out_note_section_used (void);
 static bool clobbers_queued_reg_save (const_rtx);
 static void dwarf2out_frame_debug_expr (rtx, const char *);
+static void dwarf2out_end_prologue (unsigned int, const char*);
 
 /* Support for complex CFA locations.  */
 static void output_cfa_loc (dw_cfi_ref, int);
@@ -4125,6 +4128,21 @@  dwarf2out_begin_prologue (unsigned int line ATTRIBUTE_UNUSED,
     }
 }
 
+/* Implementation of the gcc_debug_hooks::end_prologue hook.
+
+   If the underlying assembler supports it, emit a .loc asm directive
+   with a 'end_prologue' argument, effectively marking the point where
+   debugger should set a breakpoint when requested to set one on a
+   function.  */
+
+static void
+dwarf2out_end_prologue (unsigned int line, const char *filename)
+{
+  output_source_line_asm_info (line, filename, 0,
+			       /*is_stmt=*/true,
+			       /*is_prologue_end*/true);
+}
+
 /* Output a marker (i.e. a label) for the end of the generated code
    for a function prologue.  This gets called *after* the prologue code has
    been generated.  */
@@ -5767,7 +5785,7 @@  const struct gcc_debug_hooks dwarf2_debug_hooks =
   dwarf2out_vms_end_prologue,
   dwarf2out_vms_begin_epilogue,
 #else
-  debug_nothing_int_charstar,
+  dwarf2out_end_prologue,
   debug_nothing_int_charstar,
 #endif
   dwarf2out_end_epilogue,
@@ -22129,14 +22147,22 @@  dwarf2out_begin_function (tree fun)
 
 /* Output a label to mark the beginning of a source code line entry
    and record information relating to this source line, in
-   'line_info_table' for later output of the .debug_line section.  */
+   'line_info_table' for later output of the .debug_line section.
+
+   If the underlying assembler supports the .loc directive, use it
+   instead of emiting .debug_line state machine operations ourselves.
+
+   The arguments of the functions set their counterpart into the
+   .debug_line state machine registers.
+
+   This is a subroutine of dwarf2out_source_line and and
+   dwarf2out_end_prologue.  */
 
 static void
-dwarf2out_source_line (unsigned int line, const char *filename,
-                       int discriminator, bool is_stmt)
+output_source_line_asm_info (unsigned int line, const char *filename,
+			     int discriminator, bool is_stmt,
+			     bool is_prologue_end)
 {
-  static bool last_is_stmt = true;
-
   if (debug_info_level >= DINFO_LEVEL_NORMAL
       && line != 0)
     {
@@ -22151,19 +22177,42 @@  dwarf2out_source_line (unsigned int line, const char *filename,
 
       if (DWARF2_ASM_LINE_DEBUG_INFO)
 	{
-	  /* Emit the .loc directive understood by GNU as.  */
-	  fprintf (asm_out_file, "\t.loc %d %d 0", file_num, line);
-	  if (is_stmt != last_is_stmt)
+	  static int last_file_num = -1;
+	  static unsigned last_line = 0;
+	  static int last_discriminator = -1;
+	  static bool last_is_stmt = true;
+	  static int last_is_prologue_end = -1;
+
+	  if (last_file_num != file_num
+	      || last_line != line
+	      || last_is_stmt != is_stmt
+	      || last_discriminator != discriminator
+	      || last_is_prologue_end != is_prologue_end)
 	    {
-	      fprintf (asm_out_file, " is_stmt %d", is_stmt ? 1 : 0);
-	      last_is_stmt = is_stmt;
-	    }
-	  if (SUPPORTS_DISCRIMINATOR && discriminator != 0)
-	    fprintf (asm_out_file, " discriminator %d", discriminator);
-	  fputc ('\n', asm_out_file);
+	      /* Emit the .loc directive understood by GNU as.  */
+	      fprintf (asm_out_file, "\t.loc %d %d 0", file_num, line);
 
-	  /* Indicate that line number info exists.  */
-	  line_info_table_in_use++;
+	      if (is_stmt != last_is_stmt)
+		{
+		  fprintf (asm_out_file, " is_stmt %d", is_stmt ? 1 : 0);
+		  last_is_stmt = is_stmt;
+		}
+
+	      if (is_prologue_end)
+		fprintf (asm_out_file, " prologue_end");
+
+	      if (SUPPORTS_DISCRIMINATOR && discriminator != 0)
+		fprintf (asm_out_file, " discriminator %d", discriminator);
+	      fputc ('\n', asm_out_file);
+
+	      /* Indicate that line number info exists.  */
+	      line_info_table_in_use++;
+
+	      last_file_num = file_num;
+	      last_line = line;
+	      last_discriminator = discriminator;
+	      last_is_prologue_end = is_prologue_end;
+	    }
 	}
       else if (function_section (current_function_decl) != text_section)
 	{
@@ -22245,6 +22294,18 @@  dwarf2out_start_source_file (unsigned int lineno, const char *filename)
     }
 }
 
+/* Output a label to mark the beginning of a source code line entry
+   and record information relating to this source line, in
+   'line_info_table' for later output of the .debug_line section.  */
+
+static void
+dwarf2out_source_line (unsigned int line, const char *filename,
+                       int discriminator, bool is_stmt)
+{
+  output_source_line_asm_info (line, filename, discriminator,
+			       is_stmt, /*is_prolog_end=*/false);
+}
+
 /* Record the end of a source file.  */
 
 static void