Bug 36690

Summary: [4.3/4.4 Regression] .debug_line first line is behind the first instruction
Product: gcc Reporter: Jan Kratochvil <jan.kratochvil>
Component: debugAssignee: Jakub Jelinek <jakub>
Status: RESOLVED FIXED    
Severity: normal CC: cnstar9988, ebotcazou, gcc-bugs, hubicka, jakub, jason, rakdver
Priority: P2    
Version: 4.4.0   
Target Milestone: 4.4.0   
URL: http://gcc.gnu.org/ml/gcc-patches/2008-10/msg00017.html
Host: x86_64-unknown-linux-gnu Target: x86_64-unknown-linux-gnu
Build: x86_64-unknown-linux-gnu Known to work:
Known to fail: Last reconfirmed: 2008-10-03 08:55:03
Attachments: gcc44-pr36690.patch

Description Jan Kratochvil 2008-07-01 15:34:17 UTC
So far -O0 code could be safely debugged.  With 4.3.1 and HEAD if you `break func' in GDB the breakpoint can be missed despite the function got executed.  The line number info is wrong and as GCC does not produce prologue-end GDB is using the line number information to skip the prologue.

Version-Release number of selected component (if applicable):
gcc-4.3.1-3.x86_64 (Fedora 9, broken)
Fedora 8 was correct: gcc-4.1.2-33.x86_64
Verified as broken on:
GNU C (GCC) version 4.4.0 20080701 (experimental) (x86_64-unknown-linux-gnu)
	compiled by GNU C version 3.4.6 20060404 (Red Hat 3.4.6-9), GMP version 4.2.2, MPFR version 2.3.0-p2.
GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096

Steps to Reproduce:
cat >whilemain.c <<EOH
int i;

void func (void)
{
  while (i == 1)
    i = 0;
}
EOH
gcc -c -o whilemain.o whilemain.c -Wall -g

new broken gccs:
000000000040047c <func>:
int i;

void func (void)
{
  40047c:	55                   	push   %rbp
  40047d:	48 89 e5             	mov    %rsp,%rbp
  400480:	eb 0a                	jmp    40048c <func+0x10>
  while (i == 1)
    i = 0;
  400482:	c7 05 fc 03 20 00 00 	movl   $0x0,0x2003fc(%rip)        # 600888 <i>
  400489:	00 00 00 

gcc-4.1.2-33.x86_64 (Fedora 8):
0000000000400468 <func>:
int i;

void func (void)
{
  400468:	55                   	push   %rbp
  400469:	48 89 e5             	mov    %rsp,%rbp
  while (i == 1)
  40046c:	eb 0a                	jmp    400478 <func+0x10>
    i = 0;
  40046e:	c7 05 fc 03 20 00 00 	movl   $0x0,0x2003fc(%rip)        # 600874 <i>
  400475:	00 00 00
Comment 1 Joseph S. Myers 2008-07-01 15:52:34 UTC
I've also observed this problem (as causing a number of GDB testsuite failures).

With both 4.2 and 4.3, the function prologue is followed by a jump,
the loop body, and then the test of the loop condition which the jump
jumps to.  The loop body has the correct line number, as does the
condition.  With 4.2, the jump has the same line number as the
condition, so that's the line number to which GDB assigns a breakpoint
on the function.  With 4.3, the jump insn does not have a line number
and GDB decides to use the line number of the loop body, and the GDB
tests fail.
Comment 2 Jakub Jelinek 2008-07-02 09:22:13 UTC
This is caused by the implicit gotos at the end of basic blocks.
See http://gcc.gnu.org/ml/gcc-patches/2007-04/msg01840.html
During cfgexpand this can be IMHO fixed by setting curr_location before
emitting the jump, not after it (otherwise e->goto_locus is IMHO useless,
as most likely the following statements will change curr_location before
anything is emitted):
--- gcc/cfgexpand.c.jj     2008-06-06 09:17:07.000000000 +0200
+++ gcc/cfgexpand.c 2008-07-02 10:43:54.000000000 +0200
@@ -1610,9 +1610,9 @@ expand_gimple_basic_block (basic_block b
 
   if (e && e->dest != bb->next_bb)
     {
-      emit_jump (label_rtx_for_bb (e->dest));
       if (e->goto_locus)
         set_curr_insn_source_location (e->goto_locus);
+      emit_jump (label_rtx_for_bb (e->dest));
       e->flags &= ~EDGE_FALLTHRU;
     }
 
Not sure if expand_gimple_cond_expr should stay as is or be modified as well.

This fixes this problem for the few early RTL passes, but then we go into
cfglayout mode and the JUMP is removed again.
force_nonfallthru_and_redirect then when going out of cfglayout mode recreates
the JUMP again, but completely ignores e->goto_locus:

  if (target == EXIT_BLOCK_PTR)
    {
#ifdef HAVE_return
        emit_jump_insn_after_noloc (gen_return (), BB_END (jump_block));
#else
        gcc_unreachable ();
#endif
    }
  else
    {
      rtx label = block_label (target);
      emit_jump_insn_after_noloc (gen_jump (label), BB_END (jump_block));
      JUMP_LABEL (BB_END (jump_block)) = label;
      LABEL_NUSES (label)++;
    }

One problem is that we have just locus for the edges, not BLOCK, so if
force_nonfallthru_and_redirect were to call emit_jump_insn_after_setloc instead,
the question is what BLOCK should be used for it (e.g. pick the block from insn before the jump (if any)?  What to do if there is none in the basic block?).
Comment 3 Jakub Jelinek 2008-07-10 12:01:15 UTC
Created attachment 15888 [details]
gcc44-pr36690.patch

Unfinished patch which solves the testcase in this PR and a couple of other problems, but still there are plenty of issues.

It adds goto_block field to edges, because sometimes an edge is the only place
that uses certain scope.  Consider say:
void bar (int);

void foo (int i)
{
  if (!i)
    bar (0);
  else
    {
      static int z = 5;
      goto f1;
    }
  bar (1);
f1:
  bar (2);
}
At -O0 -g we really should be able to put breakpoint on goto f1 and see the z
variable.  Another testcase I've been playing with is:
void
bar (int i)
{
}

void
foo (int i, int j)
{
  if (j)
    {
      bar (i + 1);
      goto f1;
    }
  bar (i + 2);
  goto f2;
f1:
  if (i > 10)
    goto f3;
f2:
  if (i > 40)
    goto f4;
  else
    goto f5;
f3:
  bar (i);
f4:
  bar (i);
f5:
  bar (i);
}

int
main (void)
{
  foo (0, 1);
  foo (11, 1);
  foo (21, 0);
  foo (41, 0);
  return 0;
}

Here IMNSHO at -O0 -g one should be able to step through all the gotos, so there should be some instructions with the locations of the gotos.  With the attached patch one goto gets a location in the assembly, but the rest don't.  One problem
is that already the into_cfglayout pass does some optimizations I wouldn't think are appropriate for -O0, like merging basic blocks.  That's e.g. where the
goto_locus of goto f1; is gone.  Either we should prevent that kind of optimization if (!optimize) and the edge has goto_locus, or e.g. could insert
a nop insn with goto_locus as INSN_LOCATOR and hope at -O0 nothing later on optimizes that out.  Another problem is with the conditional branches.  For them
I'm afraid we can't put goto_locus on the conditional jump.  If just one
edge has goto_locus and the other does not, at (!optimize) we could possibly
invert the condition and thus let the unconditional jump be the insn that
holds the goto_locus INSN_LOCATOR.  But for the case where both edges have
goto_locus, I think we should just insert an extra conditional jump, so that
there are two unconditional jumps, each with its goto_locus.
Comment 4 Jan Hubicka 2008-07-10 13:43:52 UTC
Subject: Re:  [4.3/4.4 Regression] .debug_line first line is behind the first instruction

> One problem
> is that already the into_cfglayout pass does some optimizations I wouldn't
> think are appropriate for -O0, like merging basic blocks.  That's e.g. where
> the

Yes, this is long lasting problem.  Basically even at -O0 we can
elliminate some of user code completely, while in theory we probably
ought to maintain even stuff like empty statements.

I am not big friend of idea of making edges sort of "abnormal" ie blocks
unmergeable just to preserve debug info.  I guess making -O0 to not
elliminate NOP instruction and merge block to insert one when last
instruction of former block is not having same locator is better way
to go given that there are more general problems like this.  On tree
level we will likely need something similar (ie empty statements holding
as placeholders).
> goto_locus of goto f1; is gone.  Either we should prevent that kind of
> optimization if (!optimize) and the edge has goto_locus, or e.g. could insert
> a nop insn with goto_locus as INSN_LOCATOR and hope at -O0 nothing later on
> optimizes that out.  Another problem is with the conditional branches.  For
> them
> I'm afraid we can't put goto_locus on the conditional jump.  If just one
> edge has goto_locus and the other does not, at (!optimize) we could possibly
> invert the condition and thus let the unconditional jump be the insn that
> holds the goto_locus INSN_LOCATOR.  But for the case where both edges have
> goto_locus, I think we should just insert an extra conditional jump, so that
> there are two unconditional jumps, each with its goto_locus.

Gimplifier already ought to do COND expr jumping to GOTO expr since I
run into this problem in gcov output. There is code in cfgcleanup
preventing forwarding the edges before gcoving is done.  Later we
forward across the forwarder block that is probably optimiztion we need
to disable at O0 if we worry about this (ie forward across forwarder
block only if the forwarders block edge is having no locator or same
locator as the edge we are forwarding).

Honza
> 
> 
> -- 
> 
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36690
> 
> ------- You are receiving this mail because: -------
> You are on the CC list for the bug, or are watching someone who is.
Comment 5 Joseph S. Myers 2008-08-27 22:04:56 UTC
4.3.2 is released, changing milestones to 4.3.3.
Comment 6 Jakub Jelinek 2008-10-07 18:50:03 UTC
Subject: Bug 36690

Author: jakub
Date: Tue Oct  7 18:48:40 2008
New Revision: 140948

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=140948
Log:
	PR debug/29609
	PR debug/36690
	PR debug/37616
	* basic-block.h (struct edge_def): Add goto_block field.
	* cfglayout.c (fixup_reorder_chain): Ensure that there is at least
	one insn with locus corresponding to edge's goto_locus if !optimize.
	* profile.c (branch_prob): Copy edge's goto_block.
	* cfgrtl.c (force_nonfallthru_and_redirect): Use goto_locus for
	emitted jumps.
	(cfg_layout_merge_blocks): Emit a nop with edge's goto_locus
	locator in between the merged basic blocks if !optimize and needed.
	* cfgexpand.c (expand_gimple_cond): Convert goto_block and
	goto_locus into RTL locator.  For unconditional jump use that
	locator for the jump insn.
	(expand_gimple_cond): Convert goto_block and goto_locus into
	RTL locator for all remaining edges.  For unconditional jump
	use that locator for the jump insn.
	* cfgcleanup.c (try_forward_edges): Avoid the optimization if
	there is more than one edge or insn locator along the forwarding
	edges and !optimize.  If there is just one, set e->goto_locus.
	* tree-cfg.c (make_cond_expr_edges, make_goto_expr_edges): Set also
	edge's goto_block.
	(move_block_to_fn): Adjust edge's goto_block.

	* gcc.dg/debug/pr29609-1.c: New test.
	* gcc.dg/debug/pr29609-2.c: New test.
	* gcc.dg/debug/pr36690-1.c: New test.
	* gcc.dg/debug/pr36690-2.c: New test.
	* gcc.dg/debug/pr36690-3.c: New test.
	* gcc.dg/debug/pr37616.c: New test.
	* gcc.dg/debug/dwarf2/pr29609-1.c: New test.
	* gcc.dg/debug/dwarf2/pr29609-2.c: New test.
	* gcc.dg/debug/dwarf2/pr36690-1.c: New test.
	* gcc.dg/debug/dwarf2/pr36690-2.c: New test.
	* gcc.dg/debug/dwarf2/pr36690-3.c: New test.
	* gcc.dg/debug/dwarf2/pr37616.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/debug/dwarf2/pr29609-1.c
    trunk/gcc/testsuite/gcc.dg/debug/dwarf2/pr29609-2.c
    trunk/gcc/testsuite/gcc.dg/debug/dwarf2/pr36690-1.c
    trunk/gcc/testsuite/gcc.dg/debug/dwarf2/pr36690-2.c
    trunk/gcc/testsuite/gcc.dg/debug/dwarf2/pr36690-3.c
    trunk/gcc/testsuite/gcc.dg/debug/dwarf2/pr37616.c
    trunk/gcc/testsuite/gcc.dg/debug/pr29609-1.c
    trunk/gcc/testsuite/gcc.dg/debug/pr29609-2.c
    trunk/gcc/testsuite/gcc.dg/debug/pr36690-1.c
    trunk/gcc/testsuite/gcc.dg/debug/pr36690-2.c
    trunk/gcc/testsuite/gcc.dg/debug/pr36690-3.c
    trunk/gcc/testsuite/gcc.dg/debug/pr37616.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/basic-block.h
    trunk/gcc/cfgcleanup.c
    trunk/gcc/cfgexpand.c
    trunk/gcc/cfglayout.c
    trunk/gcc/cfgrtl.c
    trunk/gcc/profile.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-cfg.c

Comment 7 Jakub Jelinek 2008-10-07 19:00:45 UTC
Fixed.
Comment 8 littlestar 2008-10-10 07:28:25 UTC
Target Milestone 4.3.3?
But this patch didn't committed to 4.3 branch.
why closed?