Skip to content

Conversation

@jaidTw
Copy link
Contributor

@jaidTw jaidTw commented Nov 7, 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.

After the separation of hardware and software shadow stack options, we
no longer need this option.
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:RISC-V clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Nov 7, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 7, 2024

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

@llvm/pr-subscribers-clang

Author: Jesse Huang (jaidTw)

Changes

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.


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

3 Files Affected:

  • (modified) clang/docs/ShadowCallStack.rst (+4-5)
  • (modified) clang/include/clang/Driver/Options.td (-4)
  • (modified) clang/test/Driver/riscv-features.c (-6)
diff --git a/clang/docs/ShadowCallStack.rst b/clang/docs/ShadowCallStack.rst
index d7ece11b352606..fc8bea83e11452 100644
--- a/clang/docs/ShadowCallStack.rst
+++ b/clang/docs/ShadowCallStack.rst
@@ -59,11 +59,10 @@ and destruction would need to be intercepted by the application.
 
 The instrumentation makes use of the platform register ``x18`` on AArch64,
 ``x3`` (``gp``) on RISC-V with software shadow stack and ``ssp`` on RISC-V with
-hardware shadow stack, which needs `Zicfiss`_ and ``-mno-forced-sw-shadow-stack``
-(default option). Note that with ``Zicfiss``_ the RISC-V backend will default to
-the hardware based shadow call stack. Users can force the RISC-V backend to
-generate the software shadow call stack with ``Zicfiss``_ by passing
-``-mforced-sw-shadow-stack``.
+hardware shadow stack, which needs `Zicfiss`_ and ``-fcf-protection=return``.
+Users can choose between the software and hardware based shadow stack
+implementation on RISC-V backend by passing ``-fsanitize=shadowcallstack``
+or ``Zicfiss`` with ``-fcf-protection=return``.
 For simplicity we will refer to this as the ``SCSReg``. On some platforms,
 ``SCSReg`` is reserved, and on others, it is designated as a scratch register.
 This generally means that any code that may run on the same thread as code
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 805b79491e6ea4..8887e0c1495d2a 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4930,10 +4930,6 @@ def msave_restore : Flag<["-"], "msave-restore">, Group<m_riscv_Features_Group>,
   HelpText<"Enable using library calls for save and restore">;
 def mno_save_restore : Flag<["-"], "mno-save-restore">, Group<m_riscv_Features_Group>,
   HelpText<"Disable using library calls for save and restore">;
-def mforced_sw_shadow_stack : Flag<["-"], "mforced-sw-shadow-stack">, Group<m_riscv_Features_Group>,
-  HelpText<"Force using software shadow stack when shadow-stack enabled">;
-def mno_forced_sw_shadow_stack : Flag<["-"], "mno-forced-sw-shadow-stack">, Group<m_riscv_Features_Group>,
-  HelpText<"Not force using software shadow stack when shadow-stack enabled">;
 } // let Flags = [TargetSpecific]
 let Flags = [TargetSpecific] in {
 def menable_experimental_extensions : Flag<["-"], "menable-experimental-extensions">, Group<m_Group>,
diff --git a/clang/test/Driver/riscv-features.c b/clang/test/Driver/riscv-features.c
index b4fad5177c5f76..b58ec3b523e5c1 100644
--- a/clang/test/Driver/riscv-features.c
+++ b/clang/test/Driver/riscv-features.c
@@ -29,12 +29,6 @@
 // DEFAULT-NOT: "-target-feature" "-save-restore"
 // DEFAULT-NOT: "-target-feature" "+save-restore"
 
-// RUN: %clang --target=riscv32-unknown-elf -### %s -mforced-sw-shadow-stack 2>&1 | FileCheck %s -check-prefix=FORCE-SW-SCS
-// RUN: %clang --target=riscv32-unknown-elf -### %s -mno-forced-sw-shadow-stack 2>&1 | FileCheck %s -check-prefix=NO-FORCE-SW-SCS
-// FORCE-SW-SCS: "-target-feature" "+forced-sw-shadow-stack"
-// NO-FORCE-SW-SCS: "-target-feature" "-forced-sw-shadow-stack"
-// DEFAULT-NOT: "-target-feature" "+forced-sw-shadow-stack"
-
 // RUN: %clang --target=riscv32-unknown-elf -### %s -mno-strict-align 2>&1 | FileCheck %s -check-prefixes=FAST-SCALAR-UNALIGNED-ACCESS,FAST-VECTOR-UNALIGNED-ACCESS
 // RUN: %clang --target=riscv32-unknown-elf -### %s -mstrict-align 2>&1 | FileCheck %s -check-prefixes=NO-FAST-SCALAR-UNALIGNED-ACCESS,NO-FAST-VECTOR-UNALIGNED-ACCESS
 // RUN: %clang --target=riscv32-unknown-elf -### %s -mno-scalar-strict-align 2>&1 | FileCheck %s -check-prefix=FAST-SCALAR-UNALIGNED-ACCESS

@llvmbot
Copy link
Member

llvmbot commented Nov 7, 2024

@llvm/pr-subscribers-clang-driver

Author: Jesse Huang (jaidTw)

Changes

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.


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

3 Files Affected:

  • (modified) clang/docs/ShadowCallStack.rst (+4-5)
  • (modified) clang/include/clang/Driver/Options.td (-4)
  • (modified) clang/test/Driver/riscv-features.c (-6)
diff --git a/clang/docs/ShadowCallStack.rst b/clang/docs/ShadowCallStack.rst
index d7ece11b352606..fc8bea83e11452 100644
--- a/clang/docs/ShadowCallStack.rst
+++ b/clang/docs/ShadowCallStack.rst
@@ -59,11 +59,10 @@ and destruction would need to be intercepted by the application.
 
 The instrumentation makes use of the platform register ``x18`` on AArch64,
 ``x3`` (``gp``) on RISC-V with software shadow stack and ``ssp`` on RISC-V with
-hardware shadow stack, which needs `Zicfiss`_ and ``-mno-forced-sw-shadow-stack``
-(default option). Note that with ``Zicfiss``_ the RISC-V backend will default to
-the hardware based shadow call stack. Users can force the RISC-V backend to
-generate the software shadow call stack with ``Zicfiss``_ by passing
-``-mforced-sw-shadow-stack``.
+hardware shadow stack, which needs `Zicfiss`_ and ``-fcf-protection=return``.
+Users can choose between the software and hardware based shadow stack
+implementation on RISC-V backend by passing ``-fsanitize=shadowcallstack``
+or ``Zicfiss`` with ``-fcf-protection=return``.
 For simplicity we will refer to this as the ``SCSReg``. On some platforms,
 ``SCSReg`` is reserved, and on others, it is designated as a scratch register.
 This generally means that any code that may run on the same thread as code
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 805b79491e6ea4..8887e0c1495d2a 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4930,10 +4930,6 @@ def msave_restore : Flag<["-"], "msave-restore">, Group<m_riscv_Features_Group>,
   HelpText<"Enable using library calls for save and restore">;
 def mno_save_restore : Flag<["-"], "mno-save-restore">, Group<m_riscv_Features_Group>,
   HelpText<"Disable using library calls for save and restore">;
-def mforced_sw_shadow_stack : Flag<["-"], "mforced-sw-shadow-stack">, Group<m_riscv_Features_Group>,
-  HelpText<"Force using software shadow stack when shadow-stack enabled">;
-def mno_forced_sw_shadow_stack : Flag<["-"], "mno-forced-sw-shadow-stack">, Group<m_riscv_Features_Group>,
-  HelpText<"Not force using software shadow stack when shadow-stack enabled">;
 } // let Flags = [TargetSpecific]
 let Flags = [TargetSpecific] in {
 def menable_experimental_extensions : Flag<["-"], "menable-experimental-extensions">, Group<m_Group>,
diff --git a/clang/test/Driver/riscv-features.c b/clang/test/Driver/riscv-features.c
index b4fad5177c5f76..b58ec3b523e5c1 100644
--- a/clang/test/Driver/riscv-features.c
+++ b/clang/test/Driver/riscv-features.c
@@ -29,12 +29,6 @@
 // DEFAULT-NOT: "-target-feature" "-save-restore"
 // DEFAULT-NOT: "-target-feature" "+save-restore"
 
-// RUN: %clang --target=riscv32-unknown-elf -### %s -mforced-sw-shadow-stack 2>&1 | FileCheck %s -check-prefix=FORCE-SW-SCS
-// RUN: %clang --target=riscv32-unknown-elf -### %s -mno-forced-sw-shadow-stack 2>&1 | FileCheck %s -check-prefix=NO-FORCE-SW-SCS
-// FORCE-SW-SCS: "-target-feature" "+forced-sw-shadow-stack"
-// NO-FORCE-SW-SCS: "-target-feature" "-forced-sw-shadow-stack"
-// DEFAULT-NOT: "-target-feature" "+forced-sw-shadow-stack"
-
 // RUN: %clang --target=riscv32-unknown-elf -### %s -mno-strict-align 2>&1 | FileCheck %s -check-prefixes=FAST-SCALAR-UNALIGNED-ACCESS,FAST-VECTOR-UNALIGNED-ACCESS
 // RUN: %clang --target=riscv32-unknown-elf -### %s -mstrict-align 2>&1 | FileCheck %s -check-prefixes=NO-FAST-SCALAR-UNALIGNED-ACCESS,NO-FAST-VECTOR-UNALIGNED-ACCESS
 // RUN: %clang --target=riscv32-unknown-elf -### %s -mno-scalar-strict-align 2>&1 | FileCheck %s -check-prefix=FAST-SCALAR-UNALIGNED-ACCESS

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

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

LGTM

@mylai-mtk
Copy link
Contributor

LGTM

@jaidTw jaidTw merged commit 3ad6403 into llvm:main Nov 8, 2024
13 checks passed
@jaidTw jaidTw deleted the remove-force-sw-shadow-stack branch November 8, 2024 05:48
jaidTw added a commit that referenced this pull request Nov 8, 2024
This patch removes forced-sw-shadow-stack related statements in
RISCVFeatures.td, which was missed in the last patch
#115355
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.
Groverkss pushed a commit to iree-org/llvm-project that referenced this pull request Nov 15, 2024
This patch removes forced-sw-shadow-stack related statements in
RISCVFeatures.td, which was missed in the last patch
llvm#115355
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend:RISC-V clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants