* [C++ RFC] magic_varargs_p issues (PR c++/70144)
@ 2016-03-09 17:18 Jakub Jelinek
2016-03-10 17:08 ` Jakub Jelinek
0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2016-03-09 17:18 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1381 bytes --]
Hi!
The following testcase results in ICE in C++, while is properly rejected in
C. The problem is that the C++ FE treats some varargs builtins as magic and
doesn't perform any conversion on their args.
The first patch is just minimal, just ensures that we reject the builtins
without library implementation there. But, as the second testcase shows,
e.g. for __builtin_classify_type_p there is disagreement between C and C++,
where the former applies function-to-pointer and array-to-pointer
conversions even for those magic builtins, but C++ does not.
Unfortunately the second patch breaks some Cilk+ tests, so I'd probably need
to tweak it slightly, e.g. by magic_varargs_p returning 0 / 1 / 2 levels,
0 would mean no magic, 1 would mean do decay_conversion, 2 would mean do
just mark_type_use + reject_gcc_builtin, and return 2 just for Cilk+
reductions and 1 for all other magic varargs functions (for type generic
I believe function-to-pointer and array-to-pointer are desirable, aren't
they).
So, what approach do you prefer? I've so far bootstrapped/regtested the
second patch, which showed those
+FAIL: g++.dg/cilk-plus/AN/builtin_fn_custom_tplt.cc
+UNRESOLVED: g++.dg/cilk-plus/AN/builtin_fn_custom_tplt.cc
+FAIL: g++.dg/cilk-plus/AN/builtin_fn_mutating_tplt.cc
+UNRESOLVED: g++.dg/cilk-plus/AN/builtin_fn_mutating_tplt.cc
(for all opt/-g levels) regressions.
Jakub
[-- Attachment #2: V805a --]
[-- Type: text/plain, Size: 1318 bytes --]
2016-03-09 Jakub Jelinek <jakub@redhat.com>
PR c++/70144
* call.c (build_over_call): For magic_varargs_p arguments,
call reject_gcc_builtin if the argument is a FUNCTION_DECL.
* c-c++-common/pr70144.c: New test.
--- gcc/cp/call.c.jj 2016-03-04 08:23:29.000000000 +0100
+++ gcc/cp/call.c 2016-03-09 13:04:34.280011774 +0100
@@ -7516,8 +7516,12 @@ build_over_call (struct z_candidate *can
{
tree a = (*args)[arg_index];
if (magic_varargs_p (fn))
- /* Do no conversions for magic varargs. */
- a = mark_type_use (a);
+ {
+ /* Do no conversions for magic varargs. */
+ a = mark_type_use (a);
+ if (TREE_CODE (a) == FUNCTION_DECL && reject_gcc_builtin (a))
+ return error_mark_node;
+ }
else if (DECL_CONSTRUCTOR_P (fn)
&& same_type_ignoring_top_level_qualifiers_p (DECL_CONTEXT (fn),
TREE_TYPE (a)))
--- gcc/testsuite/c-c++-common/pr70144.c.jj 2016-03-09 13:10:58.246778355 +0100
+++ gcc/testsuite/c-c++-common/pr70144.c 2016-03-09 13:10:04.000000000 +0100
@@ -0,0 +1,9 @@
+/* PR c++/70144 */
+/* { dg-do compile } */
+
+void
+foo ()
+{
+ __builtin_constant_p (__builtin_constant_p) ?: ({ unsigned t = 0; t; }); /* { dg-error "must be directly called" } */
+ __builtin_classify_type (__builtin_expect); /* { dg-error "must be directly called" } */
+}
[-- Attachment #3: V805b --]
[-- Type: text/plain, Size: 1973 bytes --]
2016-03-09 Jakub Jelinek <jakub@redhat.com>
PR c++/70144
* call.c (build_over_call): For magic_varargs_p, call decay_conversion
instead of mark_type_use. Don't store error_mark_node arguments to
argarray, instead return error_mark_node.
* c-c++-common/pr70144-1.c: New test.
* c-c++-common/pr70144-2.c: New test.
--- gcc/cp/call.c.jj 2016-03-04 08:23:29.000000000 +0100
+++ gcc/cp/call.c 2016-03-09 13:29:40.674522135 +0100
@@ -7516,8 +7516,8 @@ build_over_call (struct z_candidate *can
{
tree a = (*args)[arg_index];
if (magic_varargs_p (fn))
- /* Do no conversions for magic varargs. */
- a = mark_type_use (a);
+ /* For magic varargs only do decay_conversion. */
+ a = decay_conversion (a, complain);
else if (DECL_CONSTRUCTOR_P (fn)
&& same_type_ignoring_top_level_qualifiers_p (DECL_CONTEXT (fn),
TREE_TYPE (a)))
@@ -7530,6 +7530,8 @@ build_over_call (struct z_candidate *can
}
else
a = convert_arg_to_ellipsis (a, complain);
+ if (a == error_mark_node)
+ return error_mark_node;
argarray[j++] = a;
}
--- gcc/testsuite/c-c++-common/pr70144-1.c.jj 2016-03-09 13:10:58.246778355 +0100
+++ gcc/testsuite/c-c++-common/pr70144-1.c 2016-03-09 13:10:04.000000000 +0100
@@ -0,0 +1,9 @@
+/* PR c++/70144 */
+/* { dg-do compile } */
+
+void
+foo ()
+{
+ __builtin_constant_p (__builtin_constant_p) ?: ({ unsigned t = 0; t; }); /* { dg-error "must be directly called" } */
+ __builtin_classify_type (__builtin_expect); /* { dg-error "must be directly called" } */
+}
--- gcc/testsuite/c-c++-common/pr70144-2.c.jj 2016-03-09 13:31:28.354062276 +0100
+++ gcc/testsuite/c-c++-common/pr70144-2.c 2016-03-09 13:31:49.673773235 +0100
@@ -0,0 +1,12 @@
+/* PR c++/70144 */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+int
+main ()
+{
+ if (__builtin_constant_p (__builtin_memset) != 0
+ || __builtin_classify_type (__builtin_memset) != 5)
+ __builtin_abort ();
+ return 0;
+}
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [C++ RFC] magic_varargs_p issues (PR c++/70144)
2016-03-09 17:18 [C++ RFC] magic_varargs_p issues (PR c++/70144) Jakub Jelinek
@ 2016-03-10 17:08 ` Jakub Jelinek
0 siblings, 0 replies; 2+ messages in thread
From: Jakub Jelinek @ 2016-03-10 17:08 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
On Wed, Mar 09, 2016 at 06:18:31PM +0100, Jakub Jelinek wrote:
> So, what approach do you prefer? I've so far bootstrapped/regtested the
> second patch, which showed those
> +FAIL: g++.dg/cilk-plus/AN/builtin_fn_custom_tplt.cc
> +UNRESOLVED: g++.dg/cilk-plus/AN/builtin_fn_custom_tplt.cc
> +FAIL: g++.dg/cilk-plus/AN/builtin_fn_mutating_tplt.cc
> +UNRESOLVED: g++.dg/cilk-plus/AN/builtin_fn_mutating_tplt.cc
> (for all opt/-g levels) regressions.
I've also successfully bootstrapped/regtested the other patch (though,
at least the
+ if (a == error_mark_node)
+ return error_mark_node;
hunk should be added in there too, and finally attached patch, which is
the combination of the two, use the first patch for Cilk+ reductions
and second otherwise. I've noticed that convert_arguments also checks
magic_varargs_p, but I don't understand much the relationship of
build_over_call and convert_arguments, are those used either one, or another
one, and so all this should be in both?
2016-03-09 Jakub Jelinek <jakub@redhat.com>
PR c++/70144
* cp-tree.h (magic_varargs_p): Return int instead of bool.
* call.c (magic_varargs_p): Return int instead of bool, return 2 for
Cilk+ reductions, otherwise 1 for magic varargs and 0 for normal
varargs.
(build_over_call): If magic_varargs_p == 2, call reject_gcc_builtin,
if magic_varargs_p == 1, call decay_conversion
instead of mark_type_use. Don't store error_mark_node arguments to
argarray, instead return error_mark_node.
* c-c++-common/pr70144-1.c: New test.
* c-c++-common/pr70144-2.c: New test.
--- gcc/cp/cp-tree.h.jj 2016-03-05 07:46:50.000000000 +0100
+++ gcc/cp/cp-tree.h 2016-03-10 16:17:21.121129641 +0100
@@ -5563,7 +5563,7 @@ public:
/* in call.c */
extern bool check_dtor_name (tree, tree);
-bool magic_varargs_p (tree);
+int magic_varargs_p (tree);
extern tree build_conditional_expr (location_t, tree, tree, tree,
tsubst_flags_t);
--- gcc/cp/call.c.jj 2016-03-09 15:06:21.697396705 +0100
+++ gcc/cp/call.c 2016-03-10 16:08:12.850558154 +0100
@@ -7040,15 +7040,17 @@ convert_for_arg_passing (tree type, tree
return val;
}
-/* Returns true iff FN is a function with magic varargs, i.e. ones for
- which no conversions at all should be done. This is true for some
- builtins which don't act like normal functions. */
+/* Returns non-zero iff FN is a function with magic varargs, i.e. ones for
+ which just decay_conversion or no conversions at all should be done.
+ This is true for some builtins which don't act like normal functions.
+ Return 2 if no conversions at all should be done, 1 if just
+ decay_conversion. */
-bool
+int
magic_varargs_p (tree fn)
{
if (flag_cilkplus && is_cilkplus_reduce_builtin (fn) != BUILT_IN_NONE)
- return true;
+ return 2;
if (DECL_BUILT_IN (fn))
switch (DECL_FUNCTION_CODE (fn))
@@ -7057,14 +7059,14 @@ magic_varargs_p (tree fn)
case BUILT_IN_CONSTANT_P:
case BUILT_IN_NEXT_ARG:
case BUILT_IN_VA_START:
- return true;
+ return 1;
default:;
return lookup_attribute ("type generic",
TYPE_ATTRIBUTES (TREE_TYPE (fn))) != 0;
}
- return false;
+ return 0;
}
/* Returns the decl of the dispatcher function if FN is a function version. */
@@ -7515,9 +7517,17 @@ build_over_call (struct z_candidate *can
for (; arg_index < vec_safe_length (args); ++arg_index)
{
tree a = (*args)[arg_index];
- if (magic_varargs_p (fn))
- /* Do no conversions for magic varargs. */
- a = mark_type_use (a);
+ int magic = magic_varargs_p (fn);
+ if (magic == 2)
+ {
+ /* Do no conversions for certain magic varargs. */
+ a = mark_type_use (a);
+ if (TREE_CODE (a) == FUNCTION_DECL && reject_gcc_builtin (a))
+ return error_mark_node;
+ }
+ else if (magic == 1)
+ /* For other magic varargs only do decay_conversion. */
+ a = decay_conversion (a, complain);
else if (DECL_CONSTRUCTOR_P (fn)
&& same_type_ignoring_top_level_qualifiers_p (DECL_CONTEXT (fn),
TREE_TYPE (a)))
@@ -7530,6 +7540,8 @@ build_over_call (struct z_candidate *can
}
else
a = convert_arg_to_ellipsis (a, complain);
+ if (a == error_mark_node)
+ return error_mark_node;
argarray[j++] = a;
}
--- gcc/testsuite/c-c++-common/pr70144-1.c.jj 2016-03-09 13:10:58.246778355 +0100
+++ gcc/testsuite/c-c++-common/pr70144-1.c 2016-03-09 13:10:04.000000000 +0100
@@ -0,0 +1,9 @@
+/* PR c++/70144 */
+/* { dg-do compile } */
+
+void
+foo ()
+{
+ __builtin_constant_p (__builtin_constant_p) ?: ({ unsigned t = 0; t; }); /* { dg-error "must be directly called" } */
+ __builtin_classify_type (__builtin_expect); /* { dg-error "must be directly called" } */
+}
--- gcc/testsuite/c-c++-common/pr70144-2.c.jj 2016-03-09 13:31:28.354062276 +0100
+++ gcc/testsuite/c-c++-common/pr70144-2.c 2016-03-09 13:31:49.673773235 +0100
@@ -0,0 +1,12 @@
+/* PR c++/70144 */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+int
+main ()
+{
+ if (__builtin_constant_p (__builtin_memset) != 0
+ || __builtin_classify_type (__builtin_memset) != 5)
+ __builtin_abort ();
+ return 0;
+}
Jakub
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-03-10 17:08 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-09 17:18 [C++ RFC] magic_varargs_p issues (PR c++/70144) Jakub Jelinek
2016-03-10 17:08 ` Jakub Jelinek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).