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

[Rui Ueyama via llvm-commits]

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171106/f3e03bac/attachment.html>