Page MenuHomePhabricator

Faster stack-protector for Android/AArch64.
ClosedPublic

Authored by eugenis on Mar 30 2016, 4:32 PM.

Details

Summary

Bionic has a defined thread-local location for the stack protector
cookie. Emit a direct load instead of going through __stack_chk_guard.

The current signature of getStackCookieLocation (returning addressspace + offset) is very x86 specific; let the function generate arbitrary IR for the cookie access instead. This is the same way getSafeStackPointerLocation works.

Diff Detail

Repository
rL LLVM

Event Timeline

eugenis updated this revision to Diff 52149.Mar 30 2016, 4:32 PM
eugenis retitled this revision from to Faster stack-protector for Android/AArch64..
eugenis updated this object.
eugenis added reviewers: echristo, danalbert.
eugenis set the repository for this revision to rL LLVM.
eugenis added subscribers: llvm-commits, pcc.

Adding Tim since he's in here as well.

LGTM, glad that AArch64 has Intrinsic::aarch64_thread_pointer to make your job easier. :)

I'm not the owner of SSP, but I'm recently working on it. Let's wait for others comments for a while, then I'm happy to stamp the patch.

For a bit background: I'm currently working on a cleanup on SSP (D17736), which also involves a similar change to yours. No worries, I will rebase to include your change.

Cool. Would it be better to avoid the use of IRBuilder, and switch the targetlowering hooks to the same signature as in D17736?
Value *..(Module &);

I slightly prefer taking a Module&, since by taking a Module&, getStackCookieLocation doesn't have a chance to insert a concrete instruction to a potentially specified place. That's taking Module& makes the interface narrower. But I don't have strong opinion on this.

Good point. AArch64 implementation actually needs to insert instructions for the intrinsic call. I guess we keep IRBuilder then, or replace it with Instruction *InsertBefore argument.

eugenis updated this revision to Diff 52737.Apr 5 2016, 2:25 PM
eugenis edited edge metadata.

Friendly ping.

timshen accepted this revision.Apr 5 2016, 3:04 PM
timshen edited edge metadata.
This revision is now accepted and ready to land.Apr 5 2016, 3:04 PM
eugenis closed this revision.Apr 5 2016, 3:47 PM

Thanks for the review.
r265481