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

[Uros Bizjak]

On Tue, Aug 8, 2017 at 6:54 PM, Uros Bizjak  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
movq8(%rsp), %rax
xorq%gs:my_guard(%rip), %rax

2017-08-09  Uros Bizjak  

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  

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.
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} 

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

[Uros Bizjak]

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  

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  

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.
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, , 0);
+#else
+  offset = strtoll (str, , 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 (-0x8000),
+HOST_WIDE_INT_C (0x7fff)))
+   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