[lld] r316863 - Fix ubsan error that shift amount 64 is too large.

[Rafael Avila de Espindola via llvm-commits]

Rui Ueyama <ruiu at google.com> writes:

> On Fri, Nov 3, 2017 at 9:06 PM, Rafael Avila de Espindola <
> rafael.espindola at gmail.com> wrote:
>
>> Rui Ueyama via llvm-commits <llvm-commits at lists.llvm.org> writes:
>>
>> > Modified: lld/trunk/ELF/InputFiles.cpp
>> > URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/
>> InputFiles.cpp?rev=316863&r1=316862&r2=316863&view=diff
>> > ============================================================
>> ==================
>> > --- lld/trunk/ELF/InputFiles.cpp (original)
>> > +++ lld/trunk/ELF/InputFiles.cpp Sun Oct 29 09:49:42 2017
>> > @@ -763,7 +763,9 @@ template <class ELFT> void SharedFile<EL
>> >      // files because the loader takes care of it. However, if we
>> promote a
>> >      // DSO symbol to point to .bss due to copy relocation, we need to
>> keep
>> >      // the original alignment requirements. We infer it here.
>> > -    uint32_t Alignment = 1ULL << countTrailingZeros((uint64_t)
>> Sym.st_value);
>> > +    uint32_t Alignment = 1;
>> > +    if (Sym.st_value)
>> > +      Alignment = 1ULL << countTrailingZeros((uint64_t)Sym.st_value);
>>
>> This still implicitly discard information if st_value is aligned to more
>> than 32 bits, no? Should Alignment be a uint_64?
>>
>
> Yes, but we use uint32_t to represent alignments everywhere in lld. In
> practice, I don't think there's a point to support alignments larger than 4
> GiB because I've never seen such large alignments. (If we really need it,
> we probably should consider representing alignments in the form of N in 2^N
> to save bits to store alignments.)

I fully agree that we should not support alignments that are that
big. What we should do is produce an error when we see an alignment we
don't support. Right now we just silently use the low 32 bits.

Cheers,
Rafael