[PATCH, i386]: Make stack canary location customizable (PR target/81708)

[PATCH, i386]: Make stack canary location customizable (PR target/81708)

[Uros Bizjak]

[-- Attachment #1: Type: text/plain, Size: 1100 bytes --]

Hello!

Attached patch introduces -mstack-protector-guard-reg=  and
-mstack-protector-guard-offset= options to make stack canary location
customizable. These are the same options powerpc has.

2017-08-08  Uros Bizjak  <ubizjak@gmail.com>

    PR target/81708
    * config/i386/i386.opt (mstack-protector-guard-reg=): New option
    (mstack-protector-guard-offset=): Ditto.
    * config/i386/i386.c (ix86_option_override): Handle
    -mstack-protector-guard-reg= and -mstack-protector-guard-offset=
    options.
    (ix86_stack_protect_guard): Use ix86_stack_protect_guard_reg and
    ix86_stack_protect_guard_offset variables.
    (TARGET_STACK_PROTECT_GUARD): Always define.
    * doc/invoke.texi (x86 Options): Document -mstack-protector-guard-reg=
    and -mstack-protector-guard-offset= options.

testsuite/ChangeLog:

2017-08-08  Uros Bizjak  <ubizjak@gmail.com>

    PR target/81708
    * gcc.target/i386/stack-prot-guard.c: New test.

Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

Committed to mainline SVN. I will mention new options in gcc-8 changes
webpage later.

Uros.

[-- Attachment #2: p.diff.txt --]
[-- Type: text/plain, Size: 7351 bytes --]

Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c	(revision 250960)
+++ config/i386/i386.c	(working copy)
@@ -6662,6 +6662,69 @@ ix86_option_override_internal (bool main_args_p,
     opts->x_ix86_stack_protector_guard
       = TARGET_HAS_BIONIC ? SSP_GLOBAL : SSP_TLS;
 
+#ifdef TARGET_THREAD_SSP_OFFSET
+  ix86_stack_protector_guard_offset = TARGET_THREAD_SSP_OFFSET;
+#endif
+
+  if (global_options_set.x_ix86_stack_protector_guard_offset_str)
+    {
+      char *endp;
+      const char *str = ix86_stack_protector_guard_offset_str;
+
+      errno = 0;
+      int64_t offset;
+
+#if defined(INT64_T_IS_LONG)
+      offset = strtol (str, &endp, 0);
+#else
+      offset = strtoll (str, &endp, 0);
+#endif
+
+      if (!*str || *endp || errno)
+	error ("%qs is not a valid number "
+	       "in -mstack-protector-guard-offset=", str);
+
+      if (!IN_RANGE (offset, HOST_WIDE_INT_C (-0x80000000),
+		     HOST_WIDE_INT_C (0x7fffffff)))
+	error ("%qs is not a valid offset "
+	       "in -mstack-protector-guard-offset=", str);
+
+      ix86_stack_protector_guard_offset = offset;
+    }
+
+  ix86_stack_protector_guard_reg = DEFAULT_TLS_SEG_REG;
+
+  /* The kernel uses a different segment register for performance
+     reasons; a system call would not have to trash the userspace
+     segment register, which would be expensive.  */
+  if (ix86_cmodel == CM_KERNEL)
+    ix86_stack_protector_guard_reg = ADDR_SPACE_SEG_GS;
+
+  if (global_options_set.x_ix86_stack_protector_guard_reg_str)
+    {
+      const char *str = ix86_stack_protector_guard_reg_str;
+      addr_space_t seg = ADDR_SPACE_GENERIC;
+
+      /* Discard optional register prefix.  */
+      if (str[0] == '%')
+	str++;
+
+      if (strlen (str) == 2 && str[1] == 's')
+	{
+	  if (str[0] == 'f')
+	    seg = ADDR_SPACE_SEG_FS;
+	  else if (str[0] == 'g')
+	    seg = ADDR_SPACE_SEG_GS;
+	}
+
+      if (seg == ADDR_SPACE_GENERIC)
+	error ("%qs is not a valid base register "
+	       "in -mstack-protector-guard-reg=",
+	       ix86_stack_protector_guard_reg_str);
+
+      ix86_stack_protector_guard_reg = seg;
+    }
+
   /* Handle -mmemcpy-strategy= and -mmemset-strategy=  */
   if (opts->x_ix86_tune_memcpy_strategy)
     {
@@ -45795,7 +45858,6 @@ ix86_mangle_type (const_tree type)
     }
 }
 
-#ifdef TARGET_THREAD_SSP_OFFSET
 static tree
 ix86_stack_protect_guard (void)
 {
@@ -45802,20 +45864,13 @@ ix86_stack_protect_guard (void)
   if (TARGET_SSP_TLS_GUARD)
     {
       tree type_node = lang_hooks.types.type_for_mode (ptr_mode, 1);
-      addr_space_t as = DEFAULT_TLS_SEG_REG;
 
-      /* The kernel uses a different segment register for performance
-	 reasons; a system call would not have to trash the userspace
-	 segment register, which would be expensive.  */
-      if (ix86_cmodel == CM_KERNEL)
-	as = ADDR_SPACE_SEG_GS;
+      int qual = ENCODE_QUAL_ADDR_SPACE (ix86_stack_protector_guard_reg);
 
-      int qual = ENCODE_QUAL_ADDR_SPACE (as);
-
       tree type = build_qualified_type (type_node, qual);
       tree asptrtype = build_pointer_type (type);
-      tree sspoff = build_int_cst (asptrtype, TARGET_THREAD_SSP_OFFSET);
-
+      tree sspoff = build_int_cst (asptrtype,
+				   ix86_stack_protector_guard_offset);
       tree t = build2 (MEM_REF, asptrtype, sspoff,
 		       build_int_cst (asptrtype, 0));
       return t;
@@ -45823,7 +45878,6 @@ ix86_stack_protect_guard (void)
 
   return default_stack_protect_guard ();
 }
-#endif
 
 /* For 32-bit code we can save PIC register setup by using
    __stack_chk_fail_local hidden function instead of calling
@@ -52831,10 +52885,8 @@ ix86_run_selftests (void)
 #undef TARGET_MANGLE_TYPE
 #define TARGET_MANGLE_TYPE ix86_mangle_type
 
-#ifdef TARGET_THREAD_SSP_OFFSET
 #undef TARGET_STACK_PROTECT_GUARD
 #define TARGET_STACK_PROTECT_GUARD ix86_stack_protect_guard
-#endif
 
 #if !TARGET_MACHO
 #undef TARGET_STACK_PROTECT_FAIL
Index: config/i386/i386.opt
===================================================================
--- config/i386/i386.opt	(revision 250960)
+++ config/i386/i386.opt	(working copy)
@@ -924,6 +924,20 @@ Enum(stack_protector_guard) String(tls) Value(SSP_
 EnumValue
 Enum(stack_protector_guard) String(global) Value(SSP_GLOBAL)
 
+mstack-protector-guard-reg=
+Target RejectNegative Joined Var(ix86_stack_protector_guard_reg_str)
+Use the given base register for addressing the stack-protector guard.
+
+TargetVariable
+addr_space_t ix86_stack_protector_guard_reg = ADDR_SPACE_GENERIC
+
+mstack-protector-guard-offset=
+Target RejectNegative Joined Integer Var(ix86_stack_protector_guard_offset_str)
+Use the given offset for addressing the stack-protector guard.
+
+TargetVariable
+HOST_WIDE_INT ix86_stack_protector_guard_offset = 0
+
 mmitigate-rop
 Target Var(flag_mitigate_rop) Init(0)
 Attempt to avoid generating instruction sequences containing ret bytes.
Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 250960)
+++ doc/invoke.texi	(working copy)
@@ -1215,7 +1215,9 @@ See RS/6000 and PowerPC Options.
 -msse2avx  -mfentry  -mrecord-mcount  -mnop-mcount  -m8bit-idiv @gol
 -mavx256-split-unaligned-load  -mavx256-split-unaligned-store @gol
 -malign-data=@var{type}  -mstack-protector-guard=@var{guard} @gol
--mmitigate-rop  -mgeneral-regs-only  -mcall-ms2sysv-xlogues}
+-mstack-protector-guard-reg=@var{reg} @gol
+-mstack-protector-guard-offset=@var{offset}  -mmitigate-rop @gol
+-mgeneral-regs-only  -mcall-ms2sysv-xlogues}
 
 @emph{x86 Windows Options}
 @gccoptlist{-mconsole  -mcygwin  -mno-cygwin  -mdll @gol
@@ -26147,12 +26149,23 @@ to 255, 8-bit unsigned integer divide is used inst
 Split 32-byte AVX unaligned load and store.
 
 @item -mstack-protector-guard=@var{guard}
-@opindex 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 global canary or @samp{tls} for per-thread
 canary in the TLS block (the default).  This option has effect only when
 @option{-fstack-protector} or @option{-fstack-protector-all} is specified.
 
+With the latter choice the options
+@option{-mstack-protector-guard-reg=@var{reg}} and
+@option{-mstack-protector-guard-offset=@var{offset}} furthermore specify
+which segment register (@code{%fs} or @code{%gs}) to use as base register
+for reading the canary, and from what offset from that base register.
+The default for those is as specified in the relevant ABI.
+
 @item -mmitigate-rop
 @opindex mmitigate-rop
 Try to avoid generating code sequences that contain unintended return
Index: testsuite/gcc.target/i386/stack-prot-guard.c
===================================================================
--- testsuite/gcc.target/i386/stack-prot-guard.c	(nonexistent)
+++ testsuite/gcc.target/i386/stack-prot-guard.c	(working copy)
@@ -0,0 +1,6 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fstack-protector-all -mstack-protector-guard=tls -mstack-protector-guard-reg=gs -mstack-protector-guard-offset=0x3038" } */
+
+void f(void) { }
+
+/* { dg-final { scan-assembler "gs:12344" } } */

Re: [PATCH, i386]: Make stack canary location customizable (PR target/81708)

[Uros Bizjak]

[-- Attachment #1: Type: text/plain, Size: 1458 bytes --]

On Tue, Aug 8, 2017 at 6:54 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> Hello!
>
> Attached patch introduces -mstack-protector-guard-reg=  and
> -mstack-protector-guard-offset= options to make stack canary location
> customizable. These are the same options powerpc has.

Attached addition adds -mstack-protector-guard-symbol= option that
overrides the offset to TLS stack protector canary with a symbol name.
Using this option, stack protector canary can be loaded from specified
symbol, relative to guard reg:

gcc -O2 -fstack-protector-all -mstack-protector-guard=tls
-mstack-protector-guard-reg=gs -mstack-protector-guard-symbol=my_guard

        movq    %gs:my_guard(%rip), %rax
        movq    %rax, 8(%rsp)
        xorl    %eax, %eax
        movq    8(%rsp), %rax
        xorq    %gs:my_guard(%rip), %rax

2017-08-09  Uros Bizjak  <ubizjak@gmail.com>

    PR target/81708
    * config/i386/i386.opt (mstack-protector-guard-symbol=): New option
    * config/i386/i386.c (ix86_stack_protect_guard): Use
    ix86_stack_protect_guard_symbol_str to generate varible declaration.
    * doc/invoke.texi (x86 Options): Document
    -mstack-protector-guard-symbol= option.

testsuite/ChangeLog:

2017-08-09  Uros Bizjak  <ubizjak@gmail.com>

    PR target/81708
    * gcc.target/i386/stack-prot-sym.c: New test.

Patch was bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

I plan to commit the patch to mainline SVN in a couple of days.

Uros.

[-- Attachment #2: p.diff.txt --]
[-- Type: text/plain, Size: 4831 bytes --]

Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c	(revision 250999)
+++ config/i386/i386.c	(working copy)
@@ -45858,6 +45858,8 @@ ix86_mangle_type (const_tree type)
     }
 }
 
+static GTY(()) tree ix86_tls_stack_chk_guard_decl;
+
 static tree
 ix86_stack_protect_guard (void)
 {
@@ -45864,15 +45866,47 @@ ix86_stack_protect_guard (void)
   if (TARGET_SSP_TLS_GUARD)
     {
       tree type_node = lang_hooks.types.type_for_mode (ptr_mode, 1);
-
       int qual = ENCODE_QUAL_ADDR_SPACE (ix86_stack_protector_guard_reg);
+      tree type = build_qualified_type (type_node, qual);
+      tree t;
 
-      tree type = build_qualified_type (type_node, qual);
-      tree asptrtype = build_pointer_type (type);
-      tree sspoff = build_int_cst (asptrtype,
-				   ix86_stack_protector_guard_offset);
-      tree t = build2 (MEM_REF, asptrtype, sspoff,
-		       build_int_cst (asptrtype, 0));
+      if (global_options_set.x_ix86_stack_protector_guard_symbol_str)
+	{
+	  t = ix86_tls_stack_chk_guard_decl;
+
+	  if (t == NULL)
+	    {
+	      rtx x;
+
+	      t = build_decl
+		(UNKNOWN_LOCATION, VAR_DECL,
+		 get_identifier (ix86_stack_protector_guard_symbol_str),
+		 type);
+	      TREE_STATIC (t) = 1;
+	      TREE_PUBLIC (t) = 1;
+	      DECL_EXTERNAL (t) = 1;
+	      TREE_USED (t) = 1;
+	      TREE_THIS_VOLATILE (t) = 1;
+	      DECL_ARTIFICIAL (t) = 1;
+	      DECL_IGNORED_P (t) = 1;
+
+	      /* Do not share RTL as the declaration is visible outside of
+		 current function.  */
+	      x = DECL_RTL (t);
+	      RTX_FLAG (x, used) = 1;
+
+	      ix86_tls_stack_chk_guard_decl = t;
+	    }
+	}
+      else
+	{
+	  tree asptrtype = build_pointer_type (type);
+
+	  t = build_int_cst (asptrtype, ix86_stack_protector_guard_offset);
+	  t = build2 (MEM_REF, asptrtype, t,
+		      build_int_cst (asptrtype, 0));
+	}
+
       return t;
     }
 
Index: config/i386/i386.opt
===================================================================
--- config/i386/i386.opt	(revision 250999)
+++ config/i386/i386.opt	(working copy)
@@ -938,6 +938,10 @@ Use the given offset for addressing the stack-prot
 TargetVariable
 HOST_WIDE_INT ix86_stack_protector_guard_offset = 0
 
+mstack-protector-guard-symbol=
+Target RejectNegative Joined Integer Var(ix86_stack_protector_guard_symbol_str)
+Use the given symbol for addressing the stack-protector guard.
+
 mmitigate-rop
 Target Var(flag_mitigate_rop) Init(0)
 Attempt to avoid generating instruction sequences containing ret bytes.
Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 250999)
+++ doc/invoke.texi	(working copy)
@@ -1216,7 +1216,8 @@ See RS/6000 and PowerPC Options.
 -mavx256-split-unaligned-load  -mavx256-split-unaligned-store @gol
 -malign-data=@var{type}  -mstack-protector-guard=@var{guard} @gol
 -mstack-protector-guard-reg=@var{reg} @gol
--mstack-protector-guard-offset=@var{offset}  -mmitigate-rop @gol
+-mstack-protector-guard-offset=@var{offset} @gol
+-mstack-protector-guard-symbol=@var{symbol} -mmitigate-rop @gol
 -mgeneral-regs-only  -mcall-ms2sysv-xlogues}
 
 @emph{x86 Windows Options}
@@ -22753,9 +22754,11 @@ The @option{-mno-compat-align-parm} option is the
 @item -mstack-protector-guard=@var{guard}
 @itemx -mstack-protector-guard-reg=@var{reg}
 @itemx -mstack-protector-guard-offset=@var{offset}
+@itemx -mstack-protector-guard-symbol=@var{symbol}
 @opindex mstack-protector-guard
 @opindex mstack-protector-guard-reg
 @opindex mstack-protector-guard-offset
+@opindex mstack-protector-guard-symbol
 Generate stack protection code using canary at @var{guard}.  Supported
 locations are @samp{global} for global canary or @samp{tls} for per-thread
 canary in the TLS block (the default with GNU libc version 2.4 or later).
@@ -22765,7 +22768,8 @@ With the latter choice the options
 @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. The default for those is as specified in the
-relevant ABI.
+relevant ABI.  @option{-mstack-protector-guard-symbol=@var{symbol}} overrides
+the offset with a symbol reference to a canary in the TLS block.
 @end table
 
 @node RX Options
Index: testsuite/gcc.target/i386/stack-prot-sym.c
===================================================================
--- testsuite/gcc.target/i386/stack-prot-sym.c	(nonexistent)
+++ testsuite/gcc.target/i386/stack-prot-sym.c	(working copy)
@@ -0,0 +1,6 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fstack-protector-all -mstack-protector-guard=tls -mstack-protector-guard-reg=gs -mstack-protector-guard-symbol=my_guard" } */
+
+void f(void) { }
+
+/* { dg-final { scan-assembler "gs:my_guard" } } */