| Message ID | AM4PR0701MB2162C062AA26298037E7B692E4080@AM4PR0701MB2162.eurprd07.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Jul 20, 2016 at 09:50:03PM +0000, Bernd Edlinger wrote: > But the built-in alloca is still recognized because the builtin > does have ECF_MAY_BE_ALLOCA and ECF_MALLOC. But __builtin_alloca_with_align likely doesn't have ECF_MALLOC set (even when it should). Jakub
On 07/21/16 00:00, Jakub Jelinek wrote: > On Wed, Jul 20, 2016 at 09:50:03PM +0000, Bernd Edlinger wrote: >> But the built-in alloca is still recognized because the builtin >> does have ECF_MAY_BE_ALLOCA and ECF_MALLOC. > > But __builtin_alloca_with_align likely doesn't have ECF_MALLOC set (even > when it should). > > Jakub > DEF_BUILTIN_STUB (BUILT_IN_ALLOCA_WITH_ALIGN, "__builtin_alloca_with_align") do you know what the attributes are instead, or where that is constructed? At least in this example it seems to work: extern "C" void *alloca(unsigned long); void test(void*); void bar(unsigned long n) { char *x = (char*) __builtin_alloca_with_align(n,64); if (x) *x = 0; } g++ -O3 -S -Wall test.cc -ansi results in _Z3barm: .LFB0: .cfi_startproc rep ret .cfi_endproc and make check-c has no regressions. Thanks Bernd.
On 07/21/16 00:19, Bernd Edlinger wrote: > On 07/21/16 00:00, Jakub Jelinek wrote: >> On Wed, Jul 20, 2016 at 09:50:03PM +0000, Bernd Edlinger wrote: >>> But the built-in alloca is still recognized because the builtin >>> does have ECF_MAY_BE_ALLOCA and ECF_MALLOC. >> >> But __builtin_alloca_with_align likely doesn't have ECF_MALLOC set (even >> when it should). >> >> Jakub >> > > > DEF_BUILTIN_STUB (BUILT_IN_ALLOCA_WITH_ALIGN, > "__builtin_alloca_with_align") > > do you know what the attributes are instead, > or where that is constructed? > tree.c: /* If we're checking the stack, `alloca' can throw. */ const int alloca_flags = ECF_MALLOC | ECF_LEAF | (flag_stack_check ? 0 : ECF_NOTHROW); if (!builtin_decl_explicit_p (BUILT_IN_ALLOCA)) { ftype = build_function_type_list (ptr_type_node, size_type_node, NULL_TREE); local_define_builtin ("__builtin_alloca", ftype, BUILT_IN_ALLOCA, "alloca", alloca_flags); } ftype = build_function_type_list (ptr_type_node, size_type_node, size_type_node, NULL_TREE); local_define_builtin ("__builtin_alloca_with_align", ftype, BUILT_IN_ALLOCA_WITH_ALIGN, "__builtin_alloca_with_align", alloca_flags); looks like ECF_MALLOC and ECF_LEAF are always there. Right? Bernd.
On Wed, 20 Jul 2016, Bernd Edlinger wrote: > On 07/20/16 20:08, Richard Biener wrote: > > On July 20, 2016 6:54:48 PM GMT+02:00, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote: > >> > >> But I think that alloca just should not be recognized by name any > >> more. > > > > It was introduced to mark calls that should not be duplicated by inlining or unrolling to avoid increasing stack usage too much. Sth worthwhile to keep even with -ffreestanding. > > > > Richard. > > > > On second thought I start to think that an external alloca function > might still work. And returning ECF_MAY_BE_ALLOCA just based on the > name could be made safe by checking the malloc attribute at the right > places. > > With this new incremental patch the example > > extern "C" > void *alloca(unsigned long); > void bar(unsigned long n) > { > char *x = (char*) alloca(n); > if (x) > *x = 0; > } > > might actually work when -ansi is used, > i.e. it does no longer assume that alloca cannot return null, > but still creates a frame pointer, which it would not have done > for allocb for instance. > > But the built-in alloca is still recognized because the builtin > does have ECF_MAY_BE_ALLOCA and ECF_MALLOC. > > > Is it OK for trunk after boot-strap and reg-testing? Hmm, but ECF_MALLOC doesn't guarantee non-NULL return. I think the two calls you patched simply shouldn't use the predicates (which are misnamed as they check for _maybe_alloca_call_p). Instead they have to check for the respective builtins (BUILT_IN_ALLOCA, BUILT_IN_ALLOCA_WITH_ALIGN). Richard. > > Thanks > Bernd. >
2016-07-19 Bernd Edlinger <bernd.edlinger@hotmail.de> PR middle-end/71876 * fold-const.c (tree_expr_nonzero_warnv_p): Check for real built-in alloca. * tree-vrp.c (gimple_stmt_nonzero_warnv_p): Likewise. Index: gcc/fold-const.c =================================================================== --- gcc/fold-const.c (revision 238513) +++ gcc/fold-const.c (working copy) @@ -9018,7 +9018,7 @@ tree_expr_nonzero_warnv_p (tree t, bool *strict_ov && lookup_attribute ("returns_nonnull", TYPE_ATTRIBUTES (TREE_TYPE (fndecl)))) return true; - return alloca_call_p (t); + return alloca_call_p (t) && DECL_IS_MALLOC (fndecl); } default: Index: gcc/tree-vrp.c =================================================================== --- gcc/tree-vrp.c (revision 238513) +++ gcc/tree-vrp.c (working copy) @@ -1065,7 +1065,7 @@ gimple_stmt_nonzero_warnv_p (gimple *stmt, bool *s lookup_attribute ("returns_nonnull", TYPE_ATTRIBUTES (gimple_call_fntype (stmt)))) return true; - return gimple_alloca_call_p (stmt); + return gimple_alloca_call_p (stmt) && DECL_IS_MALLOC (fndecl); } default: gcc_unreachable ();