This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: C++ PATCH for c++/70153 (unhandled UNARY_PLUS_EXPR causes stack overflow)
- From: Marek Polacek <polacek at redhat dot com>
- To: Jason Merrill <jason at redhat dot com>
- Cc: Jakub Jelinek <jakub at redhat dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Thu, 10 Mar 2016 15:53:34 +0100
- Subject: Re: C++ PATCH for c++/70153 (unhandled UNARY_PLUS_EXPR causes stack overflow)
- Authentication-results: sourceware.org; auth=none
- References: <20160310124645 dot GW10006 at redhat dot com> <20160310125641 dot GU3017 at tucnak dot redhat dot com> <20160310130702 dot GX10006 at redhat dot com> <56E185E9 dot 6040609 at redhat dot com>
On Thu, Mar 10, 2016 at 09:34:17AM -0500, Jason Merrill wrote:
> On 03/10/2016 08:07 AM, Marek Polacek wrote:
> >On Thu, Mar 10, 2016 at 01:56:41PM +0100, Jakub Jelinek wrote:
> >>On Thu, Mar 10, 2016 at 01:46:45PM +0100, Marek Polacek wrote:
> >>>2016-03-10 Marek Polacek <polacek@redhat.com>
> >>>
> >>> PR c++/70153
> >>> * cp-gimplify.c (cp_fold): Handle UNARY_PLUS_EXPR.
> >>>
> >>> * g++.dg/delayedfold/unary-plus1.C: New test.
> >>>
> >>>diff --git gcc/cp/cp-gimplify.c gcc/cp/cp-gimplify.c
> >>>index 6af3760..db23efe 100644
> >>>--- gcc/cp/cp-gimplify.c
> >>>+++ gcc/cp/cp-gimplify.c
> >>>@@ -2009,6 +2009,8 @@ cp_fold (tree x)
> >>> else
> >>> x = fold_build1_loc (loc, code, TREE_TYPE (x), op0);
> >>> }
> >>>+ else if (code == UNARY_PLUS_EXPR)
> >>>+ x = fold_convert (TREE_TYPE (x), op0);
> >>> else
> >>> x = fold (x);
> >>>
> >>
> >>Won't this still leak UNARY_PLUS_EXPR into the folded result if
> >>you could fold the operand of that? It will take the
> >> x = fold_build1_loc (loc, code, TREE_TYPE (x), op0);
> >>path...
> >
> >...of course :(. Testcase for that:
> >return 2ULL * ((1 + (unsigned long int) +(1 + 0)) * i);
> >
> >>Wouldn't it be better to just handle case UNARY_PLUS_EXPR:
> >>separately, and always optimize it away?
> >>So like:
> >> case UNARY_PLUS_EXPR:
> >> loc = EXPR_LOCATION (x);
> >> op0 = cp_fold_maybe_rvalue (TREE_OPERAND (x, 0), rval_ops);
> >> if (op0 == error_mark_node)
> >> x = error_mark_node;
> >> else
> >> x = fold_convert_loc (loc, TREE_TYPE (x), op0);
> >> break;
> >>or so?
> >
> >Let's ask Jason. If he prefers this approach, I'll get it done.
>
> Sounds good.
So I went ahead and regtested this one. It uses fold_convert instead
of fold_convert_loc because I think that's what's desirable here.
I've extended the test so that it also checks +(1 + 0).
Ok if the bootstrap passes as well?
2016-03-10 Marek Polacek <polacek@redhat.com>
PR c++/70153
* cp-gimplify.c (cp_fold): Handle UNARY_PLUS_EXPR.
* g++.dg/delayedfold/unary-plus1.C: New test.
diff --git gcc/cp/cp-gimplify.c gcc/cp/cp-gimplify.c
index 6af3760..71ac588 100644
--- gcc/cp/cp-gimplify.c
+++ gcc/cp/cp-gimplify.c
@@ -1996,7 +1996,6 @@ cp_fold (tree x)
case BIT_NOT_EXPR:
case TRUTH_NOT_EXPR:
case FIXED_CONVERT_EXPR:
- case UNARY_PLUS_EXPR:
case INDIRECT_REF:
loc = EXPR_LOCATION (x);
@@ -2016,6 +2015,14 @@ cp_fold (tree x)
|| !VOID_TYPE_P (TREE_TYPE (TREE_OPERAND (x, 0))));
break;
+ case UNARY_PLUS_EXPR:
+ op0 = cp_fold_maybe_rvalue (TREE_OPERAND (x, 0), rval_ops);
+ if (op0 == error_mark_node)
+ x = error_mark_node;
+ else
+ x = fold_convert (TREE_TYPE (x), op0);
+ break;
+
case POSTDECREMENT_EXPR:
case POSTINCREMENT_EXPR:
case INIT_EXPR:
diff --git gcc/testsuite/g++.dg/delayedfold/unary-plus1.C gcc/testsuite/g++.dg/delayedfold/unary-plus1.C
index e69de29..ebf3493 100644
--- gcc/testsuite/g++.dg/delayedfold/unary-plus1.C
+++ gcc/testsuite/g++.dg/delayedfold/unary-plus1.C
@@ -0,0 +1,22 @@
+// PR c++/70153
+// { dg-do run }
+
+unsigned long long int
+fn1 (unsigned long long int i)
+{
+ return 2ULL * ((1 + (unsigned long int) +1) * i);
+}
+
+unsigned long long int
+fn2 (unsigned long long int i)
+{
+ return 2ULL * ((1 + (unsigned long int) +(1 + 0)) * i);
+}
+
+int
+main (void)
+{
+ if (fn1 (3ULL) != 12ULL
+ || fn2 (3ULL) != 12ULL)
+ __builtin_abort ();
+}
Marek