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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Let me try to restate what's happening here so that I can see if I understand it.

There are two concepts of "section" in play here:

- a unit in an object file with a particular section name, which need not be unique within the object file, and which can have interesting per-unit attributes like mergeability; and
- the high-level layout of the final linked image, where all of the units with the same section name (within a single image) are supposed to be contiguous, and where the image will contain metadata describing the location and size of the section.

I'll call the first a "section unit" and the second an "image section" just for clarity; my apologies if there's more standard jargon.

Marking a section unit as mergeable in ELF opts in to a link-time optimization where the linker can avoid unnecessary duplication in the image section by combining identical data that would have ended up in it.  Essentially, the section unit is broken up into entries according to an entry size that's part of the mergeable attribute, and if the linker sees that two entries will be identical (whether they come from different section units or not), it can simply remove the redundant entry from the final image section.  (Presumably there's some rule about the order of entries, but it doesn't really matter for this analysis.)  This is done as a relatively late pass; any sort of mandatory "merging" from e.g. COMDAT, weak, and common symbols will already have been applied, so we don't need to worry about this interfering with language-mandated symbol coalescing.

When LLVM is emitting ELF, it will try to place an object in a mergeable section unit if the object is `unnamed_addr`.  It will also generally emit objects into the same section unit if they share the same section name.  I assume this takes attributes into account to at least some degree, or else we might be lumping non-`unnamed_addr` into a mergeable section just because the first object we processed with that section name was `unnamed_addr`.  But it must not take entry size into account, because PR 43457 shows us clearly emitting a single section unit for objects of different sizes.

Given all that, this patch seems far too aggressive.  While mergeable sections can be useful for optimizing arbitrary code that might not use a section, they are also extremely useful for optimizing the sorts of global tables that programmers frequently use explicit sections for.  It seems to me that the right fix is to find the place that ensures that we don't put mergeable and non-mergeable objects in the same section unit (or at least conservatively makes the section unit non-mergeable) and fix it to consider entry size as well.  That should be straightforward unless that place doesn't exist, in which case we have very serious problems.


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

https://reviews.llvm.org/D68101