Re: [PATCH] PR debug/47471 (set prologue_end in .debug_line)
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)
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)
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)
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