[COMMITTED] gas: sframe: do not test whether offsetT exceeds INT64_MIN..INT64_MAX
Jens Remus
jremus@linux.ibm.com
Tue Jan 13 16:07:18 GMT 2026
On 1/13/2026 4:52 PM, Jan Beulich wrote:
> On 13.01.2026 16:46, Jens Remus wrote:
>> 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.
Condensed excerpt from gas/as.h:
#ifdef BFD64
typedef int64_t bfd_signed_vma;
#else /* not BFD64 */
typedef int32_t bfd_signed_vma;
#endif /* not BFD64 */
typedef bfd_signed_vma offsetT;
If BFD64, then offsetT is a signed 64-bit integer. Otherwise it is a
signed 32-bit integer. As a result it is impossible for !BFD64 to have
a 64-bit offsetT.
Or were you concerned whether SFrame would be broken for !BFD64 targets
in general? AFAIK SFrame does only allow 8-bit, 16-bit, and 32-bit
SFrame offsets. 64-bit ones are not supported:
Excerpt from gas/gen-sframe.c:
#define SFRAME_FRE_OFFSET_FUNC_MAP_INDEX_1B 0 /* SFRAME_FRE_OFFSET_1B. */
#define SFRAME_FRE_OFFSET_FUNC_MAP_INDEX_2B 1 /* SFRAME_FRE_OFFSET_2B. */
#define SFRAME_FRE_OFFSET_FUNC_MAP_INDEX_4B 2 /* SFRAME_FRE_OFFSET_4B. */
#define SFRAME_FRE_OFFSET_FUNC_MAP_INDEX_8B 3 /* Not supported in SFrame. */
Regards,
Jens
--
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com
IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/
More information about the Binutils
mailing list