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
