Re: [PATCH] [RFC/RFT]SCS:Add gcc plugin to support Shadow Call Stack

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Nathan,

Thanks for your comments.
I rewrite the configuration as follows:

1) Change the plugin to be enabled by default, and add this option to CC_FLAGS_SCS to keep its behavior consistent with clang
---
diff --git a/Makefile b/Makefile
@@ -923,12 +923,6 @@ KBUILD_CFLAGS_KERNEL += -ffunction-sections -fdata-sections
-ifdef CONFIG_SHADOW_CALL_STACK
-CC_FLAGS_SCS	:= -fsanitize=shadow-call-stack
-KBUILD_CFLAGS	+= $(CC_FLAGS_SCS)
-export CC_FLAGS_SCS
-endif

@@ -1034,6 +1028,20 @@ include-$(CONFIG_GCC_PLUGINS)	+= scripts/Makefile.gcc-plugins
 include $(addprefix $(srctree)/, $(include-y))
+ifdef CONFIG_SHADOW_CALL_STACK
+
+ifdef CONFIG_CC_IS_CLANG
+CC_FLAGS_SCS	:= -fsanitize=shadow-call-stack
+endif
+
+ifdef CONFIG_CC_IS_GCC
+CC_FLAGS_SCS	:= $(ENABLE_SHADOW_CALL_STACK_PLUGIN)
+endif
+
+KBUILD_CFLAGS	+= $(CC_FLAGS_SCS)
+export CC_FLAGS_SCS
+endif

diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
@@ -46,6 +46,13 @@ ifdef CONFIG_GCC_PLUGIN_ARM_SSP_PER_TASK
+gcc-plugin-$(CONFIG_GCC_PLUGIN_SHADOW_CALL_STACK) += arm64_scs_plugin.so
+gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_SHADOW_CALL_STACK)	\
+		+= -DSHADOW_CALL_STACK_PLUGIN
+ifdef CONFIG_GCC_PLUGIN_SHADOW_CALL_STACK
+    ENABLE_SHADOW_CALL_STACK_PLUGIN += -fplugin-arg-arm64_scs_plugin-enable
+endif

2) Whether SCS is turned on or not is determined by CONFIG_SHADOW_CALL_STACK
   * GCC_PLUGIN_SHADOW_CALL_STACK is only used to indicate whether current platform needs the support of the gcc SCS plugin
     - It only enabled on ARM64 platform with gcc which does not support SCS(!CC_HAVE_SHADOW_CALL_STACK)
     - If one compiler supports SCS (clang or gcc), then CC_HAVE_SHADOW_CALL_STACK should be true at this time, and the plugin is automatically closed
   * As long as the current platform can support SCS(compiler or plugin), ARCH_SUPPORTS_SHADOW_CALL_STACK is always selected
   * CONFIG_SHADOW_CALL_STACK no longer depends on CC_IS_CLANG
---
diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
@@ -19,6 +19,15 @@ menuconfig GCC_PLUGINS
+config GCC_PLUGIN_SHADOW_CALL_STACK
+	bool "GCC Shadow Call Stack plugin"
+	depends on (!CC_HAVE_SHADOW_CALL_STACK) && ARM64
+	default y
+	help	....

diff --git a/arch/Kconfig b/arch/Kconfig
@@ -594,7 +594,7 @@ config ARCH_SUPPORTS_SHADOW_CALL_STACK
config SHADOW_CALL_STACK
 	bool "Clang Shadow Call Stack"
-	depends on CC_IS_CLANG && ARCH_SUPPORTS_SHADOW_CALL_STACK
+	depends on ARCH_SUPPORTS_SHADOW_CALL_STACK
 	depends on DYNAMIC_FTRACE_WITH_REGS || !FUNCTION_GRAPH_TRACER
 	help
 	  This option enables Clang's Shadow Call Stack, which uses a
	
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
@@ -81,7 +81,7 @@ config ARM64
-	select ARCH_SUPPORTS_SHADOW_CALL_STACK if CC_HAVE_SHADOW_CALL_STACK
+	select ARCH_SUPPORTS_SHADOW_CALL_STACK if (CC_HAVE_SHADOW_CALL_STACK || GCC_PLUGIN_SHADOW_CALL_STACK)
 	select ARCH_SUPPORTS_LTO_CLANG if CPU_LITTLE_ENDIAN
@@ -1060,9 +1060,13 @@ config HW_PERF_EVENTS
 # Supported by clang >= 7.0
 config CC_HAVE_SHADOW_CALL_STACK
-	def_bool $(cc-option, -fsanitize=shadow-call-stack -ffixed-x18)
+	def_bool (CC_IS_CLANG && $(cc-option, -fsanitize=shadow-call-stack -ffixed-x18))


On 9/20/21 5:45 AM, Nathan Chancellor wrote:
Hi Dan,
diff --git a/Makefile b/Makefile
  ifdef CONFIG_SHADOW_CALL_STACK

I would rather see this become

ifeq ($(CONFIG_SHADOW_CALL_STACK)$(CONFIG_CC_IS_CLANG), yy)
...
endif

rather than just avoiding assigning to CC_FLAGS_SCS.

However, how does disabling the shadow call stack plugin work for a
whole translation unit or directory? There are a few places where
CC_FLAGS_SCS are filtered out and I am not sure I see where that happens
here? It looks like the plugin has a disabled option but I do not see it
hooked in anywhere.
  In the new code, translation unit can only enable SCS when CC_FLAGS_SCS is specified.
  This behavior will be consistent with clang.
  If there are other problems in the future, those two can be modified together.

-CC_FLAGS_SCS	:= -fsanitize=shadow-call-stack
+CC_FLAGS_SCS	:= $(if $(CONFIG_CC_IS_CLANG),-fsanitize=shadow-call-stack,)

  KBUILD_CFLAGS	+= $(CC_FLAGS_SCS)
  export CC_FLAGS_SCS
  endif
diff --git a/arch/Kconfig b/arch/Kconfig
index 98db634..81ff127 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -594,7 +594,7 @@ config ARCH_SUPPORTS_SHADOW_CALL_STACK
config SHADOW_CALL_STACK
  	bool "Clang Shadow Call Stack"
-	depends on CC_IS_CLANG && ARCH_SUPPORTS_SHADOW_CALL_STACK
+	depends on (CC_IS_CLANG && ARCH_SUPPORTS_SHADOW_CALL_STACK) || GCC_PLUGIN_SHADOW_CALL_STACK

Is this logic right? SHADOW_CALL_STACK is only supported by arm64 (as
they set ARCH_SUPPORTS_SHADOW_CALL_STACK) but now you are enabling it
for any architecture, even though it seems like it still only works on
arm64. I think this wants to be

depends on (CC_IS_CLANG || GCC_PLUGIN_SHADOW_CALL_STACK) && ARCH_SUPPORTS_SHADOW_CALL_STACK

  It's modified to rely only on ARCH_SUPPORTS_SHADOW_CALL_STACK	
--- a/scripts/gcc-plugins/Kconfig
+++ b/scripts/gcc-plugins/Kconfig
@@ -19,6 +19,14 @@ menuconfig GCC_PLUGINS
if GCC_PLUGINS +config GCC_PLUGIN_SHADOW_CALL_STACK
+	bool "GCC Shadow Call Stack plugin"

This should also have a

depends on ARCH_SUPPORTS_SHADOW_CALL_STACK

if you are selecting SHADOW_CALL_STACK, as selecting does not account
for dependencies.
  Select is removed from the code above
+	select SHADOW_CALL_STACK
+	help
+	  This plugin is used to support the kernel CONFIG_SHADOW_CALL_STACK
+	  compiled by gcc. Its principle is basically the same as that of CLANG.
+	  For more information, please refer to "config SHADOW_CALL_STACK"
+
  config GCC_PLUGIN_CYC_COMPLEXITY
  	bool "Compute the cyclomatic complexity of a function" if EXPERT
  	depends on !COMPILE_TEST	# too noisy

Cheers,
Nathan




[Index of Archives]     [Linux&nblp;USB Development]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite Secrets]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux