This is an archive of the discontinued LLVM Phabricator instance.

[lld][AArch64] Add BTI landing pad to PLT when it is accessed by a range extension thunk.
ClosedPublic

Authored by danielkiss on Apr 19 2023, 3:43 AM.

Details

Summary

Adding BTI to those PLT's which accessed with by a range extension thunk due to those preform an indirect call.
Fixes: #62140

Diff Detail

Event Timeline

danielkiss created this revision.Apr 19 2023, 3:43 AM
danielkiss requested review of this revision.Apr 19 2023, 3:43 AM

From the Arm side this looks good to me. If it turns out that we can't use the flag an alternative could be adding a Target::registerThunk(Destination) function that could be defined by the AArch64 to record the symbol destinations, these could then be looked up by the PLT writing code.

lld/test/ELF/aarch64-btiplt.s
2

I think the convention now is to use the split-file command so that the linker script and assembler file can be generated in one file.

For an example the test added in arm-exidx-nonzero-offset.s https://reviews.llvm.org/D148033

8

Suggest:

The PLT must start with a bti c instruction due to the indirect call from the range extension thunk. Previously the call to the PLT was direct so no bti was required.
danielkiss marked 2 inline comments as done.
danielkiss added inline comments.
lld/test/ELF/aarch64-btiplt.s
2

Like the convention, thanks!

MaskRay added inline comments.Apr 19 2023, 3:24 PM
lld/ELF/Symbols.h
296

This uses the last bit of the byte. Adding another bit will increase the size of Symbol.

lld/test/ELF/aarch64-btiplt.s
2

The main feature test is aarch64-feature-bti. I'd use aarch64-feature-bti-plt.s to share a prefix, to make bti tests more discoverable.

21

Don't mix leading comment markers. Use # throughout, i.e. #---.

danielkiss marked 2 inline comments as done.
danielkiss added inline comments.Apr 20 2023, 3:08 AM
lld/ELF/Symbols.h
296

There is a bit more space after uint8_t hasVersionSuffix : 1; if we need to squeeze something in.

MaskRay added inline comments.Apr 21 2023, 9:34 AM
lld/test/ELF/aarch64-feature-bti-plt.s
8 ↗(On Diff #515265)

Delete this ^#$

24 ↗(On Diff #515265)

Best to add another function that doesn't need a bti c to show differences in the same test file.

danielkiss marked 2 inline comments as done.
MaskRay accepted this revision.Apr 21 2023, 2:02 PM
MaskRay added inline comments.
lld/test/ELF/aarch64-feature-bti-plt.s
9 ↗(On Diff #515875)

Suggest moving the comment immediately before <foo@plt>:

foo@plt is targeted by a range extension thunk with an indirect branch.
Add a bti c instruction.

28 ↗(On Diff #515875)
This revision is now accepted and ready to land.Apr 21 2023, 2:02 PM
danielkiss marked 2 inline comments as done.
This revision was landed with ongoing or failed builds.Apr 23 2023, 2:17 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 23 2023, 2:17 PM