Make LLVM output robust to -dead_strip on mach-o platforms
ClosedPublic

Authored by angerman on Mar 7 2017, 3:43 AM.

Details

Summary

This reverses commit 1686f30951292e94bf3076ce8b3eafb0bcbba91d (Mangle .subsections_via_symbols away., D3287),
and implements proper support for -dead_strip via the injection of .alt_entry symbols for the function
definition pointing to the beginning of the prefix data.

This is the result of a lengthy discussion with rwbarton, and the following llvm-dev mailing list thread:
http://lists.llvm.org/pipermail/llvm-dev/2017-March/110733.html

The essential problem is that there is no reference from a function to its info table.
This combined with .subsections_via_symbols, which llvm emits unconditionally, leads
the linker to believe that the prefix data is unnecessary and stripping it away if presented with the
-dead_strip flag.

The NCG has for this purpose special $dsp (dead strip preventer) symbols and adds a relocation
to the end of each function body pointing to that function's $dsp symbol. We cannot easily do the
same thing via LLVM. Instead we use the .alt_entry directive on the function symbol, which causes
the linker to treat it as a continuation of the previous symbol, namely the $dsp symbol. As a result
the function body will not be separated from its info table.

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
angerman created this revision.Mar 7 2017, 3:43 AM
rwbarton retitled this revision from Allow -dead_strip on mach-o platforms to Make LLVM output robust to -dead_strip on mach-o platforms.Mar 7 2017, 8:31 AM
rwbarton edited the summary of this revision. (Show Details)Mar 7 2017, 8:38 AM
angerman planned changes to this revision.Mar 7 2017, 10:05 PM

This is currently not working as expected. Additional changes that make the visibility of symbols clearer to the linker are necessary, such that the linker does not strip symbols that are essential (e.g. _stg_STACK_info$def).

angerman updated this revision to Diff 11635.Mar 8 2017, 6:47 AM
  • don’t dead_strip dsps.

This does work, and has the same effect as the mangling of the .subsection_via_symbols directive. This is all rather unfortunate. However I'd rather work towards dropping the Mangler for good,
and have every symbol explicitly marked .no_dead_strip, rather than having to mangle the .subsections_via_symbols in the Mangler, in some hot patch post process way.

rwbarton edited edge metadata.Mar 8 2017, 7:19 AM

You don't need .alt_entry any more if you are putting .no_dead_strip on the info tables (it could only be harmful, by causing even more things to stay live).

You don't need .alt_entry any more if you are putting .no_dead_strip on the info tables (it could only be harmful, by causing even more things to stay live).

I fear, I'm not following.

Without the .alt_entry, we don't have a handle on the start of the prefix data.

To take the diagram from the comment:

.------------. <- @<name>$def$dsp      (A)
| Info Table |
|------------| <- @<name>, @<name>$def (B)
| Fn Body    |
'------------' <- end                  (C)

While we probably do not want to mark everything as .no_dead_strip, this is now more a replacement for the mangler,
which essentially dropped -dead_strip via the removal of .subsectioions_via_symbols.

However, with the .alt_entry we now have a handle at (A). And we basically want (A) to (C) dead_strip protected.

You don't need .alt_entry any more if you are putting .no_dead_strip on the info tables (it could only be harmful, by causing even more things to stay live).

I fear, I'm not following.

Without the .alt_entry, we don't have a handle on the start of the prefix data.

This doesn't make sense. Whether or not you include an .alt_entry directive is quite independent of whether you put a symbol which points to the start of the info table.

To take the diagram from the comment:

.------------. <- @<name>$def$dsp      (A)
| Info Table |
|------------| <- @<name>, @<name>$def (B)
| Fn Body    |
'------------' <- end                  (C)

While we probably do not want to mark everything as .no_dead_strip, this is now more a replacement for the mangler,
which essentially dropped -dead_strip via the removal of .subsectioions_via_symbols.

However, with the .alt_entry we now have a handle at (A). And we basically want (A) to (C) dead_strip protected.

No, it's writing the alias that gives you a symbol that points at (A). Has nothing to do with the .alt_entry directive.

rwbarton added inline comments.Mar 8 2017, 8:16 AM
compiler/llvmGen/LlvmCodeGen/Ppr.hs
225

To clarify, I am just talking about removing this single line. The one that adds an .alt_entry directive.

I might have had a wrong interpretation of .alt_entry. I'll give this a build, and update the diff, unless something falls apart along the way.

angerman updated this revision to Diff 11638.Mar 8 2017, 9:14 AM
  • drop alt_entry
michalt added a subscriber: michalt.Mar 8 2017, 1:40 PM
angerman updated this revision to Diff 11663.Mar 9 2017, 7:56 PM
  • rebase
bgamari updated this revision to Diff 12160.Apr 17 2017, 11:29 AM
bgamari edited edge metadata.

Rebase for validate on OS X

angerman updated this revision to Diff 12275.Apr 25 2017, 8:58 PM
  • don’t dead_strip dsps.
  • drop alt_entry
bgamari accepted this revision.Apr 27 2017, 11:32 AM

I'm still not entirely sure why this is better than the mangler approach given that ultimately we will eventually need to move to some more permanent solution anyways, but fair enough.

compiler/llvmGen/LlvmCodeGen/Ppr.hs
184

Note that this is also explained in Note [Llvm Forward References] in LlvmCodeGen.Base.

This revision is now accepted and ready to land.Apr 27 2017, 11:32 AM

I'm still not entirely sure why this is better than the mangler approach given that ultimately we will eventually need to move to some more permanent solution anyways, but fair enough.

Hopefully with llvm4 we'll have that permanent solution. Removing this item from the mangler list, (the function/object rewrite is elf specific, and for the avx rewrite I have no test case),
allows to *not* have to run the mangler. And the mangler is prohibitive towards any chance of making bitcode a object file target, as the mangler operates on the assembly (e.g. after llc).

Ideally we'd like to kill of the mangler entirely. Thus any step towards that end that does not require processing all produced assembly seems like a win to me.

compiler/llvmGen/LlvmCodeGen/Ppr.hs
184

Yes it is, but as I had precisely this code lost from the llvm backend once already. I'd rather reiterate it
here for the time beeing, so that the chances of having it lost again are reduced.

Another way to look at it is that the mangler is a gigantic hack, whereas this diff addresses the problem at it's core. We use specific features and need to guard them properly.

This change also means that in the mean time we get to strip dead code--we just don't get to strip the info tables attached to dead code.

Alright, I suppose this sounds like a win. It would be interesting to know how much this affects object file sizes in practice.

This revision was automatically updated to reflect the committed changes.