This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] specify large command line option arguments (PR 82063)
- From: Jeff Law <law at redhat dot com>
- To: Martin Sebor <msebor at gmail dot com>, Gcc Patch List <gcc-patches at gcc dot gnu dot org>
- Date: Thu, 19 Jul 2018 16:31:56 -0600
- Subject: Re: [PATCH] specify large command line option arguments (PR 82063)
- References: <8f19164f-2b12-2e52-493a-37ec412debcd@gmail.com>
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