[PATCH 04/11] elf: Adjust DT_EXTRATAGIDX to avoid undefined shifts

[Adhemerval Zanella Netto]

On 02/06/25 07:48, Florian Weimer wrote:
> * Adhemerval Zanella Netto:
> 
>> On 20/05/25 09:48, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>> From: Richard Henderson <rth@twiddle.net>
>>>>
>>>> When building with --enable-ubsan, the relocation code triggers:
>>>>
>>>> UBSAN: Undefined behaviour in get-dynamic-info.h:56:30 left shift of 1879047925 by 1 cannot be represented in type 'int'
>>>>
>>>> Originally from
>>>> https://sourceware.org/pipermail/libc-alpha/2015-August/063015.html.
>>>> ---
>>>>  elf/elf.h | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/elf/elf.h b/elf/elf.h
>>>> index 1e1a59c14d..d48cf47b9a 100644
>>>> --- a/elf/elf.h
>>>> +++ b/elf/elf.h
>>>> @@ -995,7 +995,7 @@ typedef struct
>>>>     range.  Be compatible.  */
>>>>  #define DT_AUXILIARY    0x7ffffffd      /* Shared object to load before self */
>>>>  #define DT_FILTER       0x7fffffff      /* Shared object to get values from */
>>>> -#define DT_EXTRATAGIDX(tag)	((Elf32_Word)-((Elf32_Sword) (tag) <<1>>1)-1)
>>>> +#define DT_EXTRATAGIDX(tag)  (-((Elf32_Sword)((Elf32_Word)(tag) * 2) / 2 + 1))
>>>>  #define DT_EXTRANUM	3
>>>>  
>>>>  /* Values of `d_un.d_val' in the DT_FLAGS entry.  */
>>>
>>> Doesn't this change the type of the constant?  And rely on a GCC
>>> extension?
>>
>> Indeed, I think an extra cast to Elf32_Word should keep the same type.
>> Wrt to GCC extension, this definition is already gated through _GNU_SOURCE
>> and afaik only used internally on glibc (I couldn't find any other project
>> that actually uses it), so I think it should be fine to rely on it.
> 
> If it's an internal definition, should we move it into elf/include.h
> instead (with a NEWS entry)?

I am not sure since the search I did for its usage might not be that wide.
I don't have a strong opinion about that.