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 Fri, Jul 20, 2018 at 1:57 PM, Martin Sebor <msebor@gmail.com> wrote:
> On 07/19/2018 04:31 PM, Jeff Law wrote:
>>
>> 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.
>
>
> As far as I can tell they're still valid.
>
>
>>> +  /* 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/
>
>
> Fixed.
>
>>
>> OK with the nit fixed.  IF you need to update the doc changes as a
>> result of the -faligned-* changes, those are pre-approved.
>
>
> I had to adjust a few more tests and make a couple of minor
> tweaks as I noticed a coupled of test failures that I had
> previously missed but the result was didn't show any more
> regressions on x86_64.
>
> Committed in r262910.
>

This caused:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86624

This may impact all 32-bit hosts.


-- 
H.J.


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