Re: [patch] Enhance support for -Wstack-usage/-Wvla-larger-than/-Walloca-larger-than

[Richard Biener]

On Thu, Oct 19, 2017 at 12:28 PM, Eric Botcazou  wrote:
>> Looks ok.  I wonder if you want to explicitely document that max_size < size
>> doesn't have any effect on actual code generation and is not checked for.
>
> Documentation amended to that effect:
>
>  -- Built-in Function: void *__builtin_alloca_with_align_and_max
>   (size_t size, size_t alignment, size_t max_size)
>  Similar to `__builtin_alloca_with_align' but takes an extra
>  argument specifying an upper bound for SIZE in case its value
>  cannot be computed at compile time, for use by `-fstack-usage',
>  `-Wstack-usage' and `-Walloca-larger-than' computations.  MAX_SIZE
>  has no effect on code generation and no attempt is made to check
>  its compatibility with SIZE.

works for me.

>> Also it seems that __builtin_alloca_with_align (20, 8, 16) will still
>> account 20 as the size and not 16 compared to 20 arriving in a variable
>> which is when 16 will be used.  So at least for accounting always use MIN
>> (size, max_size)?
>
> The implementation is in keeping with the above documentation, i.e. SIZE will
> prevail if its value can be computed at compile time.

OK.  Just wanted to mention it, there'll likely be cases where -O0 then
reports a smaller stack usage than -O2 because of this.

Thanks,
Richard.

> --
> Eric Botcazou

Re: [patch] Enhance support for -Wstack-usage/-Wvla-larger-than/-Walloca-larger-than

[Eric Botcazou]

> Looks ok.  I wonder if you want to explicitely document that max_size < size
> doesn't have any effect on actual code generation and is not checked for.

Documentation amended to that effect:

 -- Built-in Function: void *__builtin_alloca_with_align_and_max
  (size_t size, size_t alignment, size_t max_size)
 Similar to `__builtin_alloca_with_align' but takes an extra
 argument specifying an upper bound for SIZE in case its value
 cannot be computed at compile time, for use by `-fstack-usage',
 `-Wstack-usage' and `-Walloca-larger-than' computations.  MAX_SIZE
 has no effect on code generation and no attempt is made to check
 its compatibility with SIZE.

> Also it seems that __builtin_alloca_with_align (20, 8, 16) will still
> account 20 as the size and not 16 compared to 20 arriving in a variable
> which is when 16 will be used.  So at least for accounting always use MIN
> (size, max_size)?

The implementation is in keeping with the above documentation, i.e. SIZE will 
prevail if its value can be computed at compile time.

-- 
Eric Botcazou

Re: [patch] Enhance support for -Wstack-usage/-Wvla-larger-than/-Walloca-larger-than

[Richard Biener]

On Mon, Oct 16, 2017 at 10:35 AM, Eric Botcazou  wrote:
> Hi,
>
> a big limitation of -Wstack-usage/-Wvla-larger-than/-Walloca-larger-than is
> that you need -O2 (or more precisely -ftree-vrp) in order to be able to say
> something sensible for dynamically-sized objects/VLAs/calls to alloca.  That
> can be problematic, for example if the coding guidelines prevents you from
> using anything beyond -O1 for production builds.
>
> Now in Ada it is very easy and common to use integer types with custom bounds
> (the compiler automatically generates the appropriate run-time bound checks)
> so it is very easy to be able to say something sensible about dynamically-
> sized objects and VLAs (Ada doesn't have alloca) even at -O0 or -O1.
>
> That's why the attached patch introduces a way for front-ends to communicate
> an upper bound for the size of dynamically-sized objects/VLAs/calls to alloca
> to the -Wstack-usage/-Wvla-larger-than/-Walloca-larger-than machineries, based
> on a 3rd form of the BUILT_IN_ALLOCA builtin which takes a 3rd parameter in
> addition to the 2 parameters of BUILT_IN_ALLOCA_WITH_ALIGN.  This 3rd form is
> only used when the front-end can put an upper bound via max_int_size_in_bytes,
> which invokes lang_hooks.types.max_size, for the time being, but its usage
> could easily be extended.
>
> Macros and helper function are provided to manipulate the variants as a single
> builtin, so that code not directly tied to their support is little modified.
> The -Wstack-usage and -Wvla-larger-than/-Walloca-larger-than machineries are
> enhanced to take into account the upper bound, if it exists.
>
> Bootstrapped/regtested on x86_64-suse-linux, OK for the mainline?

Looks ok.  I wonder if you want to explicitely document that max_size < size
doesn't have any effect on actual code generation and is not checked for.
Also it seems that __builtin_alloca_with_align (20, 8, 16) will still account 20
as the size and not 16 compared to 20 arriving in a variable which is when 16
will be used.  So at least for accounting always use MIN (size, max_size)?

Richard.

>
> 2017-10-16  Eric Botcazou  
>
> * asan.c (handle_builtin_alloca): Deal with all alloca variants.
> (get_mem_refs_of_builtin_call): Likewise.
> * builtins.c (expand_builtin_apply): Adjust call to
> allocate_dynamic_stack_space.
> (expand_builtin_alloca): For __builtin_alloca_with_align_and_max, pass
> the third argument to allocate_dynamic_stack_space, otherwise -1.
> (expand_builtin): Deal with all alloca variants.
> (is_inexpensive_builtin): Likewise.
> * builtins.def (BUILT_IN_ALLOCA_WITH_ALIGN_AND_MAX): New.
> * calls.c (special_function_p): Deal with all alloca variants.
> (initialize_argument_information): Adjust call to
> allocate_dynamic_stack_space.
> (expand_call): Likewise.
> * cfgexpand.c (expand_stack_vars): Likewise.
> (expand_call_stmt): Deal with all alloca variants.
> * doc/extend.texi (Built-ins): Add __builtin_alloca_with_align_and_max
> * explow.c (allocate_dynamic_stack_space): Add MAX_SIZE parameter and
> use it for the stack usage computation.
> * explow.h (allocate_dynamic_stack_space): Adjust prototype.
> * function.c (gimplify_parameters): Turn BUILT_IN_ALLOCA_WITH_ALIGN
> into BUILT_IN_ALLOCA_WITH_ALIGN_AND_MAX and pass maximum size.
> * gimple-ssa-warn-alloca.c (alloca_call_type): Simplify control flow.
> Take into account 3rd argument of __builtin_alloca_with_align_and_max.
> (in_loop_p): Remove first argument and useless check.
> (pass_walloca::execute): Remove useless test and adjust call to above.
> * gimple.c (gimple_build_call_from_tree): Deal with all alloc variants
> * gimplify.c (gimplify_vla_decl): Turn BUILT_IN_ALLOCA_WITH_ALIGN into
> BUILT_IN_ALLOCA_WITH_ALIGN_AND_MAX and pass maximum size.
> (gimplify_call_expr): Deal with all alloca variants.
> * hsa-gen.c (gen_hsa_alloca): Likewise.
> (gen_hsa_insns_for_call): Likewise.
> * ipa-pure-const.c (special_builtin_state): Likewise.
> * tree-chkp.c (chkp_build_returned_bound): Likewise.
> * tree-object-size.c (alloc_object_size): Likewise.
> * tree-ssa-alias.c (ref_maybe_used_by_call_p_1): Likewise.
> (call_may_clobber_ref_p_1): Likewise.
> * tree-ssa-ccp.c (evaluate_stmt): Likewise.
> (ccp_fold_stmt): Likewise.
> (optimize_stack_restore): Likewise.
> * tree-ssa-dce.c (mark_stmt_if_obviously_necessary): Likewise.
> (mark_all_reaching_defs_necessary_1): Likewise.
> (propagate_necessity): Likewise.
> (eliminate_unnecessary_stmts): Likewise.
> * tree.c (build_common_builtin_nodes): Build
> BUILT_IN_ALLOCA_WITH_ALIGN_AND_MAX.
> * tree.h (ALLOCA_FUNCTION_CODE_P): 

[patch] Enhance support for -Wstack-usage/-Wvla-larger-than/-Walloca-larger-than

[Eric Botcazou]

Hi,

a big limitation of -Wstack-usage/-Wvla-larger-than/-Walloca-larger-than is 
that you need -O2 (or more precisely -ftree-vrp) in order to be able to say 
something sensible for dynamically-sized objects/VLAs/calls to alloca.  That 
can be problematic, for example if the coding guidelines prevents you from 
using anything beyond -O1 for production builds.

Now in Ada it is very easy and common to use integer types with custom bounds 
(the compiler automatically generates the appropriate run-time bound checks) 
so it is very easy to be able to say something sensible about dynamically-
sized objects and VLAs (Ada doesn't have alloca) even at -O0 or -O1.

That's why the attached patch introduces a way for front-ends to communicate 
an upper bound for the size of dynamically-sized objects/VLAs/calls to alloca 
to the -Wstack-usage/-Wvla-larger-than/-Walloca-larger-than machineries, based 
on a 3rd form of the BUILT_IN_ALLOCA builtin which takes a 3rd parameter in 
addition to the 2 parameters of BUILT_IN_ALLOCA_WITH_ALIGN.  This 3rd form is 
only used when the front-end can put an upper bound via max_int_size_in_bytes, 
which invokes lang_hooks.types.max_size, for the time being, but its usage 
could easily be extended.

Macros and helper function are provided to manipulate the variants as a single 
builtin, so that code not directly tied to their support is little modified.
The -Wstack-usage and -Wvla-larger-than/-Walloca-larger-than machineries are 
enhanced to take into account the upper bound, if it exists.

Bootstrapped/regtested on x86_64-suse-linux, OK for the mainline?


2017-10-16  Eric Botcazou  

* asan.c (handle_builtin_alloca): Deal with all alloca variants.
(get_mem_refs_of_builtin_call): Likewise.
* builtins.c (expand_builtin_apply): Adjust call to
allocate_dynamic_stack_space.
(expand_builtin_alloca): For __builtin_alloca_with_align_and_max, pass
the third argument to allocate_dynamic_stack_space, otherwise -1.
(expand_builtin): Deal with all alloca variants.
(is_inexpensive_builtin): Likewise.
* builtins.def (BUILT_IN_ALLOCA_WITH_ALIGN_AND_MAX): New.
* calls.c (special_function_p): Deal with all alloca variants.
(initialize_argument_information): Adjust call to
allocate_dynamic_stack_space.
(expand_call): Likewise.
* cfgexpand.c (expand_stack_vars): Likewise.
(expand_call_stmt): Deal with all alloca variants.
* doc/extend.texi (Built-ins): Add __builtin_alloca_with_align_and_max
* explow.c (allocate_dynamic_stack_space): Add MAX_SIZE parameter and
use it for the stack usage computation.
* explow.h (allocate_dynamic_stack_space): Adjust prototype.
* function.c (gimplify_parameters): Turn BUILT_IN_ALLOCA_WITH_ALIGN
into BUILT_IN_ALLOCA_WITH_ALIGN_AND_MAX and pass maximum size.
* gimple-ssa-warn-alloca.c (alloca_call_type): Simplify control flow.
Take into account 3rd argument of __builtin_alloca_with_align_and_max.
(in_loop_p): Remove first argument and useless check.
(pass_walloca::execute): Remove useless test and adjust call to above.
* gimple.c (gimple_build_call_from_tree): Deal with all alloc variants
* gimplify.c (gimplify_vla_decl): Turn BUILT_IN_ALLOCA_WITH_ALIGN into
BUILT_IN_ALLOCA_WITH_ALIGN_AND_MAX and pass maximum size.
(gimplify_call_expr): Deal with all alloca variants.
* hsa-gen.c (gen_hsa_alloca): Likewise.
(gen_hsa_insns_for_call): Likewise.
* ipa-pure-const.c (special_builtin_state): Likewise.
* tree-chkp.c (chkp_build_returned_bound): Likewise.
* tree-object-size.c (alloc_object_size): Likewise.
* tree-ssa-alias.c (ref_maybe_used_by_call_p_1): Likewise.
(call_may_clobber_ref_p_1): Likewise.
* tree-ssa-ccp.c (evaluate_stmt): Likewise.
(ccp_fold_stmt): Likewise.
(optimize_stack_restore): Likewise.
* tree-ssa-dce.c (mark_stmt_if_obviously_necessary): Likewise.
(mark_all_reaching_defs_necessary_1): Likewise.
(propagate_necessity): Likewise.
(eliminate_unnecessary_stmts): Likewise.
* tree.c (build_common_builtin_nodes): Build
BUILT_IN_ALLOCA_WITH_ALIGN_AND_MAX.
* tree.h (ALLOCA_FUNCTION_CODE_P): New macro.
(CASE_BUILT_IN_ALLOCA): Likewise.
* varasm.c (incorporeal_function_p): Deal with all alloca variants.
ada/
* gcc-interface/utils.c (max_size): Deal with SSA names.
c-family/
* c-common.c (check_builtin_function_arguments): Also check arguments
of __builtin_alloca_with_align_and_max.


2017-10-16  Eric Botcazou  

* gcc.dg/Walloca-15.c: New test.
* gnat.dg/stack_usage4.adb: Likewise.
* gnat.dg/stack_usage4_pkg.ads: New helper.

-- 
Eric BotcazouIndex: ada/gcc-interface/utils.c