This is an archive of the discontinued LLVM Phabricator instance.

[ELF] Write IPLT header in -static -z retpolineplt mode
ClosedPublic

Authored by MaskRay on Nov 20 2018, 11:19 PM.

Details

Summary

This fixes PR39711: -static -z retpolineplt does not produce retpoline header as IPLT header.
-z now is not relevant.

Statically linked executable does not have PLT, but may have IPLT with no header. When -z retpolineplt is specified, however, the repoline header should still be emitted.

I've checked that this fixes (the program prints "Hi" rather than SIGSEGV) the FreeBSD reproduce in PR39711 and a Linux program linked against glibc.

getPltEntryOffset may look dirty after this patch, but it can be cleaned up later.

Diff Detail

Repository
rL LLVM

Event Timeline

MaskRay created this revision.Nov 20 2018, 11:19 PM
MaskRay updated this revision to Diff 174876.Nov 20 2018, 11:50 PM
MaskRay retitled this revision from [ELF] Take IPLT header size in account when -z retpolineplt is specified to [ELF] Take IPLT header size into account in -static -z retpolineplt mode.
MaskRay edited the summary of this revision. (Show Details)

Add test

MaskRay updated this revision to Diff 174877.Nov 20 2018, 11:51 PM
MaskRay added reviewers: ruiu, chandlerc, emaste.
MaskRay removed subscribers: jfb, emaste.

.

emaste accepted this revision.Nov 21 2018, 6:23 AM

Confirmed this fixes my bug.

ELF/SyntheticSections.cpp
2283 ↗(On Diff #174878)

At first this seemed slightly confusing to me vs. the test at line 2294; maybe the test could be the same?
!IsIplt || Config->ZRetpolineplt ? Target->PltHeaderSize : 0

2292 ↗(On Diff #174878)

Perhaps update the comment too? e.g. beginning of PLT but not non-retpoline IPLT, although we're basically just repeating the code. Not sure of a succinct way to express this.

This revision is now accepted and ready to land.Nov 21 2018, 6:23 AM
MaskRay updated this revision to Diff 174939.Nov 21 2018, 8:59 AM
MaskRay retitled this revision from [ELF] Take IPLT header size into account in -static -z retpolineplt mode to [ELF] Write IPLT header in -static -z retpolineplt mode.
MaskRay removed reviewers: ruiu, chandlerc, emaste.
MaskRay added a subscriber: emaste.

Address comment and use

if (HeaderSize > 0)
  Target->writePltHeader(Buf);
This revision now requires review to proceed.Nov 21 2018, 8:59 AM
MaskRay updated this revision to Diff 174940.Nov 21 2018, 9:01 AM
MaskRay marked an inline comment as done.

Comment

MaskRay marked an inline comment as done.Nov 21 2018, 9:01 AM
MaskRay updated this revision to Diff 174941.Nov 21 2018, 9:08 AM
MaskRay added reviewers: emaste, chandlerc, ruiu.

Sorry I didn't know how to use arc diff --verbatim ...

emaste accepted this revision.Nov 21 2018, 9:40 AM
This revision is now accepted and ready to land.Nov 21 2018, 9:40 AM
This revision was automatically updated to reflect the committed changes.
ruiu added inline comments.Nov 26 2018, 12:58 PM
lld/trunk/ELF/Symbols.cpp
148–149

This is nontrivial code, which needs a comment.