Re: [C++ Patch] PR 71464 ("[6/7/8 Regression] ICE on invalid code (with redeclared constructor) at -Os: Segmentation fault")

[Jason Merrill]

OK.

On Sat, Mar 3, 2018 at 4:50 AM, Paolo Carlini  wrote:
> Hi,
>
> On 03/03/2018 06:13, Jason Merrill wrote:
>>
>> On Fri, Mar 2, 2018 at 4:02 PM, Paolo Carlini 
>> wrote:
>>>
>>> this error recovery ICE happens only with -Os and is just a P5 - on the
>>> other hand I would argue the reproducer isn't that exotic! - but seems
>>> fixable easily and safely: cdtor_comdat_group immediately calls
>>> DECL_ASSEMBLER_NAME on both arguments and of course crashes if they are
>>> null. Tested x86_64-linux.
>>
>> It would make more sense to me to do this check and return right after
>> populate_clone_array.
>
> Oh nice. Then, given that the existing 'if (fns[0] && ...' looked a bit
> weird to me, to be super safe I also ran the the testsuite with 'gcc_assert
> (fns[0] && fns[1]);' and everything went well, it only triggered for the new
> testcase. Is the below OK?
>
> Thanks,
> Paolo.
>
> //

Re: [C++ Patch] PR 71464 ("[6/7/8 Regression] ICE on invalid code (with redeclared constructor) at -Os: Segmentation fault")

[Paolo Carlini]

Hi,

On 03/03/2018 06:13, Jason Merrill wrote:

On Fri, Mar 2, 2018 at 4:02 PM, Paolo Carlini  wrote:

this error recovery ICE happens only with -Os and is just a P5 - on the
other hand I would argue the reproducer isn't that exotic! - but seems
fixable easily and safely: cdtor_comdat_group immediately calls
DECL_ASSEMBLER_NAME on both arguments and of course crashes if they are
null. Tested x86_64-linux.

It would make more sense to me to do this check and return right after
populate_clone_array.
Oh nice. Then, given that the existing 'if (fns[0] && ...' looked a bit 
weird to me, to be super safe I also ran the the testsuite with 
'gcc_assert (fns[0] && fns[1]);' and everything went well, it only 
triggered for the new testcase. Is the below OK?


Thanks,
Paolo.

//
Index: cp/optimize.c
===
--- cp/optimize.c   (revision 258165)
+++ cp/optimize.c   (working copy)
@@ -261,8 +261,12 @@ maybe_thunk_body (tree fn, bool force)
 
   populate_clone_array (fn, fns);
 
+  /* Can happen during error recovery (c++/71464).  */
+  if (!fns[0] || !fns[1])
+return 0;
+
   /* Don't use thunks if the base clone omits inherited parameters.  */
-  if (fns[0] && ctor_omit_inherited_parms (fns[0]))
+  if (ctor_omit_inherited_parms (fns[0]))
 return 0;
 
   DECL_ABSTRACT_P (fn) = false;
Index: testsuite/g++.dg/torture/pr71464.C
===
--- testsuite/g++.dg/torture/pr71464.C  (nonexistent)
+++ testsuite/g++.dg/torture/pr71464.C  (working copy)
@@ -0,0 +1,7 @@
+struct A {}; 
+
+struct B : virtual A
+{
+  B () {};
+  B () {};  // { dg-error "cannot be overloaded" }
+};

Re: [C++ Patch] PR 71464 ("[6/7/8 Regression] ICE on invalid code (with redeclared constructor) at -Os: Segmentation fault")

[Jason Merrill]

On Fri, Mar 2, 2018 at 4:02 PM, Paolo Carlini  wrote:
> this error recovery ICE happens only with -Os and is just a P5 - on the
> other hand I would argue the reproducer isn't that exotic! - but seems
> fixable easily and safely: cdtor_comdat_group immediately calls
> DECL_ASSEMBLER_NAME on both arguments and of course crashes if they are
> null. Tested x86_64-linux.

It would make more sense to me to do this check and return right after
populate_clone_array.

Jason

[C++ Patch] PR 71464 ("[6/7/8 Regression] ICE on invalid code (with redeclared constructor) at -Os: Segmentation fault")

[Paolo Carlini]

Hi,

this error recovery ICE happens only with -Os and is just a P5 - on the 
other hand I would argue the reproducer isn't that exotic! - but seems 
fixable easily and safely: cdtor_comdat_group immediately calls 
DECL_ASSEMBLER_NAME on both arguments and of course crashes if they are 
null. Tested x86_64-linux.


Thanks, Paolo.

//

/cp
2018-03-02  Paolo Carlini  

PR c++/71464
* optimize.c (maybe_thunk_body): When HAVE_COMDAT_GROUP is true,
bail out immediately if either fns[1] or fns[0] is null.

/testsuite
2018-03-02  Paolo Carlini  

PR c++/71464
* g++.dg/torture/pr71464.C: New.
Index: cp/optimize.c
===
--- cp/optimize.c   (revision 258151)
+++ cp/optimize.c   (working copy)
@@ -276,6 +276,9 @@ maybe_thunk_body (tree fn, bool force)
 {
   /* At eof, defer creation of mangling aliases temporarily.  */
   bool save_defer_mangling_aliases = defer_mangling_aliases;
+  if (!fns[1] || !fns[0])
+   /* Can happen during error recovery (c++/71464).  */
+   return 0;
   defer_mangling_aliases = true;
   tree comdat_group = cdtor_comdat_group (fns[1], fns[0]);
   defer_mangling_aliases = save_defer_mangling_aliases;
Index: testsuite/g++.dg/torture/pr71464.C
===
--- testsuite/g++.dg/torture/pr71464.C  (nonexistent)
+++ testsuite/g++.dg/torture/pr71464.C  (working copy)
@@ -0,0 +1,7 @@
+struct A {}; 
+
+struct B : virtual A
+{
+  B () {};
+  B () {};  // { dg-error "cannot be overloaded" }
+};