[PATCH] D74006: [MC][ELF] Make linked-to symbol name part of ELFSectionKey

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 13 21:18:28 PST 2020


MaskRay added a subscriber: psmith.
MaskRay added a comment.

In D73999#1866680 <https://reviews.llvm.org/D73999#1866680>, @psmith wrote:

> In D73999#1866168 <https://reviews.llvm.org/D73999#1866168>, @MaskRay wrote:
>
> > Address review comments.
> >
> > I tend to agree with Alan Modra's arguement in
> >  https://sourceware.org/ml/binutils/2020-02/msg00093.html
> >
> > > I don't think so.  User assembly often gets section attributes wrong
> > >  or leaves them off entirely for special sections known to the
> > >  assembler.  ie. the first .section .foo above is a built-in rather
> > >  than user input.
> >
> > Add more folks for feedback.
> >
> > Please see my binutils post. For a .section directive with the same name but a different field. If the field is:
> >
> > - sh_flags or sh_type: warn
> > - sh_link due to SHF_LINK_ORDER: no warning. produce separate sections
> > - sh_entsize due to SHF_MERGE: still controversial
>
>
> I agree with warning for sh_flags, I think there is too much legacy code out there that would behave differently.
>  I agree with sh_link producing separate sections. In an ideal world no-one writes this by hand in assembly.
>  For sh_entsize SHF_MERGE, this is informing the linker that it can produce an optimisation, which it is permitted to ignore. I tend towards an error message if it is implementable as it is a strong message to fix the assembler. The next best thing is a warning then clearing SHF_MERGE and setting sh_entsize to 0. I don't think a warning on its own is safe.


Does this patch look good? The consensus is that we should make sh_link producing separate sections.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74006/new/

https://reviews.llvm.org/D74006





More information about the llvm-commits mailing list