Message ID | nycvar.YFH.7.76.1911081318570.5566@zhemvz.fhfr.qr |
---|---|
State | New |
Headers | show |
Series | Fix PR92088 | expand |
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 <rguenther@suse.de> > > 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 (); > +} >
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.
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 (); +}