This is an archive of the discontinued LLVM Phabricator instance.

[ELF][X86] Don't create IBT .plt if there is no PLT entry
ClosedPublic

Authored by joaomoreira on Feb 25 2022, 4:23 PM.

Details

Summary

https://github.com/ClangBuiltLinux/linux/issues/1606
When GNU_PROPERTY_X86_FEATURE_1_IBT is enabled, ld.lld will create .plt output
section even if there is no PLT entry. Fix this by implementing
IBTPltSection::isNeeded instead of using the default code path (which always
returns true).

Diff Detail

Event Timeline

joaomoreira created this revision.Feb 25 2022, 4:23 PM
joaomoreira requested review of this revision.Feb 25 2022, 4:23 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 25 2022, 4:23 PM

This is correct. I can add a test for you. I'll use a much conciser description.

Hm. I got tests almost ready (fixed the broken one and am working on finishing a new one). I'll update the diff in a few minutes. I can also update the description, let me know if you have suggestions.

MaskRay updated this revision to Diff 411566.Feb 25 2022, 7:41 PM
MaskRay retitled this revision from [X86][LLD] Fix PLT emission by LLD when -fcf-protection=branch is used to [ELF][X86] Don't create IBT .plt if there is no PLT entry.
MaskRay edited the summary of this revision. (Show Details)

Fix description and add tests

Hm. I got tests almost ready (fixed the broken one and am working on finishing a new one). I'll update the diff in a few minutes. I can also update the description, let me know if you have suggestions.

OK:) I usually have a preference on how the test should be written :)

No problem! And thanks! The test looks better than the one I had half-way implemented. I'm also updating the test which got broken by this fix... let me know if you see a problem with it!

MaskRay accepted this revision.Feb 25 2022, 7:55 PM

LGTM.

This revision is now accepted and ready to land.Feb 25 2022, 7:55 PM
This revision was landed with ongoing or failed builds.Feb 25 2022, 7:55 PM
This revision was automatically updated to reflect the committed changes.

LGTM +1 , thanks a lot!