OK.
On Wed, Jan 17, 2018 at 12:27 PM, David Malcolm <dmalc...@redhat.com> wrote:
> On Wed, 2018-01-17 at 09:28 -0500, Jason Merrill wrote:
>> On Wed, Jan 17, 2018 at 5:34 AM, Jakub Jelinek <ja...@redhat.com>
>> wrote:
>> > On Fri, Jan 12, 2018 at 05:09:24PM -0500, David Malcolm wrote:
>> > > PR c++/83814 reports an ICE introduced by the location wrapper
>> > > patch
>> > > (r256448), affecting certain memset calls within templates.
>> >
>> > Note, I think this issue sadly affects a lot of code, so it is
>> > quite urgent.
>> >
>> > That said, wonder if we really can't do any folding when
>> > processing_template_decl, could we e.g. do at least
>> > maybe_constant_value,
>> > or fold if the expression is not type nor value dependent?
>>
>> Yes, in a template we should call fold_non_dependent_expr.
>>
>> > BTW, never know if cp_fold_rvalue is a superset of
>> > maybe_constant_value or not.
>>
>> It is.
>>
>> Jason
>
> Thanks. Here's an updated version of the patch.
>
> Changed in v2:
> * use fold_non_dependent_expr in the C++ impl of fold_for_warn
> * add some test coverage of folding to g++.dg/wrappers/pr83814.C
> * added another testcase (from PR c++/83902)
>
> Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
> OK for trunk?
>
>
> gcc/c-family/ChangeLog:
> PR c++/83814
> * c-common.c (fold_for_warn): Move to c/c-fold.c and cp/expr.c.
>
> gcc/c/ChangeLog:
> PR c++/83814
> * c-fold.c (fold_for_warn): Move from c-common.c, reducing to just
> the C part.
>
> gcc/cp/ChangeLog:
> PR c++/83814
> * expr.c (fold_for_warn): Move from c-common.c, reducing to just
> the C++ part. If processing a template, call
> fold_non_dependent_expr rather than fully folding.
>
> gcc/testsuite/ChangeLog:
> PR c++/83814
> PR c++/83902
> * g++.dg/wrappers/pr83814.C: New test case.
> * g++.dg/wrappers/pr83902.C: New test case.
> ---
> gcc/c-family/c-common.c | 13 ------
> gcc/c/c-fold.c | 10 +++++
> gcc/cp/expr.c | 15 +++++++
> gcc/testsuite/g++.dg/wrappers/pr83814.C | 70
> +++++++++++++++++++++++++++++++++
> gcc/testsuite/g++.dg/wrappers/pr83902.C | 9 +++++
> 5 files changed, 104 insertions(+), 13 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/wrappers/pr83814.C
> create mode 100644 gcc/testsuite/g++.dg/wrappers/pr83902.C
>
> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
> index 097d192..858ed68 100644
> --- a/gcc/c-family/c-common.c
> +++ b/gcc/c-family/c-common.c
> @@ -868,19 +868,6 @@ c_get_substring_location (const substring_loc
> &substr_loc,
> }
>
>
> -/* Fold X for consideration by one of the warning functions when checking
> - whether an expression has a constant value. */
> -
> -tree
> -fold_for_warn (tree x)
> -{
> - if (c_dialect_cxx ())
> - return c_fully_fold (x, /*for_init*/false, /*maybe_constp*/NULL);
> - else
> - /* The C front-end has already folded X appropriately. */
> - return x;
> -}
> -
> /* Return true iff T is a boolean promoted to int. */
>
> bool
> diff --git a/gcc/c/c-fold.c b/gcc/c/c-fold.c
> index 5776f1b..be6a0fc 100644
> --- a/gcc/c/c-fold.c
> +++ b/gcc/c/c-fold.c
> @@ -668,3 +668,13 @@ c_fully_fold_internal (tree expr, bool in_init, bool
> *maybe_const_operands,
> }
> return ret;
> }
> +
> +/* Fold X for consideration by one of the warning functions when checking
> + whether an expression has a constant value. */
> +
> +tree
> +fold_for_warn (tree x)
> +{
> + /* The C front-end has already folded X appropriately. */
> + return x;
> +}
> diff --git a/gcc/cp/expr.c b/gcc/cp/expr.c
> index 7d79215..b1ab453 100644
> --- a/gcc/cp/expr.c
> +++ b/gcc/cp/expr.c
> @@ -315,3 +315,18 @@ mark_exp_read (tree exp)
> }
> }
>
> +/* Fold X for consideration by one of the warning functions when checking
> + whether an expression has a constant value. */
> +
> +tree
> +fold_for_warn (tree x)
> +{
> + /* C++ implementation. */
> +
> + /* It's not generally safe to fully fold inside of a template, so
> + call fold_non_dependent_expr instead. */
> + if (processing_template_decl)
> + return fold_non_dependent_expr (x);
> +
> + return c_fully_fold (x, /*for_init*/false, /*maybe_constp*/NULL);
> +}
> diff --git a/gcc/testsuite/g++.dg/wrappers/pr83814.C
> b/gcc/testsuite/g++.dg/wrappers/pr83814.C
> new file mode 100644
> index 0000000..b9f8faa
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/wrappers/pr83814.C
> @@ -0,0 +1,70 @@
> +/* Verify that our memset warnings don't crash when folding
> + arguments within a template (PR c++/83814). */
> +
> +// { dg-options "-Wno-int-to-pointer-cast -Wmemset-transposed-args
> -Wmemset-elt-size" }
> +
> +template <class>
> +void test_1()
> +{
> + __builtin_memset (int() - char(), 0, 0);
> +}
> +
> +template <class>
> +void test_2()
> +{
> + __builtin_memset (0, 0, int() - char());
> +}
> +
> +template <class>
> +void test_3 (unsigned a, int c)
> +{
> + __builtin_memset((char *)c + a, 0, a);
> +}
> +
> +template <class>
> +void test_4 (unsigned a, int c)
> +{
> + __builtin_memset(0, 0, (char *)c + a);
> +}
> +
> +/* Verify that we warn for -Wmemset-transposed-args inside
> + a template. */
> +
> +char buf[1024];
> +
> +template <class>
> +void test_5 ()
> +{
> + __builtin_memset (buf, sizeof buf, 0); // { dg-warning "transposed
> parameters" }
> +}
> +
> +/* Adapted from c-c++-common/memset-array.c; verify that
> + -Wmemset-elt-size works within a template. */
> +
> +enum a {
> + a_1,
> + a_2,
> + a_n
> +};
> +int t1[20];
> +int t2[a_n];
> +
> +struct s
> +{
> + int t[20];
> +};
> +
> +template<class>
> +void foo (struct s *s)
> +{
> + __builtin_memset (t1, 0, 20); // { dg-warning "element size" }
> +
> + // This case requires reading through an enum value:
> + __builtin_memset (t2, 0, a_n); // { dg-warning "element size" }
> +
> + __builtin_memset (s->t, 0, 20); // { dg-warning "element size" }
> +
> + // These cases require folding of arg2 within a template:
> + __builtin_memset (t2, 0, a_n + 0); // { dg-warning "element size" }
> + __builtin_memset (t2, 0, a_n * 1); // { dg-warning "element size" }
> +}
> diff --git a/gcc/testsuite/g++.dg/wrappers/pr83902.C
> b/gcc/testsuite/g++.dg/wrappers/pr83902.C
> new file mode 100644
> index 0000000..3d334e7
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/wrappers/pr83902.C
> @@ -0,0 +1,9 @@
> +extern "C" void *memset (void *, int, __SIZE_TYPE__);
> +void *p;
> +
> +template <int T>
> +struct B
> +{
> + void foo () { memset (p, 0, 4 * T * sizeof(float)); }
> +};
> +
> --
> 1.8.5.3
>
