Re: [PING] Re: [PATCH] Fix PR92088
On Tue, 19 Nov 2019, Richard Biener wrote:
> > +/* For nested functions disqualify ones taking VLAs by value
> > + from inlining since the middle-end cannot deal with this.
> > + ??? We should arrange for those to be passed by reference
> > + with emitting the copy on the caller side in the frontend. */
> > +if (storage_class == csc_none
> > + && TREE_CODE (type) == FUNCTION_TYPE)
> > + for (tree al = TYPE_ARG_TYPES (type); al; al = TREE_CHAIN (al))
> > + {
> > + tree arg = TREE_VALUE (al);
> > + if (arg != error_mark_node
> > + && C_TYPE_VARIABLE_SIZE (TREE_VALUE (al)))
The second use of TREE_VALUE (al) looks like it would be better as a
reference to arg. OK with that change.
--
Joseph S. Myers
jos...@codesourcery.com
[PING] Re: [PATCH] Fix PR92088
On Fri, 8 Nov 2019, Richard Biener wrote:
>
> The following works around a middle-end limitation not being able
> to deal with by value-passing of VLAs transparently during inlining
> (but only DECL_BY_REFERENCE is handled) in the C frontend by marking
> said functions as not inlinable. This avoids ICEs later.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
>
> OK for trunk?
Ping.
> Thanks,
> Richard.
>
> 2019-11-09 Richard Biener
>
> PR c/92088
> c/
> * c-decl.c (grokdeclarator): Prevent inlining of nested
> function with VLA arguments.
>
> * builtins.c (compute_objsize): Deal with VLAs.
>
> * gcc.dg/torture/pr92088-1.c: New testcase.
> * gcc.dg/torture/pr92088-2.c: Likewise.
>
> Index: gcc/builtins.c
> ===
> --- gcc/builtins.c(revision 277906)
> +++ gcc/builtins.c(working copy)
> @@ -3708,7 +3708,8 @@ compute_objsize (tree dest, int ostype,
>if (DECL_P (ref))
> {
>*pdecl = ref;
> - return DECL_SIZE_UNIT (ref);
> + if (tree size = DECL_SIZE_UNIT (ref))
> + return TREE_CODE (size) == INTEGER_CST ? size : NULL_TREE;
> }
>
>tree type = TREE_TYPE (dest);
> Index: gcc/c/c-decl.c
> ===
> --- gcc/c/c-decl.c(revision 277906)
> +++ gcc/c/c-decl.c(working copy)
> @@ -7304,6 +7304,23 @@ grokdeclarator (const struct c_declarato
> "no linkage");
>}
>
> +/* For nested functions disqualify ones taking VLAs by value
> + from inlining since the middle-end cannot deal with this.
> + ??? We should arrange for those to be passed by reference
> + with emitting the copy on the caller side in the frontend. */
> +if (storage_class == csc_none
> + && TREE_CODE (type) == FUNCTION_TYPE)
> + for (tree al = TYPE_ARG_TYPES (type); al; al = TREE_CHAIN (al))
> + {
> + tree arg = TREE_VALUE (al);
> + if (arg != error_mark_node
> + && C_TYPE_VARIABLE_SIZE (TREE_VALUE (al)))
> + {
> + DECL_UNINLINABLE (decl) = 1;
> + break;
> + }
> + }
> +
> /* Record `register' declaration for warnings on &
> and in case doing stupid register allocation. */
>
> Index: gcc/testsuite/gcc.dg/torture/pr92088-1.c
> ===
> --- gcc/testsuite/gcc.dg/torture/pr92088-1.c (revision 0)
> +++ gcc/testsuite/gcc.dg/torture/pr92088-1.c (working copy)
> @@ -0,0 +1,22 @@
> +/* { dg-do run } */
> +
> +int __attribute__((noipa))
> +g (char *p)
> +{
> + return p[9];
> +}
> +int main (int argc, char **argv)
> +{
> + struct S {
> +char toto[argc + 16];
> + };
> + int f (struct S arg) {
> + __builtin_strcpy(arg.toto, "helloworld");
> + return g (arg.toto);
> + }
> + struct S bob;
> + __builtin_strcpy(bob.toto, "coucoucoucou");
> + if (f(bob) != 'd' || __builtin_strcmp (bob.toto, "coucoucoucou"))
> +__builtin_abort ();
> + return 0;
> +}
> Index: gcc/testsuite/gcc.dg/torture/pr92088-2.c
> ===
> --- gcc/testsuite/gcc.dg/torture/pr92088-2.c (revision 0)
> +++ gcc/testsuite/gcc.dg/torture/pr92088-2.c (working copy)
> @@ -0,0 +1,17 @@
> +/* { dg-do compile } */
> +
> +void foo(int n)
> +{
> + struct X { int a[n]; } y;
> +
> + struct X baz (struct X x)
> +{
> + x.a[0] = 1;
> + return x;
> +}
> +
> + y.a[0] = 0;
> + y = baz(y);
> + if (y.a[0] != 1)
> +__builtin_abort ();
> +}
>
--
Richard Biener
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
[PATCH] Fix PR92088
The following works around a middle-end limitation not being able
to deal with by value-passing of VLAs transparently during inlining
(but only DECL_BY_REFERENCE is handled) in the C frontend by marking
said functions as not inlinable. This avoids ICEs later.
Bootstrapped and tested on x86_64-unknown-linux-gnu.
OK for trunk?
Thanks,
Richard.
2019-11-09 Richard Biener
PR c/92088
c/
* c-decl.c (grokdeclarator): Prevent inlining of nested
function with VLA arguments.
* builtins.c (compute_objsize): Deal with VLAs.
* gcc.dg/torture/pr92088-1.c: New testcase.
* gcc.dg/torture/pr92088-2.c: Likewise.
Index: gcc/builtins.c
===
--- gcc/builtins.c (revision 277906)
+++ gcc/builtins.c (working copy)
@@ -3708,7 +3708,8 @@ compute_objsize (tree dest, int ostype,
if (DECL_P (ref))
{
*pdecl = ref;
- return DECL_SIZE_UNIT (ref);
+ if (tree size = DECL_SIZE_UNIT (ref))
+ return TREE_CODE (size) == INTEGER_CST ? size : NULL_TREE;
}
tree type = TREE_TYPE (dest);
Index: gcc/c/c-decl.c
===
--- gcc/c/c-decl.c (revision 277906)
+++ gcc/c/c-decl.c (working copy)
@@ -7304,6 +7304,23 @@ grokdeclarator (const struct c_declarato
"no linkage");
}
+/* For nested functions disqualify ones taking VLAs by value
+ from inlining since the middle-end cannot deal with this.
+ ??? We should arrange for those to be passed by reference
+ with emitting the copy on the caller side in the frontend. */
+if (storage_class == csc_none
+ && TREE_CODE (type) == FUNCTION_TYPE)
+ for (tree al = TYPE_ARG_TYPES (type); al; al = TREE_CHAIN (al))
+ {
+ tree arg = TREE_VALUE (al);
+ if (arg != error_mark_node
+ && C_TYPE_VARIABLE_SIZE (TREE_VALUE (al)))
+ {
+ DECL_UNINLINABLE (decl) = 1;
+ break;
+ }
+ }
+
/* Record `register' declaration for warnings on &
and in case doing stupid register allocation. */
Index: gcc/testsuite/gcc.dg/torture/pr92088-1.c
===
--- gcc/testsuite/gcc.dg/torture/pr92088-1.c(revision 0)
+++ gcc/testsuite/gcc.dg/torture/pr92088-1.c(working copy)
@@ -0,0 +1,22 @@
+/* { dg-do run } */
+
+int __attribute__((noipa))
+g (char *p)
+{
+ return p[9];
+}
+int main (int argc, char **argv)
+{
+ struct S {
+char toto[argc + 16];
+ };
+ int f (struct S arg) {
+ __builtin_strcpy(arg.toto, "helloworld");
+ return g (arg.toto);
+ }
+ struct S bob;
+ __builtin_strcpy(bob.toto, "coucoucoucou");
+ if (f(bob) != 'd' || __builtin_strcmp (bob.toto, "coucoucoucou"))
+__builtin_abort ();
+ return 0;
+}
Index: gcc/testsuite/gcc.dg/torture/pr92088-2.c
===
--- gcc/testsuite/gcc.dg/torture/pr92088-2.c(revision 0)
+++ gcc/testsuite/gcc.dg/torture/pr92088-2.c(working copy)
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+
+void foo(int n)
+{
+ struct X { int a[n]; } y;
+
+ struct X baz (struct X x)
+{
+ x.a[0] = 1;
+ return x;
+}
+
+ y.a[0] = 0;
+ y = baz(y);
+ if (y.a[0] != 1)
+__builtin_abort ();
+}
