[llvm][patch] Adjust behavior of FDE cross-section relocs for targets that don't support abs-differences.

[Iain Sandoe]

Hi all and Happy New Year!

It seems we all agree on the main engineering elements here - so the issue is to decide whether to apply the patch or not.

@ Nick and Jim

- my point here is that the current behaviour is wrong for all ld variants (incl binutils ones) - so I'd think we should apply this completely independently of supporting legacy OSX versions.

@ Nick - if you have metrics for the improvements in code-size or link speed - it might be interesting to see if the binutils folks would like to conform to this approach too?

so the question is:

Is this patch good to apply?

Iain


On 19 Dec 2013, at 00:39, David Fang wrote:


> 	FWIW, this patch set fixes most exception throwing test cases in libc++'s test suite.  Logs from my recent tests (powerpc-darwin8 branch):
> 
> powerpc-darwin8 branch at r197439:
> http://csl.cornell.edu/~fang/sw/llvm/logs/libcxx-check-20131217.log.bz2
> 
> powerpc-darwin8 branch at r197439 +FDE patches:
> http://csl.cornell.edu/~fang/sw/llvm/logs/libcxx-check-20131218.log.bz2
> 
> I am using odcctools-2009's ld and as. flags: -B/sw/lib/odcctools/bin -std=c++11 -stdlib=libc++ -save-temps -no-integrated-as
> 
> vote: I would very much like this fix to go in upstream.  This is necessary to unblock libc++abi or libc++(libsupc++).  It doesn't break anything, and we are willing to continue to maintain and test it.
> We need to include some llvm-dwarfdump test cases though.

>> 
>> On 17 Dec 2013, at 22:28, Nick Kledzik wrote:
>> 
>>> On Dec 12, 2013, at 3:57 PM, Iain Sandoe <iain at codesourcery.com> wrote:
>>>> bump
>>>> 
>>>> @jim - although this was reported against an old target, I believe it potentially effects any target using non-trivial relocs for Start-PC and LSDAs in FDEs.
>>>> Iain
>>>> 
>>>> On 3 Dec 2013, at 15:59, Iain Sandoe wrote:
>>>> 
>>>>> Hi,
>>>>> (I hope I have directed this to the correct reviewers)
>>>>> 
>>>>> At the dev. conf. David reported to me that EH was not working on ppc-darwin8.  Once we checked a few more platforms, it turns out it's not working with any ld64 < 97.17 (so OSX < 10.6 on both x86/ppc).
>>>>> 
>>>>> The reason is that the older ld/ld64 versions require proper scattered relocs for expressions in CIE/FDE symbol refs that cross section boundaries.
>>>>> 
>>>>> Later versions of the ld64 allow that these may be replaced by an absolute difference***
>>> I can tell you the motivation for the linker change.  ld64 takes the ?smart linker? approach.  If fully breaks up the __eh_frame section and associates each FDE with its owning function.  Since it knows how to parse an FDE, a relocation is redundant and only slows down the linker.  On the other hand, a ?dumb linker? which does not know anything special about the __eh_frame section would need relocations.
>> 
>> indeed, but (as far as I am aware) the only such smart linker is ld64 >= 97.1 ?
>> ?  so should the patch be applied to allow other linkers to function?
>> 
>> Iain
>> 
>> 
>>> 
>>> -Nick
>>> 
>>>>> 
>>>>> AFAIK, this behaviour is specific to Darwin/OSX (and ld64 >= 97.17), I suspect that any other target that elected to use a non-trivial FDE symbol encoding would likely be surprised by the abs values.  However, these are hardwired in MCDwarf.cpp, as of now.
>>>>> 
>>>>> NOTE1: in general, this would not cause a linkage error for the target - 'just' a failure to unwind at runtime.
>>>>> NOTE2: it also seems that the abs-ification is not done even for x86-64-darwin12 for compact unwind.
>>>>> 
>>>>> The solution I am proposing is to introduce a "DwarfFDESymbolsUseAbsDiff" boolean value in MCAsmInfo and to use that to control this behaviour.  In the patch, this is only switched on for X86 Darwin/OSX & for OSX >= 10.6.  It should really be contingent on -target-linker-version >= 97.17, but this information is not yet passed to the target.
>>>>> 
>>>>> With the patch EH works on *-darwin9 with the native linker (and still emits the same set of relocs on darwin12).
>>>>> 
>>>>> Is this a reasonable solution?
>>>>> Iain
>>>>> 
>>>>> *** P.S.
>>>>> I assume that these extra hoops and inconsistencies (and presumably matching ones in ld64) are really worthwhile?
>>>>> I.E. that there is some measurable performance gain?
>>>>> (if not, we could just elect to emit the relocs and drop this special casing).
>>>>> 
>>>>> <llvm-eh-fde-relocs-diff-v1.txt>
>>>>> 
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at cs.uiuc.edu
>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>> 
>>>> 
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>> 
>>> 
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>> 
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 
> 
> -- 
> David Fang
> http://www.csl.cornell.edu/~fang/
>