This is the mail archive of the 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] treat -Wxxx-larger-than=HWI_MAX special (PR 86631)

On 08/20/2018 06:14 AM, Richard Biener wrote:
On Thu, Jul 26, 2018 at 10:52 PM Martin Sebor <> wrote:

On 07/26/2018 08:58 AM, Martin Sebor wrote:
On 07/26/2018 02:38 AM, Richard Biener wrote:
On Wed, Jul 25, 2018 at 5:54 PM Martin Sebor <> wrote:

On 07/25/2018 08:57 AM, Jakub Jelinek wrote:
On Wed, Jul 25, 2018 at 08:54:13AM -0600, Martin Sebor wrote:
I don't mean for the special value to be used except internally
for the defaults.  Otherwise, users wanting to override the default
will choose a value other than it.  I'm happy to document it in
the .opt file for internal users though.

-1 has the documented effect of disabling the warnings altogether
(-1 is SIZE_MAX) so while I agree that -1 looks better it doesn't
work.  (It would need more significant changes.)

The variable is signed, so -1 is not SIZE_MAX.  Even if -1 disables
it, you
could use e.g. -2 or other negative value for the other special case.

The -Wxxx-larger-than=N distinguish three ranges of argument
values (treated as unsigned):

   1.  [0, HOST_WIDE_INT_MAX)
   3.  [HOST_WIDE_INT_MAX + 1, Infinity)

But it doesn't make sense for those to be host dependent.

It isn't when the values are handled by each warning.  That's
also the point of this patch: to remove this (unintended)

I think numerical user input should be limited to [0, ptrdiff_max]
and cases (1) and (2) should be simply merged, I see no value
in distinguishing them.  -Wxxx-larger-than should be aliased
to [0, ptrdiff_max], case (3) is achieved by -Wno-xxx-larger-than.

To be clear: this is also close to what this patch does.

The only wrinkle is that we don't know the value of PTRDIFF_MAX
either at the time the option initial value is set in the .opt
file or when the option is processed when it's specified either
on the command line or as an alias in the .opt file (as all
-Wno-xxx-larger-than options are).

But then why not make that special value accessible and handle
it as PTRDIFF_MAX when that is available (at users of the params)?

That is,

Index: gcc/calls.c
--- gcc/calls.c (revision 262951)
+++ gcc/calls.c (working copy)
@@ -1222,9 +1222,12 @@ alloc_max_size (void)
   if (alloc_object_size_limit)
     return alloc_object_size_limit;

-  alloc_object_size_limit
-    = build_int_cst (size_type_node, warn_alloc_size_limit);
+  HOST_WIDE_INT limit = warn_alloc_size_limit;
+  if (limit == HOST_WIDE_INT_MAX)
+    limit = tree_to_shwi (TYPE_MAX_VALUE (ptrdiff_type_node));

+  alloc_object_size_limit = build_int_cst (size_type_node, limit);
   return alloc_object_size_limit;

use sth like

 if (warn_alloc_size_limit == -1)
   alloc_object_size_limit = fold_convert (size_type_node,
TYPE_MAX_VALUE (ptrdiff_type_node));
   alloc_object_size_limit = size_int (warn_alloc_size_limit);

?  Also removing the need to have > int params values.

Not sure I understand this last part.  Remove the enhancement?
(We do need to handle option arguments in excess of INT_MAX.)

It's that HOST_WIDE_INT_MAX use that is problematic IMHO.  Why not use -1?

-1 is a valid/documented value of the argument of all these
options because it's treated as unsigned HWI:

  Warnings controlled by the option can be disabled either
  by specifying byte-size of ‘SIZE_MAX’

It has an intuitive meaning: warning for any size larger than
the maximum means not warning at all.  Treating -1 as special
instead of HOST_WIDE_INT_MAX would replace that meaning with
"warn on any size in excess of PTRDIFF_MAX."

A reasonable way to disable the warning is like so:

  gcc -Walloc-size-larger-than=$(getconf ULONG_MAX) ...

That would not work anymore.

Treating HOST_WIDE_INT_MAX as PTRDIFF_MAX is the natural choice
on LP64: they have the same value.  It's only less than perfectly
natural in ILP32 and even there it's not a problem in practice
because it's either far out of the range of valid values [0, 4GB]
(i.e., where HWI is a 64-bit long long), or it's also equal to
PTRDIFF_MAX (on hosts with no 64-bit type, if GCC even supports

I'm not trying to be stubborn here but I just don't see why
you think that setting aside HOST_WIDE_INT_MAX is problematic.
Anything else is worse from a user-interface POV.  It makes
little difference inside GCC as long as we want to let users
choose to warn for allocation sizes over some value in
[PTRDIFF_MAX, SIZE_MAX] -- e.g., for malloc() over 3GB but
not for less.  There's also the -Walloca-size-larger-than=
case where PTRDIFF_MAX means only warn for known sizes over
than but not for unknown sizes.



 Case (2) above is only
used by the implementation as a placeholder for PTRDIFF_MAX.
It's not part of the interface -- it's an internal workaround
for lack of a better word.

(There is an additional wrinkle in the -Walloca-larger-than=
has two modes both of which are controlled by a single option:
(a) diagnose only allocations >= PTRDIFF_MAX (default), and
(b) diagnose allocations > limit plus also unbounded/unknown
allocations.  I think these modes should be split up and (b)
controlled by a separate option (say something like

I think you are over-engineering this and the user-interface is

Thank you.

I agree that what you describe would be the ideal solution.
As I explained in the description of the patch, I did consider
handling PTRDIFF_MAX but the target-dependent value is not
available at the time the option argument is processed.  We
don't even know yet what the target data model is.

This is the best I came up with.  What do you suggest instead?


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