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

[Rafael Avila de Espindola via llvm-commits]

The alignment we use is bounded by the section alignment, so I think we
can just produce an error if the final alignment turns out to be too
large.

Cheers,
Rafael

Rui Ueyama <ruiu at google.com> writes:

> What if a symbol happens to have an address that is aligned to a very large
> number? What we are doing here is making a guess, and there's no way to
> make it precise.
>
> Maybe we should limit an alignment to 32 (or a reasonable maximum alignment
> that makes sense on any ISA) if a guessed alignment is too big?
>
> On Mon, Nov 6, 2017 at 1:03 PM, Rafael Avila de Espindola <
> rafael.espindola at gmail.com> wrote:
>
>> 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
>>