This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Add BTI to CFI jumptables.
ClosedPublic

Authored by danielkiss on Jun 5 2020, 4:28 AM.

Details

Summary

With branch protection the jump to the jump table entries requires a landing pad.

Diff Detail

Event Timeline

danielkiss created this revision.Jun 5 2020, 4:28 AM

This patch depends on D80791 and D75181.

eugenis added inline comments.Jun 5 2020, 11:26 AM
llvm/lib/Transforms/IPO/LowerTypeTests.cpp
1220

I'd rather this was a named constant.

1244

Do we need to check the function attribute here as well? What happens if a function opts out of BTI?

danielkiss marked 2 inline comments as done.Jun 8 2020, 2:10 AM
danielkiss added inline comments.
llvm/lib/Transforms/IPO/LowerTypeTests.cpp
1220

I'll update it.

1244

No, direct jump does not need a landing pad. So this jump will be fine even that target function opted out.

danielkiss updated this revision to Diff 269251.Jun 8 2020, 8:36 AM
danielkiss marked 2 inline comments as done.
eugenis accepted this revision.Jun 8 2020, 5:05 PM

LGTM

llvm/lib/Transforms/IPO/LowerTypeTests.cpp
1244

What I meant is, if a function has ignore-branch-target-enforcement, do not we want to skip the BTI hint in that function's CFI stub?
Or, rather, replace it with NOP to preserve the stub size.

Thinking about it some more - no, probably not. The attribute can be used on a "naked" function that already has BTI in the assembly; in that case we'd need BTI in the jump table as well.

This revision is now accepted and ready to land.Jun 8 2020, 5:05 PM
danielkiss planned changes to this revision.Aug 29 2020, 6:37 AM

I need to update it due to changes in the dependent patches.

chill added inline comments.Sep 1 2020, 4:38 AM
llvm/lib/Transforms/IPO/LowerTypeTests.cpp
1410

Why are we disabling BTI here?

1411

Here we should use sign-return-address=none, and, if needed, I suggest (re-)introducing branch-target-enforcement=true|false ?

danielkiss added inline comments.Sep 1 2020, 4:59 AM
llvm/lib/Transforms/IPO/LowerTypeTests.cpp
1410

This function is a jump table and each entry will have a landing pad ( see line 1235).
If it is not disabled then the entry will get an additional landing pad that miss-aligns the table.

Worst case we could add special condition for the first entry but IMHO that would be ugly.

1411

Agree , I will update once the D85649 is updated as proposed there.

danielkiss marked an inline comment as not done.Sep 16 2020, 1:52 PM
danielkiss added inline comments.
llvm/lib/Transforms/IPO/LowerTypeTests.cpp
1245

since D81257 we can use bti c in assembly.

rebasing to the new patch series.

This revision is now accepted and ready to land.Sep 16 2020, 1:56 PM

update module flag handling due to the change is merged in: D85649.

LGTM after the minor change

tamas.petz accepted this revision.Sep 29 2020, 4:32 AM
This revision was automatically updated to reflect the committed changes.