[RFA] Patch PR debug/37801

[RFA] Patch PR debug/37801

[Dodji Seketeli]

[-- Attachment #1: Type: text/plain, Size: 2469 bytes --]

Hello,

Consider this example:

inline void
third (int arg3)
{
  int var3 = arg3;
  int* a = 0;
  a[0] = var3;
}

inline void
second (int arg2)
{
  int var2 = arg2;
  third (var2+1);
}

inline void
first (int arg1)
{
  int var1 = arg1;
  second (var1+1);
}

int main ()
{
  int some_int = 1;
  first (some_int);
  return 0;
}

In this PR, abstract instances of second and first contain DIEs
representing the inlined calls to third and second. That is not correct. On
top of that, those inlined calls were represented as DW_TAG_lexical_blocks.

Only the concrete instances of second, first and main should contain DIEs
representing inlined calls.

Here is what I think the problem was.

During inlining,  expand_call_inline calls dwarf2out_abstract_function
asking the
dwarf emitter to generate debug info for the abstract instance of the
function being inlined.

The emitter recursively flags alls the blocks of the inline function as
being abstract
and then emits debug info for those. Normally, as all the blocks of the
abstract function are flagged abstract, no block should contain any
descendent DIE that represents an inline call.

At some point though, gen_block_die wrongly calls gen_inlined_subroutine_die,
asking it to emit debug info for an inlined call to an abstract function
block. That
gen_inlined_subroutine_die then wrongly walks through the abstract function
block, emitting a DW_TAG_lexical_block for it along the way.

This patch prevents gen_block_die to call gen_inlined_subroutine_die with
an abstract function block. It also asserts that gen_inlined_subroutine_die
should not be passed an abstract function block.

I was concerned about the possible regression caused by
gen_inlined_subroutine_die not walking through abstract function blocks any
more. I might be mistaken, but I think it should be fine because for every
single inlined call, expand_call_inline calls dwarf2out_abstract_function
on the matching abstract instance. So we don't miss any abstract block.

This patch updates the test gcc.dg/debug/20020224-1.c that was supposed to
catch that later possible problem. I noticed functions in that test
wouldn't be inlined at all (they are static inline) unless they are
actually called from a non static function. So I just did that. I believe
the debug info for that test is properly generated now.

Bootstraped and tested on trunk and 4.4  for x86_64-unknown-linux-gnu.

Ok for those branches ?

Thanks.

-- 
Dodji Seketeli
Red Hat


[-- Attachment #2: gcc-PR37801-patch.txt --]
[-- Type: text/plain, Size: 7003 bytes --]

commit a495ae090a6e99a0cbd9a75d6d59fdec4050bf2c
Author: Dodji Seketeli <dodji@redhat.com>
Date:   Sat Aug 15 11:21:51 2009 +0200

    Fix for PR debug/37801
    
    gcc/ChangeLog:
    	* gcc/dwarf2out.c (gen_inlined_subroutine_die): Concentrate on
    	generating inlined subroutine die only. We shouldn't be
    	called for anything else.
    	(gen_block_die): Don't generate inline subroutine debug info for
    	abstract blocks.
    
    gcc/testsuite/ChangeLog:
    	* gcc/testsuite/gcc.dg/debug/20020224-1.c: Adjust the comment.
    	Make sure to trigger inlining optimizations.
    	* gcc/testsuite/gcc.dg/debug/dwarf2/inline2.c: New test.

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index af97bb0..2ee1df8 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -15003,7 +15003,13 @@ gen_lexical_block_die (tree stmt, dw_die_ref context_die, int depth)
 static void
 gen_inlined_subroutine_die (tree stmt, dw_die_ref context_die, int depth)
 {
-  tree decl = block_ultimate_origin (stmt);
+  tree decl;
+
+  /* The instance of function that is effectively being inlined shall not
+     be abstract.  */
+  gcc_assert (! BLOCK_ABSTRACT (stmt));
+
+  decl = block_ultimate_origin (stmt);
 
   /* Emit info for the abstract instance first, if we haven't yet.  We
      must emit this even if the block is abstract, otherwise when we
@@ -15024,20 +15030,6 @@ gen_inlined_subroutine_die (tree stmt, dw_die_ref context_die, int depth)
       decls_for_scope (stmt, subr_die, depth);
       current_function_has_inlines = 1;
     }
-  else
-    /* We may get here if we're the outer block of function A that was
-       inlined into function B that was inlined into function C.  When
-       generating debugging info for C, dwarf2out_abstract_function(B)
-       would mark all inlined blocks as abstract, including this one.
-       So, we wouldn't (and shouldn't) expect labels to be generated
-       for this one.  Instead, just emit debugging info for
-       declarations within the block.  This is particularly important
-       in the case of initializers of arguments passed from B to us:
-       if they're statement expressions containing declarations, we
-       wouldn't generate dies for their abstract variables, and then,
-       when generating dies for the real variables, we'd die (pun
-       intended :-)  */
-    gen_lexical_block_die (stmt, context_die, depth);
 }
 
 /* Generate a DIE for a field in a record, or structure.  */
@@ -15666,7 +15658,23 @@ gen_block_die (tree stmt, dw_die_ref context_die, int depth)
   if (must_output_die)
     {
       if (inlined_func)
-	gen_inlined_subroutine_die (stmt, context_die, depth);
+	{
+	  /* If STMT block is abstract, that means we have been called
+	     indirectly from dwarf2out_abstract_function.
+	     That function rightfully marks the descendent blocks (of
+	     the abstract function it is dealing with) as being abstract,
+	     precisely to prevent us from emitting any
+	     DW_TAG_inlined_subroutine DIE as a descendent
+	     of an abstract function instance. So in that case, we should
+	     not call gen_inlined_subroutine_die.
+
+	     Later though, when cgraph asks dwarf2out to emit info
+	     for the concrete instance of the function decl into which
+	     the concrete instance of STMT got inlined, the later will lead
+	     to the generation of a DW_TAG_inlined_subroutine DIE.  */
+	  if (! BLOCK_ABSTRACT (stmt))
+	    gen_inlined_subroutine_die (stmt, context_die, depth);
+	}
       else
 	gen_lexical_block_die (stmt, context_die, depth);
     }
diff --git a/gcc/testsuite/gcc.dg/debug/20020224-1.c b/gcc/testsuite/gcc.dg/debug/20020224-1.c
index c61a17a..32e4f9d 100644
--- a/gcc/testsuite/gcc.dg/debug/20020224-1.c
+++ b/gcc/testsuite/gcc.dg/debug/20020224-1.c
@@ -1,9 +1,13 @@
+/* { dg-options "-g3 -O -dA" } */
 /* { dg-do compile } */
 
-/* Here's the deal: f3 is not inlined because it's too big, but f2 and
-   f1 are inlined into it.  We used to fail to emit debugging info for
-   t1, because it was moved inside the (inlined) block of f1, marked
-   as abstract, then we'd crash.  */
+/* Here's the deal: f4 is inlined into main, f3 is inlined into f4, f2 is
+   inlined into f1. The DIE of main should contain DW_TAG_inlined_subroutines
+   children for f4, f3, f2 and f1. Also, there should be a DIE representing
+   and out of line instance of f4, aside the DIE representing its abstract
+   instance.
+   We used to fail to emit debugging info for t1, because it was moved
+   inside the (inlined) block of f1, marked as abstract, then we'd crash.  */
 
 #define UNUSED __attribute__((unused))
 #define EXT __extension__
@@ -58,3 +62,10 @@ f4 (void)
 
   return;
 }
+
+int
+main ()
+{
+    int foo = 1;
+    f4 ();
+}
diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/inline2.c b/gcc/testsuite/gcc.dg/debug/dwarf2/inline2.c
new file mode 100644
index 0000000..0935857
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/debug/dwarf2/inline2.c
@@ -0,0 +1,69 @@
+/* Contributed by Dodji Seketeli <dodji@redhat.com>
+   Origin: PR debug/37801
+
+  Abstract instances (DW_TAG_subroutines having the DW_AT_inline attribute)
+  of second and first were having a DW_TAG_lexical_block DIE wrongly
+  representing the inlined calls to third (in second) and to
+  second (in first). At the same time, main did'nt have children
+  DW_TAG_inlined_subroutine DIEs representing the inlined calls to
+  first, second and third.
+
+  The ideal goal here is to test that we have no superfluous
+  DW_TAG_lexical_block DIE anymore, that abstract instances DIEs have
+  no descendant DIE with a DW_AT_abstract_origin attribute, and that main has
+  properly nested DW_TAG_inlined_subroutine DIEs for third, second and first.
+*/
+
+/* { dg-options "-O -g3 -dA" } */
+/* { dg-do compile } */
+
+/* There are 6 inlined subroutines, one per inlined call:
+   - One for each subroutine inlined into main, that's 3.
+   - One for earch subroutine inline into the out of line instances
+     of third, second and first.  */
+/* { dg-final { scan-assembler-times "\\(DIE \\(.*?\\) DW_TAG_inlined_subroutine" 6 } } */
+
+/* Likewise e should have 6 DW_TAG_lexical_block DIEs:
+   - One for each subroutine inlined into main, so that's 3.
+   - One for each subroutine inlined in the out of line instances
+     of third, second and first, that's 3.
+*/
+/* { dg-final { scan-assembler-times "\\(DIE \\(.*?\\) DW_TAG_lexical_block" 6 } } */
+
+
+/* There are 3 DW_AT_inline attributes: one per abstract inline instance.
+   The value of the attribute must be 0x3, meaning the function was
+   actually inlined.  */
+/* { dg-final { scan-assembler-times "byte.*?0x3.*? DW_AT_inline" 3 } } */
+
+
+inline void
+third (int arg3)
+{
+  int var3 = arg3;
+  int* a = 0;
+  a[0] = var3;
+}
+
+inline void
+second (int arg2)
+{
+  int var2 = arg2;
+  third (var2+1);
+}
+
+inline void
+first (int arg1)
+{
+  int var1 = arg1;
+  second (var1+1);
+}
+
+int main ()
+{
+  int some_int = 1;
+  first (some_int);
+  return 0;
+}
+
+

Re: [RFA] Patch PR debug/37801

[Jason Merrill]

On 08/15/2009 11:36 AM, Dodji Seketeli wrote:
> Ok for those branches ?

Just a few nits about the tests:

> +++ b/gcc/testsuite/gcc.dg/debug/20020224-1.c
> @@ -1,9 +1,13 @@
> +/* { dg-options "-g3 -O -dA" } */

You don't need -dA for this test.

> +/* Here's the deal: f4 is inlined into main, f3 is inlined into f4, f2 is
> +   inlined into f1. The DIE of main should contain DW_TAG_inlined_subroutines
> +   children for f4, f3, f2 and f1. Also, there should be a DIE representing
> +   and out of line instance of f4, aside the DIE representing its abstract
       an

> +  second (in first). At the same time, main did'nt have children

didn't

OK with those changes.

Jason