[llvm] r239552 - Generalize emitAbsoluteSymbolDiff.

[Duncan P. N. Exon Smith]

> On 2015-Jun-11, at 12:26, Frédéric Riss <friss at apple.com> wrote:
> 
> 
>> On Jun 11, 2015, at 12:19 PM, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
>> 
>>> When Duncan was working on this I was wondering if it couldn’t be generalized. I thought the ELF linkers relied on the relocations this would generate eg. for strings in the dwarf_str section. I guess all the strings will be in the same fragment. Without relocations, how can a standard ELF linker rewrite the references to the dwarf_str section when combining multiple input files?
>> 
>> The elf linker needs a relocation if one section refers to another. If
>> two symbols are in different sections, then they are in different
>> fragments.
>> 
>> So the case of dwarf_str, only if the two symbols are in dwarf_str
>> would this function avoid creating the expression.
> 
> Sorry for the noise. I thought the logic that did a difference between the string symbol and the start of section symbol was common to all platforms. I see that ELF will emits directly the string symbol which makes it totally irrelevant to this patch :-)

What a mess.  These functions' assumptions are pretty convoluted.  I
just spent some time trying to follow this as well (a little behind on
email).  I'm not quite convinced this patch is right.

I hadn't realized that (almost!) all the logic to use
emitLabelDifference() from section starts was Darwin-specific.  With
that new knowledge, here are a few useful assumptions we can make in
emitLabelDifference():

 1. It's only used when emitting DWARF.  (I have an out-of-tree patch
    to rename it to emitDwarfLabelDifference()... I'll commit that soon
    I hope.)
 2. emitSectionOffset(), which calls into this function only on Mach-O,
    is only used to describe DWARF sections.  (The same patch fixes its
    name.)
 3. On ELF, sections can't be split.  Since emitLabelDifference() is
    only used between two symbols within a section (caveat below for
    split-DWARF!!), we can safely emit it absolutely.
 4. On Mach-O, DWARF isn't relocatable: the DWARF is always grabbed
    from the original object file.  The target symbols are always
    either (1) DWARF, which won't move, or (2) some offset of an atom,
    and atoms can't be split.  Safe here too.

So aside from my caveat, and assuming COFF somehow falls in the ELF
bucket (or doesn't use DWARF), this seems good.

The one case I can't quite prove to myself is "okay" is split-DWARF.
There's a bunch of logic that calls `emitLabelDifference()` instead of
`emitSectionOffset()` on split-DWARF/DWO.  `DIELocList::EmitValue()`
is one suspicious example, since it's emitting a symbol offset into
the TEXT section:

    void DIELocList::EmitValue(const AsmPrinter *AP, dwarf::Form Form) const {
      DwarfDebug *DD = AP->getDwarfDebug();
      MCSymbol *Label = DD->getDebugLocs().getList(Index).Label;
    
      if (AP->MAI->doesDwarfUseRelocationsAcrossSections() && !DD->useSplitDwarf())
        AP->emitSectionOffset(Label);
      else
        AP->EmitLabelDifference(Label, Label->getSection().getBeginSymbol(), 4);
    }

I *think* this case is alright, but only because, currently, every
instruction happens to be in a separate RelaxableFragment, so
emitAbsoluteLabelDifference() is going to "fail".  What I don't like
is that whoever breaks that assumption is going to have no idea that
the correctness of "emitAbsoluteLabelDifference()" depends on this.

To rephrase, emitAbsoluteLabelDifference() used to only be called
when it was correct to emit an absolute difference -- if it "failed",
it was only a missed optimization.  It looks to me like we're now
*depending* on it "failing" for split-DWARF.

Or am I wrong that we're depending on that?  Does ELF not require
relocations here?

(There are a couple of other calls to emitLabelDifference() that come
from split-DWARF/DWO logic.  I didn't look at them as deeply, but I
imagine they could have similar problems?)