[PATCH] D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D68101#1806135 <https://reviews.llvm.org/D68101#1806135>, @nickdesaulniers wrote:

> In D68101#1802220 <https://reviews.llvm.org/D68101#1802220>, @bd1976llvm wrote:
>
> > Below is the code comment from the new patch explaining the new approach, please take a look and see if you have any questions/comments:
> >
> >   // If two globals with differing sizes end up in the same mergeable
> >   // section that section can be assigned an incorrect entry size. Normally,
> >   // the assembler avoids this by putting incompatible globals into
> >   // differently named sections. However, globals can be explicitly assigned
> >   // to a section by specifying the section name. In this case, if unique
> >   // section names are available (-unique-section-names in LLVM) then we
> >   // bin compatible globals into different mergeable sections with the same name.
>
>
> Looks good up to here.
>
> >   // Otherwise, if incompatible globals have been explicitly assigned to section by a
> >   // fine-grained/per-symbol mechanism (e.g. via  _attribute_((section(“myname”)))) then
> >   // we issue an error and the user can then change the section assignment. If the
>
> No bueno.  The Linux kernel has code where there are 2D global arrays of different entity sizes explicitly placed in different sections (together). GCC does not error for this, (it doesn't mark the section merge-able), neither should we.
>
> In D68101#1802280 <https://reviews.llvm.org/D68101#1802280>, @rjmccall wrote:
>
> > I laid out a series of three options before:
> >
> > - Emit different object-file sections for different mergeability settings.
> > - Only mark an object-file section as mergeable if all the symbols in it would have the same mergeability settings.
> > - Stop implicitly using mergeability for "ordinary" sections (i.e. sections other than the string section).
>
>
> Of the above, I really think #2 is the only complete solution.


Can you explain why you think #1 is not "complete"?  All three seem to establish correctness; I can see how giving up on the optimization (#3) is not a great solution, but #1 doesn't have that problem and actually preserves it in more places.  To be clear, this is just taking advantage of the ability to have multiple sections with the same name in an ELF object file; they will still be assembled into a single section in the linked image.

My understanding is that changing the flags on an MCSection retroactively is pretty counter to the architecture, so I'm not sure how we'd actually achieve #2.


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

https://reviews.llvm.org/D68101