Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support -mstack-protector-guard=sysreg for riscv #46685

Closed
kees opened this issue Aug 28, 2020 · 10 comments · Fixed by #108942
Closed

Support -mstack-protector-guard=sysreg for riscv #46685

kees opened this issue Aug 28, 2020 · 10 comments · Fixed by #108942
Labels
backend:RISC-V bugzilla Issues migrated from bugzilla clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl'

Comments

@kees
Copy link
Contributor

kees commented Aug 28, 2020

Bugzilla Link 47341
Version trunk
OS Linux
Blocks #4440
CC @efriedma-quic,@nickdesaulniers,@zygoloid,@stephenhines

Extended Description

The Linux kernel uses this for having a per-thread stack canary. Implementing this for both arm64 and riscv is desired.

For example, gcc's arm64 support is used like this in the kernel:

-mstack-protector-guard=sysreg
-mstack-protector-guard-reg=sp_el0
-mstack-protector-guard-offset=0

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0a1213fa7432778b71a1c0166bf56660a3aab030

Here are discussions on the start of riscv support:

https://lore.kernel.org/lkml/CAJF2gTTr_ENhGRmjqNbGBhEc8y4D3YSqAResvwHuB1ykemKTUw@mail.gmail.com/

@nickdesaulniers
Copy link
Member

SelectionDAGBuilder::visitSPDescriptorParent is probably the most relevant method for this. I'm not sure yet how best to modify getLoadStackGuard in order to change this from a load of a global Value to an aarch64 specific mrs instruction.

Here's a basic test case demonstrating the differences between these 3 new flags, vs without them (but still with -fstack-protector-strong).

https://godbolt.org/z/oEc1fvMPo

@nickdesaulniers
Copy link
Member

Ah looks like I can just add custom expansion for AArch64::LOAD_STACK_GUARD in AArch64InstrInfo::expandPostRAPseudo() and that seems to work.

@nickdesaulniers
Copy link
Member

aarch64: https://reviews.llvm.org/D100919

@nickdesaulniers
Copy link
Member

https://reviews.llvm.org/rG0f417789192e74f9d2fad0f6aee4efc394257176 has landed for aarch64 with the caveats that we only support sp_el0 for valid sysreg, and we don't try too hard to support arbitrary int32_t's for offsets.

@nickdesaulniers
Copy link
Member

nickdesaulniers commented May 17, 2021

For the Linux kernel, it looks like risc-v uses

-mstack-protector-guard=tls
-mstack-protector-guard-reg=tp
-mstack-protector-guard-offset=

It looks like ppc uses:

-mstack-protector-guard=tls
-mstack-protector-guard-reg={r13|r2}
-mstack-protector-guard-offset=

x86 uses:

-mstack-protector-guard-reg=fs
-mstack-protector-guard-symbol=__stack_chk_guard
-mstack-protector-guard=global

See #48553 for -mstack-protector-guard-symbol=.

@nickdesaulniers
Copy link
Member

mentioned in issue llvm/llvm-bugzilla-archive#49209

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
@EugeneZelenko EugeneZelenko added clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' and removed clang Clang issues not falling into any other category labels Jun 18, 2022
@llvmbot
Copy link
Member

llvmbot commented Jun 18, 2022

@llvm/issue-subscribers-clang-driver

@nickdesaulniers
Copy link
Member

cc @asb

@xgupta
Copy link
Contributor

xgupta commented Jan 27, 2023

I thought to look at RISCV but it seems they don't have basic stack guard protection. so that needs to be implemented first (like powerpc or sparc) and then customization of stack protector guard.

@llvmbot
Copy link
Member

llvmbot commented Jan 28, 2023

@llvm/issue-subscribers-backend-risc-v

@kees kees changed the title Support -mstack-protector-guard=sysreg for arm64 and riscv Support -mstack-protector-guard=sysreg for riscv Nov 6, 2023
@topperc topperc closed this as completed in ca57e8f Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:RISC-V bugzilla Issues migrated from bugzilla clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl'
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants