Page MenuHomePhabricator

CodeGen: Use PLT relocations for relative references to unnamed_addr functions.
ClosedPublic

Authored by pcc on Mar 7 2016, 12:57 PM.

Details

Summary

The relative vtable ABI (PR26723) needs PLT relocations to refer to virtual
functions defined in other DSOs. The unnamed_addr attribute means that the
function's address is not significant, so we're allowed to substitute it
with the address of a PLT entry.

Also includes a bonus feature: addends for COFF image-relative references.

Diff Detail

Repository
rL LLVM

Event Timeline

pcc updated this revision to Diff 49989.Mar 7 2016, 12:57 PM
pcc retitled this revision from to CodeGen: Use PLT relocations for relative references to unnamed_addr functions..
pcc updated this object.
pcc added a reviewer: rafael.
pcc added a subscriber: llvm-commits.
pcc updated this revision to Diff 49996.Mar 7 2016, 2:23 PM
  • Add test coverage for references to non-functions
majnemer added inline comments.
lib/Target/X86/X86TargetObjectFile.cpp
78–91 ↗(On Diff #49996)

Removing this logic seems problematic, no?

Won't this fire on x * __ImageBase ?

lib/Target/X86/X86TargetObjectFile.h
44–45 ↗(On Diff #49996)

Should this be:

X86ELFTargetObjectFile() : PLTRelativeVariantKind(MCSymbolRefExpr::VK_PLT) {}

?

pcc added inline comments.Mar 7 2016, 2:35 PM
lib/Target/X86/X86TargetObjectFile.cpp
78–91 ↗(On Diff #49996)

This part of the logic has been moved to AsmPrinter::lowerConstant, which will only call lowerRelativeReference if the expression is a substraction.

lib/Target/X86/X86TargetObjectFile.h
44–45 ↗(On Diff #49996)

No, because PLTRelativeVariantKind is a field of a base, so we can't initialize it that way.

pcc updated this revision to Diff 51242.Mar 21 2016, 4:12 PM
  • Replace getSymbolPlusOffset with an existing function that does the same thing
pcc added a reviewer: rnk.Apr 19 2016, 11:46 AM
pcc added a subscriber: rnk.

@rnk can you please take a look?

rnk accepted this revision.Apr 21 2016, 4:17 PM
rnk edited edge metadata.

lgtm

This revision is now accepted and ready to land.Apr 21 2016, 4:17 PM
This revision was automatically updated to reflect the committed changes.