Re: Another GLIBC build error with GCC6

[Richard Henderson <rth at twiddle dot net>]

> It looks like it is due to some GCC changes involving -Wshift-overflow
> (PR c++/55095)
> 
> Here is a cut-down test case that gives the warning with the latest GCC in
> 32 bit mode:
> 
>   #include <stdlib.h>
>   unsigned long int foo ()
>   {
>     unsigned long int longword, magic_bits;
>     switch (sizeof (longword))
>       {
>       case 4: magic_bits = 0x7efefeffL; break;
>       case 8: magic_bits = ((0x7efefefeL << 16) << 16) | 0xfefefeffL; break;
>       default: abort ();
>       }
>       return magic_bits;
>   }
> 
> This seems like a reasonable error in 32 bit mode, I wonder if the switch
> statement should be ifdef's instead so that the LP64 code is not seen in
> 32 bit mode?

No, I don't think ifdefs are required.  All that's needed is to make these
constants unsigned, so that we're not shifting set bits into the sign bit.  I
believe you can just change L to UL and it should all Just Work.


> The definition of
> DT_EXTRATAGIDX in elf/elf.h also gives a shift overflow warning and
> elf/dl-dl-deps.c now gives an array out-of-bounds message.  Here is a
> not fully tested, not ready patch that allowed me to do a complete
> build.  I think this new definition of DT_EXTRATAGIDX would give the
> same result as the old definition.  I couldn't see anything to do with
> dl-deps.c except ignore the warning, maybe someone else has a better
> idea.

I think the out-of-bounds message is due to...

> -#define DT_EXTRATAGIDX(tag)	((Elf32_Word)-((Elf32_Sword) (tag) <<1>>1)-1)
> +#define DT_EXTRATAGIDX(tag) ((Elf32_Word)-((Elf32_Sword) (tag) & 0x7fffffff)-1)

This change is wrong.  The double shift isn't masking, as you do here, but
sign-extending.  DT_AUXILIARY (0x7ffffffd) becomes 0xfffffffd = -3, and from
there ((-(-3)) - 1) = 2.

You're producing 0x80000002, which is indeed out of bounds.

Which is probably way more complicated that it needs to be, given that the
formulation is only good for the last few entries of DT_HIPROC.  Probably a
formulation like

#define DT_EXTRATAGIDX(tag)  ((tag) - (DT_HIPROC - DT_EXTRANUM))

would be a lot better.  And understandable, from a "what the hell is this
trying to do" standpoint.


r~