Re: [PATCH] PR debug/47471 (set prologue_end in .debug_line)

2011-03-31 Thread Jan Kratochvil
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


Re: [PATCH] PR debug/47471 (set prologue_end in .debug_line)

2011-03-31 Thread Dodji Seketeli
Richard,

Thank you for the crystal clear explanation.  Now I understand why we
were not using the end_prologue debug hook before :-).

From what you say and from what Jan said, I think we could just keep the
part (of my earlier patch) that avoids emitting two consecutive
redundant .loc directives.  It seems to me that in the case of direct
output (i.e when we the underlying assembler doesn't support the .loc
directive) we already avoid the duplication.  And that avoidance fixes
the immediate issue GDB is facing, with -g -O0.

With -g -O0, GDB doesn't have the issue as the DW_AT_location
attributes of the variable DIEs are locations that are valid in the
region of the prologue, so it doesn't need to know where the prologue
ends.  Just trusting DW_AT_location is enough.

I am currently testing the stripped down patch below.

Thanks.

-- 
Dodji

From 87f97cc32bfac37264aa414c43d4ad47a9a35d72 Mon Sep 17 00:00:00 2001
From: Dodji Seketeli do...@redhat.com
Date: Tue, 29 Mar 2011 16:56:20 +0200
Subject: [PATCH] PR debug/47471 (set prologue_end in .debug_line)

gcc/
* dwarf2out.c (dwarf2out_source_line):
Avoid emitting redundant consecutive .loc asm directives.

gcc/testsuite/

* gcc.dg/debug/dwarf2/line-prog-1.c: New test.
---
 gcc/dwarf2out.c |   37 +
 1 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 91be9a4..7134315 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -22145,8 +22145,6 @@ static void
 dwarf2out_source_line (unsigned int line, const char *filename,
int discriminator, bool is_stmt)
 {
-  static bool last_is_stmt = true;
-
   if (debug_info_level = DINFO_LEVEL_NORMAL
line != 0)
 {
@@ -22161,19 +22159,34 @@ 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 bool last_is_stmt = true;
+ static int last_file_num = -1;
+ static unsigned last_line = 0;
+ static int last_discriminator = -1;
+
+ if (last_file_num != file_num
+ || last_line != line
+ || last_is_stmt != is_stmt
+ || last_discriminator != discriminator)
{
- fprintf (asm_out_file,  is_stmt %d, is_stmt ? 1 : 0);
+ /* 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)
+   fprintf (asm_out_file,  is_stmt %d, is_stmt ? 1 : 0);
+
+ 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_stmt = is_stmt;
}
- 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++;
}
   else if (function_section (current_function_decl) != text_section)
{
-- 
1.7.3.4



Re: [PATCH] PR debug/47471 (set prologue_end in .debug_line)

2011-03-31 Thread Richard Henderson
On 03/31/2011 07:35 AM, Dodji Seketeli wrote:
 redundant .loc directives.  It seems to me that in the case of direct
 output (i.e when we the underlying assembler doesn't support the .loc
 directive) we already avoid the duplication.  And that avoidance fixes
 the immediate issue GDB is facing, with -g -O0.

There's no avoidance with direct output either.  See the #if 0 bits
inside output_line_info itself.

#if 0
  /* Don't emit anything for redundant notes.  */
  if (line_info-dw_line_num == current_line
   line_info-dw_file_num == current_file
   line_info-function == function)
goto cont;
#endif

 With -g -O0, GDB doesn't have the issue as the DW_AT_location
 attributes of the variable DIEs are locations that are valid in the
 region of the prologue, so it doesn't need to know where the prologue
 ends.  Just trusting DW_AT_location is enough.

Ah, I see.  And I can see how having end_prologue might help there.
Well, let's see if we can clean things up here then.

As for your patch itself, you're not checking to see if you're outputting
to the same section, which could cause line info to go missing at the
beginning of a different section.

That said, the patch I'm working on will do this elimination in one place
for both direct output and assembler output, and be mindful of sections.


r~


[PATCH] PR debug/47471 (set prologue_end in .debug_line)

2011-03-30 Thread Dodji Seketeli
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.

-- 
Dodji

From dc3dea429f1471540867a0b7e694a60494062ac0 Mon Sep 17 00:00:00 2001
From: Dodji Seketeli do...@redhat.com
Date: Tue, 29 Mar 2011 16:56:20 +0200
Subject: [PATCH] PR debug/47471 (set prologue_end in .debug_line)

gcc/
* dwarf2out.c (output_source_line_asm_info):  Split out of
dwarf2out_source_line.  Add a new is_prologue_end parameter.
Avoid emitting redundant consecutive .loc asm directives.
(dwarf2out_source_line):  Use output_source_line_asm.
(dwarf2out_end_prologue): New function.
(dwarf2_debug_hooks-end_prologue): Set to dwarf2out_end_prologue.

gcc/testsuite/
* gcc.dg/debug/dwarf2/line-prog-1.c: New test.
---
 gcc/dwarf2out.c |   95 +--
 1 files changed, 78 insertions(+), 17 deletions(-)

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