On Wed, Nov 21, 2018 at 11:21 PM Jakub Jelinek <ja...@redhat.com> wrote:
>
> Hi!
>
> As I wrote in the PR, before PR81708 commits,
> while i386 defaulted to SSP_TLS rather than SSP_GLOBAL on everything but
> Android, the -mstack-protector-guard= switch controlled pretty much
> whether the i386.md special stack protector patterns are used (if tls)
> or whether generic code is used (global). These special stack protector
> patterns did one thing if TARGET_THREAD_SSP_OFFSET macro was defined
> (only defined on glibc targets) - code like:
> movq %fs:40, %rax
> movq %rax, -8(%rbp)
> xorl %eax, %eax
> in the prologue and
> movq -8(%rbp), %rdx
> xorq %fs:40, %rdx
> je .L4
> in the epilogue. If TARGET_THREAD_SSP_OFFSET macro wasn't defined, it would
> do instead:
> movq .refptr.__stack_chk_guard(%rip), %rax
> movq (%rax), %rcx
> movq %rcx, -8(%rbp)
> xorl %ecx, %ecx
> and
> movq .refptr.__stack_chk_guard(%rip), %rdx
> movq -8(%rbp), %rcx
> xorq (%rdx), %rcx
> je .L4
> (this is taken from 7.x cross to mingw).
> Finally, for Android or when -mstack-protector-guard=global was used, it
> emitted:
> movq __stack_chk_guard(%rip), %rax
> movq %rax, -8(%rbp)
> and
> movq __stack_chk_guard(%rip), %rdx
> cmpq %rdx, %rcx
> je .L4
> Note, apart from OS specific details, those =global sequences are similar
> to the =tls ones when TARGET_THREAD_SSP_OFFSET is not defined, the main
> difference is that the =tls ones are more secure as they clear registers
> containing the guard as quickly as possible. The PR81708 changes dropped
> the non-tls special stack_protector_* patterns from i386.md and now =tls
> implies really tls, but the default remained, so mingw32 or darwin still
> default to tls and just use 0 offset by default.
>
> So, this patch changes the default for mingw32, darwin and everything else
> except gnu-user*.h to be =global, and just forces those special i386.md
> more secure patterns unconditionally (slightly changing the generated code
> on Android, but it is one extra insn in prologue and one fewer in the
> epilogue).
>
> With this patch -mstack-protector-guard=tls is really for tls and =global
> for pure var access and user can override the defaults on non-glibc targets,
> but they should get a default that works there.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, plus tested with a
> cross to mingw, ok for trunk?
>
> 2018-11-21 Jakub Jelinek <ja...@redhat.com>
>
> PR target/85644
> PR target/86832
> * config/i386/i386.c (ix86_option_override_internal): Default
> ix86_stack_protector_guard to SSP_TLS only if TARGET_THREAD_SSP_OFFSET
> is defined.
> * config/i386/i386.md (stack_protect_set, stack_protect_set_<mode>,
> stack_protect_test, stack_protect_test_<mode>): Use empty condition
> instead of TARGET_SSP_TLS_GUARD.
OK for mainline and backports, with a bit of bikeshedding below.
Thanks,
Uros.
> --- gcc/config/i386/i386.c.jj 2018-11-20 21:39:00.905577452 +0100
> +++ gcc/config/i386/i386.c 2018-11-21 18:02:49.448049161 +0100
> @@ -4557,8 +4557,13 @@ ix86_option_override_internal (bool main
>
> /* Handle stack protector */
> if (!opts_set->x_ix86_stack_protector_guard)
> - opts->x_ix86_stack_protector_guard
> - = TARGET_HAS_BIONIC ? SSP_GLOBAL : SSP_TLS;
> + {
> + opts->x_ix86_stack_protector_guard = SSP_GLOBAL;
> +#ifdef TARGET_THREAD_SSP_OFFSET
> + if (!TARGET_HAS_BIONIC)
> + opts->x_ix86_stack_protector_guard = SSP_TLS;
> +#endif
> + }
How about:
--cut here--
/* Handle stack protector */
if (!opts_set->x_ix86_stack_protector_guard)
{
#ifdef TARGET_THREAD_SSP_OFFSET
if (!TARGET_HAS_BIONIC)
opts->x_ix86_stack_protector_guard = SSP_TLS;
else
#endif
opts->x_ix86_stack_protector_guard = SSP_GLOBAL;
}
--cut here--
> #ifdef TARGET_THREAD_SSP_OFFSET
> ix86_stack_protector_guard_offset = TARGET_THREAD_SSP_OFFSET;
> --- gcc/config/i386/i386.md.jj 2018-11-21 11:45:12.090721862 +0100
> +++ gcc/config/i386/i386.md 2018-11-21 18:03:46.166119350 +0100
> @@ -19010,7 +19010,7 @@ (define_insn "*prefetch_prefetchwt1"
> (define_expand "stack_protect_set"
> [(match_operand 0 "memory_operand")
> (match_operand 1 "memory_operand")]
> - "TARGET_SSP_TLS_GUARD"
> + ""
> {
> rtx (*insn)(rtx, rtx);
>
> @@ -19028,7 +19028,7 @@ (define_insn "stack_protect_set_<mode>"
> UNSPEC_SP_SET))
> (set (match_scratch:PTR 2 "=&r") (const_int 0))
> (clobber (reg:CC FLAGS_REG))]
> - "TARGET_SSP_TLS_GUARD"
> + ""
> "mov{<imodesuffix>}\t{%1, %2|%2, %1}\;mov{<imodesuffix>}\t{%2, %0|%0,
> %2}\;xor{l}\t%k2, %k2"
> [(set_attr "type" "multi")])
>
> @@ -19036,7 +19036,7 @@ (define_expand "stack_protect_test"
> [(match_operand 0 "memory_operand")
> (match_operand 1 "memory_operand")
> (match_operand 2)]
> - "TARGET_SSP_TLS_GUARD"
> + ""
> {
> rtx flags = gen_rtx_REG (CCZmode, FLAGS_REG);
>
> @@ -19059,7 +19059,7 @@ (define_insn "stack_protect_test_<mode>"
> (match_operand:PTR 2 "memory_operand" "m")]
> UNSPEC_SP_TEST))
> (clobber (match_scratch:PTR 3 "=&r"))]
> - "TARGET_SSP_TLS_GUARD"
> + ""
> "mov{<imodesuffix>}\t{%1, %3|%3, %1}\;xor{<imodesuffix>}\t{%2, %3|%3, %2}"
> [(set_attr "type" "multi")])
>
>
> Jakub
