[C++ Patch] PR 71737

[C++ Patch] PR 71737

[Paolo Carlini]

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

Hi,

in this error recovery issue get_underlying_template crashes when 
TYPE_TEMPLATE_INFO_MAYBE_ALIAS is applied to a null orig_type. Simply 
checking for that condition appears to solve the issue in a 
straightforward way. Tested x86_64-linux.

Thanks, Paolo.

/////////////////////


[-- Attachment #2: CL_71737 --]
[-- Type: text/plain, Size: 261 bytes --]

/cp
2017-01-13  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/71737
	* pt.c (get_underlying_template): Don't crash if orig_type
	is NULL_TREE.

/testsuite
2017-01-13  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/71737
	* g++.dg/cpp0x/pr71737.C: New.

[-- Attachment #3: patch_71737 --]
[-- Type: text/plain, Size: 1024 bytes --]

Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 244405)
+++ cp/pt.c	(working copy)
@@ -5916,6 +5916,8 @@ get_underlying_template (tree tmpl)
     {
       /* Determine if the alias is equivalent to an underlying template.  */
       tree orig_type = DECL_ORIGINAL_TYPE (DECL_TEMPLATE_RESULT (tmpl));
+      if (!orig_type)
+	break;
       tree tinfo = TYPE_TEMPLATE_INFO_MAYBE_ALIAS (orig_type);
       if (!tinfo)
 	break;
Index: testsuite/g++.dg/cpp0x/pr71737.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr71737.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/pr71737.C	(working copy)
@@ -0,0 +1,13 @@
+// PR c++/78765
+// { dg-do compile { target c++11 } }
+
+template <template <typename ...> class TT>
+struct quote {
+  template <typename ...Ts>
+  using apply = TT<Ts...>;  // { dg-error "pack expansion" }
+};
+
+template <typename>
+using to_int_t = int;
+
+using t = quote<quote<to_int_t>::apply>::apply<int>;

Re: [C++ Patch] PR 71737

[Nathan Sidwell]

On 01/13/2017 09:45 AM, Paolo Carlini wrote:
> Hi,
>
> in this error recovery issue get_underlying_template crashes when
> TYPE_TEMPLATE_INFO_MAYBE_ALIAS is applied to a null orig_type. Simply
> checking for that condition appears to solve the issue in a
> straightforward way. Tested x86_64-linux.

Wouldn't it be better if a scrogged alias got error_mark_node as the 
underlying type?  (I have no idea whether that's an easy thing to 
accomplish)

nathan
-- 
Nathan Sidwell

Re: [C++ Patch] PR 71737

[Paolo Carlini]

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

Hi,

On 13/01/2017 15:51, Nathan Sidwell wrote:
> On 01/13/2017 09:45 AM, Paolo Carlini wrote:
>> Hi,
>>
>> in this error recovery issue get_underlying_template crashes when
>> TYPE_TEMPLATE_INFO_MAYBE_ALIAS is applied to a null orig_type. Simply
>> checking for that condition appears to solve the issue in a
>> straightforward way. Tested x86_64-linux.
>
> Wouldn't it be better if a scrogged alias got error_mark_node as the 
> underlying type?  (I have no idea whether that's an easy thing to 
> accomplish)
Your reply, Nathan, led me to investigate where exactly 
DECL_ORIGINAL_TYPE becomes null, and turns out that in tsubst_decl we 
have code actively doing that. That same code, a few lines below, only 
sets TYPE_DEPENDENT_P_VALID to false if type != error_mark_node. I 
cannot say to fully understand yet all the details, but certainly the 
patchlet below also passes testing. Do you have comments about this too?

Thanks!
Paolo.

///////////////////

[-- Attachment #2: p --]
[-- Type: text/plain, Size: 559 bytes --]

Index: pt.c
===================================================================
--- pt.c	(revision 244405)
+++ pt.c	(working copy)
@@ -12868,7 +12868,8 @@ tsubst_decl (tree t, tree args, tsubst_flags_t com
 	/* Preserve a typedef that names a type.  */
 	if (is_typedef_decl (r))
 	  {
-	    DECL_ORIGINAL_TYPE (r) = NULL_TREE;
+	    if (type != error_mark_node)
+	      DECL_ORIGINAL_TYPE (r) = NULL_TREE;
 	    set_underlying_type (r);
 	    if (TYPE_DECL_ALIAS_P (r) && type != error_mark_node)
 	      /* An alias template specialization can be dependent

Re: [C++ Patch] PR 71737

[Jason Merrill]

On Fri, Jan 13, 2017 at 11:42 AM, Paolo Carlini
<paolo.carlini@oracle.com> wrote:
> Hi,
>
> On 13/01/2017 15:51, Nathan Sidwell wrote:
>>
>> On 01/13/2017 09:45 AM, Paolo Carlini wrote:
>>>
>>> Hi,
>>>
>>> in this error recovery issue get_underlying_template crashes when
>>> TYPE_TEMPLATE_INFO_MAYBE_ALIAS is applied to a null orig_type. Simply
>>> checking for that condition appears to solve the issue in a
>>> straightforward way. Tested x86_64-linux.
>>
>>
>> Wouldn't it be better if a scrogged alias got error_mark_node as the
>> underlying type?  (I have no idea whether that's an easy thing to
>> accomplish)
>
> Your reply, Nathan, led me to investigate where exactly DECL_ORIGINAL_TYPE
> becomes null, and turns out that in tsubst_decl we have code actively doing
> that. That same code, a few lines below, only sets TYPE_DEPENDENT_P_VALID to
> false if type != error_mark_node. I cannot say to fully understand yet all
> the details, but certainly the patchlet below also passes testing. Do you
> have comments about this too?

The clearing of DECL_ORIGINAL_TYPE is to allow set_underlying_type to
then set it to something more appropriate.  That function currently
avoids setting DECL_ORIGINAL_TYPE to error_mark_node, perhaps that
should be changed.

Jason

Re: [C++ Patch] PR 71737

[Paolo Carlini]

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

Hi,

On 13/01/2017 18:33, Jason Merrill wrote:
> On Fri, Jan 13, 2017 at 11:42 AM, Paolo Carlini
> <paolo.carlini@oracle.com> wrote:
>> Hi,
>>
>> On 13/01/2017 15:51, Nathan Sidwell wrote:
>>> On 01/13/2017 09:45 AM, Paolo Carlini wrote:
>>>> Hi,
>>>>
>>>> in this error recovery issue get_underlying_template crashes when
>>>> TYPE_TEMPLATE_INFO_MAYBE_ALIAS is applied to a null orig_type. Simply
>>>> checking for that condition appears to solve the issue in a
>>>> straightforward way. Tested x86_64-linux.
>>>
>>> Wouldn't it be better if a scrogged alias got error_mark_node as the
>>> underlying type?  (I have no idea whether that's an easy thing to
>>> accomplish)
>> Your reply, Nathan, led me to investigate where exactly DECL_ORIGINAL_TYPE
>> becomes null, and turns out that in tsubst_decl we have code actively doing
>> that. That same code, a few lines below, only sets TYPE_DEPENDENT_P_VALID to
>> false if type != error_mark_node. I cannot say to fully understand yet all
>> the details, but certainly the patchlet below also passes testing. Do you
>> have comments about this too?
> The clearing of DECL_ORIGINAL_TYPE is to allow set_underlying_type to
> then set it to something more appropriate.  That function currently
> avoids setting DECL_ORIGINAL_TYPE to error_mark_node, perhaps that
> should be changed.
I see, thanks a lot. The below passes testing on x86_64-linux.

Paolo.

///////////////////////

[-- Attachment #2: patch_71737_2 --]
[-- Type: text/plain, Size: 1518 bytes --]

Index: c-family/c-common.c
===================================================================
--- c-family/c-common.c	(revision 244405)
+++ c-family/c-common.c	(working copy)
@@ -7419,16 +7419,18 @@ set_underlying_type (tree x)
       if (TYPE_NAME (TREE_TYPE (x)) == 0)
 	TYPE_NAME (TREE_TYPE (x)) = x;
     }
-  else if (TREE_TYPE (x) != error_mark_node
-	   && DECL_ORIGINAL_TYPE (x) == NULL_TREE)
+  else if (DECL_ORIGINAL_TYPE (x) == NULL_TREE)
     {
       tree tt = TREE_TYPE (x);
       DECL_ORIGINAL_TYPE (x) = tt;
-      tt = build_variant_type_copy (tt);
-      TYPE_STUB_DECL (tt) = TYPE_STUB_DECL (DECL_ORIGINAL_TYPE (x));
-      TYPE_NAME (tt) = x;
-      TREE_USED (tt) = TREE_USED (x);
-      TREE_TYPE (x) = tt;
+      if (tt != error_mark_node)
+	{
+	  tt = build_variant_type_copy (tt);
+	  TYPE_STUB_DECL (tt) = TYPE_STUB_DECL (DECL_ORIGINAL_TYPE (x));
+	  TYPE_NAME (tt) = x;
+	  TREE_USED (tt) = TREE_USED (x);
+	  TREE_TYPE (x) = tt;
+	}
     }
 }
 
Index: testsuite/g++.dg/cpp0x/pr71737.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr71737.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/pr71737.C	(working copy)
@@ -0,0 +1,13 @@
+// PR c++/78765
+// { dg-do compile { target c++11 } }
+
+template <template <typename ...> class TT>
+struct quote {
+  template <typename ...Ts>
+  using apply = TT<Ts...>;  // { dg-error "pack expansion" }
+};
+
+template <typename>
+using to_int_t = int;
+
+using t = quote<quote<to_int_t>::apply>::apply<int>;

Re: [C++ Patch] PR 71737

[Jason Merrill]

OK.

On Fri, Jan 13, 2017 at 5:58 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi,
>
> On 13/01/2017 18:33, Jason Merrill wrote:
>>
>> On Fri, Jan 13, 2017 at 11:42 AM, Paolo Carlini
>> <paolo.carlini@oracle.com> wrote:
>>>
>>> Hi,
>>>
>>> On 13/01/2017 15:51, Nathan Sidwell wrote:
>>>>
>>>> On 01/13/2017 09:45 AM, Paolo Carlini wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> in this error recovery issue get_underlying_template crashes when
>>>>> TYPE_TEMPLATE_INFO_MAYBE_ALIAS is applied to a null orig_type. Simply
>>>>> checking for that condition appears to solve the issue in a
>>>>> straightforward way. Tested x86_64-linux.
>>>>
>>>>
>>>> Wouldn't it be better if a scrogged alias got error_mark_node as the
>>>> underlying type?  (I have no idea whether that's an easy thing to
>>>> accomplish)
>>>
>>> Your reply, Nathan, led me to investigate where exactly
>>> DECL_ORIGINAL_TYPE
>>> becomes null, and turns out that in tsubst_decl we have code actively
>>> doing
>>> that. That same code, a few lines below, only sets TYPE_DEPENDENT_P_VALID
>>> to
>>> false if type != error_mark_node. I cannot say to fully understand yet
>>> all
>>> the details, but certainly the patchlet below also passes testing. Do you
>>> have comments about this too?
>>
>> The clearing of DECL_ORIGINAL_TYPE is to allow set_underlying_type to
>> then set it to something more appropriate.  That function currently
>> avoids setting DECL_ORIGINAL_TYPE to error_mark_node, perhaps that
>> should be changed.
>
> I see, thanks a lot. The below passes testing on x86_64-linux.
>
> Paolo.
>
> ///////////////////////

Re: [C++ Patch] PR 71737

[Paolo Carlini]

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

Hi again,

On 14/01/2017 15:43, Jason Merrill wrote:
> OK.
As you may or may not have noticed, I had to revert the patch because it 
caused the regression of 
tr1/2_general_utilities/shared_ptr/cons/43820_neg.cc during error 
recovery: an ICE happens in remap_decl when build_variant_type_copy is 
called on a TREE_TYPE (== DECL_ORIGINAL_TYPE) which is error_mark_node. 
A possible straightforward and safe fix for the new issue would be 
simply checking for the special condition, as I did in patch_71737_3 
below, which passes testing. Alternately, I have been fiddling also with 
older approaches, and patch_71737_4 below also passes testing: it simply 
renounces to preserve a typedef which names an error_mark_node type.

Thanks again!
Paolo.

//////////////////////////

[-- Attachment #2: patch_71737_3 --]
[-- Type: text/plain, Size: 2216 bytes --]

Index: c-family/c-common.c
===================================================================
--- c-family/c-common.c	(revision 245274)
+++ c-family/c-common.c	(working copy)
@@ -7420,16 +7420,18 @@ set_underlying_type (tree x)
       if (TYPE_NAME (TREE_TYPE (x)) == 0)
 	TYPE_NAME (TREE_TYPE (x)) = x;
     }
-  else if (TREE_TYPE (x) != error_mark_node
-	   && DECL_ORIGINAL_TYPE (x) == NULL_TREE)
+  else if (DECL_ORIGINAL_TYPE (x) == NULL_TREE)
     {
       tree tt = TREE_TYPE (x);
       DECL_ORIGINAL_TYPE (x) = tt;
-      tt = build_variant_type_copy (tt);
-      TYPE_STUB_DECL (tt) = TYPE_STUB_DECL (DECL_ORIGINAL_TYPE (x));
-      TYPE_NAME (tt) = x;
-      TREE_USED (tt) = TREE_USED (x);
-      TREE_TYPE (x) = tt;
+      if (tt != error_mark_node)
+	{
+	  tt = build_variant_type_copy (tt);
+	  TYPE_STUB_DECL (tt) = TYPE_STUB_DECL (DECL_ORIGINAL_TYPE (x));
+	  TYPE_NAME (tt) = x;
+	  TREE_USED (tt) = TREE_USED (x);
+	  TREE_TYPE (x) = tt;
+	}
     }
 }
 
Index: testsuite/g++.dg/cpp0x/pr71737.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr71737.C	(revision 245274)
+++ testsuite/g++.dg/cpp0x/pr71737.C	(working copy)
@@ -0,0 +1,13 @@
+// PR c++/78765
+// { dg-do compile { target c++11 } }
+
+template <template <typename ...> class TT>
+struct quote {
+  template <typename ...Ts>
+  using apply = TT<Ts...>;  // { dg-error "pack expansion" }
+};
+
+template <typename>
+using to_int_t = int;
+
+using t = quote<quote<to_int_t>::apply>::apply<int>;
Index: tree-inline.c
===================================================================
--- tree-inline.c	(revision 245274)
+++ tree-inline.c	(working copy)
@@ -375,7 +375,8 @@ remap_decl (tree decl, copy_body_data *id)
 	  /* Preserve the invariant that DECL_ORIGINAL_TYPE != TREE_TYPE,
 	     which is enforced in gen_typedef_die when DECL_ABSTRACT_ORIGIN
 	     is not set on the TYPE_DECL, for example in LTO mode.  */
-	  if (DECL_ORIGINAL_TYPE (t) == TREE_TYPE (t))
+	  if (DECL_ORIGINAL_TYPE (t) == TREE_TYPE (t)
+	      && TREE_TYPE (t) != error_mark_node)
 	    {
 	      tree x = build_variant_type_copy (TREE_TYPE (t));
 	      TYPE_STUB_DECL (x) = TYPE_STUB_DECL (TREE_TYPE (t));

[-- Attachment #3: patch_71737_4 --]
[-- Type: text/plain, Size: 1260 bytes --]

Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 245274)
+++ cp/pt.c	(working copy)
@@ -12876,11 +12876,11 @@ tsubst_decl (tree t, tree args, tsubst_flags_t com
 					args, complain, in_decl);
 
 	/* Preserve a typedef that names a type.  */
-	if (is_typedef_decl (r))
+	if (is_typedef_decl (r) && type != error_mark_node)
 	  {
 	    DECL_ORIGINAL_TYPE (r) = NULL_TREE;
 	    set_underlying_type (r);
-	    if (TYPE_DECL_ALIAS_P (r) && type != error_mark_node)
+	    if (TYPE_DECL_ALIAS_P (r))
 	      /* An alias template specialization can be dependent
 		 even if its underlying type is not.  */
 	      TYPE_DEPENDENT_P_VALID (TREE_TYPE (r)) = false;
Index: testsuite/g++.dg/cpp0x/pr71737.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr71737.C	(revision 245274)
+++ testsuite/g++.dg/cpp0x/pr71737.C	(working copy)
@@ -0,0 +1,13 @@
+// PR c++/78765
+// { dg-do compile { target c++11 } }
+
+template <template <typename ...> class TT>
+struct quote {
+  template <typename ...Ts>
+  using apply = TT<Ts...>;  // { dg-error "pack expansion" }
+};
+
+template <typename>
+using to_int_t = int;
+
+using t = quote<quote<to_int_t>::apply>::apply<int>;

Re: [C++ Patch] PR 71737

[Jason Merrill]

On Thu, Feb 9, 2017 at 4:08 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> On 14/01/2017 15:43, Jason Merrill wrote:
>>
>> OK.
>
> As you may or may not have noticed, I had to revert the patch because it
> caused the regression of
> tr1/2_general_utilities/shared_ptr/cons/43820_neg.cc during error recovery:
> an ICE happens in remap_decl when build_variant_type_copy is called on a
> TREE_TYPE (== DECL_ORIGINAL_TYPE) which is error_mark_node. A possible
> straightforward and safe fix for the new issue would be simply checking for
> the special condition, as I did in patch_71737_3 below, which passes
> testing. Alternately, I have been fiddling also with older approaches, and
> patch_71737_4 below also passes testing: it simply renounces to preserve a
> typedef which names an error_mark_node type.

_4 seems consistent with the set_underlying_type behavior, I guess
let's go with that.

Jason

Re: [C++ Patch] PR 71737

[Paolo Carlini]

On 16/01/2017 16:11, David Edelsohn wrote:
> This patch caused a libstdc++ regression on AIX
>
> libstdc++-v3/include/tr1/shared_ptr.h:556: internal compiler error:
> tree check: expected class 'type', have 'exceptional' (error_mark) in
> build_variant_type_copy, at tree.c:6737
I noticed, patch reverted for the time being.

Sorry about the annoyance,
Paolo.

Re: [C++ Patch] PR 71737

[David Edelsohn]

This patch caused a libstdc++ regression on AIX

libstdc++-v3/include/tr1/shared_ptr.h:556: internal compiler error:
tree check: expected class 'type', have 'exceptional' (error_mark) in
build_variant_type_copy, at tree.c:6737