[COMMITTED] gas: sframe: do not test whether offsetT exceeds INT64_MIN..INT64_MAX
Jan Beulich
jbeulich@suse.com
Tue Jan 13 15:52:46 GMT 2026
On 13.01.2026 16:46, Jens Remus wrote:
> Hello Jan,
>
> thank you for the prompt response!
>
> On 1/13/2026 4:37 PM, Jan Beulich wrote:
>> On 13.01.2026 16:27, Jens Remus wrote:
>>> On 1/13/2026 2:59 PM, Jan Beulich wrote:
>>>> On 13.01.2026 14:24, Jens Remus wrote:
>>>>> A value of type offsetT, which is either a signed 32-bit or 64-bit
>>>>> integer, cannot exceed the range of INT64_MIN..INT64_MAX. This
>>>>> resolves the following compile error:
>>>>>
>>>>> ../../binutils-gdb/gas/gen-sframe.c: In function ‘get_offset_size_in_bytes’:
>>>>> ../../binutils-gdb/gas/gen-sframe.c:213:45: error: comparison is always true due to limited range of data type [-Werror=type-limits]
>>>>> 213 | else if ((sizeof (offsetT) > 4) && (value <= INT64_MAX && value >= INT64_MIN))
>>>>> | ^~
>>>>> ../../binutils-gdb/gas/gen-sframe.c:213:67: error: comparison is always true due to limited range of data type [-Werror=type-limits]
>>>>> 213 | else if ((sizeof (offsetT) > 4) && (value <= INT64_MAX && value >= INT64_MIN))
>>>>
>>>> This same error ...
>>>>
>>>>> --- a/gas/gen-sframe.c
>>>>> +++ b/gas/gen-sframe.c
>>>>> @@ -210,7 +210,7 @@ get_offset_size_in_bytes (offsetT value)
>>>>> size = 2;
>>>>> else if (value <= INT32_MAX && value >= INT32_MIN)
>>>>
>>>> ... will trigger here on a !BFD64 build.
>>>
>>> Argh! Good catch! Thanks for letting me know! How do I resolve this
>>> properly? Shall I commit another fix "gas: sframe: do not test whether
>>> 32-bit offsetT exceeds INT32_MIN..INT32_MAX" or shall I better revert
>>> both commits and start fresh with a v3 review of the initial patch?
>>
>> Incrementally is fine with me.
>>
>>> I propose the following as solution:
>>>
>>> if (value <= INT8_MAX && value >= INT8_MIN)
>>> size = 1;
>>> else if (value <= INT16_MAX && value >= INT16_MIN)
>>> size = 2;
>>> else if ((sizeof (offsetT) == 4) || (value <= INT32_MAX && value >= INT32_MIN))
>>
>> Depending on the internal workings of compilers, this may still trigger
>> the same diagnostic. I.e. depending on whether the rhs of the || is
>> eliminated ahead of or after when these kinds of warnings are issued.
>> A pre-processor conditional involving BFD64 may be better.
>
> Is the following patch ok?
In principle yes; the part I need to defer to Indu or you is whether ...
> --- a/gas/gen-sframe.c
> +++ b/gas/gen-sframe.c
> @@ -208,10 +208,15 @@ get_offset_size_in_bytes (offsetT value)
> size = 1;
> else if (value <= INT16_MAX && value >= INT16_MIN)
> size = 2;
> +#ifdef BFD64
> else if (value <= INT32_MAX && value >= INT32_MIN)
> size = 4;
> - else if (sizeof (offsetT) > 4)
> + else
> size = 8;
> +#else
> + else
> + size = 4;
> +#endif
... 8-byte sizes really are entirely impossible to use when !BFD64. Since
it's an offsetT that is passed in, this would seem logical, but I'd rather
have Indu / you double-check.
Jan
More information about the Binutils
mailing list