This is an archive of the discontinued LLVM Phabricator instance.

[mips] Correct the ordering of HI/LO pairs in the relocation table.
ClosedPublic

Authored by dsanders on Apr 29 2016, 3:16 AM.

Details

Summary

There seems to have been a misunderstanding as to the meaning of 'offset' in
the rules laid down by our ABI. The previous code believed that 'offset' meant
the offset within the section that the relocation is applied to. However, it
should have meant the offset from the symbol used in the relocation expression.

This patch adds two fields to ELFRelocationEntry and uses them to correct the
order of relocations for MIPS. These fields contain:

  • The original symbol before shouldRelocateWithSymbol() is considered. This ensures that R_MIPS_GOT16 is able to correctly distinguish between local and external symbols, allowing us to tell whether %got() requires a matching %lo() or not (local symbols require one, external symbols don't). It also prevents confusing cases where the fuzzy matching rules cause things like %hi(foo)/%lo(foo+3) and %hi(bar)/%lo(bar+1) to swap their %lo()'s.
  • The original offset before shouldRelocateWithSymbol() is considered. The existing Addend field is always zero when the object uses in place addends (because it's already moved it to the encoding) but MIPS needs to use the original offset to ensure that the linker correctly calculates the carry-in bit for %hi() and %got().

IAS ensures that unmatchable %hi()/%got() relocations are placed at the end of
the table to ensure that the linker rejects the table (we're unable to report
such errors directly). The alternatives to this risk accidental matching
against inappropriate relocations which may silently compute incorrect values
due to an incorrect carry bit between the %lo() and %hi()/%got().

Depends on D19716 and fixes the bug revealed by D19016.

Diff Detail

Event Timeline

dsanders updated this revision to Diff 55560.Apr 29 2016, 3:16 AM
dsanders retitled this revision from to [mips] Correct the ordering of HI/LO pairs in the relocation table. There seems to have been a misunderstanding as to the meaning of 'offset' in the rules laid down by our ABI. The previous code believed that 'offset' meant the offset within the....
dsanders updated this object.
dsanders added a reviewer: sdardis.
dsanders added subscribers: llvm-commits, rafael.
dsanders updated this revision to Diff 55561.Apr 29 2016, 3:19 AM

Include a last minute correction.

dsanders retitled this revision from [mips] Correct the ordering of HI/LO pairs in the relocation table. There seems to have been a misunderstanding as to the meaning of 'offset' in the rules laid down by our ABI. The previous code believed that 'offset' meant the offset within the... to [mips] Correct the ordering of HI/LO pairs in the relocation table..Apr 29 2016, 3:19 AM
dsanders updated this object.
sdardis accepted this revision.May 4 2016, 7:48 AM
sdardis edited edge metadata.

LGTM with nits addressed.

lib/Target/Mips/MCTargetDesc/MipsELFObjectWriter.cpp
86–88

Nit: Align the top two comments lines with the bottom line.

156

Nit: The first sentence here is a somewhat fragmented. I assume you're saying "It's type matches that of a corresponding low part".

178

warning: extra ';' [-Wpendantic]

This revision is now accepted and ready to land.May 4 2016, 7:48 AM
dsanders closed this revision.May 6 2016, 6:55 AM