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.

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

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

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 ↗(On Diff #174949)

This is nontrivial code, which needs a comment.