[PATCH v2] [RISC-V] Add support for TLS stack protector canary access

[PATCH v2] [RISC-V] Add support for TLS stack protector canary access

[cooper]

gcc/
	* config/riscv/riscv-opts.h (stack_protector_guard): New enum.
	* config/riscv/riscv.c (riscv_option_override): Handle
	the new options.
	* config/riscv/riscv.md (stack_protect_set): New pattern to handle
	flexible stack protector guard settings.
	(stack_protect_set_<mode>): Ditto.
	(stack_protect_test): Ditto.
	(stack_protect_test_<mode>): Ditto.
	* config/riscv/riscv.opt (mstack-protector-guard=,
	mstack-protector-guard-reg=, mstack-protector-guard-offset=): New
	options.
	* doc/invoke.texi (Option Summary) [RISC-V Options]:
	Add -mstack-protector-guard=, -mstack-protector-guard-reg=, and
	-mstack-protector-guard-offset=.
	(RISC-V Options): Ditto.

Signed-off-by: cooper <cooper.qu@linux.alibaba.com>
Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
---
 gcc/config/riscv/riscv-opts.h |  6 +++
 gcc/config/riscv/riscv.c      | 47 ++++++++++++++++++++
 gcc/config/riscv/riscv.md     | 80 +++++++++++++++++++++++++++++++++++
 gcc/config/riscv/riscv.opt    | 28 ++++++++++++
 gcc/doc/invoke.texi           | 22 +++++++++-
 5 files changed, 182 insertions(+), 1 deletion(-)

diff --git a/gcc/config/riscv/riscv-opts.h b/gcc/config/riscv/riscv-opts.h
index 8f12e50b9f1..2a3f9d9eef5 100644
--- a/gcc/config/riscv/riscv-opts.h
+++ b/gcc/config/riscv/riscv-opts.h
@@ -51,4 +51,10 @@ enum riscv_align_data {
   riscv_align_data_type_natural
 };
 
+/* Where to get the canary for the stack protector.  */
+enum stack_protector_guard {
+  SSP_TLS,			/* per-thread canary in TLS block */
+  SSP_GLOBAL			/* global canary */
+};
+
 #endif /* ! GCC_RISCV_OPTS_H */
diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
index bfb3885ed08..63b0c3877b0 100644
--- a/gcc/config/riscv/riscv.c
+++ b/gcc/config/riscv/riscv.c
@@ -4775,6 +4775,53 @@ riscv_option_override (void)
 	   " [%<-mriscv-attribute%>]");
 #endif
 
+  if (riscv_stack_protector_guard == SSP_GLOBAL
+      && global_options_set.x_riscv_stack_protector_guard_offset_str)
+    {
+      error ("incompatible options %<-mstack-protector-guard=global%> and "
+	     "%<-mstack-protector-guard-offset=%s%>",
+	     riscv_stack_protector_guard_offset_str);
+    }
+
+  if (riscv_stack_protector_guard == SSP_TLS
+      && !(global_options_set.x_riscv_stack_protector_guard_offset_str
+	   && global_options_set.x_riscv_stack_protector_guard_reg_str))
+    {
+      error ("both %<-mstack-protector-guard-offset%> and "
+	     "%<-mstack-protector-guard-reg%> must be used "
+	     "with %<-mstack-protector-guard=sysreg%>");
+    }
+
+  if (global_options_set.x_riscv_stack_protector_guard_reg_str)
+    {
+      const char *str = riscv_stack_protector_guard_reg_str;
+      int reg = decode_reg_name (str);
+
+      if (!IN_RANGE (reg, GP_REG_FIRST + 1, GP_REG_LAST))
+	error ("%qs is not a valid base register in %qs", str,
+	       "-mstack-protector-guard-reg=");
+
+      riscv_stack_protector_guard_reg = reg;
+    }
+
+  if (global_options_set.x_riscv_stack_protector_guard_offset_str)
+    {
+      char *end;
+      const char *str = riscv_stack_protector_guard_offset_str;
+      errno = 0;
+      long offs = strtol (riscv_stack_protector_guard_offset_str, &end, 0);
+
+      if (!*str || *end || errno)
+	error ("%qs is not a valid number in %qs", str,
+	       "-mstack-protector-guard-offset=");
+
+      if (!SMALL_OPERAND (offs))
+	error ("%qs is not a valid offset in %qs", str,
+	       "-mstack-protector-guard-offset=");
+
+      riscv_stack_protector_guard_offset = offs;
+    }
+
 }
 
 /* Implement TARGET_CONDITIONAL_REGISTER_USAGE.  */
diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index 95a02ecaa34..f15bad3b29e 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -65,6 +65,10 @@
   UNSPECV_BLOCKAGE
   UNSPECV_FENCE
   UNSPECV_FENCE_I
+
+  ;; Stack Smash Protector
+  UNSPEC_SSP_SET
+  UNSPEC_SSP_TEST
 ])
 
 (define_constants
@@ -2523,6 +2527,82 @@
   ""
 {})
 
+;; Named patterns for stack smashing protection.
+
+(define_expand "stack_protect_set"
+  [(match_operand 0 "memory_operand")
+   (match_operand 1 "memory_operand")]
+  ""
+{
+  machine_mode mode = GET_MODE (operands[0]);
+  if (riscv_stack_protector_guard == SSP_TLS)
+  {
+    rtx reg = gen_rtx_REG (Pmode, riscv_stack_protector_guard_reg);
+    rtx offset = GEN_INT (riscv_stack_protector_guard_offset);
+    rtx addr = gen_rtx_PLUS (Pmode, reg, offset);
+    operands[1] = gen_rtx_MEM (Pmode, addr);
+  }
+
+  emit_insn ((mode == DImode
+	      ? gen_stack_protect_set_di
+	      : gen_stack_protect_set_si) (operands[0], operands[1]));
+  DONE;
+})
+
+;; DO NOT SPLIT THIS PATTERN.  It is important for security reasons that the
+;; canary value does not live beyond the life of this sequence.
+(define_insn "stack_protect_set_<mode>"
+  [(set (match_operand:GPR 0 "memory_operand" "=m")
+	(unspec:GPR [(match_operand:GPR 1 "memory_operand" "m")]
+	 UNSPEC_SSP_SET))
+   (set (match_scratch:GPR 2 "=&r") (const_int 0))]
+  ""
+  "<load>\\t%2, %1\;<store>\\t%2, %0\;li\t%2, 0"
+  [(set_attr "length" "12")])
+
+(define_expand "stack_protect_test"
+  [(match_operand 0 "memory_operand")
+   (match_operand 1 "memory_operand")
+   (match_operand 2)]
+  ""
+{
+  rtx result;
+  machine_mode mode = GET_MODE (operands[0]);
+
+  result = gen_reg_rtx(mode);
+  if (riscv_stack_protector_guard == SSP_TLS)
+  {
+      rtx reg = gen_rtx_REG (Pmode, riscv_stack_protector_guard_reg);
+      rtx offset = GEN_INT (riscv_stack_protector_guard_offset);
+      rtx addr = gen_rtx_PLUS (Pmode, reg, offset);
+      operands[1] = gen_rtx_MEM (Pmode, addr);
+  }
+  emit_insn ((mode == DImode
+		  ? gen_stack_protect_test_di
+		  : gen_stack_protect_test_si) (result,
+					        operands[0],
+					        operands[1]));
+
+  if (mode == DImode)
+    emit_jump_insn (gen_cbranchdi4 (gen_rtx_EQ (VOIDmode, result, const0_rtx),
+				    result, const0_rtx, operands[2]));
+  else
+    emit_jump_insn (gen_cbranchsi4 (gen_rtx_EQ (VOIDmode, result, const0_rtx),
+				    result, const0_rtx, operands[2]));
+
+  DONE;
+})
+
+(define_insn "stack_protect_test_<mode>"
+  [(set (match_operand:GPR 0 "register_operand" "=r")
+	(unspec:GPR [(match_operand:GPR 1 "memory_operand" "m")
+		     (match_operand:GPR 2 "memory_operand" "m")]
+	 UNSPEC_SSP_TEST))
+   (clobber (match_scratch:GPR 3 "=&r"))]
+  ""
+  "<load>\t%3, %1\;<load>\t%0, %2\;xor\t%0, %3, %0\;li\t%3, 0"
+  [(set_attr "length" "12")])
+
 (include "sync.md")
 (include "peephole.md")
 (include "pic.md")
diff --git a/gcc/config/riscv/riscv.opt b/gcc/config/riscv/riscv.opt
index e4bfcb86f51..f01d3ab79c3 100644
--- a/gcc/config/riscv/riscv.opt
+++ b/gcc/config/riscv/riscv.opt
@@ -151,3 +151,31 @@ Enum(riscv_align_data) String(xlen) Value(riscv_align_data_type_xlen)
 
 EnumValue
 Enum(riscv_align_data) String(natural) Value(riscv_align_data_type_natural)
+
+mstack-protector-guard=
+Target RejectNegative Joined Enum(stack_protector_guard) Var(riscv_stack_protector_guard) Init(SSP_GLOBAL)
+Use given stack-protector guard.
+
+Enum
+Name(stack_protector_guard) Type(enum stack_protector_guard)
+Valid arguments to -mstack-protector-guard=:
+
+EnumValue
+Enum(stack_protector_guard) String(tls) Value(SSP_TLS)
+
+EnumValue
+Enum(stack_protector_guard) String(global) Value(SSP_GLOBAL)
+
+mstack-protector-guard-reg=
+Target RejectNegative Joined Var(riscv_stack_protector_guard_reg_str)
+Use the given base register for addressing the stack-protector guard.
+
+TargetVariable
+int riscv_stack_protector_guard_reg = 0
+
+mstack-protector-guard-offset=
+Target RejectNegative Joined Integer Var(riscv_stack_protector_guard_offset_str)
+Use the given offset for addressing the stack-protector guard.
+
+TargetVariable
+long riscv_stack_protector_guard_offset = 0
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 09bcc5b0f78..3bb124ae6ed 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -1138,7 +1138,9 @@ See RS/6000 and PowerPC Options.
 -mexplicit-relocs  -mno-explicit-relocs @gol
 -mrelax  -mno-relax @gol
 -mriscv-attribute  -mmo-riscv-attribute @gol
--malign-data=@var{type}}
+-malign-data=@var{type} @gol
++-mstack-protector-guard=@var{guard} -mstack-protector-guard-reg=@var{reg} @gol
++-mstack-protector-guard-offset=@var{offset}}
 
 @emph{RL78 Options}
 @gccoptlist{-msim  -mmul=none  -mmul=g13  -mmul=g14  -mallregs @gol
@@ -25723,6 +25725,24 @@ Control how GCC aligns variables and constants of array, structure, or union
 types.  Supported values for @var{type} are @samp{xlen} which uses x register
 width as the alignment value, and @samp{natural} which uses natural alignment.
 @samp{xlen} is the default.
+
+@item -mstack-protector-guard=@var{guard}
+@itemx -mstack-protector-guard-reg=@var{reg}
+@itemx -mstack-protector-guard-offset=@var{offset}
+@opindex mstack-protector-guard
+@opindex mstack-protector-guard-reg
+@opindex mstack-protector-guard-offset
+Generate stack protection code using canary at @var{guard}.  Supported
+locations are @samp{global} for a global canary or @samp{tls} for per-thread
+canary in the TLS block.
+
+With the latter choice the options
+@option{-mstack-protector-guard-reg=@var{reg}} and
+@option{-mstack-protector-guard-offset=@var{offset}} furthermore specify
+which register to use as base register for reading the canary,
+and from what offset from that base register. There is no default
+register or offset as this is entirely for use within the Linux
+kernel.
 @end table
 
 @node RL78 Options
-- 
2.26.2

Re: [PATCH v2] [RISC-V] Add support for TLS stack protector canary access

[cooper]

Ping

On 2020/7/13 下午4:15, cooper wrote:
> gcc/
> 	* config/riscv/riscv-opts.h (stack_protector_guard): New enum.
> 	* config/riscv/riscv.c (riscv_option_override): Handle
> 	the new options.
> 	* config/riscv/riscv.md (stack_protect_set): New pattern to handle
> 	flexible stack protector guard settings.
> 	(stack_protect_set_<mode>): Ditto.
> 	(stack_protect_test): Ditto.
> 	(stack_protect_test_<mode>): Ditto.
> 	* config/riscv/riscv.opt (mstack-protector-guard=,
> 	mstack-protector-guard-reg=, mstack-protector-guard-offset=): New
> 	options.
> 	* doc/invoke.texi (Option Summary) [RISC-V Options]:
> 	Add -mstack-protector-guard=, -mstack-protector-guard-reg=, and
> 	-mstack-protector-guard-offset=.
> 	(RISC-V Options): Ditto.
>
> Signed-off-by: cooper <cooper.qu@linux.alibaba.com>
> Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> ---
>   gcc/config/riscv/riscv-opts.h |  6 +++
>   gcc/config/riscv/riscv.c      | 47 ++++++++++++++++++++
>   gcc/config/riscv/riscv.md     | 80 +++++++++++++++++++++++++++++++++++
>   gcc/config/riscv/riscv.opt    | 28 ++++++++++++
>   gcc/doc/invoke.texi           | 22 +++++++++-
>   5 files changed, 182 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/config/riscv/riscv-opts.h b/gcc/config/riscv/riscv-opts.h
> index 8f12e50b9f1..2a3f9d9eef5 100644
> --- a/gcc/config/riscv/riscv-opts.h
> +++ b/gcc/config/riscv/riscv-opts.h
> @@ -51,4 +51,10 @@ enum riscv_align_data {
>     riscv_align_data_type_natural
>   };
>   
> +/* Where to get the canary for the stack protector.  */
> +enum stack_protector_guard {
> +  SSP_TLS,			/* per-thread canary in TLS block */
> +  SSP_GLOBAL			/* global canary */
> +};
> +
>   #endif /* ! GCC_RISCV_OPTS_H */
> diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
> index bfb3885ed08..63b0c3877b0 100644
> --- a/gcc/config/riscv/riscv.c
> +++ b/gcc/config/riscv/riscv.c
> @@ -4775,6 +4775,53 @@ riscv_option_override (void)
>   	   " [%<-mriscv-attribute%>]");
>   #endif
>   
> +  if (riscv_stack_protector_guard == SSP_GLOBAL
> +      && global_options_set.x_riscv_stack_protector_guard_offset_str)
> +    {
> +      error ("incompatible options %<-mstack-protector-guard=global%> and "
> +	     "%<-mstack-protector-guard-offset=%s%>",
> +	     riscv_stack_protector_guard_offset_str);
> +    }
> +
> +  if (riscv_stack_protector_guard == SSP_TLS
> +      && !(global_options_set.x_riscv_stack_protector_guard_offset_str
> +	   && global_options_set.x_riscv_stack_protector_guard_reg_str))
> +    {
> +      error ("both %<-mstack-protector-guard-offset%> and "
> +	     "%<-mstack-protector-guard-reg%> must be used "
> +	     "with %<-mstack-protector-guard=sysreg%>");
> +    }
> +
> +  if (global_options_set.x_riscv_stack_protector_guard_reg_str)
> +    {
> +      const char *str = riscv_stack_protector_guard_reg_str;
> +      int reg = decode_reg_name (str);
> +
> +      if (!IN_RANGE (reg, GP_REG_FIRST + 1, GP_REG_LAST))
> +	error ("%qs is not a valid base register in %qs", str,
> +	       "-mstack-protector-guard-reg=");
> +
> +      riscv_stack_protector_guard_reg = reg;
> +    }
> +
> +  if (global_options_set.x_riscv_stack_protector_guard_offset_str)
> +    {
> +      char *end;
> +      const char *str = riscv_stack_protector_guard_offset_str;
> +      errno = 0;
> +      long offs = strtol (riscv_stack_protector_guard_offset_str, &end, 0);
> +
> +      if (!*str || *end || errno)
> +	error ("%qs is not a valid number in %qs", str,
> +	       "-mstack-protector-guard-offset=");
> +
> +      if (!SMALL_OPERAND (offs))
> +	error ("%qs is not a valid offset in %qs", str,
> +	       "-mstack-protector-guard-offset=");
> +
> +      riscv_stack_protector_guard_offset = offs;
> +    }
> +
>   }
>   
>   /* Implement TARGET_CONDITIONAL_REGISTER_USAGE.  */
> diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
> index 95a02ecaa34..f15bad3b29e 100644
> --- a/gcc/config/riscv/riscv.md
> +++ b/gcc/config/riscv/riscv.md
> @@ -65,6 +65,10 @@
>     UNSPECV_BLOCKAGE
>     UNSPECV_FENCE
>     UNSPECV_FENCE_I
> +
> +  ;; Stack Smash Protector
> +  UNSPEC_SSP_SET
> +  UNSPEC_SSP_TEST
>   ])
>   
>   (define_constants
> @@ -2523,6 +2527,82 @@
>     ""
>   {})
>   
> +;; Named patterns for stack smashing protection.
> +
> +(define_expand "stack_protect_set"
> +  [(match_operand 0 "memory_operand")
> +   (match_operand 1 "memory_operand")]
> +  ""
> +{
> +  machine_mode mode = GET_MODE (operands[0]);
> +  if (riscv_stack_protector_guard == SSP_TLS)
> +  {
> +    rtx reg = gen_rtx_REG (Pmode, riscv_stack_protector_guard_reg);
> +    rtx offset = GEN_INT (riscv_stack_protector_guard_offset);
> +    rtx addr = gen_rtx_PLUS (Pmode, reg, offset);
> +    operands[1] = gen_rtx_MEM (Pmode, addr);
> +  }
> +
> +  emit_insn ((mode == DImode
> +	      ? gen_stack_protect_set_di
> +	      : gen_stack_protect_set_si) (operands[0], operands[1]));
> +  DONE;
> +})
> +
> +;; DO NOT SPLIT THIS PATTERN.  It is important for security reasons that the
> +;; canary value does not live beyond the life of this sequence.
> +(define_insn "stack_protect_set_<mode>"
> +  [(set (match_operand:GPR 0 "memory_operand" "=m")
> +	(unspec:GPR [(match_operand:GPR 1 "memory_operand" "m")]
> +	 UNSPEC_SSP_SET))
> +   (set (match_scratch:GPR 2 "=&r") (const_int 0))]
> +  ""
> +  "<load>\\t%2, %1\;<store>\\t%2, %0\;li\t%2, 0"
> +  [(set_attr "length" "12")])
> +
> +(define_expand "stack_protect_test"
> +  [(match_operand 0 "memory_operand")
> +   (match_operand 1 "memory_operand")
> +   (match_operand 2)]
> +  ""
> +{
> +  rtx result;
> +  machine_mode mode = GET_MODE (operands[0]);
> +
> +  result = gen_reg_rtx(mode);
> +  if (riscv_stack_protector_guard == SSP_TLS)
> +  {
> +      rtx reg = gen_rtx_REG (Pmode, riscv_stack_protector_guard_reg);
> +      rtx offset = GEN_INT (riscv_stack_protector_guard_offset);
> +      rtx addr = gen_rtx_PLUS (Pmode, reg, offset);
> +      operands[1] = gen_rtx_MEM (Pmode, addr);
> +  }
> +  emit_insn ((mode == DImode
> +		  ? gen_stack_protect_test_di
> +		  : gen_stack_protect_test_si) (result,
> +					        operands[0],
> +					        operands[1]));
> +
> +  if (mode == DImode)
> +    emit_jump_insn (gen_cbranchdi4 (gen_rtx_EQ (VOIDmode, result, const0_rtx),
> +				    result, const0_rtx, operands[2]));
> +  else
> +    emit_jump_insn (gen_cbranchsi4 (gen_rtx_EQ (VOIDmode, result, const0_rtx),
> +				    result, const0_rtx, operands[2]));
> +
> +  DONE;
> +})
> +
> +(define_insn "stack_protect_test_<mode>"
> +  [(set (match_operand:GPR 0 "register_operand" "=r")
> +	(unspec:GPR [(match_operand:GPR 1 "memory_operand" "m")
> +		     (match_operand:GPR 2 "memory_operand" "m")]
> +	 UNSPEC_SSP_TEST))
> +   (clobber (match_scratch:GPR 3 "=&r"))]
> +  ""
> +  "<load>\t%3, %1\;<load>\t%0, %2\;xor\t%0, %3, %0\;li\t%3, 0"
> +  [(set_attr "length" "12")])
> +
>   (include "sync.md")
>   (include "peephole.md")
>   (include "pic.md")
> diff --git a/gcc/config/riscv/riscv.opt b/gcc/config/riscv/riscv.opt
> index e4bfcb86f51..f01d3ab79c3 100644
> --- a/gcc/config/riscv/riscv.opt
> +++ b/gcc/config/riscv/riscv.opt
> @@ -151,3 +151,31 @@ Enum(riscv_align_data) String(xlen) Value(riscv_align_data_type_xlen)
>   
>   EnumValue
>   Enum(riscv_align_data) String(natural) Value(riscv_align_data_type_natural)
> +
> +mstack-protector-guard=
> +Target RejectNegative Joined Enum(stack_protector_guard) Var(riscv_stack_protector_guard) Init(SSP_GLOBAL)
> +Use given stack-protector guard.
> +
> +Enum
> +Name(stack_protector_guard) Type(enum stack_protector_guard)
> +Valid arguments to -mstack-protector-guard=:
> +
> +EnumValue
> +Enum(stack_protector_guard) String(tls) Value(SSP_TLS)
> +
> +EnumValue
> +Enum(stack_protector_guard) String(global) Value(SSP_GLOBAL)
> +
> +mstack-protector-guard-reg=
> +Target RejectNegative Joined Var(riscv_stack_protector_guard_reg_str)
> +Use the given base register for addressing the stack-protector guard.
> +
> +TargetVariable
> +int riscv_stack_protector_guard_reg = 0
> +
> +mstack-protector-guard-offset=
> +Target RejectNegative Joined Integer Var(riscv_stack_protector_guard_offset_str)
> +Use the given offset for addressing the stack-protector guard.
> +
> +TargetVariable
> +long riscv_stack_protector_guard_offset = 0
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 09bcc5b0f78..3bb124ae6ed 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -1138,7 +1138,9 @@ See RS/6000 and PowerPC Options.
>   -mexplicit-relocs  -mno-explicit-relocs @gol
>   -mrelax  -mno-relax @gol
>   -mriscv-attribute  -mmo-riscv-attribute @gol
> --malign-data=@var{type}}
> +-malign-data=@var{type} @gol
> ++-mstack-protector-guard=@var{guard} -mstack-protector-guard-reg=@var{reg} @gol
> ++-mstack-protector-guard-offset=@var{offset}}
>   
>   @emph{RL78 Options}
>   @gccoptlist{-msim  -mmul=none  -mmul=g13  -mmul=g14  -mallregs @gol
> @@ -25723,6 +25725,24 @@ Control how GCC aligns variables and constants of array, structure, or union
>   types.  Supported values for @var{type} are @samp{xlen} which uses x register
>   width as the alignment value, and @samp{natural} which uses natural alignment.
>   @samp{xlen} is the default.
> +
> +@item -mstack-protector-guard=@var{guard}
> +@itemx -mstack-protector-guard-reg=@var{reg}
> +@itemx -mstack-protector-guard-offset=@var{offset}
> +@opindex mstack-protector-guard
> +@opindex mstack-protector-guard-reg
> +@opindex mstack-protector-guard-offset
> +Generate stack protection code using canary at @var{guard}.  Supported
> +locations are @samp{global} for a global canary or @samp{tls} for per-thread
> +canary in the TLS block.
> +
> +With the latter choice the options
> +@option{-mstack-protector-guard-reg=@var{reg}} and
> +@option{-mstack-protector-guard-offset=@var{offset}} furthermore specify
> +which register to use as base register for reading the canary,
> +and from what offset from that base register. There is no default
> +register or offset as this is entirely for use within the Linux
> +kernel.
>   @end table
>   
>   @node RL78 Options

Re: [PATCH v2] [RISC-V] Add support for TLS stack protector canary access

[Kito Cheng]

Hi Cooper:

Could you add testcases like ppc[3-4]?

[3] https://github.com/gcc-mirror/gcc/blob/master/gcc/testsuite/gcc.target/powerpc/ssp-1.c
[4] https://github.com/gcc-mirror/gcc/blob/master/gcc/testsuite/gcc.target/powerpc/ssp-2.c

On Mon, Jul 20, 2020 at 10:04 AM cooper via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Ping
>
> On 2020/7/13 下午4:15, cooper wrote:
> > gcc/
> >       * config/riscv/riscv-opts.h (stack_protector_guard): New enum.
> >       * config/riscv/riscv.c (riscv_option_override): Handle
> >       the new options.
> >       * config/riscv/riscv.md (stack_protect_set): New pattern to handle
> >       flexible stack protector guard settings.
> >       (stack_protect_set_<mode>): Ditto.
> >       (stack_protect_test): Ditto.
> >       (stack_protect_test_<mode>): Ditto.
> >       * config/riscv/riscv.opt (mstack-protector-guard=,
> >       mstack-protector-guard-reg=, mstack-protector-guard-offset=): New
> >       options.
> >       * doc/invoke.texi (Option Summary) [RISC-V Options]:
> >       Add -mstack-protector-guard=, -mstack-protector-guard-reg=, and
> >       -mstack-protector-guard-offset=.
> >       (RISC-V Options): Ditto.
> >
> > Signed-off-by: cooper <cooper.qu@linux.alibaba.com>
> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > ---
> >   gcc/config/riscv/riscv-opts.h |  6 +++
> >   gcc/config/riscv/riscv.c      | 47 ++++++++++++++++++++
> >   gcc/config/riscv/riscv.md     | 80 +++++++++++++++++++++++++++++++++++
> >   gcc/config/riscv/riscv.opt    | 28 ++++++++++++
> >   gcc/doc/invoke.texi           | 22 +++++++++-
> >   5 files changed, 182 insertions(+), 1 deletion(-)
> >
> > diff --git a/gcc/config/riscv/riscv-opts.h b/gcc/config/riscv/riscv-opts.h
> > index 8f12e50b9f1..2a3f9d9eef5 100644
> > --- a/gcc/config/riscv/riscv-opts.h
> > +++ b/gcc/config/riscv/riscv-opts.h
> > @@ -51,4 +51,10 @@ enum riscv_align_data {
> >     riscv_align_data_type_natural
> >   };
> >
> > +/* Where to get the canary for the stack protector.  */
> > +enum stack_protector_guard {
> > +  SSP_TLS,                   /* per-thread canary in TLS block */
> > +  SSP_GLOBAL                 /* global canary */
> > +};
> > +
> >   #endif /* ! GCC_RISCV_OPTS_H */
> > diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
> > index bfb3885ed08..63b0c3877b0 100644
> > --- a/gcc/config/riscv/riscv.c
> > +++ b/gcc/config/riscv/riscv.c
> > @@ -4775,6 +4775,53 @@ riscv_option_override (void)
> >          " [%<-mriscv-attribute%>]");
> >   #endif
> >
> > +  if (riscv_stack_protector_guard == SSP_GLOBAL
> > +      && global_options_set.x_riscv_stack_protector_guard_offset_str)
> > +    {
> > +      error ("incompatible options %<-mstack-protector-guard=global%> and "
> > +          "%<-mstack-protector-guard-offset=%s%>",
> > +          riscv_stack_protector_guard_offset_str);
> > +    }
> > +
> > +  if (riscv_stack_protector_guard == SSP_TLS
> > +      && !(global_options_set.x_riscv_stack_protector_guard_offset_str
> > +        && global_options_set.x_riscv_stack_protector_guard_reg_str))
> > +    {
> > +      error ("both %<-mstack-protector-guard-offset%> and "
> > +          "%<-mstack-protector-guard-reg%> must be used "
> > +          "with %<-mstack-protector-guard=sysreg%>");
> > +    }
> > +
> > +  if (global_options_set.x_riscv_stack_protector_guard_reg_str)
> > +    {
> > +      const char *str = riscv_stack_protector_guard_reg_str;
> > +      int reg = decode_reg_name (str);
> > +
> > +      if (!IN_RANGE (reg, GP_REG_FIRST + 1, GP_REG_LAST))
> > +     error ("%qs is not a valid base register in %qs", str,
> > +            "-mstack-protector-guard-reg=");
> > +
> > +      riscv_stack_protector_guard_reg = reg;
> > +    }
> > +
> > +  if (global_options_set.x_riscv_stack_protector_guard_offset_str)
> > +    {
> > +      char *end;
> > +      const char *str = riscv_stack_protector_guard_offset_str;
> > +      errno = 0;
> > +      long offs = strtol (riscv_stack_protector_guard_offset_str, &end, 0);
> > +
> > +      if (!*str || *end || errno)
> > +     error ("%qs is not a valid number in %qs", str,
> > +            "-mstack-protector-guard-offset=");
> > +
> > +      if (!SMALL_OPERAND (offs))
> > +     error ("%qs is not a valid offset in %qs", str,
> > +            "-mstack-protector-guard-offset=");
> > +
> > +      riscv_stack_protector_guard_offset = offs;
> > +    }
> > +
> >   }
> >
> >   /* Implement TARGET_CONDITIONAL_REGISTER_USAGE.  */
> > diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
> > index 95a02ecaa34..f15bad3b29e 100644
> > --- a/gcc/config/riscv/riscv.md
> > +++ b/gcc/config/riscv/riscv.md
> > @@ -65,6 +65,10 @@
> >     UNSPECV_BLOCKAGE
> >     UNSPECV_FENCE
> >     UNSPECV_FENCE_I
> > +
> > +  ;; Stack Smash Protector
> > +  UNSPEC_SSP_SET
> > +  UNSPEC_SSP_TEST
> >   ])
> >
> >   (define_constants
> > @@ -2523,6 +2527,82 @@
> >     ""
> >   {})
> >
> > +;; Named patterns for stack smashing protection.
> > +
> > +(define_expand "stack_protect_set"
> > +  [(match_operand 0 "memory_operand")
> > +   (match_operand 1 "memory_operand")]
> > +  ""
> > +{
> > +  machine_mode mode = GET_MODE (operands[0]);
> > +  if (riscv_stack_protector_guard == SSP_TLS)
> > +  {
> > +    rtx reg = gen_rtx_REG (Pmode, riscv_stack_protector_guard_reg);
> > +    rtx offset = GEN_INT (riscv_stack_protector_guard_offset);
> > +    rtx addr = gen_rtx_PLUS (Pmode, reg, offset);
> > +    operands[1] = gen_rtx_MEM (Pmode, addr);
> > +  }
> > +
> > +  emit_insn ((mode == DImode
> > +           ? gen_stack_protect_set_di
> > +           : gen_stack_protect_set_si) (operands[0], operands[1]));
> > +  DONE;
> > +})
> > +
> > +;; DO NOT SPLIT THIS PATTERN.  It is important for security reasons that the
> > +;; canary value does not live beyond the life of this sequence.
> > +(define_insn "stack_protect_set_<mode>"
> > +  [(set (match_operand:GPR 0 "memory_operand" "=m")
> > +     (unspec:GPR [(match_operand:GPR 1 "memory_operand" "m")]
> > +      UNSPEC_SSP_SET))
> > +   (set (match_scratch:GPR 2 "=&r") (const_int 0))]
> > +  ""
> > +  "<load>\\t%2, %1\;<store>\\t%2, %0\;li\t%2, 0"
> > +  [(set_attr "length" "12")])
> > +
> > +(define_expand "stack_protect_test"
> > +  [(match_operand 0 "memory_operand")
> > +   (match_operand 1 "memory_operand")
> > +   (match_operand 2)]
> > +  ""
> > +{
> > +  rtx result;
> > +  machine_mode mode = GET_MODE (operands[0]);
> > +
> > +  result = gen_reg_rtx(mode);
> > +  if (riscv_stack_protector_guard == SSP_TLS)
> > +  {
> > +      rtx reg = gen_rtx_REG (Pmode, riscv_stack_protector_guard_reg);
> > +      rtx offset = GEN_INT (riscv_stack_protector_guard_offset);
> > +      rtx addr = gen_rtx_PLUS (Pmode, reg, offset);
> > +      operands[1] = gen_rtx_MEM (Pmode, addr);
> > +  }
> > +  emit_insn ((mode == DImode
> > +               ? gen_stack_protect_test_di
> > +               : gen_stack_protect_test_si) (result,
> > +                                             operands[0],
> > +                                             operands[1]));
> > +
> > +  if (mode == DImode)
> > +    emit_jump_insn (gen_cbranchdi4 (gen_rtx_EQ (VOIDmode, result, const0_rtx),
> > +                                 result, const0_rtx, operands[2]));
> > +  else
> > +    emit_jump_insn (gen_cbranchsi4 (gen_rtx_EQ (VOIDmode, result, const0_rtx),
> > +                                 result, const0_rtx, operands[2]));
> > +
> > +  DONE;
> > +})
> > +
> > +(define_insn "stack_protect_test_<mode>"
> > +  [(set (match_operand:GPR 0 "register_operand" "=r")
> > +     (unspec:GPR [(match_operand:GPR 1 "memory_operand" "m")
> > +                  (match_operand:GPR 2 "memory_operand" "m")]
> > +      UNSPEC_SSP_TEST))
> > +   (clobber (match_scratch:GPR 3 "=&r"))]
> > +  ""
> > +  "<load>\t%3, %1\;<load>\t%0, %2\;xor\t%0, %3, %0\;li\t%3, 0"
> > +  [(set_attr "length" "12")])
> > +
> >   (include "sync.md")
> >   (include "peephole.md")
> >   (include "pic.md")
> > diff --git a/gcc/config/riscv/riscv.opt b/gcc/config/riscv/riscv.opt
> > index e4bfcb86f51..f01d3ab79c3 100644
> > --- a/gcc/config/riscv/riscv.opt
> > +++ b/gcc/config/riscv/riscv.opt
> > @@ -151,3 +151,31 @@ Enum(riscv_align_data) String(xlen) Value(riscv_align_data_type_xlen)
> >
> >   EnumValue
> >   Enum(riscv_align_data) String(natural) Value(riscv_align_data_type_natural)
> > +
> > +mstack-protector-guard=
> > +Target RejectNegative Joined Enum(stack_protector_guard) Var(riscv_stack_protector_guard) Init(SSP_GLOBAL)
> > +Use given stack-protector guard.
> > +
> > +Enum
> > +Name(stack_protector_guard) Type(enum stack_protector_guard)
> > +Valid arguments to -mstack-protector-guard=:
> > +
> > +EnumValue
> > +Enum(stack_protector_guard) String(tls) Value(SSP_TLS)
> > +
> > +EnumValue
> > +Enum(stack_protector_guard) String(global) Value(SSP_GLOBAL)
> > +
> > +mstack-protector-guard-reg=
> > +Target RejectNegative Joined Var(riscv_stack_protector_guard_reg_str)
> > +Use the given base register for addressing the stack-protector guard.
> > +
> > +TargetVariable
> > +int riscv_stack_protector_guard_reg = 0
> > +
> > +mstack-protector-guard-offset=
> > +Target RejectNegative Joined Integer Var(riscv_stack_protector_guard_offset_str)
> > +Use the given offset for addressing the stack-protector guard.
> > +
> > +TargetVariable
> > +long riscv_stack_protector_guard_offset = 0
> > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > index 09bcc5b0f78..3bb124ae6ed 100644
> > --- a/gcc/doc/invoke.texi
> > +++ b/gcc/doc/invoke.texi
> > @@ -1138,7 +1138,9 @@ See RS/6000 and PowerPC Options.
> >   -mexplicit-relocs  -mno-explicit-relocs @gol
> >   -mrelax  -mno-relax @gol
> >   -mriscv-attribute  -mmo-riscv-attribute @gol
> > --malign-data=@var{type}}
> > +-malign-data=@var{type} @gol
> > ++-mstack-protector-guard=@var{guard} -mstack-protector-guard-reg=@var{reg} @gol
> > ++-mstack-protector-guard-offset=@var{offset}}
> >
> >   @emph{RL78 Options}
> >   @gccoptlist{-msim  -mmul=none  -mmul=g13  -mmul=g14  -mallregs @gol
> > @@ -25723,6 +25725,24 @@ Control how GCC aligns variables and constants of array, structure, or union
> >   types.  Supported values for @var{type} are @samp{xlen} which uses x register
> >   width as the alignment value, and @samp{natural} which uses natural alignment.
> >   @samp{xlen} is the default.
> > +
> > +@item -mstack-protector-guard=@var{guard}
> > +@itemx -mstack-protector-guard-reg=@var{reg}
> > +@itemx -mstack-protector-guard-offset=@var{offset}
> > +@opindex mstack-protector-guard
> > +@opindex mstack-protector-guard-reg
> > +@opindex mstack-protector-guard-offset
> > +Generate stack protection code using canary at @var{guard}.  Supported
> > +locations are @samp{global} for a global canary or @samp{tls} for per-thread
> > +canary in the TLS block.
> > +
> > +With the latter choice the options
> > +@option{-mstack-protector-guard-reg=@var{reg}} and
> > +@option{-mstack-protector-guard-offset=@var{offset}} furthermore specify
> > +which register to use as base register for reading the canary,
> > +and from what offset from that base register. There is no default
> > +register or offset as this is entirely for use within the Linux
> > +kernel.
> >   @end table
> >
> >   @node RL78 Options

Re: [PATCH v2] [RISC-V] Add support for TLS stack protector canary access

[Jim Wilson]

On Sun, Jul 19, 2020 at 7:04 PM cooper <cooper.qu@linux.alibaba.com> wrote:
> Ping
>
> On 2020/7/13 下午4:15, cooper wrote:
> > gcc/
> >       * config/riscv/riscv-opts.h (stack_protector_guard): New enum.
> >       * config/riscv/riscv.c (riscv_option_override): Handle
> >       the new options.
> >       * config/riscv/riscv.md (stack_protect_set): New pattern to handle
> >       flexible stack protector guard settings.
> >       (stack_protect_set_<mode>): Ditto.
> >       (stack_protect_test): Ditto.
> >       (stack_protect_test_<mode>): Ditto.
> >       * config/riscv/riscv.opt (mstack-protector-guard=,
> >       mstack-protector-guard-reg=, mstack-protector-guard-offset=): New
> >       options.
> >       * doc/invoke.texi (Option Summary) [RISC-V Options]:
> >       Add -mstack-protector-guard=, -mstack-protector-guard-reg=, and
> >       -mstack-protector-guard-offset=.
> >       (RISC-V Options): Ditto.

The v2 patch looks fine to me.  Meanwhile, Kito asked for testcases
which would be nice to have but I don't think is critical considering
that this has already been tested with a kernel build.  Maybe the
testcases can be a follow on patch?  I'd like to see forward movement
on this, even if we accept a patch without the testcases.

Jim

Re: [PATCH v2] [RISC-V] Add support for TLS stack protector canary access

[Kito Cheng]

Add testcase later is OK to me.

On Tue, Jul 28, 2020 at 6:55 AM Jim Wilson <jimw@sifive.com> wrote:
>
> On Sun, Jul 19, 2020 at 7:04 PM cooper <cooper.qu@linux.alibaba.com> wrote:
> > Ping
> >
> > On 2020/7/13 下午4:15, cooper wrote:
> > > gcc/
> > >       * config/riscv/riscv-opts.h (stack_protector_guard): New enum.
> > >       * config/riscv/riscv.c (riscv_option_override): Handle
> > >       the new options.
> > >       * config/riscv/riscv.md (stack_protect_set): New pattern to handle
> > >       flexible stack protector guard settings.
> > >       (stack_protect_set_<mode>): Ditto.
> > >       (stack_protect_test): Ditto.
> > >       (stack_protect_test_<mode>): Ditto.
> > >       * config/riscv/riscv.opt (mstack-protector-guard=,
> > >       mstack-protector-guard-reg=, mstack-protector-guard-offset=): New
> > >       options.
> > >       * doc/invoke.texi (Option Summary) [RISC-V Options]:
> > >       Add -mstack-protector-guard=, -mstack-protector-guard-reg=, and
> > >       -mstack-protector-guard-offset=.
> > >       (RISC-V Options): Ditto.
>
> The v2 patch looks fine to me.  Meanwhile, Kito asked for testcases
> which would be nice to have but I don't think is critical considering
> that this has already been tested with a kernel build.  Maybe the
> testcases can be a follow on patch?  I'd like to see forward movement
> on this, even if we accept a patch without the testcases.
>
> Jim

Re: [PATCH v2] [RISC-V] Add support for TLS stack protector canary access

[Cooper Qu]

Sorry for later replay, I will add testcases on a following patch if the 
patch is accepted.


Regards,

Cooper

On 2020/7/28 上午9:23, Kito Cheng wrote:
> Add testcase later is OK to me.
>
> On Tue, Jul 28, 2020 at 6:55 AM Jim Wilson <jimw@sifive.com> wrote:
>> On Sun, Jul 19, 2020 at 7:04 PM cooper <cooper.qu@linux.alibaba.com> wrote:
>>> Ping
>>>
>>> On 2020/7/13 下午4:15, cooper wrote:
>>>> gcc/
>>>>        * config/riscv/riscv-opts.h (stack_protector_guard): New enum.
>>>>        * config/riscv/riscv.c (riscv_option_override): Handle
>>>>        the new options.
>>>>        * config/riscv/riscv.md (stack_protect_set): New pattern to handle
>>>>        flexible stack protector guard settings.
>>>>        (stack_protect_set_<mode>): Ditto.
>>>>        (stack_protect_test): Ditto.
>>>>        (stack_protect_test_<mode>): Ditto.
>>>>        * config/riscv/riscv.opt (mstack-protector-guard=,
>>>>        mstack-protector-guard-reg=, mstack-protector-guard-offset=): New
>>>>        options.
>>>>        * doc/invoke.texi (Option Summary) [RISC-V Options]:
>>>>        Add -mstack-protector-guard=, -mstack-protector-guard-reg=, and
>>>>        -mstack-protector-guard-offset=.
>>>>        (RISC-V Options): Ditto.
>> The v2 patch looks fine to me.  Meanwhile, Kito asked for testcases
>> which would be nice to have but I don't think is critical considering
>> that this has already been tested with a kernel build.  Maybe the
>> testcases can be a follow on patch?  I'd like to see forward movement
>> on this, even if we accept a patch without the testcases.
>>
>> Jim

Re: [PATCH v2] [RISC-V] Add support for TLS stack protector canary access

[Kito Cheng]

Hi Cooper:

Thanks for your patch! committed to trunk.

https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=c931e8d5a96463427040b0d11f9c4352ac22b2b0

On Wed, Jul 29, 2020 at 8:34 PM Cooper Qu via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Sorry for later replay, I will add testcases on a following patch if the
> patch is accepted.
>
>
> Regards,
>
> Cooper
>
> On 2020/7/28 上午9:23, Kito Cheng wrote:
> > Add testcase later is OK to me.
> >
> > On Tue, Jul 28, 2020 at 6:55 AM Jim Wilson <jimw@sifive.com> wrote:
> >> On Sun, Jul 19, 2020 at 7:04 PM cooper <cooper.qu@linux.alibaba.com> wrote:
> >>> Ping
> >>>
> >>> On 2020/7/13 下午4:15, cooper wrote:
> >>>> gcc/
> >>>>        * config/riscv/riscv-opts.h (stack_protector_guard): New enum.
> >>>>        * config/riscv/riscv.c (riscv_option_override): Handle
> >>>>        the new options.
> >>>>        * config/riscv/riscv.md (stack_protect_set): New pattern to handle
> >>>>        flexible stack protector guard settings.
> >>>>        (stack_protect_set_<mode>): Ditto.
> >>>>        (stack_protect_test): Ditto.
> >>>>        (stack_protect_test_<mode>): Ditto.
> >>>>        * config/riscv/riscv.opt (mstack-protector-guard=,
> >>>>        mstack-protector-guard-reg=, mstack-protector-guard-offset=): New
> >>>>        options.
> >>>>        * doc/invoke.texi (Option Summary) [RISC-V Options]:
> >>>>        Add -mstack-protector-guard=, -mstack-protector-guard-reg=, and
> >>>>        -mstack-protector-guard-offset=.
> >>>>        (RISC-V Options): Ditto.
> >> The v2 patch looks fine to me.  Meanwhile, Kito asked for testcases
> >> which would be nice to have but I don't think is critical considering
> >> that this has already been tested with a kernel build.  Maybe the
> >> testcases can be a follow on patch?  I'd like to see forward movement
> >> on this, even if we accept a patch without the testcases.
> >>
> >> Jim