[patch c++]: Fix PR/53904

[patch c++]: Fix PR/53904

[Kai Tietz]

Hello,

this issue fixes a type-overflow issue caused by trying to cast a UHWI
via tree_to_shwi.
As soon as value gets larger then SHWI_MAX, we get an error for it.
So we need to cast it
via tree_to_uhwi, and then casting it to the signed variant.

ChangeLog

2014-11-20  Kai Tietz  <ktietz@redhat.com>

    PR c++/63904
    * constexpr.c (cxx_eval_vec_init_1): Avoid
    type-overflow issue.

2014-11-20  Kai Tietz  <ktietz@redhat.com>

    PR c++/63904
    * g++.dg/cpp0x/pr63904.C: New.


Regression tested for x86_64-unknown-linux-gnu.  Ok for apply?

Regards,
Kai

Index: gcc/gcc/cp/constexpr.c
===================================================================
--- gcc.orig/gcc/cp/constexpr.c
+++ gcc/gcc/cp/constexpr.c
@@ -2006,12 +2050,12 @@ cxx_eval_vec_init_1 (const constexpr_ctx
              bool *non_constant_p, bool *overflow_p)
 {
   tree elttype = TREE_TYPE (atype);
-  int max = tree_to_shwi (array_type_nelts (atype));
+  HOST_WIDE_INT max = (HOST_WIDE_INT) tree_to_uhwi (array_type_nelts (atype));
   verify_ctor_sanity (ctx, atype);
   vec<constructor_elt, va_gc> **p = &CONSTRUCTOR_ELTS (ctx->ctor);
   vec_alloc (*p, max + 1);
   bool pre_init = false;
-  int i;
+  HOST_WIDE_INT i;

   /* For the default constructor, build up a call to the default
      constructor of the element type.  We only need to handle class types
Index: gcc/gcc/testsuite/g++.dg/cpp0x/pr63904.C
===================================================================
--- /dev/null
+++ gcc/gcc/testsuite/g++.dg/cpp0x/pr63904.C
@@ -0,0 +1,13 @@
+// { dg-do compile { target c++11 } }
+
+template<int N>
+struct foo {
+    constexpr foo() : a() {}
+    int a[N];
+};
+
+int main() {
+  foo< (foo<1>{}).a[0] > f;
+  return 0;
+}
+

Re: [patch c++]: Fix PR/53904

[Richard Biener]

On Thu, Nov 20, 2014 at 8:48 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
> Hello,
>
> this issue fixes a type-overflow issue caused by trying to cast a UHWI
> via tree_to_shwi.
> As soon as value gets larger then SHWI_MAX, we get an error for it.
> So we need to cast it
> via tree_to_uhwi, and then casting it to the signed variant.

I think it's better to handle the degenerate case (no element) explicitely.
And I would think that sth like "nelts" should have a positive result,
thus why is 'max' not unsigned?  Also 'max' and using 'nelts' looks
like a mismatch?  max == nelts - 1.  Ah, because array_type_nelts
returns nelts - 1 ... how useful ;)

Still you want to special-case the array_type_nelts == -1 case.

Richard.

> ChangeLog
>
> 2014-11-20  Kai Tietz  <ktietz@redhat.com>
>
>     PR c++/63904
>     * constexpr.c (cxx_eval_vec_init_1): Avoid
>     type-overflow issue.
>
> 2014-11-20  Kai Tietz  <ktietz@redhat.com>
>
>     PR c++/63904
>     * g++.dg/cpp0x/pr63904.C: New.
>
>
> Regression tested for x86_64-unknown-linux-gnu.  Ok for apply?
>
> Regards,
> Kai
>
> Index: gcc/gcc/cp/constexpr.c
> ===================================================================
> --- gcc.orig/gcc/cp/constexpr.c
> +++ gcc/gcc/cp/constexpr.c
> @@ -2006,12 +2050,12 @@ cxx_eval_vec_init_1 (const constexpr_ctx
>               bool *non_constant_p, bool *overflow_p)
>  {
>    tree elttype = TREE_TYPE (atype);
> -  int max = tree_to_shwi (array_type_nelts (atype));
> +  HOST_WIDE_INT max = (HOST_WIDE_INT) tree_to_uhwi (array_type_nelts (atype));
>    verify_ctor_sanity (ctx, atype);
>    vec<constructor_elt, va_gc> **p = &CONSTRUCTOR_ELTS (ctx->ctor);
>    vec_alloc (*p, max + 1);
>    bool pre_init = false;
> -  int i;
> +  HOST_WIDE_INT i;
>
>    /* For the default constructor, build up a call to the default
>       constructor of the element type.  We only need to handle class types
> Index: gcc/gcc/testsuite/g++.dg/cpp0x/pr63904.C
> ===================================================================
> --- /dev/null
> +++ gcc/gcc/testsuite/g++.dg/cpp0x/pr63904.C
> @@ -0,0 +1,13 @@
> +// { dg-do compile { target c++11 } }
> +
> +template<int N>
> +struct foo {
> +    constexpr foo() : a() {}
> +    int a[N];
> +};
> +
> +int main() {
> +  foo< (foo<1>{}).a[0] > f;
> +  return 0;
> +}
> +

Re: [patch c++]: Fix PR/53904

[Jason Merrill]

On 11/20/2014 02:48 PM, Kai Tietz wrote:
> this issue fixes a type-overflow issue caused by trying to cast a UHWI
> via tree_to_shwi.
> As soon as value gets larger then SHWI_MAX, we get an error for it.
> So we need to cast it via tree_to_uhwi, and then casting it to the signed variant.

The problem seems to be with zero-length arrays getting -1 from 
array_type_nelts.  Let's use array_type_nelts_top instead so we don't 
ever see negative values.

Jason

Re: [patch c++]: Fix PR/53904

[Kai Tietz]

Ok.  Adjusted patch attached.  Nevertheless we should use here
unsigned HWI instead of possible truncation to signed int.  I admit
that it is unlikely to have more then 2^31 elements, but well ....

Ok for apply with adjusted ChangeLog?

Regards,
Kai

Index: constexpr.c
===================================================================
--- constexpr.c (Revision 218076)
+++ constexpr.c (Arbeitskopie)
@@ -2013,12 +2013,12 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tre
                     bool *non_constant_p, bool *overflow_p)
 {
   tree elttype = TREE_TYPE (atype);
-  int max = tree_to_shwi (array_type_nelts (atype));
+  unsigned HOST_WIDE_INT max = tree_to_uhwi (array_type_nelts_top (atype));
   verify_ctor_sanity (ctx, atype);
   vec<constructor_elt, va_gc> **p = &CONSTRUCTOR_ELTS (ctx->ctor);
   vec_alloc (*p, max + 1);
   bool pre_init = false;
-  int i;
+  unsigned HOST_WIDE_INT i;

   /* For the default constructor, build up a call to the default
      constructor of the element type.  We only need to handle class types
@@ -2043,7 +2043,7 @@ cxx_eval_vec_init_1 (const constexpr_ctx *ctx, tre
       pre_init = true;
     }

-  for (i = 0; i <= max; ++i)
+  for (i = 0; i < max; ++i)
     {
       tree idx = build_int_cst (size_type_node, i);
       tree eltinit;

Re: [patch c++]: Fix PR/53904

[Jason Merrill]

OK, thanks.

Jason

Re: [patch c++]: Fix PR/53904

[H.J. Lu]

On Wed, Nov 26, 2014 at 9:52 AM, Kai Tietz <ktietz70@googlemail.com> wrote:
> Ok.  Adjusted patch attached.  Nevertheless we should use here
> unsigned HWI instead of possible truncation to signed int.  I admit
> that it is unlikely to have more then 2^31 elements, but well ....
>
> Ok for apply with adjusted ChangeLog?
>
> Regards,
> Kai
>

On Linux/x86, the testcase fails with

output is:
/export/gnu/import/git/gcc/gcc/testsuite/g++.dg/cpp0x/pr63904.C: In
instantiation of 'struct foo<0>':^M
/export/gnu/import/git/gcc/gcc/testsuite/g++.dg/cpp0x/pr63904.C:10:26:
  required from here^M
/export/gnu/import/git/gcc/gcc/testsuite/g++.dg/cpp0x/pr63904.C:6:12:
error: ISO C++ forbids zero-size array [-Wpedantic]^M

FAIL: g++.dg/cpp0x/pr63904.C  -std=c++11 (test for excess errors)

H.J.

Re: [patch c++]: Fix PR/53904

[Kai Tietz]

Sorry, missed to add needed hunk to disable pedantic warnings for this testcase.
Committed it as obvious fix at rev 218130.

Kai