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

[Clang][RISCV] Support -fcf-protection=return for RISC-V #112477

Merged
merged 5 commits into from
Oct 29, 2024

Conversation

jaidTw
Copy link
Contributor

@jaidTw jaidTw commented Oct 16, 2024

This patches enables the support of -fcf-protection=return on RISC-V, and add a string attribute "hw-shadow-stack" to every function if the option is set on RISC-V

@jaidTw jaidTw requested review from kito-cheng and topperc October 16, 2024 04:50
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:RISC-V clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen labels Oct 16, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 16, 2024

@llvm/pr-subscribers-backend-risc-v
@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Jesse Huang (jaidTw)

Changes

This patches add a string attribute "hw-shadow-stack" to every function if -fcf-protection=return is set on RISC-V


Full diff: https://github.com/llvm/llvm-project/pull/112477.diff

2 Files Affected:

  • (modified) clang/lib/Basic/Targets/RISCV.h (+6)
  • (modified) clang/lib/CodeGen/CodeGenFunction.cpp (+4)
diff --git a/clang/lib/Basic/Targets/RISCV.h b/clang/lib/Basic/Targets/RISCV.h
index bf40edb8683b3e..3165623593fdf7 100644
--- a/clang/lib/Basic/Targets/RISCV.h
+++ b/clang/lib/Basic/Targets/RISCV.h
@@ -141,6 +141,12 @@ class RISCVTargetInfo : public TargetInfo {
     return true;
   }
 
+  bool checkCFProtectionReturnSupported(DiagnosticsEngine &Diags) const override {
+    if (ISAInfo->hasExtension("zimop"))
+      return true;
+    return TargetInfo::checkCFProtectionReturnSupported(Diags);
+  }
+
   CFBranchLabelSchemeKind getDefaultCFBranchLabelScheme() const override {
     return CFBranchLabelSchemeKind::FuncSig;
   }
diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp
index 2306043c90f406..d8f0f7c14f6b40 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -899,6 +899,10 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, QualType RetTy,
   if (CodeGenOpts.PointerAuth.IndirectGotos)
     Fn->addFnAttr("ptrauth-indirect-gotos");
 
+  // Add return control flow integrity attributes.
+  if (CodeGenOpts.CFProtectionReturn)
+    Fn->addFnAttr("hw-shadow-stack");
+
   // Apply xray attributes to the function (as a string, for now)
   bool AlwaysXRayAttr = false;
   if (const auto *XRayAttr = D ? D->getAttr<XRayInstrumentAttr>() : nullptr) {

Copy link

github-actions bot commented Oct 16, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@jaidTw jaidTw force-pushed the riscv-cf-protection-return branch from 2a7a6ef to fe4a28f Compare October 16, 2024 05:37
@jaidTw jaidTw force-pushed the riscv-cf-protection-return branch from 449c0d9 to 7dc168a Compare October 23, 2024 06:45
@mylai-mtk
Copy link
Contributor

LGTM, but please wait for others. I'm too junior to this community that I'm not sure if my words are enough 😂

@jaidTw jaidTw force-pushed the riscv-cf-protection-return branch 3 times, most recently from bb59a70 to 3cdb6c5 Compare October 24, 2024 08:22
Copy link
Member

@kito-cheng kito-cheng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test?

@jaidTw jaidTw force-pushed the riscv-cf-protection-return branch from 3cdb6c5 to 42132c2 Compare October 25, 2024 05:16
@jaidTw
Copy link
Contributor Author

jaidTw commented Oct 25, 2024

Addressed comments

Copy link
Member

@kito-cheng kito-cheng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jaidTw
Copy link
Contributor Author

jaidTw commented Oct 28, 2024

After a meeting with Kito, we decided to use Zicfiss as the requirement in this patch.
I will change the requirement of both patches (this and 112478) to Zicfis.s
And after taking a look at other tests, I think -triple=riscv64/32 is sufficient for the tuples so I also changed it

@jaidTw jaidTw force-pushed the riscv-cf-protection-return branch from 421a369 to f272724 Compare October 28, 2024 08:13
@jaidTw
Copy link
Contributor Author

jaidTw commented Oct 29, 2024

@mylai-mtk could you take a look at the latest version?
I'm gonna merge it if it looks good to you too

@mylai-mtk
Copy link
Contributor

LGTM. Thanks.

@jaidTw jaidTw merged commit 335e68d into llvm:main Oct 29, 2024
8 checks passed
@jaidTw jaidTw deleted the riscv-cf-protection-return branch October 29, 2024 07:47
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
Enables the support of `-fcf-protection=return` on RISC-V, which
requires Zicfiss. It also adds a string attribute "hw-shadow-stack"
to every function if the option is set on RISC-V
jaidTw added a commit that referenced this pull request Nov 7, 2024
This patch follows #112477.
Previously `-fsanitize=shadow-call-stack` (which get transform to
`Attribute::ShadowCallStack`) is used for enable both hardware and
software shadow stack, and another option `-force-sw-shadow-stack` is
needed if the user wants to use the software shadow stack where hardware
software shadow stack could be supported. It decouples both by using the
string attribute `hw-shadow-stack` to distinguish from the software
shadow stack attribute.
jaidTw added a commit that referenced this pull request Nov 8, 2024
This option was used to override the behavior of
`-fsanitize=shadowcallstack` on RISC-V backend, which by default use a
hardware implementation if possible, to use the software implementation
instead. After #112477 and #112478, now two implementation
is represented by independent options and we no longer need it.
Groverkss pushed a commit to iree-org/llvm-project that referenced this pull request Nov 15, 2024
This patch follows llvm#112477.
Previously `-fsanitize=shadow-call-stack` (which get transform to
`Attribute::ShadowCallStack`) is used for enable both hardware and
software shadow stack, and another option `-force-sw-shadow-stack` is
needed if the user wants to use the software shadow stack where hardware
software shadow stack could be supported. It decouples both by using the
string attribute `hw-shadow-stack` to distinguish from the software
shadow stack attribute.
Groverkss pushed a commit to iree-org/llvm-project that referenced this pull request Nov 15, 2024
This option was used to override the behavior of
`-fsanitize=shadowcallstack` on RISC-V backend, which by default use a
hardware implementation if possible, to use the software implementation
instead. After llvm#112477 and llvm#112478, now two implementation
is represented by independent options and we no longer need it.
mylai-mtk added a commit that referenced this pull request Feb 19, 2025
The `-fcf-protection` flag is now also used to enable CFI features for
the RISC-V target, so it's not suitable to define `__CET__` solely based
on the flag anymore. This patch moves the definition of the `__CET__`
macro into X86 target hook, so only X86 targets with the
`-fcf-protection` flag would enable the `__CET__` macro.

See #109784 and
#112477 for the adoption
of `-fcf-protection` flag for RISC-V targets.
github-actions bot pushed a commit to arm/arm-toolchain that referenced this pull request Feb 19, 2025
…27616)

The `-fcf-protection` flag is now also used to enable CFI features for
the RISC-V target, so it's not suitable to define `__CET__` solely based
on the flag anymore. This patch moves the definition of the `__CET__`
macro into X86 target hook, so only X86 targets with the
`-fcf-protection` flag would enable the `__CET__` macro.

See llvm/llvm-project#109784 and
llvm/llvm-project#112477 for the adoption
of `-fcf-protection` flag for RISC-V targets.
Prakhar-Dixit pushed a commit to Prakhar-Dixit/llvm-project that referenced this pull request Feb 19, 2025
The `-fcf-protection` flag is now also used to enable CFI features for
the RISC-V target, so it's not suitable to define `__CET__` solely based
on the flag anymore. This patch moves the definition of the `__CET__`
macro into X86 target hook, so only X86 targets with the
`-fcf-protection` flag would enable the `__CET__` macro.

See llvm#109784 and
llvm#112477 for the adoption
of `-fcf-protection` flag for RISC-V targets.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:RISC-V clang:codegen clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants