[Bug c++/81942] ICE on empty constexpr constructor with C++14

[paolo.carlini at oracle dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81942

Paolo Carlini  changed:

   What|Removed |Added

 CC||nsz at gcc dot gnu.org

--- Comment #18 from Paolo Carlini  ---
*** Bug 82023 has been marked as a duplicate of this bug. ***

[Bug c++/81942] ICE on empty constexpr constructor with C++14

[paolo.carlini at oracle dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81942

Paolo Carlini  changed:

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
 Resolution|--- |FIXED
   Target Milestone|--- |8.0

--- Comment #17 from Paolo Carlini  ---
Fixed.

[Bug c++/81942] ICE on empty constexpr constructor with C++14

[paolo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81942

--- Comment #16 from paolo at gcc dot gnu.org  ---
Author: paolo
Date: Tue Sep  5 13:33:44 2017
New Revision: 251714

URL: https://gcc.gnu.org/viewcvs?rev=251714=gcc=rev
Log:
/cp
2017-09-05  Paolo Carlini  

PR c++/81942
* cp-tree.h (LABEL_DECL_CDTOR): Add and document.
* decl.c (start_preparsed_function): Set LABEL_DECL_CDTOR when
creating cdtor_label.
* constexpr.c (returns): Add the case of a constructor/destructor
returning via a LABEL_DECL_CDTOR label.
(cxx_eval_constant_expression, case [GOTO_EXPR]): Likewise.

/testsuite
2017-09-05  Paolo Carlini  

PR c++/81942
* g++.dg/cpp1y/constexpr-return3.C: New.

Added:
trunk/gcc/testsuite/g++.dg/cpp1y/constexpr-return3.C
Modified:
trunk/gcc/cp/ChangeLog
trunk/gcc/cp/constexpr.c
trunk/gcc/cp/cp-tree.h
trunk/gcc/cp/decl.c
trunk/gcc/testsuite/ChangeLog

[Bug c++/81942] ICE on empty constexpr constructor with C++14

[thopre01 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81942

--- Comment #15 from Thomas Preud'homme  ---
(In reply to Jakub Jelinek from comment #14)
> The patch LGTM, but I'll defer the final say to Jason/Nathan.

FYI I tried the patch on arm-none-eabi toolchain and it fixes the bug reported.

Best regards.

[Bug c++/81942] ICE on empty constexpr constructor with C++14

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81942

--- Comment #14 from Jakub Jelinek  ---
The patch LGTM, but I'll defer the final say to Jason/Nathan.

[Bug c++/81942] ICE on empty constexpr constructor with C++14

[paolo.carlini at oracle dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81942

Paolo Carlini  changed:

   What|Removed |Added

 CC|paolo.carlini at oracle dot com|

--- Comment #13 from Paolo Carlini  ---
Patch passed testing on both x86_64-linux and aarch64-linux. I'll probably send
it to the mailing list with minimal modifications.

[Bug c++/81942] ICE on empty constexpr constructor with C++14

[paolo.carlini at oracle dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81942

Paolo Carlini  changed:

   What|Removed |Added

 Status|NEW |ASSIGNED
   Assignee|unassigned at gcc dot gnu.org  |paolo.carlini at oracle 
dot com

[Bug c++/81942] ICE on empty constexpr constructor with C++14

[paolo.carlini at oracle dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81942

--- Comment #12 from Paolo Carlini  ---
I think this is getting closer:

Index: constexpr.c
===
--- constexpr.c (revision 251607)
+++ constexpr.c (working copy)
@@ -3671,7 +3671,9 @@ static bool
 returns (tree *jump_target)
 {
   return *jump_target
-&& TREE_CODE (*jump_target) == RETURN_EXPR;
+&& (TREE_CODE (*jump_target) == RETURN_EXPR
+   || (TREE_CODE (*jump_target) == LABEL_DECL
+   && LABEL_DECL_CDTOR (*jump_target)));
 }

 static bool
@@ -4554,7 +4556,9 @@ cxx_eval_constant_expression (const constexpr_ctx

 case GOTO_EXPR:
   *jump_target = TREE_OPERAND (t, 0);
-  gcc_assert (breaks (jump_target) || continues (jump_target));
+  gcc_assert (breaks (jump_target) || continues (jump_target)
+ /* Allow for jumping to a cdtor_label.  */
+ || returns (jump_target));
   break;

 case LOOP_EXPR:
Index: cp-tree.h
===
--- cp-tree.h   (revision 251607)
+++ cp-tree.h   (working copy)
@@ -3833,6 +3833,11 @@ more_aggr_init_expr_args_p (const aggr_init_expr_a
 #define LABEL_DECL_CONTINUE(NODE) \
   DECL_LANG_FLAG_1 (LABEL_DECL_CHECK (NODE))

+/* Nonzero if NODE is the target for genericization of 'return' stmts
+   in constructors/destructors of targetm.cxx.cdtor_returns_this targets.  */
+#define LABEL_DECL_CDTOR(NODE) \
+  DECL_LANG_FLAG_2 (LABEL_DECL_CHECK (NODE))
+
 /* True if NODE was declared with auto in its return type, but it has
started compilation and so the return type might have been changed by
return type deduction; its declared return type should be found in
Index: decl.c
===
--- decl.c  (revision 251607)
+++ decl.c  (working copy)
@@ -15072,7 +15073,10 @@ start_preparsed_function (tree decl1, tree attrs,
   if (DECL_DESTRUCTOR_P (decl1)
   || (DECL_CONSTRUCTOR_P (decl1)
  && targetm.cxx.cdtor_returns_this ()))
-cdtor_label = create_artificial_label (input_location);
+{
+  cdtor_label = create_artificial_label (input_location);
+  LABEL_DECL_CDTOR (cdtor_label) = true;
+}

   start_fname_decls ();

[Bug c++/81942] ICE on empty constexpr constructor with C++14

[paolo.carlini at oracle dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81942

--- Comment #11 from Paolo Carlini  ---
Uhm, no, we are not completely safe. Because in general, per 10.1.5, a
constexpr function is *not* supposed to contain goto statements, and our code
reflects that in various implicit/subtle ways. Thus the only completely Ok way
I see to fully fix the problem would be modeling somehow such special goto +
return this as plain returns, for example the returns helper function should
return true for those: the cxx_eval_statement_list loop would break as soon as
one is seen, consistently for the various targets.

[Bug c++/81942] ICE on empty constexpr constructor with C++14

[paolo.carlini at oracle dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81942

Paolo Carlini  changed:

   What|Removed |Added

 CC||jakub at gcc dot gnu.org

--- Comment #10 from Paolo Carlini  ---
Well, if we are Ok with loosening a bit the gcc_assert which is firing for such
targets, I don't think we risk miscompiling anything, I think we are already
handling correctly the case of a return in user code which boils down to a goto
to a label immediately followed by return this.

I'm adding Jakub in CC, he hacked quite a bit related constexpr.c code in order
to completely fix constexpr-77467.C

Index: constexpr.c
===
--- constexpr.c (revision 251607)
+++ constexpr.c (working copy)
@@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "config.h"
 #include "system.h"
 #include "coretypes.h"
+#include "target.h"
 #include "cp-tree.h"
 #include "varasm.h"
 #include "c-family/c-objc.h"
@@ -4554,7 +4555,14 @@ cxx_eval_constant_expression (const constexpr_ctx

 case GOTO_EXPR:
   *jump_target = TREE_OPERAND (t, 0);
-  gcc_assert (breaks (jump_target) || continues (jump_target));
+  gcc_assert (breaks (jump_target) || continues (jump_target)
+ /* Allow for jumping to a cdtor_label.  */
+ || (targetm.cxx.cdtor_returns_this ()
+ && *jump_target
+ && TREE_CODE (*jump_target) == LABEL_DECL
+ && DECL_ARTIFICIAL (*jump_target)
+ && (DECL_CONSTRUCTOR_P (DECL_CONTEXT (*jump_target))
+ || DECL_DESTRUCTOR_P (DECL_CONTEXT
(*jump_target);
   break;

 case LOOP_EXPR:

[Bug c++/81942] ICE on empty constexpr constructor with C++14

[paolo.carlini at oracle dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81942

--- Comment #9 from Paolo Carlini  ---
I meant constexpr-77467.C

[Bug c++/81942] ICE on empty constexpr constructor with C++14

[paolo.carlini at oracle dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81942

--- Comment #8 from Paolo Carlini  ---
Thanks.

What I quickly hacked can't be completely correct because it breaks
constexpr-7747.C on x86_64-linux. That most likely implies that since we are
using DECL_ARTIFICIAL labels in other cases we need at least something more
accurate to tell the special labels created for such targets.

[Bug c++/81942] ICE on empty constexpr constructor with C++14

[thopre01 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81942

--- Comment #7 from Thomas Preud'homme  ---
Hi Paolo,

Thanks for working on this.

(In reply to Paolo Carlini from comment #6)
> It would be nice if somebody with a fully functional ARM toolchain could
> check whether something like the below at least avoids the ICE without
> causing any regressions. I hope it does.
> 
> Index: constexpr.c
> ===
> --- constexpr.c   (revision 251553)
> +++ constexpr.c   (working copy)
> @@ -3679,7 +3679,7 @@ breaks (tree *jump_target)
>  {
>return *jump_target
>  && ((TREE_CODE (*jump_target) == LABEL_DECL
> -  && LABEL_DECL_BREAK (*jump_target))
> +  && (LABEL_DECL_BREAK (*jump_target) || DECL_ARTIFICIAL (*jump_target)))
>   || TREE_CODE (*jump_target) == EXIT_EXPR);
>  }

The patch works for me on the testcase I provided, that is compilation process
returns a success error code instead of ICEing.

Best regards.

[Bug c++/81942] ICE on empty constexpr constructor with C++14

[paolo.carlini at oracle dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81942

Paolo Carlini  changed:

   What|Removed |Added

 CC||jose.marchesi at oracle dot com

--- Comment #6 from Paolo Carlini  ---
It would be nice if somebody with a fully functional ARM toolchain could check
whether something like the below at least avoids the ICE without causing any
regressions. I hope it does.

Index: constexpr.c
===
--- constexpr.c (revision 251553)
+++ constexpr.c (working copy)
@@ -3679,7 +3679,7 @@ breaks (tree *jump_target)
 {
   return *jump_target
 && ((TREE_CODE (*jump_target) == LABEL_DECL
-&& LABEL_DECL_BREAK (*jump_target))
+&& (LABEL_DECL_BREAK (*jump_target) || DECL_ARTIFICIAL
(*jump_target)))
|| TREE_CODE (*jump_target) == EXIT_EXPR);
 }

[Bug c++/81942] ICE on empty constexpr constructor with C++14

[paolo.carlini at oracle dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81942

--- Comment #5 from Paolo Carlini  ---
Such labels are special: they are DECL_ARTIFICIAL and are created by
create_artificial_label called by start_preparsed_function.

[Bug c++/81942] ICE on empty constexpr constructor with C++14

[paolo.carlini at oracle dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81942

--- Comment #4 from Paolo Carlini  ---
Adding more details.
The GOTO_EXPR is generated for such targets in finish_return_stmt:

 903   if (DECL_DESTRUCTOR_P (current_function_decl)
 904   || (DECL_CONSTRUCTOR_P (current_function_decl)
 905   && targetm.cxx.cdtor_returns_this ()))
 906 {
 907   /* Similarly, all destructors must run destructors for
 908  base-classes before returning.  So, all returns in a
 909  destructor get sent to the DTOR_LABEL; finish_function emits
 910  code to return a value there.  */
 911   return finish_goto_stmt (cdtor_label);
 912 }

[Bug c++/81942] ICE on empty constexpr constructor with C++14

[paolo.carlini at oracle dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81942

Thomas Preud'homme  changed:

   What|Removed |Added

   Keywords|ice-on-invalid-code |ice-on-valid-code

Paolo Carlini  changed:

   What|Removed |Added

 Status|UNCONFIRMED |NEW
   Last reconfirmed||2017-08-31
 CC||paolo.carlini at oracle dot com
 Ever confirmed|0   |1

--- Comment #2 from Thomas Preud'homme  ---
(In reply to TC from comment #1)
> Why is this tagged ice-on-invalid? The test case is valid C++14.

My bad, got confused by the error message when building for C++11.

--- Comment #3 from Paolo Carlini  ---
We ICE when cxx_eval_constant_expression encounters a GOTO_EXPR to a LABEL_DECL
target which is neither a break nor a continue. This is the .original:

;; Function constexpr A::A() (null)
;; enabled by -tree-original


{
  // predicted unlikely by goto predictor.;
  goto ;
}
:;
return this;

For comparison, for x86_64 we generate none of that, simply:

;; Function constexpr A::A() (null)
;; enabled by -tree-original


{
  return;
}

A guess an ARM expert is welcome, to understand where the goto is coming
from...

[Bug c++/81942] ICE on empty constexpr constructor with C++14

[thopre01 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81942

Thomas Preud'homme  changed:

   What|Removed |Added

   Keywords|ice-on-invalid-code |ice-on-valid-code

--- Comment #2 from Thomas Preud'homme  ---
(In reply to TC from comment #1)
> Why is this tagged ice-on-invalid? The test case is valid C++14.

My bad, got confused by the error message when building for C++11.

[Bug c++/81942] ICE on empty constexpr constructor with C++14

[rs2740 at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81942

TC  changed:

   What|Removed |Added

 CC||rs2740 at gmail dot com

--- Comment #1 from TC  ---
Why is this tagged ice-on-invalid? The test case is valid C++14.