[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