This is an archive of the discontinued LLVM Phabricator instance.

Differential D26401

[mips][ias] N32/N64 must not sort the relocation table.
ClosedPublic

Authored by sdardis on Nov 8 2016, 7:56 AM.

Details

Reviewers
vkalintiris
atanasyan
Commits
rG8fe36cd77cb6: [mips][ias] N32/N64 must not sort the relocation table.
rL288666: [mips][ias] N32/N64 must not sort the relocation table.
Summary

Doing so changes the evaluation order for relocation composition.

Patch By: Daniel Sanders

Diff Detail

Event Timeline

sdardis updated this revision to Diff 77195.Nov 8 2016, 7:56 AM
sdardis retitled this revision from to [mips][ias] N32/N64 must not sort the relocation table..
sdardis updated this object.
sdardis added reviewers: vkalintiris, atanasyan.
sdardis set the repository for this revision to rL LLVM.
sdardis added a subscriber: llvm-commits.
Herald edited edge metadata. · View Herald TranscriptNov 8 2016, 7:56 AM
atanasyan accepted this revision.Nov 8 2016, 11:07 AM
atanasyan edited edge metadata.

LGTM with a nit.

lib/Target/Mips/MCTargetDesc/MipsELFObjectWriter.cpp
422

I would add a comment here to describe why we do not need to sort relocation if hasRelocationAddend() return true.

This revision is now accepted and ready to land.Nov 8 2016, 11:07 AM
Closed by commit rL288666: [mips][ias] N32/N64 must not sort the relocation table. (authored by sdardis). · Explain WhyDec 5 2016, 5:05 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
lib/
Target/
Mips/
MCTargetDesc/
3 lines
test/
MC/
Mips/
12 lines

Diff 77195

lib/Target/Mips/MCTargetDesc/MipsELFObjectWriter.cpp

Show First 20 Lines • Show All 413 Lines • ▼ Show 20 Lines
/// R_MIPS_GOT16 and similar only have AHL addends if they refer to local /// R_MIPS_GOT16 and similar only have AHL addends if they refer to local
/// symbols. /// symbols.
/// ///
/// It should also be noted that this function is not affected by whether /// It should also be noted that this function is not affected by whether
/// the symbol was kept or rewritten into a section-relative equivalent. We /// the symbol was kept or rewritten into a section-relative equivalent. We
/// always match using the expressions from the source. /// always match using the expressions from the source.
void MipsELFObjectWriter::sortRelocs(const MCAssembler &Asm, void MipsELFObjectWriter::sortRelocs(const MCAssembler &Asm,
std::vector<ELFRelocationEntry> &Relocs) { std::vector<ELFRelocationEntry> &Relocs) {
if (hasRelocationAddend())
atanasyanUnsubmitted
Not Done
ReplyInline Actions

I would add a comment here to describe why we do not need to sort relocation if hasRelocationAddend() return true.

atanasyan: I would add a comment here to describe why we do not need to sort relocation if…
return;
if (Relocs.size() < 2) if (Relocs.size() < 2)
return; return;
// Sort relocations by the address they are applied to. // Sort relocations by the address they are applied to.
std::sort(Relocs.begin(), Relocs.end(), std::sort(Relocs.begin(), Relocs.end(),
[](const ELFRelocationEntry &A, const ELFRelocationEntry &B) { [](const ELFRelocationEntry &A, const ELFRelocationEntry &B) {
return A.Offset < B.Offset; return A.Offset < B.Offset;
}); });
▲ Show 20 Lines • Show All 226 Lines • Show Last 20 Lines

test/MC/Mips/reloc-directive.s

Show All 37 Lines
# incorrectly use the 3-reloc encoding (and ELF64). mips64-* triples # incorrectly use the 3-reloc encoding (and ELF64). mips64-* triples
# are closest to being correct so we use them for now. # are closest to being correct so we use them for now.
# This should be corrected once the triple bugfixes allow us to be ABI # This should be corrected once the triple bugfixes allow us to be ABI
# dependent rather than triple dependent. # dependent rather than triple dependent.
# OBJ-N32-LABEL: Name: .text # OBJ-N32-LABEL: Name: .text
# OBJ-N32: 0000: 00000000 00000000 00000000 # OBJ-N32: 0000: 00000000 00000000 00000000
# OBJ-N32-LABEL: } # OBJ-N32-LABEL: }
# OBJ-N32-LABEL: Relocations [ # OBJ-N32-LABEL: Relocations [
# OBJ-N32: 0x0 R_MIPS_NONE/R_MIPS_NONE/R_MIPS_NONE .text 0x4
# OBJ-N32: 0x4 R_MIPS_NONE/R_MIPS_NONE/R_MIPS_NONE .text 0x0 # OBJ-N32: 0x4 R_MIPS_NONE/R_MIPS_NONE/R_MIPS_NONE .text 0x0
# OBJ-N32: 0x0 R_MIPS_NONE/R_MIPS_NONE/R_MIPS_NONE .text 0x4
# OBJ-N32: 0x8 R_MIPS_32/R_MIPS_NONE/R_MIPS_NONE .text 0x8 # OBJ-N32: 0x8 R_MIPS_32/R_MIPS_NONE/R_MIPS_NONE .text 0x8
# OBJ-N32: 0xC R_MIPS_NONE/R_MIPS_NONE/R_MIPS_NONE - 0x0 # OBJ-N32: 0xC R_MIPS_NONE/R_MIPS_NONE/R_MIPS_NONE - 0x0
# FIXME:This is the correct output for reference.
# OBJ-N32-FIXME: 0x4 R_MIPS_NONE .text 0x0
# OBJ-N32-FIXME: 0x0 R_MIPS_NONE .text 0x4
# OBJ-N32-FIXME: 0x8 R_MIPS_32 .text 0x8
# OBJ-N32-FIXME: 0xC R_MIPS_NONE - 0x0
# OBJ-N64-LABEL: Name: .text # OBJ-N64-LABEL: Name: .text
# OBJ-N64: 0000: 00000000 00000000 00000000 # OBJ-N64: 0000: 00000000 00000000 00000000
# OBJ-N64-LABEL: } # OBJ-N64-LABEL: }
# OBJ-N64-LABEL: Relocations [ # OBJ-N64-LABEL: Relocations [
# OBJ-N64: 0x0 R_MIPS_NONE/R_MIPS_NONE/R_MIPS_NONE .text 0x4
# OBJ-N64: 0x4 R_MIPS_NONE/R_MIPS_NONE/R_MIPS_NONE .text 0x0 # OBJ-N64: 0x4 R_MIPS_NONE/R_MIPS_NONE/R_MIPS_NONE .text 0x0
# OBJ-N64: 0x0 R_MIPS_NONE/R_MIPS_NONE/R_MIPS_NONE .text 0x4
# OBJ-N64: 0x8 R_MIPS_32/R_MIPS_NONE/R_MIPS_NONE .text 0x8 # OBJ-N64: 0x8 R_MIPS_32/R_MIPS_NONE/R_MIPS_NONE .text 0x8
# OBJ-N64: 0xC R_MIPS_NONE/R_MIPS_NONE/R_MIPS_NONE - 0x0 # OBJ-N64: 0xC R_MIPS_NONE/R_MIPS_NONE/R_MIPS_NONE - 0x0