Page MenuHomePhabricator

[ELF] Implement RISCV::getImplicitAddend()
AcceptedPublic

Authored by arichardson on Apr 28 2021, 6:59 AM.

Details

Reviewers
MaskRay
jrtc27
Summary

This allows checking dynamic relocation addends for -z rel and
--apply-dynamic-relocs output.

Depends on D101451 (not functionally, just to apply cleanly)

Diff Detail

Event Timeline

arichardson created this revision.Apr 28 2021, 6:59 AM
arichardson requested review of this revision.Apr 28 2021, 6:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 28 2021, 6:59 AM

Honestly this seems rather academic, surely we should just mandate Elf_Rela for RISC-V? I've filed https://github.com/riscv/riscv-elf-psabi-doc/issues/186.

Honestly this seems rather academic, surely we should just mandate Elf_Rela for RISC-V? I've filed https://github.com/riscv/riscv-elf-psabi-doc/issues/186.

I don't think this should be a supported configuration, but it appears that -z rel is being used with AArch64, so it might make sense to allow it for RISC-V too.

Honestly this seems rather academic, surely we should just mandate Elf_Rela for RISC-V? I've filed https://github.com/riscv/riscv-elf-psabi-doc/issues/186.

I don't think this should be a supported configuration, but it appears that -z rel is being used with AArch64, so it might make sense to allow it for RISC-V too.

AAELF64 explicitly permits both, whereas RISC-V is silent on the matter and AFAIK nobody is using Elf_Rel because why would you inflict that pain upon yourself.

MaskRay accepted this revision.Apr 30 2021, 2:36 PM

Looks reasonable to me. The base patches need a bit more work, though.

I think the overall complexity (generic code plus target-specific code) supporting REL isn't that high.
This supports both (1) -z rel and (2) check our dynamic relocation addends are correct.

lld/ELF/Arch/RISCV.cpp
155

The value at this location is not used.

ld.so implementations don't use addend for JUMP_SLOT/GLOB_DAT. NONE is just ignored.

This revision is now accepted and ready to land.Apr 30 2021, 2:36 PM