Page MenuHomePhabricator
Differential D102712 Diff 347153 llvm/lib/Target/BPF/MCTargetDesc/BPFELFObjectWriter.cpp

Changeset View

Standalone View

View Options

llvm/lib/Target/BPF/MCTargetDesc/BPFELFObjectWriter.cpp

Show All 36 Lines
unsigned BPFELFObjectWriter::getRelocType(MCContext &Ctx, const MCValue &Target, unsigned BPFELFObjectWriter::getRelocType(MCContext &Ctx, const MCValue &Target,
const MCFixup &Fixup, const MCFixup &Fixup,
bool IsPCRel) const { bool IsPCRel) const {
// determine the type of the relocation // determine the type of the relocation
switch (Fixup.getKind()) { switch (Fixup.getKind()) {
default: default:
llvm_unreachable("invalid fixup kind!"); llvm_unreachable("invalid fixup kind!");
case FK_SecRel_8: case FK_SecRel_8:
// LD_imm64 instruction.
MaskRayUnsubmitted
Not Done
ReplyInline Actions

FK_SecRel_4 is used by PE-COFF to represent SECREL relocation types.

"The 32-bit offset of the target from the beginning of its section. This is used to support debugging information and static thread local storage."

Why does it map to a regular looking R_BPF_64_64?

MaskRay: FK_SecRel_4 is used by PE-COFF to represent SECREL relocation types. "The 32-bit offset of the…
yonghong-songAuthorUnsubmitted
Done
ReplyInline Actions

In AMDGPU/MCTargetDesc/R600MCCodeEmitter.cpp, I see

Fixups.push_back(MCFixup::create(offset, MO.getExpr(), FK_SecRel_4, MI.getLoc()));

It looks like backend can choose create a MCFixup with FK_SecRel_4. Do we have restrictions that we have to use a different one? Use FK_SecRel_4 seems a good fit here as only 4 bytes are permitted to write anyway.

yonghong-song: In AMDGPU/MCTargetDesc/R600MCCodeEmitter.cpp, I see ``` Fixups.push_back(MCFixup::create(offset…
yonghong-songAuthorUnsubmitted
Done
ReplyInline Actions

I can certainly go back with FK_SecRel_8, but want to understand why I cannot use FK_SecRel_4.

yonghong-song: I can certainly go back with FK_SecRel_8, but want to understand why I cannot use FK_SecRel_4.
MaskRayUnsubmitted
Not Done
ReplyInline Actions

My concern here is that FK_SecRel_* is no overloaded for entirely different semantics. Other targets use different variant kinds for such cases.

MaskRay: My concern here is that `FK_SecRel_*` is no overloaded for entirely different semantics. Other…
yonghong-songAuthorUnsubmitted
Done
ReplyInline Actions

Ok. Sounds reasonable since they are all related to similar tls use case. I will switch to FK_SecRel_8 in the next version. Thanks!

yonghong-song: Ok. Sounds reasonable since they are all related to similar tls use case. I will switch to…
return ELF::R_BPF_64_64; return ELF::R_BPF_64_64;
case FK_PCRel_4: case FK_PCRel_4:
case FK_SecRel_4: // CALL instruction.
return ELF::R_BPF_64_32; return ELF::R_BPF_64_32;
case FK_Data_8: case FK_Data_8:
return ELF::R_BPF_64_64; return ELF::R_BPF_64_ABS64;
case FK_Data_4: case FK_Data_4:
if (const MCSymbolRefExpr *A = Target.getSymA()) { if (const MCSymbolRefExpr *A = Target.getSymA()) {
const MCSymbol &Sym = A->getSymbol(); const MCSymbol &Sym = A->getSymbol();
if (Sym.isDefined()) { if (Sym.isDefined()) {
MCSection &Section = Sym.getSection(); MCSection &Section = Sym.getSection();
const MCSectionELF *SectionELF = dyn_cast<MCSectionELF>(&Section); const MCSectionELF *SectionELF = dyn_cast<MCSectionELF>(&Section);
assert(SectionELF && "Null section for reloc symbol"); assert(SectionELF && "Null section for reloc symbol");
unsigned Flags = SectionELF->getFlags(); unsigned Flags = SectionELF->getFlags();
if (Sym.isTemporary()) { if (Sym.isTemporary()) {
// .BTF.ext generates FK_Data_4 relocations for // .BTF.ext generates FK_Data_4 relocations for
// insn offset by creating temporary labels. // insn offset by creating temporary labels.
// The insn offset is within the code section and
// already been fulfilled by applyFixup(). No
// further relocation is needed.
// The reloc symbol should be in text section. // The reloc symbol should be in text section.
// Use a different relocation to instruct ExecutionEngine
// RuntimeDyld not to do relocation for it, yet still to
// allow lld to do proper adjustment when merging sections.
if ((Flags & ELF::SHF_ALLOC) && (Flags & ELF::SHF_EXECINSTR)) if ((Flags & ELF::SHF_ALLOC) && (Flags & ELF::SHF_EXECINSTR))
return ELF::R_BPF_NONE; return ELF::R_BPF_64_NODYLD32;
MaskRayUnsubmitted
Not Done
ReplyInline Actions

This doesn't make sense to me. The relocation type names shouldn't be aware of SHF_ALLOC/non-SHF_ALLOC.

MaskRay: This doesn't make sense to me. The relocation type names shouldn't be aware of SHF_ALLOC/non…
yonghong-songAuthorUnsubmitted
Done
ReplyInline Actions

I just wanted to choose a best-fit name. How about R_BPF_64_NODYLD_32 implying no runtime dynamic linking?

yonghong-song: I just wanted to choose a best-fit name. How about R_BPF_64_NODYLD_32 implying no runtime…
} else { } else {
// .BTF generates FK_Data_4 relocations for variable // .BTF generates FK_Data_4 relocations for variable
// offset in DataSec kind. Similar to the above .BTF.ext // offset in DataSec kind.
// insn offset, no further relocation is needed.
// The reloc symbol should be in data section. // The reloc symbol should be in data section.
if ((Flags & ELF::SHF_ALLOC) && (Flags & ELF::SHF_WRITE)) if ((Flags & ELF::SHF_ALLOC) && (Flags & ELF::SHF_WRITE))
return ELF::R_BPF_NONE; return ELF::R_BPF_64_NODYLD32;
} }
} }
} }
return ELF::R_BPF_64_32; return ELF::R_BPF_64_ABS32;
} }
} }
std::unique_ptr<MCObjectTargetWriter> std::unique_ptr<MCObjectTargetWriter>
llvm::createBPFELFObjectWriter(uint8_t OSABI) { llvm::createBPFELFObjectWriter(uint8_t OSABI) {
return std::make_unique<BPFELFObjectWriter>(OSABI); return std::make_unique<BPFELFObjectWriter>(OSABI);
} }