[PATCH] D73999: [MC][ELF] Warn changed section type or flags

ben via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 10 09:32:56 PST 2020


bd1976llvm 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.


After @MaskRay's discussion with the GNU binutils people I think the following will work for sh_entsize:

- In the assembler emit an error (as long as https://sourceware.org/ml/binutils/2020-02/msg00129.html goes through) for two section directives with the same section name but incompatible sh_entsize values.
- When lowering symbols "normally" there isn't a problem as we just bin compatible symbols into differently named sections e.g. .rodata.cst4 or .rodata.str4.4.
- When lowering symbols that are explicitly given a section name we will need a fix for https://bugs.llvm.org/show_bug.cgi?id=43457, see also comments in https://reviews.llvm.org/D72194.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73999





More information about the llvm-commits mailing list