[PATCH] avoid using types wider than int for width and precision (PR 80364)

Martin Sebor msebor@gmail.com
Mon Apr 10 22:16:00 GMT 2017


>> @@ -972,11 +972,11 @@ get_int_range (tree arg, tree type, HOST_WIDE_INT *pmin, HOST_WIDE_INT *pmax,
>>  	  if (range_type == VR_RANGE)
>>  	    {
>>  	      HOST_WIDE_INT type_min
>> -		= (TYPE_UNSIGNED (type)
>> -		   ? tree_to_uhwi (TYPE_MIN_VALUE (type))
>> -		   : tree_to_shwi (TYPE_MIN_VALUE (type)));
>> +		= (TYPE_UNSIGNED (argtype)
>> +		   ? tree_to_uhwi (TYPE_MIN_VALUE (argtype))
>> +		   : tree_to_shwi (TYPE_MIN_VALUE (argtype)));
>>
>> -	      HOST_WIDE_INT type_max = tree_to_uhwi (TYPE_MAX_VALUE (type));
>> +	      HOST_WIDE_INT type_max = tree_to_uhwi (TYPE_MAX_VALUE (argtype));
>
> For the start, consider what happens if argtype is __int128_t or __uint128_t
> or unsigned int here.  For the first two, those tree_to_uhwi or tree_to_shwi
> will just ICE above (if you have VR_RANGE for those, shouldn't be hard to
> write testcase).  So likely you need to ignore range info for invalid
> args with precision greater than that of integer_type_node (i.e. INT_TYPE_SIZE).

Right.  It's too bad that tree_to_uhwi is so brittle.

>
> unsigned int is I think not UB when passed to var-args and read as int, it is just
> implementation defined how is it converted into int (I could be wrong).
> In that case (i.e. TYPE_UNSIGNED and precision equal to INT_TYPE_SIZE) I
> think you need to either handle it like you do only if the max is smaller or
> equal than INT_MAX and punt to full int range otherwise, or need to take the
> unsigned -> int conversion into account.  I think passing unsigned int arg
> bigger than __INT_MAX__ is not well defined, because that means negative
> precision in the end.

C requires the type of the argument passed to the asterisk to be
int and so, strictly speaking, passing in an argument of any other
type is undefined just as is passing in any value that's not
representable in int.  The same is true for %d and %i.  But mixing
and matching signed and unsigned is common and GCC doesn't warn
about either unless -Wformat-signedness is used, so it makes sense
to handle this case more gracefully.

>
> And, if you never change type, so type == integer_type_node, you might
> consider not passing that argument at all, the behavior is hardcoded for
> that type anyway (assumption that it fits into shwi or uhwi etc.).

I can do that.  I had initially intended the function to be more
general than that but in the end wound up only using it where int
is expected.

>
> Perhaps in addition to the type_min/type_max check you do to determine
> knownrange follow it by similar range check for integer_type_node range
> and don't set unknown to false if it is not within integer_type_node range?
> The TYPE_PRECISION <= INT_TYPE_SIZE check will be still needed not to
> trigger the ICEs.

Yes, that makes sense.

Attached is an updated version with these changes.

Thanks
Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: gcc-80364.diff
Type: text/x-patch
Size: 17304 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20170410/640a6196/attachment.bin>


More information about the Gcc-patches mailing list