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

Nick Desaulniers via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 6 11:13:04 PST 2020


nickdesaulniers added a comment.

In D68101#1806213 <https://reviews.llvm.org/D68101#1806213>, @rjmccall wrote:

> In D68101#1806135 <https://reviews.llvm.org/D68101#1806135>, @nickdesaulniers wrote:
>
> > 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.


Ah, ok, I reread  https://reviews.llvm.org/D72194 and see that it's creating non-contiguous sections (option 1), with unique entity sizes.  That should be fine. Dissenting opinion retracted.  We should prefer https://reviews.llvm.org/D72194 with the commit message updated.


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

https://reviews.llvm.org/D68101





More information about the cfe-commits mailing list