[PATCH] D29315: MCMacho: Allow __thread_ptr section after dwarf sections

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added a comment.

In https://reviews.llvm.org/D29315#662118, @grosbach wrote:

> In https://reviews.llvm.org/D29315#662038, @MatzeB wrote:
>
> > In https://reviews.llvm.org/D29315#661848, @grosbach wrote:
> >
> > > Nothing that can have relocations or fixups to or from it should go after the debug info. This patch doesn't seem right to me.
> >
> >
> > What's the motivation for this rule? Less work when `strip`ing a binary?
> >
> > Also we do allow the `__DATA/__nl_symbol_ptr` section after the dwarf sections today it seems, shouldn't `__DATA/__thread_ptr` behave very similar?
>
>
> The main thing is that debug info can get so large that putting it between things results in relocations being out of range of what the file format can represent. So we put all the stuff than can reference each other w/ relocations up front next to one another.  __TEXT relocations in particular due to the references being start-of-file relative offsets. It might be OK(ish) for __DATA? That would line up with various commit messages. That we're tripping the assert() might imply it's not? Worth exploring a bit to be sure. Debugging when this stuff goes wrong is unpleasant.
>
> For some history on that part, see r231898 and r232842.


My impression is that the ordering decisions here are mostly driven by rdar://15623193 which is motivates them with binary diffing / reproducibility of the text/data section independently of dwarf being emitted or omitted. With that motivation we should indeed output `__DATA/__nl_symbol_ptr` and `__DATA/__thread_ptr` before the dwarf sections.

Note however that we never did this so far and r232842 specifically adds an exception for `__nl_symbol_ptr`, so in the short term (=clang-4.0 release) I'd like to go ahead with just adding a similar exception for `__thread_ptr` as proposed here.

It is probably easy to fix the ordering for both __nl_symbol_ptr and __thread_ptr but I'd like to attack that in a separate patch for llvm ToT only.


Repository:
  rL LLVM

https://reviews.llvm.org/D29315