[PATCH] [PR79542][Ada] Fix ICE in dwarf2out.c with nested func. inlining

[PATCH] [PR79542][Ada] Fix ICE in dwarf2out.c with nested func. inlining

[Pierre-Marie de Rodat]

Hello,

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79542 reports an ICE in
dwarf2out.c for an Ada testcase built with optimization.

This crash happens during the late generation pass because
add_gnat_descriptive_type cannot find the type DIE corresponding to some
descriptive type after having tried to generate it. This is because the
DIE was generated during the early generation pass, but then pruned by
the type pruning machinery. So why was it pruned?

We are in a situation where we have cloned types (because of inlining,
IIUC) whose TYPE_NAME have non-null DECL_ABSTRACT_ORIGIN attributes. As
a consequence:

  * In modified_type_die, the "handle C typedef types" part calls
    gen_type_die on the cloned type.

  * gen_type_die matches a typedef variant, and then calls gen_decl_die
    on its TYPE_NAME, which will end up calling gen_typedef_die.

  * gen_typedef_die checks decl_ultimate_origin for this TYPE_DECL, and
    finds one, so it only adds a DW_AT_abstract_origin attribute to the
    DW_TAG_typedef DIE, but the cloned type itself does not get its own
    DIE.

  * Back in modified_type_die, the call to lookup_type_die on the type
    passed to gen_type_die returns NULL.

In the end, whole type trees, i.e. the ones referenced by
DECL_ABSTRACT_ORIGIN attributes, are never referenced from type pruning
"roots" and are thus pruned. The descriptive type at stake here is one
of them, hence the assertion failure.

This patch attemps to fix that with what seems to be the most sensible
thing to do in my opinion: updating the "handle C typedef types" part in
modified_type_die to check decl_ultimate_origin before calling
gen_type_die: if that function returns something not null, then we know
that gen_type_die/gen_typedef_die will not generate a DIE for the input
type, so we try to process the ultimate origin instead.

I bootstrapped and regtested this successfully on x86_64-linux. I also
checked that there was no regression in GDB's testsuite and of course
the new testcase, which crashes with current trunk, passes with the
patch applied.

Is it ok to commit? Thank you in advance!

gcc/
	PR ada/79542
	* dwarf2out.c (modified_type_die): For C typedef types that have
	an ultimate origin, process the ultimate origin instead of the
	input type.

gcc/testsuite/

	* gnat.dg/debug10.ads, gnat.dg/debug10.adb: New testcase.
---
 gcc/dwarf2out.c                   | 12 ++++++++++++
 gcc/testsuite/gnat.dg/debug10.adb | 38 ++++++++++++++++++++++++++++++++++++++
 gcc/testsuite/gnat.dg/debug10.ads |  5 +++++
 3 files changed, 55 insertions(+)
 create mode 100644 gcc/testsuite/gnat.dg/debug10.adb
 create mode 100644 gcc/testsuite/gnat.dg/debug10.ads

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 0bbb90ed3aa..4088614fc18 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -12496,6 +12496,18 @@ modified_type_die (tree type, int cv_quals, bool reverse,
 
       if (qualified_type == dtype)
 	{
+	  tree origin
+	   = TYPE_NAME (qualified_type) == NULL
+	     ? NULL
+	     : decl_ultimate_origin (TYPE_NAME (qualified_type));
+
+	  /* Typedef variants that have an abstract origin don't get their own
+	     type DIE (see gen_typedef_die), so fall back on the ultimate
+	     abstract origin instead.  */
+	  if (origin != NULL)
+	    return modified_type_die (TREE_TYPE (origin), cv_quals, reverse,
+				      context_die);
+
 	  /* For a named type, use the typedef.  */
 	  gen_type_die (qualified_type, context_die);
 	  return lookup_type_die (qualified_type);
diff --git a/gcc/testsuite/gnat.dg/debug10.adb b/gcc/testsuite/gnat.dg/debug10.adb
new file mode 100644
index 00000000000..b85f3d36dd7
--- /dev/null
+++ b/gcc/testsuite/gnat.dg/debug10.adb
@@ -0,0 +1,38 @@
+--  { dg-options "-cargs -O2 -g -margs" }
+
+package body Debug10 is
+
+   procedure Compile (P : Natural)
+   is
+      Max_Pos : constant Natural := P;
+      type Position_Set is array (1 .. Max_Pos) of Boolean;
+
+      Empty  : constant Position_Set := (others => False);
+
+      type Position_Set_Array is array (1 .. Max_Pos) of Position_Set;
+
+      Follow  : Position_Set_Array := (others => Empty);
+
+      function Get_Follows return Position_Set;
+
+      procedure Make_DFA;
+
+      function Get_Follows return Position_Set is
+         Result : Position_Set := Empty;
+      begin
+         Result := Result or Follow (1);
+
+         return Result;
+      end Get_Follows;
+
+      procedure Make_DFA is
+         Next   : constant Position_Set := Get_Follows;
+      begin
+         null;
+      end Make_DFA;
+
+   begin
+      Make_DFA;
+   end Compile;
+
+end Debug10;
diff --git a/gcc/testsuite/gnat.dg/debug10.ads b/gcc/testsuite/gnat.dg/debug10.ads
new file mode 100644
index 00000000000..ecd3c8880ba
--- /dev/null
+++ b/gcc/testsuite/gnat.dg/debug10.ads
@@ -0,0 +1,5 @@
+package Debug10 is
+
+   procedure Compile (P : Natural);
+
+end Debug10;
-- 
2.11.0

[PING][PATCH][PR79542][Ada] Fix ICE in dwarf2out.c with nested func. inlining

[Pierre-Marie de Rodat]

Hello,

This is a ping for the patch I posted there: 
<https://gcc.gnu.org/ml/gcc-patches/2017-03/msg00725.html>. Thank you!

-- 
Pierre-Marie de Rodat

[PING*2][PATCH][PR79542][Ada] Fix ICE in dwarf2out.c with nested func. inlining

[Pierre-Marie de Rodat]

Hello,

I would like to ping for the patch I posted there: 
<https://gcc.gnu.org/ml/gcc-patches/2017-03/msg00725.html>. Thank you!

-- 
Pierre-Marie de Rodat

Re: [PATCH] [PR79542][Ada] Fix ICE in dwarf2out.c with nested func. inlining

[Jason Merrill]

On Tue, Mar 14, 2017 at 8:24 AM, Pierre-Marie de Rodat
<derodat@adacore.com> wrote:
> Hello,
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79542 reports an ICE in
> dwarf2out.c for an Ada testcase built with optimization.
>
> This crash happens during the late generation pass because
> add_gnat_descriptive_type cannot find the type DIE corresponding to some
> descriptive type after having tried to generate it. This is because the
> DIE was generated during the early generation pass, but then pruned by
> the type pruning machinery. So why was it pruned?
>
> We are in a situation where we have cloned types (because of inlining,
> IIUC) whose TYPE_NAME have non-null DECL_ABSTRACT_ORIGIN attributes. As
> a consequence:
>
>   * In modified_type_die, the "handle C typedef types" part calls
>     gen_type_die on the cloned type.
>
>   * gen_type_die matches a typedef variant, and then calls gen_decl_die
>     on its TYPE_NAME, which will end up calling gen_typedef_die.
>
>   * gen_typedef_die checks decl_ultimate_origin for this TYPE_DECL, and
>     finds one, so it only adds a DW_AT_abstract_origin attribute to the
>     DW_TAG_typedef DIE, but the cloned type itself does not get its own
>     DIE.

That seems like a bug; if gen_typedef_die is going to generate a DIE
for a cloned typedef, it needs to associate the type with the DIE.

>   * Back in modified_type_die, the call to lookup_type_die on the type
>     passed to gen_type_die returns NULL.

> In the end, whole type trees, i.e. the ones referenced by
> DECL_ABSTRACT_ORIGIN attributes, are never referenced from type pruning
> "roots" and are thus pruned. The descriptive type at stake here is one
> of them, hence the assertion failure.
>
> This patch attemps to fix that with what seems to be the most sensible
> thing to do in my opinion: updating the "handle C typedef types" part in
> modified_type_die to check decl_ultimate_origin before calling
> gen_type_die: if that function returns something not null, then we know
> that gen_type_die/gen_typedef_die will not generate a DIE for the input
> type, so we try to process the ultimate origin instead.

This soundsn good; the DWARF standard says that we don't need to have
a die at all for the cloned typedef.

> @@ -12496,6 +12496,18 @@ modified_type_die (tree type, int cv_quals, bool reverse,
>
>        if (qualified_type == dtype)
>         {
> +         tree origin
> +          = TYPE_NAME (qualified_type) == NULL
> +            ? NULL
> +            : decl_ultimate_origin (TYPE_NAME (qualified_type));

This is unnecessarily complicated; at this point we already know that
TYPE_NAME (qualified_type) is non-null and in the variable "name".

> +         /* Typedef variants that have an abstract origin don't get their own
> +            type DIE (see gen_typedef_die), so fall back on the ultimate

gen_typedef_die does create a DIE for them, it just doesn't do it
properly.  But we could change gen_typedef_die to abort in that case,
making this comment correct.

Jason

Re: [PATCH] [PR79542][Ada] Fix ICE in dwarf2out.c with nested func. inlining

[Jason Merrill]

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

On Wed, May 3, 2017 at 3:22 PM, Jason Merrill <jason@redhat.com> wrote:
> On Tue, Mar 14, 2017 at 8:24 AM, Pierre-Marie de Rodat
> <derodat@adacore.com> wrote:
>> Hello,
>>
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79542 reports an ICE in
>> dwarf2out.c for an Ada testcase built with optimization.
>>
>> This crash happens during the late generation pass because
>> add_gnat_descriptive_type cannot find the type DIE corresponding to some
>> descriptive type after having tried to generate it. This is because the
>> DIE was generated during the early generation pass, but then pruned by
>> the type pruning machinery. So why was it pruned?
>>
>> We are in a situation where we have cloned types (because of inlining,
>> IIUC) whose TYPE_NAME have non-null DECL_ABSTRACT_ORIGIN attributes. As
>> a consequence:
>>
>>   * In modified_type_die, the "handle C typedef types" part calls
>>     gen_type_die on the cloned type.
>>
>>   * gen_type_die matches a typedef variant, and then calls gen_decl_die
>>     on its TYPE_NAME, which will end up calling gen_typedef_die.
>>
>>   * gen_typedef_die checks decl_ultimate_origin for this TYPE_DECL, and
>>     finds one, so it only adds a DW_AT_abstract_origin attribute to the
>>     DW_TAG_typedef DIE, but the cloned type itself does not get its own
>>     DIE.
>
> That seems like a bug; if gen_typedef_die is going to generate a DIE
> for a cloned typedef, it needs to associate the type with the DIE.
>
>>   * Back in modified_type_die, the call to lookup_type_die on the type
>>     passed to gen_type_die returns NULL.
>
>> In the end, whole type trees, i.e. the ones referenced by
>> DECL_ABSTRACT_ORIGIN attributes, are never referenced from type pruning
>> "roots" and are thus pruned. The descriptive type at stake here is one
>> of them, hence the assertion failure.
>>
>> This patch attemps to fix that with what seems to be the most sensible
>> thing to do in my opinion: updating the "handle C typedef types" part in
>> modified_type_die to check decl_ultimate_origin before calling
>> gen_type_die: if that function returns something not null, then we know
>> that gen_type_die/gen_typedef_die will not generate a DIE for the input
>> type, so we try to process the ultimate origin instead.
>
> This soundsn good; the DWARF standard says that we don't need to have
> a die at all for the cloned typedef.
>
>> @@ -12496,6 +12496,18 @@ modified_type_die (tree type, int cv_quals, bool reverse,
>>
>>        if (qualified_type == dtype)
>>         {
>> +         tree origin
>> +          = TYPE_NAME (qualified_type) == NULL
>> +            ? NULL
>> +            : decl_ultimate_origin (TYPE_NAME (qualified_type));
>
> This is unnecessarily complicated; at this point we already know that
> TYPE_NAME (qualified_type) is non-null and in the variable "name".
>
>> +         /* Typedef variants that have an abstract origin don't get their own
>> +            type DIE (see gen_typedef_die), so fall back on the ultimate
>
> gen_typedef_die does create a DIE for them, it just doesn't do it
> properly.  But we could change gen_typedef_die to abort in that case,
> making this comment correct.

Something like this, which also avoids routinely creating DIEs for
local typedefs that will only be pruned away later; this patch doesn't
change the size of .debug_info in cc1plus.

[-- Attachment #2: inline-static.diff --]
[-- Type: text/plain, Size: 1267 bytes --]

commit 56cce11181d1296e43cb4d603fc8efc6ac2570fa
Author: Jason Merrill <jason@redhat.com>
Date:   Thu May 4 15:00:51 2017 -0400

    inline-static

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 6caf598..bf6a65b 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -24355,7 +24355,7 @@ gen_typedef_die (tree decl, dw_die_ref context_die)
   type_die = new_die (DW_TAG_typedef, context_die, decl);
   origin = decl_ultimate_origin (decl);
   if (origin != NULL)
-    add_abstract_origin_attribute (type_die, origin);
+    gcc_unreachable (), add_abstract_origin_attribute (type_die, origin);
   else
     {
       tree type = TREE_TYPE (decl);
@@ -24858,6 +24858,16 @@ process_scope_var (tree stmt, tree decl, tree origin, dw_die_ref context_die)
   else
     die = NULL;
 
+  if ((origin || DECL_ABSTRACT_ORIGIN (decl))
+      && (TREE_CODE (decl_or_origin) == TYPE_DECL
+	  || (VAR_P (decl_or_origin) && TREE_STATIC (decl_or_origin))))
+    {
+      origin = decl_ultimate_origin (decl_or_origin);
+      if (decl && VAR_P (decl))
+	equate_decl_number_to_die (decl, lookup_decl_die (origin));
+      return;
+    }
+
   if (die != NULL && die->die_parent == NULL)
     add_child_die (context_die, die);
   else if (TREE_CODE (decl_or_origin) == IMPORTED_DECL)

Re: [PATCH] [PR79542][Ada] Fix ICE in dwarf2out.c with nested func. inlining

[Pierre-Marie de Rodat]

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

Hello,

Thanks for your suggestions, Jason.

On 05/08/2017 06:27 PM, Jason Merrill wrote:
>> That seems like a bug; if gen_typedef_die is going to generate a DIE
>> for a cloned typedef, it needs to associate the type with the DIE.

Hm… gen_typedef_die generates a DIE for a DECL node, but 
modified_type_die look for a DIE for the corresponding TYPE, and of 
course the lookup returns NULL. So just removing the “if 
(DECL_ABSTRACT_P (decl))” condition in gen_typedef_die is not enough to 
make the crash go away.

>> This is unnecessarily complicated; at this point we already know that
>> TYPE_NAME (qualified_type) is non-null and in the variable "name".

Fixed, thanks!

>> gen_typedef_die does create a DIE for them, it just doesn't do it
>> properly.  But we could change gen_typedef_die to abort in that case,
>> making this comment correct.
> 
> Something like this, which also avoids routinely creating DIEs for
> local typedefs that will only be pruned away later; this patch doesn't
> change the size of .debug_info in cc1plus.

I tried this, but I got a crash when compiling the Ada runtime 
(g-awk.adb). I could not extract a reproducer, but the idea is that 
because of the call to set_decl_origin_self, some DECLs have themselves 
as DECL_ABSTRACT_ORIGIN. As a result, my patch in modified_type_die does 
not prevent execution from calling gen_typedef_die with a DECL that has 
a non-null abstract origin. So I have to soften the assertion so that 
this specific case was still allowed in gen_typedef_die.

So here’s the update patch: bootstrapped and regtested fine on x86_64-linux.

-- 
Pierre-Marie de Rodat

[-- Attachment #2: 0001-PR79542-Ada-Fix-ICE-in-dwarf2out.c-with-nested-func..patch --]
[-- Type: text/x-patch, Size: 6866 bytes --]

From 334338c3ac5dbdeb3981a6302259d0be0b5efc11 Mon Sep 17 00:00:00 2001
From: Pierre-Marie de Rodat <derodat@adacore.com>
Date: Tue, 9 May 2017 12:24:36 +0200
Subject: [PATCH] [PR79542][Ada] Fix ICE in dwarf2out.c with nested func.
 inlining

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79542 reports an ICE in
dwarf2out.c for an Ada testcase built with optimization.

This crash happens during the late generation pass because
add_gnat_descriptive_type cannot find the type DIE corresponding to some
descriptive type after having tried to generate it. This is because the
DIE was generated during the early generation pass, but then pruned by
the type pruning machinery. So why was it pruned?

We are in a situation where we have cloned types (because of inlining,
IIUC) whose TYPE_NAME have non-null DECL_ABSTRACT_ORIGIN attributes. As
a consequence:

  * In modified_type_die, the "handle C typedef types" part calls
    gen_type_die on the cloned type.

  * gen_type_die matches a typedef variant, and then calls gen_decl_die
    on its TYPE_NAME, which will end up calling gen_typedef_die.

  * gen_typedef_die checks decl_ultimate_origin for this TYPE_DECL, and
    finds one, so it only adds a DW_AT_abstract_origin attribute to the
    DW_TAG_typedef DIE, but the cloned type itself does not get its own
    DIE.

  * Back in modified_type_die, the call to lookup_type_die on the type
    passed to gen_type_die returns NULL.

In the end, whole type trees, i.e. the ones referenced by
DECL_ABSTRACT_ORIGIN attributes, are never referenced from type pruning
"roots" and are thus pruned. The descriptive type at stake here is one
of them, hence the assertion failure.

This patch attemps to fix that with what seems to be the most sensible
thing to do in my opinion: updating the "handle C typedef types" part in
modified_type_die to check decl_ultimate_origin before calling
gen_type_die: if that function returns something not null, then we know
that gen_type_die/gen_typedef_die will not generate a DIE for the input
type, so we try to process the ultimate origin instead. It also updates
in a similar way gen_type_die_with_usage, assert that when
gen_typedef_die is called on nodes that have an ultimate origin, this
origin is themselves.

gcc/
	PR ada/79542
	* dwarf2out.c (modified_type_die): For C typedef types that have
	an ultimate origin, process the ultimate origin instead of the
	input type.
	(gen_typedef_die): Assert that DECLs that have an ultimate
	origin are their own ultimate origin.
	(gen_type_die_with_usage): For typedef variants that have an
	ultimate origin, just call gen_decl_die on the original DECL.
	(process_scope_var): Do nothing for DECLs with an ultimate
	origin.

gcc/testsuite/

	* gnat.dg/debug11.ads, gnat.dg/debug11.adb: New testcase.
---
 gcc/dwarf2out.c                   | 30 +++++++++++++++++++++++++++---
 gcc/testsuite/gnat.dg/debug11.adb | 38 ++++++++++++++++++++++++++++++++++++++
 gcc/testsuite/gnat.dg/debug11.ads |  5 +++++
 3 files changed, 70 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gnat.dg/debug11.adb
 create mode 100644 gcc/testsuite/gnat.dg/debug11.ads

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 98c5157..b4f5a69 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -12510,6 +12510,15 @@ modified_type_die (tree type, int cv_quals, bool reverse,
 
       if (qualified_type == dtype)
 	{
+	  tree origin = decl_ultimate_origin (name);
+
+	  /* Typedef variants that have an abstract origin don't get their own
+	     type DIE (see gen_typedef_die), so fall back on the ultimate
+	     abstract origin instead.  */
+	  if (origin != NULL)
+	    return modified_type_die (TREE_TYPE (origin), cv_quals, reverse,
+				      context_die);
+
 	  /* For a named type, use the typedef.  */
 	  gen_type_die (qualified_type, context_die);
 	  return lookup_type_die (qualified_type);
@@ -24355,7 +24364,10 @@ gen_typedef_die (tree decl, dw_die_ref context_die)
   type_die = new_die (DW_TAG_typedef, context_die, decl);
   origin = decl_ultimate_origin (decl);
   if (origin != NULL)
-    add_abstract_origin_attribute (type_die, origin);
+    {
+      gcc_assert (origin == decl);
+      add_abstract_origin_attribute (type_die, origin);
+    }
   else
     {
       tree type = TREE_TYPE (decl);
@@ -24531,15 +24543,23 @@ gen_type_die_with_usage (tree type, dw_die_ref context_die,
       if (TREE_ASM_WRITTEN (type))
 	return;
 
+      tree name = TYPE_NAME (type);
+      tree origin = decl_ultimate_origin (name);
+      if (origin != NULL)
+	{
+	  gen_decl_die (origin, NULL, NULL, context_die);
+	  return;
+	}
+
       /* Prevent broken recursion; we can't hand off to the same type.  */
-      gcc_assert (DECL_ORIGINAL_TYPE (TYPE_NAME (type)) != type);
+      gcc_assert (DECL_ORIGINAL_TYPE (name) != type);
 
       /* Give typedefs the right scope.  */
       context_die = scope_die_for (type, context_die);
 
       TREE_ASM_WRITTEN (type) = 1;
 
-      gen_decl_die (TYPE_NAME (type), NULL, NULL, context_die);
+      gen_decl_die (name, NULL, NULL, context_die);
       return;
     }
 
@@ -24858,6 +24878,10 @@ process_scope_var (tree stmt, tree decl, tree origin, dw_die_ref context_die)
   else
     die = NULL;
 
+  if ((origin || DECL_ABSTRACT_ORIGIN (decl))
+      && TREE_CODE (decl_or_origin) == TYPE_DECL)
+    return;
+
   if (die != NULL && die->die_parent == NULL)
     add_child_die (context_die, die);
   else if (TREE_CODE (decl_or_origin) == IMPORTED_DECL)
diff --git a/gcc/testsuite/gnat.dg/debug11.adb b/gcc/testsuite/gnat.dg/debug11.adb
new file mode 100644
index 0000000..1e89346
--- /dev/null
+++ b/gcc/testsuite/gnat.dg/debug11.adb
@@ -0,0 +1,38 @@
+--  { dg-options "-cargs -O2 -g -margs" }
+
+package body Debug11 is
+
+   procedure Compile (P : Natural)
+   is
+      Max_Pos : constant Natural := P;
+      type Position_Set is array (1 .. Max_Pos) of Boolean;
+
+      Empty  : constant Position_Set := (others => False);
+
+      type Position_Set_Array is array (1 .. Max_Pos) of Position_Set;
+
+      Follow  : Position_Set_Array := (others => Empty);
+
+      function Get_Follows return Position_Set;
+
+      procedure Make_DFA;
+
+      function Get_Follows return Position_Set is
+         Result : Position_Set := Empty;
+      begin
+         Result := Result or Follow (1);
+
+         return Result;
+      end Get_Follows;
+
+      procedure Make_DFA is
+         Next   : constant Position_Set := Get_Follows;
+      begin
+         null;
+      end Make_DFA;
+
+   begin
+      Make_DFA;
+   end Compile;
+
+end Debug11;
diff --git a/gcc/testsuite/gnat.dg/debug11.ads b/gcc/testsuite/gnat.dg/debug11.ads
new file mode 100644
index 0000000..a53b084
--- /dev/null
+++ b/gcc/testsuite/gnat.dg/debug11.ads
@@ -0,0 +1,5 @@
+package Debug11 is
+
+   procedure Compile (P : Natural);
+
+end Debug11;
-- 
2.3.3.199.g52cae64

Re: [PATCH] [PR79542][Ada] Fix ICE in dwarf2out.c with nested func. inlining

[Pierre-Marie de Rodat]

On 05/26/2017 04:12 PM, Pierre-Marie de Rodat wrote:
> I tried this, but I got a crash when compiling the Ada runtime 
> (g-awk.adb). I could not extract a reproducer, but the idea is that 
> because of the call to set_decl_origin_self, some DECLs have themselves 
> as DECL_ABSTRACT_ORIGIN. As a result, my patch in modified_type_die does 
> not prevent execution from calling gen_typedef_die with a DECL that has 
> a non-null abstract origin. So I have to soften the assertion so that 
> this specific case was still allowed in gen_typedef_die.
> 
> So here’s the update patch: bootstrapped and regtested fine on 
> x86_64-linux.

Ping for the updated patch, originally submitted at 
https://gcc.gnu.org/ml/gcc-patches/2017-05/msg02049.html

Thanks!

-- 
Pierre-Marie de Rodat

[PING*2][PATCH] [PR79542][Ada] Fix ICE in dwarf2out.c with nested func. inlining

[Pierre-Marie de Rodat]

I would like to ping for the updated patch, originally submitted at
<https://gcc.gnu.org/ml/gcc-patches/2017-05/msg02049.html>. Thank you in 
advance!

-- 
Pierre-Marie de Rodat

[PING*3][PATCH] [PR79542][Ada] Fix ICE in dwarf2out.c with nested func. inlining

[Pierre-Marie de Rodat]

Hello,

Ping for the updated patch, originally submitted at
<https://gcc.gnu.org/ml/gcc-patches/2017-05/msg02049.html>. Thank you in 
advance!

-- 
Pierre-Marie de Rodat

Re: [PATCH] [PR79542][Ada] Fix ICE in dwarf2out.c with nested func. inlining

[Jason Merrill]

On 05/26/2017 10:12 AM, Pierre-Marie de Rodat wrote:
> On 05/08/2017 06:27 PM, Jason Merrill wrote:
>>> That seems like a bug; if gen_typedef_die is going to generate a DIE
>>> for a cloned typedef, it needs to associate the type with the DIE.
> 
> Hm… gen_typedef_die generates a DIE for a DECL node, but 
> modified_type_die look for a DIE for the corresponding TYPE, and of 
> course the lookup returns NULL. So just removing the “if 
> (DECL_ABSTRACT_P (decl))” condition in gen_typedef_die is not enough to 
> make the crash go away.

Right, the bug seems to be generating the useless DIE in the first place.

>>> gen_typedef_die does create a DIE for them, it just doesn't do it
>>> properly.  But we could change gen_typedef_die to abort in that case,
>>> making this comment correct.
>>
>> Something like this, which also avoids routinely creating DIEs for
>> local typedefs that will only be pruned away later; this patch doesn't
>> change the size of .debug_info in cc1plus.
> 
> I tried this, but I got a crash when compiling the Ada runtime 
> (g-awk.adb). I could not extract a reproducer, but the idea is that 
> because of the call to set_decl_origin_self, some DECLs have themselves 
> as DECL_ABSTRACT_ORIGIN. As a result, my patch in modified_type_die does 
> not prevent execution from calling gen_typedef_die with a DECL that has 
> a non-null abstract origin. So I have to soften the assertion so that 
> this specific case was still allowed in gen_typedef_die.

Perhaps the DECL_ABSTRACT_ORIGIN check in my patch should be 
decl_ultimate_origin instead, which should return null in that case?

Jason

Re: [PATCH] [PR79542][Ada] Fix ICE in dwarf2out.c with nested func. inlining

[Pierre-Marie de Rodat]

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

On 08/07/2017 08:42 PM, Jason Merrill wrote:
> Perhaps the DECL_ABSTRACT_ORIGIN check in my patch should be 
> decl_ultimate_origin instead, which should return null in that case?

It looks like it worked, thanks! Note that I had to tweak a bit your 
change in process_scope_vars to avoid a crash in a couple of Fortran 
tests (at -O3 -g). Also, as the gcc_unreachable appears in an IF block, 
I turned it into a gcc_assert and remove the corresponding IF.

Here is the final patch version, bootstrapped and regtested on x86_64-linux.

-- 
Pierre-Marie de Rodat

[-- Attachment #2: 0001-PR79542-Ada-Fix-ICE-in-dwarf2out.c-with-nested-func..patch --]
[-- Type: text/x-patch, Size: 11761 bytes --]

From 9272d97bfa77b073d57e99c231d271ec1bea2c26 Mon Sep 17 00:00:00 2001
From: Pierre-Marie de Rodat <derodat@adacore.com>
Date: Tue, 9 May 2017 12:24:36 +0200
Subject: [PATCH] [PR79542][Ada] Fix ICE in dwarf2out.c with nested func.
 inlining

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79542 reports an ICE in
dwarf2out.c for an Ada testcase built with optimization.

This crash happens during the late generation pass because
add_gnat_descriptive_type cannot find the type DIE corresponding to some
descriptive type after having tried to generate it. This is because the
DIE was generated during the early generation pass, but then pruned by
the type pruning machinery. So why was it pruned?

We are in a situation where we have cloned types (because of inlining,
IIUC) whose TYPE_NAME have non-null DECL_ABSTRACT_ORIGIN attributes. As
a consequence:

  * In modified_type_die, the "handle C typedef types" part calls
    gen_type_die on the cloned type.

  * gen_type_die matches a typedef variant, and then calls gen_decl_die
    on its TYPE_NAME, which will end up calling gen_typedef_die.

  * gen_typedef_die checks decl_ultimate_origin for this TYPE_DECL, and
    finds one, so it only adds a DW_AT_abstract_origin attribute to the
    DW_TAG_typedef DIE, but the cloned type itself does not get its own
    DIE.

  * Back in modified_type_die, the call to lookup_type_die on the type
    passed to gen_type_die returns NULL.

In the end, whole type trees, i.e. the ones referenced by
DECL_ABSTRACT_ORIGIN attributes, are never referenced from type pruning
"roots" and are thus pruned. The descriptive type at stake here is one
of them, hence the assertion failure.

This patch attemps to fix that with what seems to be the most sensible
thing to do in my opinion: updating the "handle C typedef types" part in
modified_type_die to check decl_ultimate_origin before calling
gen_type_die: if that function returns something not null, then we know
that gen_type_die/gen_typedef_die will not generate a DIE for the input
type, so we try to process the ultimate origin instead. It also updates
in a similar way gen_type_die_with_usage, assert that when
gen_typedef_die is called on nodes that have an ultimate origin, this
origin is themselves.

gcc/
	PR ada/79542
	* dwarf2out.c (modified_type_die): For C typedef types that have
	an ultimate origin, process the ultimate origin instead of the
	input type.
	(gen_typedef_die): Assert that input DECLs have no ultimate
	origin.
	(gen_type_die_with_usage): For typedef variants that have an
	ultimate origin, just call gen_decl_die on the original DECL.
	(process_scope_var): Avoid creating DIEs for local typedefs and
	concrete static variables.

gcc/testsuite/

	* gnat.dg/debug13.ads, gnat.dg/debug13.adb: New testcase.
---
 gcc/dwarf2out.c                   | 143 +++++++++++++++++++++++---------------
 gcc/testsuite/gnat.dg/debug13.adb |  38 ++++++++++
 gcc/testsuite/gnat.dg/debug13.ads |   5 ++
 3 files changed, 129 insertions(+), 57 deletions(-)
 create mode 100644 gcc/testsuite/gnat.dg/debug13.adb
 create mode 100644 gcc/testsuite/gnat.dg/debug13.ads

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 8e42227..917ab9f 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -12506,6 +12506,15 @@ modified_type_die (tree type, int cv_quals, bool reverse,
 
       if (qualified_type == dtype)
 	{
+	  tree origin = decl_ultimate_origin (name);
+
+	  /* Typedef variants that have an abstract origin don't get their own
+	     type DIE (see gen_typedef_die), so fall back on the ultimate
+	     abstract origin instead.  */
+	  if (origin != NULL)
+	    return modified_type_die (TREE_TYPE (origin), cv_quals, reverse,
+				      context_die);
+
 	  /* For a named type, use the typedef.  */
 	  gen_type_die (qualified_type, context_die);
 	  return lookup_type_die (qualified_type);
@@ -24296,7 +24305,7 @@ static void
 gen_typedef_die (tree decl, dw_die_ref context_die)
 {
   dw_die_ref type_die;
-  tree origin;
+  tree type;
 
   if (TREE_ASM_WRITTEN (decl))
     {
@@ -24305,75 +24314,71 @@ gen_typedef_die (tree decl, dw_die_ref context_die)
       return;
     }
 
+  /* As we avoid creating DIEs for local typedefs (see decl_ultimate_origin
+     checks in process_scope_var and modified_type_die), this should be called
+     only for original types.  */
+  gcc_assert (decl_ultimate_origin (decl) == NULL);
+
   TREE_ASM_WRITTEN (decl) = 1;
   type_die = new_die (DW_TAG_typedef, context_die, decl);
-  origin = decl_ultimate_origin (decl);
-  if (origin != NULL)
-    add_abstract_origin_attribute (type_die, origin);
-  else
+
+  add_name_and_src_coords_attributes (type_die, decl);
+  if (DECL_ORIGINAL_TYPE (decl))
     {
-      tree type = TREE_TYPE (decl);
+      type = DECL_ORIGINAL_TYPE (decl);
+      if (type == error_mark_node)
+	return;
 
+      gcc_assert (type != TREE_TYPE (decl));
+      equate_type_number_to_die (TREE_TYPE (decl), type_die);
+    }
+  else
+    {
+      type = TREE_TYPE (decl);
       if (type == error_mark_node)
 	return;
 
-      add_name_and_src_coords_attributes (type_die, decl);
-      if (DECL_ORIGINAL_TYPE (decl))
+      if (is_naming_typedef_decl (TYPE_NAME (type)))
 	{
-	  type = DECL_ORIGINAL_TYPE (decl);
+	  /* Here, we are in the case of decl being a typedef naming
+	     an anonymous type, e.g:
+		 typedef struct {...} foo;
+	     In that case TREE_TYPE (decl) is not a typedef variant
+	     type and TYPE_NAME of the anonymous type is set to the
+	     TYPE_DECL of the typedef. This construct is emitted by
+	     the C++ FE.
 
-	  if (type == error_mark_node)
-	    return;
+	     TYPE is the anonymous struct named by the typedef
+	     DECL. As we need the DW_AT_type attribute of the
+	     DW_TAG_typedef to point to the DIE of TYPE, let's
+	     generate that DIE right away. add_type_attribute
+	     called below will then pick (via lookup_type_die) that
+	     anonymous struct DIE.  */
+	  if (!TREE_ASM_WRITTEN (type))
+	    gen_tagged_type_die (type, context_die, DINFO_USAGE_DIR_USE);
 
-	  gcc_assert (type != TREE_TYPE (decl));
-	  equate_type_number_to_die (TREE_TYPE (decl), type_die);
-	}
-      else
-	{
-	  if (is_naming_typedef_decl (TYPE_NAME (type)))
-	    {
-	      /* Here, we are in the case of decl being a typedef naming
-	         an anonymous type, e.g:
-	             typedef struct {...} foo;
-	         In that case TREE_TYPE (decl) is not a typedef variant
-	         type and TYPE_NAME of the anonymous type is set to the
-	         TYPE_DECL of the typedef. This construct is emitted by
-	         the C++ FE.
-
-	         TYPE is the anonymous struct named by the typedef
-	         DECL. As we need the DW_AT_type attribute of the
-	         DW_TAG_typedef to point to the DIE of TYPE, let's
-	         generate that DIE right away. add_type_attribute
-	         called below will then pick (via lookup_type_die) that
-	         anonymous struct DIE.  */
-	      if (!TREE_ASM_WRITTEN (type))
-	        gen_tagged_type_die (type, context_die, DINFO_USAGE_DIR_USE);
-
-	      /* This is a GNU Extension.  We are adding a
-		 DW_AT_linkage_name attribute to the DIE of the
-		 anonymous struct TYPE.  The value of that attribute
-		 is the name of the typedef decl naming the anonymous
-		 struct.  This greatly eases the work of consumers of
-		 this debug info.  */
-	      add_linkage_name_raw (lookup_type_die (type), decl);
-	    }
+	  /* This is a GNU Extension.  We are adding a
+	     DW_AT_linkage_name attribute to the DIE of the
+	     anonymous struct TYPE.  The value of that attribute
+	     is the name of the typedef decl naming the anonymous
+	     struct.  This greatly eases the work of consumers of
+	     this debug info.  */
+	  add_linkage_name_raw (lookup_type_die (type), decl);
 	}
+    }
 
-      add_type_attribute (type_die, type, decl_quals (decl), false,
-			  context_die);
-
-      if (is_naming_typedef_decl (decl))
-	/* We want that all subsequent calls to lookup_type_die with
-	   TYPE in argument yield the DW_TAG_typedef we have just
-	   created.  */
-	equate_type_number_to_die (type, type_die);
+  add_type_attribute (type_die, type, decl_quals (decl), false,
+		      context_die);
 
-      type = TREE_TYPE (decl);
+  if (is_naming_typedef_decl (decl))
+    /* We want that all subsequent calls to lookup_type_die with
+       TYPE in argument yield the DW_TAG_typedef we have just
+       created.  */
+    equate_type_number_to_die (type, type_die);
 
-      add_alignment_attribute (type_die, type);
+  add_alignment_attribute (type_die, TREE_TYPE (decl));
 
-      add_accessibility_attribute (type_die, decl);
-    }
+  add_accessibility_attribute (type_die, decl);
 
   if (DECL_ABSTRACT_P (decl))
     equate_decl_number_to_die (decl, type_die);
@@ -24485,15 +24490,23 @@ gen_type_die_with_usage (tree type, dw_die_ref context_die,
       if (TREE_ASM_WRITTEN (type))
 	return;
 
+      tree name = TYPE_NAME (type);
+      tree origin = decl_ultimate_origin (name);
+      if (origin != NULL)
+	{
+	  gen_decl_die (origin, NULL, NULL, context_die);
+	  return;
+	}
+
       /* Prevent broken recursion; we can't hand off to the same type.  */
-      gcc_assert (DECL_ORIGINAL_TYPE (TYPE_NAME (type)) != type);
+      gcc_assert (DECL_ORIGINAL_TYPE (name) != type);
 
       /* Give typedefs the right scope.  */
       context_die = scope_die_for (type, context_die);
 
       TREE_ASM_WRITTEN (type) = 1;
 
-      gen_decl_die (TYPE_NAME (type), NULL, NULL, context_die);
+      gen_decl_die (name, NULL, NULL, context_die);
       return;
     }
 
@@ -24812,6 +24825,22 @@ process_scope_var (tree stmt, tree decl, tree origin, dw_die_ref context_die)
   else
     die = NULL;
 
+  /* Avoid creating DIEs for local typedefs and concrete static variables that
+     will only be pruned later.  */
+  if ((origin || decl_ultimate_origin (decl))
+      && (TREE_CODE (decl_or_origin) == TYPE_DECL
+	  || (VAR_P (decl_or_origin) && TREE_STATIC (decl_or_origin))))
+    {
+      origin = decl_ultimate_origin (decl_or_origin);
+      if (decl && VAR_P (decl) && die != NULL)
+	{
+	  die = lookup_decl_die (origin);
+	  if (die != NULL)
+	    equate_decl_number_to_die (decl, die);
+	}
+      return;
+    }
+
   if (die != NULL && die->die_parent == NULL)
     add_child_die (context_die, die);
   else if (TREE_CODE (decl_or_origin) == IMPORTED_DECL)
diff --git a/gcc/testsuite/gnat.dg/debug13.adb b/gcc/testsuite/gnat.dg/debug13.adb
new file mode 100644
index 0000000..4b94b3e
--- /dev/null
+++ b/gcc/testsuite/gnat.dg/debug13.adb
@@ -0,0 +1,38 @@
+--  { dg-options "-cargs -O2 -g -margs" }
+
+package body Debug13 is
+
+   procedure Compile (P : Natural)
+   is
+      Max_Pos : constant Natural := P;
+      type Position_Set is array (1 .. Max_Pos) of Boolean;
+
+      Empty  : constant Position_Set := (others => False);
+
+      type Position_Set_Array is array (1 .. Max_Pos) of Position_Set;
+
+      Follow  : Position_Set_Array := (others => Empty);
+
+      function Get_Follows return Position_Set;
+
+      procedure Make_DFA;
+
+      function Get_Follows return Position_Set is
+         Result : Position_Set := Empty;
+      begin
+         Result := Result or Follow (1);
+
+         return Result;
+      end Get_Follows;
+
+      procedure Make_DFA is
+         Next   : constant Position_Set := Get_Follows;
+      begin
+         null;
+      end Make_DFA;
+
+   begin
+      Make_DFA;
+   end Compile;
+
+end Debug13;
diff --git a/gcc/testsuite/gnat.dg/debug13.ads b/gcc/testsuite/gnat.dg/debug13.ads
new file mode 100644
index 0000000..512a9ef
--- /dev/null
+++ b/gcc/testsuite/gnat.dg/debug13.ads
@@ -0,0 +1,5 @@
+package Debug13 is
+
+   procedure Compile (P : Natural);
+
+end Debug13;
-- 
2.3.3.199.g52cae64

Re: [PATCH] [PR79542][Ada] Fix ICE in dwarf2out.c with nested func. inlining

[Jason Merrill]

On Fri, Aug 11, 2017 at 6:05 AM, Pierre-Marie de Rodat
<derodat@adacore.com> wrote:
> On 08/07/2017 08:42 PM, Jason Merrill wrote:
>>
>> Perhaps the DECL_ABSTRACT_ORIGIN check in my patch should be
>> decl_ultimate_origin instead, which should return null in that case?
>
>
> It looks like it worked, thanks! Note that I had to tweak a bit your change
> in process_scope_vars to avoid a crash in a couple of Fortran tests (at -O3
> -g). Also, as the gcc_unreachable appears in an IF block, I turned it into a
> gcc_assert and remove the corresponding IF.
>
> Here is the final patch version, bootstrapped and regtested on x86_64-linux.

OK.

Jason

Re: [PATCH] [PR79542][Ada] Fix ICE in dwarf2out.c with nested func. inlining

[Pierre-Marie de Rodat]

On 08/11/2017 11:29 PM, Jason Merrill wrote:
> OK.

Committed. Thank you for your sustained review effort, Jason. :-)

-- 
Pierre-Marie de Rodat

Re: [PATCH] [PR79542][Ada] Fix ICE in dwarf2out.c with nested func. inlining

[Richard Biener]

On Sat, Aug 12, 2017 at 11:09 AM, Pierre-Marie de Rodat
<derodat@adacore.com> wrote:
> On 08/11/2017 11:29 PM, Jason Merrill wrote:
>>
>> OK.
>
>
> Committed. Thank you for your sustained review effort, Jason. :-)

The way you use decl_ultimate_origin conflicts with the early LTO
debug patches which
make dwarf2out_abstract_function call set_decl_origin_self and thus the assert
in gen_typedef_die triggers (and the rest probably misbehaves).

Now I wonder whether we at any point need that self-origin?

Currently it's set via

static dw_die_ref
gen_decl_die (tree decl, tree origin, struct vlr_context *ctx,
              dw_die_ref context_die)
{
...
    case FUNCTION_DECL:
#if 0
      /* FIXME */
      /* This doesn't work because the C frontend sets DECL_ABSTRACT_ORIGIN
         on local redeclarations of global functions.  That seems broken.  */
      if (current_function_decl != decl)
        /* This is only a declaration.  */;
#endif

      /* If we're emitting a clone, emit info for the abstract instance.  */
      if (origin || DECL_ORIGIN (decl) != decl)
        dwarf2out_abstract_function (origin
                                     ? DECL_ORIGIN (origin)
                                     : DECL_ABSTRACT_ORIGIN (decl));

      /* If we're emitting an out-of-line copy of an inline function,
         emit info for the abstract instance and set up to refer to it.  */
      else if (cgraph_function_possibly_inlined_p (decl)
               && ! DECL_ABSTRACT_P (decl)
               && ! class_or_namespace_scope_p (context_die)
               /* dwarf2out_abstract_function won't emit a die if this is just
                  a declaration.  We must avoid setting DECL_ABSTRACT_ORIGIN in
                  that case, because that works only if we have a die.  */
               && DECL_INITIAL (decl) != NULL_TREE)
        {
          dwarf2out_abstract_function (decl);
          set_decl_origin_self (decl);
        }

ok, not doing this at all doesn't work, doing it only in the above case neither.

Bah.

Can anyone explain to me why we do the set_decl_origin_self dance?

Richard.

> --
> Pierre-Marie de Rodat

Re: [PATCH] [PR79542][Ada] Fix ICE in dwarf2out.c with nested func. inlining

[Richard Biener]

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

On Tue, Aug 15, 2017 at 1:16 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Sat, Aug 12, 2017 at 11:09 AM, Pierre-Marie de Rodat
> <derodat@adacore.com> wrote:
>> On 08/11/2017 11:29 PM, Jason Merrill wrote:
>>>
>>> OK.
>>
>>
>> Committed. Thank you for your sustained review effort, Jason. :-)
>
> The way you use decl_ultimate_origin conflicts with the early LTO
> debug patches which
> make dwarf2out_abstract_function call set_decl_origin_self and thus the assert
> in gen_typedef_die triggers (and the rest probably misbehaves).
>
> Now I wonder whether we at any point need that self-origin?
>
> Currently it's set via
>
> static dw_die_ref
> gen_decl_die (tree decl, tree origin, struct vlr_context *ctx,
>               dw_die_ref context_die)
> {
> ...
>     case FUNCTION_DECL:
> #if 0
>       /* FIXME */
>       /* This doesn't work because the C frontend sets DECL_ABSTRACT_ORIGIN
>          on local redeclarations of global functions.  That seems broken.  */
>       if (current_function_decl != decl)
>         /* This is only a declaration.  */;
> #endif
>
>       /* If we're emitting a clone, emit info for the abstract instance.  */
>       if (origin || DECL_ORIGIN (decl) != decl)
>         dwarf2out_abstract_function (origin
>                                      ? DECL_ORIGIN (origin)
>                                      : DECL_ABSTRACT_ORIGIN (decl));
>
>       /* If we're emitting an out-of-line copy of an inline function,
>          emit info for the abstract instance and set up to refer to it.  */
>       else if (cgraph_function_possibly_inlined_p (decl)
>                && ! DECL_ABSTRACT_P (decl)
>                && ! class_or_namespace_scope_p (context_die)
>                /* dwarf2out_abstract_function won't emit a die if this is just
>                   a declaration.  We must avoid setting DECL_ABSTRACT_ORIGIN in
>                   that case, because that works only if we have a die.  */
>                && DECL_INITIAL (decl) != NULL_TREE)
>         {
>           dwarf2out_abstract_function (decl);
>           set_decl_origin_self (decl);
>         }
>
> ok, not doing this at all doesn't work, doing it only in the above case neither.
>
> Bah.
>
> Can anyone explain to me why we do the set_decl_origin_self dance?

Ok, so I need the following incremental patch to fix the fallout.

This allows Ada LTO bootstrap to succeed with the early LTO debug patches.

I assume this change is ok ontop of the LTO debug patches unless I
hear otherwise
til Monday (when I then finally will commit the series).

Full bootstrap/testing running now.

Thanks,
Richard.

2017-08-18  Richard Biener  <rguenther@suse.de>

        * dwarf2out.c (modified_type_die): Check for self origin before
        recursing.
        (gen_type_die_with_usage): Likewise.
        (gen_typedef_die): Allow self origin.
        * tree.c (variably_modified_type_p): Guard against Ada recursive
        pointer types.

[-- Attachment #2: p --]
[-- Type: application/octet-stream, Size: 2492 bytes --]

2017-08-18  Richard Biener  <rguenther@suse.de>

	* dwarf2out.c (modified_type_die): Check for self origin before
	recursing.
	(gen_type_die_with_usage): Likewise.
	(gen_typedef_die): Allow self origin.
	* tree.c (variably_modified_type_p): Guard against Ada recursive
	pointer types.

Index: early-lto-debug/gcc/dwarf2out.c
===================================================================
--- early-lto-debug.orig/gcc/dwarf2out.c	2017-08-18 12:07:37.670609820 +0200
+++ early-lto-debug/gcc/dwarf2out.c	2017-08-18 10:16:59.261727525 +0200
@@ -12837,7 +12837,7 @@ modified_type_die (tree type, int cv_qua
 	  /* Typedef variants that have an abstract origin don't get their own
 	     type DIE (see gen_typedef_die), so fall back on the ultimate
 	     abstract origin instead.  */
-	  if (origin != NULL)
+	  if (origin != NULL && origin != name)
 	    return modified_type_die (TREE_TYPE (origin), cv_quals, reverse,
 				      context_die);
 
@@ -24545,7 +24545,8 @@ gen_typedef_die (tree decl, dw_die_ref c
   /* As we avoid creating DIEs for local typedefs (see decl_ultimate_origin
      checks in process_scope_var and modified_type_die), this should be called
      only for original types.  */
-  gcc_assert (decl_ultimate_origin (decl) == NULL);
+  gcc_assert (decl_ultimate_origin (decl) == NULL
+	      || decl_ultimate_origin (decl) == decl);
 
   TREE_ASM_WRITTEN (decl) = 1;
   type_die = new_die (DW_TAG_typedef, context_die, decl);
@@ -24720,7 +24721,7 @@ gen_type_die_with_usage (tree type, dw_d
 
       tree name = TYPE_NAME (type);
       tree origin = decl_ultimate_origin (name);
-      if (origin != NULL)
+      if (origin != NULL && origin != name)
 	{
 	  gen_decl_die (origin, NULL, NULL, context_die);
 	  return;
Index: early-lto-debug/gcc/tree.c
===================================================================
--- early-lto-debug.orig/gcc/tree.c	2017-08-18 12:07:37.786611917 +0200
+++ early-lto-debug/gcc/tree.c	2017-08-17 16:52:50.302969994 +0200
@@ -8601,8 +8601,16 @@ variably_modified_type_p (tree type, tre
     case POINTER_TYPE:
     case REFERENCE_TYPE:
     case VECTOR_TYPE:
+      /* Ada can have pointer types refering to themselves indirectly.  */
+      if (TREE_VISITED (type))
+	return false;
+      TREE_VISITED (type) = true;
       if (variably_modified_type_p (TREE_TYPE (type), fn))
-	return true;
+	{
+	  TREE_VISITED (type) = false;
+	  return true;
+	}
+      TREE_VISITED (type) = false;
       break;
 
     case FUNCTION_TYPE:

Re: [PATCH] [PR79542][Ada] Fix ICE in dwarf2out.c with nested func. inlining

[Pierre-Marie de Rodat]

On 08/18/2017 12:10 PM, Richard Biener wrote:
>> ok, not doing this at all doesn't work, doing it only in the above case neither.
>>
>> Bah.
>>
>> Can anyone explain to me why we do the set_decl_origin_self dance?
> 
> Ok, so I need the following incremental patch to fix the fallout.
> 
> This allows Ada LTO bootstrap to succeed with the early LTO debug patches.
> 
> I assume this change is ok ontop of the LTO debug patches unless I
> hear otherwise
> til Monday (when I then finally will commit the series).
> 
> Full bootstrap/testing running now.
Sorry for the late answer, I’ve been busy the last two weeks. As 
discussed on IRC, I’m not very familiar with debug info generation for 
optimized code yet anyway. ;-)

Are there still pending issues with this? Also, do you think we can port 
the fix for PR79542 on the 7 release branch?

-- 
Pierre-Marie de Rodat

Re: [PATCH] [PR79542][Ada] Fix ICE in dwarf2out.c with nested func. inlining

[Richard Biener]

On Mon, Sep 4, 2017 at 11:20 AM, Pierre-Marie de Rodat
<derodat@adacore.com> wrote:
> On 08/18/2017 12:10 PM, Richard Biener wrote:
>>>
>>> ok, not doing this at all doesn't work, doing it only in the above case
>>> neither.
>>>
>>> Bah.
>>>
>>> Can anyone explain to me why we do the set_decl_origin_self dance?
>>
>>
>> Ok, so I need the following incremental patch to fix the fallout.
>>
>> This allows Ada LTO bootstrap to succeed with the early LTO debug patches.
>>
>> I assume this change is ok ontop of the LTO debug patches unless I
>> hear otherwise
>> til Monday (when I then finally will commit the series).
>>
>> Full bootstrap/testing running now.
>
> Sorry for the late answer, I’ve been busy the last two weeks. As discussed
> on IRC, I’m not very familiar with debug info generation for optimized code
> yet anyway. ;-)
>
> Are there still pending issues with this? Also, do you think we can port the
> fix for PR79542 on the 7 release branch?

No more pending issues and yes, I guess the fix is ok for the branch.

Richard.

> --
> Pierre-Marie de Rodat

Re: [PATCH] [PR79542][Ada] Fix ICE in dwarf2out.c with nested func. inlining

[Pierre-Marie de Rodat]

On 09/04/2017 11:26 AM, Richard Biener wrote:
> No more pending issues and yes, I guess the fix is ok for the branch.

Ok, thanks! This is now comitted on the 7 release branch.

-- 
Pierre-Marie de Rodat