This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] specify large command line option arguments (PR 82063)


On 06/24/2018 03:05 PM, Martin Sebor wrote:
> Storing integer command line option arguments in type int
> limits options such as -Wlarger-than= or -Walloca-larger-than
> to at most INT_MAX (see bug 71905).  Larger values wrap around
> zero.  The value zero is considered to disable the option,
> making it impossible to specify a zero limit.
> 
> To get around these limitations, the -Walloc-size-larger-than=
> option accepts a string argument that it then parses itself
> and interprets as HOST_WIDE_INT.  The option also accepts byte
> size suffixes like KB, MB, GiB, etc. to make it convenient to
> specify very large limits.
> 
> The int limitation is obviously less than ideal in a 64-bit
> world.  The treatment of zero as a toggle is just a minor wart.
> The special treatment to make it work for just a single option
> makes option handling inconsistent.  It should be possible for
> any option that takes an integer argument to use the same logic.
> 
> The attached patch enhances GCC option processing to do that.
> It changes the storage type of option arguments from int to
> HOST_WIDE_INT and extends the existing (although undocumented)
> option property Host_Wide_Int to specify wide option arguments.
> It also introduces the ByteSize property for options for which
> specifying the byte-size suffix makes sense.
> 
> To make it possible to consider zero as a meaningful argument
> value rather than a flag indicating that an option is disabled
> the patch also adds a CLVC_SIZE enumerator to the cl_var_type
> enumeration, and modifies how options of the kind are handled.
> 
> Warning options that take large byte-size arguments can be
> disabled by specifying a value equal to or greater than
> HOST_WIDE_INT_M1U.  For convenience, aliases in the form of
> -Wno-xxx-larger-than have been provided for all the affected
> options.
> 
> In the patch all the existing -larger-than options are set
> to PTRDIFF_MAX.  This makes them effectively enabled, but
> because the setting is exceedingly permissive, and because
> some of the existing warnings are already set to the same
> value and some other checks detect and reject such exceedingly
> large values with errors, this change shouldn't noticeably
> affect what constructs are diagnosed.
> 
> Although all the options are set to PTRDIFF_MAX, I think it
> would make sense to consider setting some of them lower, say
> to PTRDIFF_MAX / 2.  I'd like to propose that in a followup
> patch.
> 
> To minimize observable changes the -Walloca-larger-than and
> -Wvla-larger-than warnings required more extensive work to
> make of the new mechanism because of the "unbounded" argument
> handling (the warnings trigger for arguments that are not
> visibly constrained), and because of the zero handling
> (the warnings also trigger
> 
> 
> Martin
> 
> 
> gcc-82063.diff
> 
> 
> PR middle-end/82063 - issues with arguments enabled by -Wall
> 
> gcc/ada/ChangeLog:
> 
> 	PR middle-end/82063
> 	* gcc-interface/misc.c (gnat_handle_option): Change function argument
> 	to HOST_WIDE_INT.
> 
> gcc/brig/ChangeLog:
> 	* brig/brig-lang.c (brig_langhook_handle_option): Change function
> 	argument to HOST_WIDE_INT.
> 
> gcc/c-family/ChangeLog:
> 
> 	PR middle-end/82063
> 	* c-common.h (c_common_handle_option): Change function argument
> 	to HOST_WIDE_INT.
> 	* c-opts.c (c_common_init_options): Same.
> 	(c_common_handle_option): Same.  Remove special handling of
> 	OPT_Walloca_larger_than_ and OPT_Wvla_larger_than_.
> 	* c.opt (-Walloc-size-larger-than, -Walloca-larger-than): Change
> 	options to take a HOST_WIDE_INT argument and accept a byte-size
> 	suffix.  Initialize.
> 	(-Wvla-larger-than): Same.
> 	(-Wno-alloc-size-larger-than, -Wno-alloca-larger-than): New.
> 	(-Wno-vla-larger-than): Same.
> 	
> 
> gcc/fortran/ChangeLog:
> 
> 	PR middle-end/82063
> 	* gfortran.h (gfc_handle_option): Change function argument
> 	to HOST_WIDE_INT.
> 	* options.c (gfc_handle_option): Same.
> 
> gcc/go/ChangeLog:
> 
> 	PR middle-end/82063
> 	* go-lang.c (go_langhook_handle_option): Change function argument
> 	to HOST_WIDE_INT.
> 
> gcc/lto/ChangeLog:
> 
> 	PR middle-end/82063
> 	* lto-lang.c (lto_handle_option): Change function argument
> 	to HOST_WIDE_INT.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR middle-end/82063
> 	* gcc.dg/Walloc-size-larger-than-16.c: Adjust.
> 	* gcc.dg/Walloca-larger-than.c: New test.
> 	* gcc.dg/Wframe-larger-than-2.c: New test.
> 	* gcc.dg/Wlarger-than3.c: New test.
> 	* gcc.dg/Wvla-larger-than-3.c: New test.
> 
> gcc/ChangeLog:
> 
> 	PR middle-end/82063
> 	* builtins.c (expand_builtin_alloca): Adjust.
> 	* calls.c (alloc_max_size): Simplify.
> 	* cgraphunit.c (cgraph_node::expand): Adjust.
> 	* common.opt (larger_than_size, warn_frame_larger_than): Remove
> 	variables.
> 	(frame_larger_than_size): Same.
> 	(-Wframe-larger-than, -Wlarger-than, -Wstack-usage): Change options
> 	to take a HOST_WIDE_INT argument and accept a byte-size suffix.
> 	Initialize.
> 	* doc/invoke.texi (GCC Command Options): Document option arguments.
> 	Explain byte-size arguments and suffixes.
> 	(-Wvla-larger-than, -Wno-alloc-size-larger-than): Update.
> 	(-Wno-alloca-larger-than, -Wno-vla-larger-than): Same.
> 	(-Wframe-larger-than, -Wlarger-than, -Wstack-usage): Same.
> 	* doc/options.texi (UInteger): Expand.
> 	(Host_Wide_Int, ByteSize): Document new properties.
> 	* final.c (final_start_function_1): Include sizes in an error message.
> 	* function.c (frame_offset_overflow): Same.
> 	* gimple-ssa-warn-alloca.c (pass_walloca::gate): Adjust.
> 	(alloca_call_type_by_arg): Change function argument to HOST_WIDE_INT.
> 	Diagnose unbounded alloca calls only for limits of less than
> 	PTRDIFF_MAX.
> 	(alloca_call_type): Adjust.  Diagnose possibly out-of-bounds alloca
> 	calls and VLA size only for limits of less than	PTRDIFF_MAX.  Same
> 	for alloca(0).
> 	(pass_walloca::execute): Adjust.  Diagnose alloca calls in loops
> 	only for limits of less than PTRDIFF_MAX.
> 	* langhooks-def.h (lhd_handle_option): Change function argument
> 	to HOST_WIDE_INT.
> 	* langhooks.c (lhd_handle_option): Same.
> 	* langhooks.h (handle_option): Same.
> 	* opt-functions.awk (switch_bit_fields): Handle Host_Wide_Int and
> 	ByteSize flags.
> 	(var_type, var_type_struct): Same.
> 	(var_set): Handle ByteSize flag.
> 	* optc-gen.awk: Add comments to output to ease debugging.  Make
> 	use of HOST_WIDE_INT where appropriate.
> 	* opts-gen-save.awk:  Use %lx to format unsigned long.
> 	* opth-gen.awk: Change function argument to HOST_WIDE_INT.
> 	* opts-common.c (integral_argument): Return HOST_WIDE_INT and add
> 	arguments.  Parse bytes-size suffixes.
> 	(enum_arg_to_value): Change function argument to HOST_WIDE_INT.
> 	(enum_value_to_arg): Same.
> 	(decode_cmdline_option): Handle cl_host_wide_int.  Adjust.
> 	(handle_option): Adjust.
> 	(generate_option): Change function argument to HOST_WIDE_INT.
> 	(cmdline_handle_error): Adjust.
> 	(read_cmdline_option): Change function argument to HOST_WIDE_INT.
> 	(set_option): Change function argument to HOST_WIDE_INT.
> 	(option_enabled): Handle cl_host_wide_int.
> 	(get_option_state): Handle CLVC_SIZE.
> 	(control_warning_option): Same.
> 	* opts.c (common_handle_option): Change function argument to
> 	HOST_WIDE_INT.  Remove handling of OPT_Walloca_larger_than_ and
> 	OPT_Wvla_larger_than_.
> 	* opts.h (enum cl_var_type): Add an enumerator.
> 	* stor-layout.c (layout_decl): Print a more meaningful warning.
> 	* toplev.c (output_stack_usage): Adjust.
In the doc changes you refer to the -falign-* options.  All that stuff
recently changed.  Please make sure your references are still sane given
those recent changes.


> diff --git a/gcc/gimple-ssa-warn-alloca.c b/gcc/gimple-ssa-warn-alloca.c
> index 327c806..a3367aa 100644
> --- a/gcc/gimple-ssa-warn-alloca.c
> +++ b/gcc/gimple-ssa-warn-alloca.c
> @@ -150,13 +153,25 @@ struct alloca_type_and_limit {
>  // in bytes we allow for arg.
>  
>  static struct alloca_type_and_limit
> -alloca_call_type_by_arg (tree arg, tree arg_casted, edge e, unsigned max_size)
> +alloca_call_type_by_arg (tree arg, tree arg_casted, edge e,
> +			 unsigned HOST_WIDE_INT max_size)
>  {
>    basic_block bb = e->src;
>    gimple_stmt_iterator gsi = gsi_last_bb (bb);
>    gimple *last = gsi_stmt (gsi);
> +
> +  const offset_int maxobjsize = tree_to_shwi (max_object_size ());
> +
> +  /* When MAX_SIZE is greater than or equal to PTRDIFF_MAX treat
> +     allocations that aren't visibly constrained as OK, otherwise
> +     report them as (potentially) unbonded.  */
s/unbonded/unbounded/

OK with the nit fixed.  IF you need to update the doc changes as a
result of the -faligned-* changes, those are pre-approved.

jeff


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]