diff mbox

[RFC,V2,AARCH64] : Re: [RFC] [PATCH, AARCH64] Machine descriptions to support stack smashing protection

Message ID CAJK_mQ3vP7+QpGRzRdmAfKG9t18yVf8csDBXBXXHq=NU5S5a-g@mail.gmail.com
State New
Headers show

Commit Message

Venkataramanan Kumar Nov. 26, 2013, 2:16 p.m. UTC
Hi Joseph/Jakub,

Attached is Version 2 patch that adds machine descriptions for stack
protection in Aarch64. I have removed the incorrect test case changes
from the previous patch.

To make GCC compatible with glibc, I have added a test for aarch64 in
"GCC/configure".
This tests for the glibc version >= 2.19, generate TLS based stack
guard access, otherwise will fall back to global variable
__stack_chk_guard based access.

Also I will be submitting a glibc patch to export __stack_chk_guard
and initialize it with proper value, to make sure that it coexists
with TLS based stack guard access for aarch64.

I have posted code snippet here.
http://gcc.gnu.org/ml/gcc-patches/2013-11/msg02968.html

ChangeLog:

2013-11-26  Venkataramanan Kumar  <venkataramanan.kumar@linaro.org>
        * configure.ac (gcc_cv_libc_provides_tls_ssp): Add test to
        check TLS support in target C library for Aarch64.
        * configure: Regenerate.
        * config.in: Regenerate.
        * config/aarch64/aarch64.md (stack_protect_set, stack_protect_test)
        (stack_protect_set_<mode>, stack_protect_test_<mode>): Add
        initial machine description for Stack Smashing Protector.
        * config/aarch64/aarch64-linux.h (TARGET_THREAD_SSP_OFFSET): Define.

2013-11-26  Venkataramanan Kumar  <venkataramanan.kumar@linaro.org>
        * g++.dg/fstack-protector-strong.C: Add aarch64 target.
        * gcc.dg/fstack-protector-strong.c: Likewise.

Let me know your feed back and please advice on improving it further.

regards,
Venkat.

On 23 November 2013 09:32, Venkataramanan Kumar
<venkataramanan.kumar@linaro.org> wrote:
> Hi Joseph,
>
> Thank you for the detail explanation.
>
>> You need to ensure that, when new glibc is built, whatever compiler it's
>> built with, it continues to export the __stack_chk_guard symbol at version
>> GLIBC_2.17.  Furthermore, if any released GCC version would generate
>> references to __stack_chk_guard when compiling code for AArch64 with stack
>> protection, you need to ensure __stack_chk_guard is a normal symbol not a
>> compat symbol so that people can continue to link against new glibc when
>> using old GCC.  If it's only a limited range of development versions of
>> GCC that could have generated code using __stack_chk_guard because
>> released versions didn't support stack protection on AArch64 at all, a
>> compat symbol would probably be OK (but you should still ensure that the
>> variable gets initialized with the correct value for any applications
>> built with those development versions of GCC).
>
> As you said when THREAD_SET_STACK_GUARD is set glibc does not export
> __stack_chk_guard. So I am planning to change the export logic by
> adding a new macro EXPORT_GLOBAL_STACK_GUARD
> and set it for Aarch64 port in glibc.
>
> ----snip----
> --- a/csu/libc-start.c
> +++ b/csu/libc-start.c
> -# ifndef THREAD_SET_STACK_GUARD
> +
> +#if !defined(THREAD_SET_STACK_GUARD) || defined(EXPORT_GLOBAL_STACK_GUARD)
>  /* Only exported for architectures that don't store the stack guard canary
>     in thread local area.  */
>  uintptr_t __stack_chk_guard attribute_relro;
> -# endif
> +#endif
> +
> ----snip----
>
> I will find a better way to version that symbol as well. I will sent a
> RFC patch to glibc mailing list.
>
> On the GCC side, trunk GCC configure script checks and sets
> TARGET_LIBC_PROVIDES_SSP support when glibc is >=2.4
>
> -----snip----
> # Test for stack protector support in target C library.
> { $as_echo "$as_me:${as_lineno-$LINENO}: checking __stack_chk_fail in
> target C library" >&5
> $as_echo_n "checking __stack_chk_fail in target C library... " >&6; }
> if test "${gcc_cv_libc_provides_ssp+set}" = set; then :
>   $as_echo_n "(cached) " >&6
> else
>   gcc_cv_libc_provides_ssp=no
>     case "$target" in
>        *-*-linux* | *-*-kfreebsd*-gnu | *-*-knetbsd*-gnu)
>       # glibc 2.4 and later provides __stack_chk_fail and
>       # either __stack_chk_guard, or TLS access to stack guard canary.
>
> if test $glibc_version_major -gt 2 \
>   || ( test $glibc_version_major -eq 2 && test $glibc_version_minor
> -ge 4 ); then :
>   gcc_cv_libc_provides_ssp=yes
>
>
> if test x$gcc_cv_libc_provides_ssp = xyes; then
>
> $as_echo "#define TARGET_LIBC_PROVIDES_SSP 1" >>confdefs.h
> fi
> ----snip-----
>
> To make GCC for AArch64 generate TLS based stack access for glibc >=
> 2.19 I need to introduce a new macro
> TARGET_LIBC_PROVIDES_TLS_SSP and check and set it for glibc >= 2.19 in
> GCC configure .
>
> Any better approach to this since it is specific to Aarch64?
>
> regards,
> Venkat.
>
> On 20 November 2013 22:38, Joseph S. Myers <joseph@codesourcery.com> wrote:
>> On Wed, 20 Nov 2013, Venkataramanan Kumar wrote:
>>
>>> > I would like to see a clear description of what happens with all eight
>>> > combinations of (glibc version does or doesn't support this, GCC building
>>> > glibc does or doesn't support this, GCC building user program does or
>>> > doesn't support this).  Which of the (GCC building glibc, glibc)
>>> > combinations will successfully build glibc?  Will all such glibcs be
>>> > binary-compatible?  Will both old and new GCC work for building user
>>> > programs with both old and new glibc?
>>>
>>> Can you please clarify why we need to consider "the gcc compiler that
>>> builds the glibc" in the combinations you want me to describe. I am
>>> not able to understand that.
>>
>> Let's imagine this support goes in GCC 4.9 and the glibc support goes in
>> glibc 2.19, whereas GCC 4.8 and glibc 2.18 are versions without this
>> support.
>>
>> * Building glibc 2.18 with GCC 4.8 already works (I presume).
>>
>> * Suppose you use GCC 4.9 to build glibc 2.18.  Does this work?  If it
>> works, it must produce a glibc binary that's ABI compatible with one built
>> with GCC 4.8, meaning same symbols exported at same symbol versions.
>>
>> * Suppose you build glibc 2.19 with GCC 4.8.  Does this work?  If it does,
>> then it must be ABI compatible with 2.18 (meaning the symbols exported at
>> GLIBC_2.18 or earlier versions must be exactly the same as exported at
>> those versions in 2.18).
>>
>> * Suppose you build glibc 2.19 with GCC 4.9.  This needs to work and must
>> again produce a binary compatible with the previous ones.
>>
>> Note: there is no current glibc support for architectures that gained
>> TLS-based stack guards after 2.4 (meaning they need both a TLS-based
>> scheme and backwards compatibility for binaries using __stack_chk_guard),
>> and your glibc patch doesn't seem to add any.  It looks to me like your
>> glibc patch would have removed the __stack_chk_guard symbol and so failed
>> ABI tests (this symbol being defined only if THREAD_SET_STACK_GUARD isn't
>> defined) - you didn't make clear if your patch was tested with the glibc
>> testsuite including passing the ABI tests.  The normal presumption is that
>> it's not acceptable to remove exported symbols from glibc as some
>> application might be using them.
>>
>> You need to ensure that, when new glibc is built, whatever compiler it's
>> built with, it continues to export the __stack_chk_guard symbol at version
>> GLIBC_2.17.  Furthermore, if any released GCC version would generate
>> references to __stack_chk_guard when compiling code for AArch64 with stack
>> protection, you need to ensure __stack_chk_guard is a normal symbol not a
>> compat symbol so that people can continue to link against new glibc when
>> using old GCC.  If it's only a limited range of development versions of
>> GCC that could have generated code using __stack_chk_guard because
>> released versions didn't support stack protection on AArch64 at all, a
>> compat symbol would probably be OK (but you should still ensure that the
>> variable gets initialized with the correct value for any applications
>> built with those development versions of GCC).
>>
>> --
>> Joseph S. Myers
>> joseph@codesourcery.com

Comments

Richard Earnshaw Nov. 26, 2013, 2:53 p.m. UTC | #1
On 26/11/13 14:16, Venkataramanan Kumar wrote:
> Index: gcc/testsuite/gcc.dg/fstack-protector-strong.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/fstack-protector-strong.c	(revision 205378)
> +++ gcc/testsuite/gcc.dg/fstack-protector-strong.c	(working copy)
> @@ -1,6 +1,6 @@
>  /* Test that stack protection is done on chosen functions. */
>  
> -/* { dg-do compile { target i?86-*-* x86_64-*-* rs6000-*-* s390x-*-* } } */
> +/* { dg-do compile { target i?86-*-* x86_64-*-* rs6000-*-* s390x-*-* aarch64-*-* } } */
>  /* { dg-options "-O2 -fstack-protector-strong" } */
>  

I don't think it's good to have long lists of targets on generic tests.
 Can we factor this out into a target-supports option?

R.
diff mbox

Patch

Index: gcc/config/aarch64/aarch64-linux.h
===================================================================
--- gcc/config/aarch64/aarch64-linux.h	(revision 205378)
+++ gcc/config/aarch64/aarch64-linux.h	(working copy)
@@ -43,4 +43,9 @@ 
     }						\
   while (0)
 
+#ifdef TARGET_LIBC_PROVIDES_TLS_SSP
+/* Aarch64 glibc provides __stack_chk_guard in [tp - 0x8].  */
+#define TARGET_THREAD_SSP_OFFSET (-1 * GET_MODE_SIZE (ptr_mode))
+#endif
+
 #endif  /* GCC_AARCH64_LINUX_H */
Index: gcc/config/aarch64/aarch64.md
===================================================================
--- gcc/config/aarch64/aarch64.md	(revision 205378)
+++ gcc/config/aarch64/aarch64.md	(working copy)
@@ -99,6 +99,10 @@ 
     UNSPEC_TLSDESC
     UNSPEC_USHL_2S
     UNSPEC_VSTRUCTDUMMY
+    UNSPEC_SP_SET
+    UNSPEC_SP_TEST
+    UNSPEC_SP_TLS_SET
+    UNSPEC_SP_TLS_TEST
 ])
 
 (define_c_enum "unspecv" [
@@ -3625,6 +3629,80 @@ 
   DONE;
 })
 
+
+;; Named patterns for stack smashing protection.
+(define_expand "stack_protect_set"
+  [(match_operand 0 "memory_operand")
+   (match_operand 1 "memory_operand")]
+  ""
+{
+  enum machine_mode mode = GET_MODE (operands[0]);
+
+#ifdef TARGET_THREAD_SSP_OFFSET
+  rtx tlsreg = gen_reg_rtx (Pmode);
+  emit_insn (gen_aarch64_load_tp_hard (tlsreg));
+  rtx addr = gen_rtx_PLUS (Pmode, tlsreg, GEN_INT (TARGET_THREAD_SSP_OFFSET));
+  operands[1] = gen_rtx_MEM (Pmode, addr);
+#endif
+
+  emit_insn ((mode == DImode
+	      ? gen_stack_protect_set_di
+	      : gen_stack_protect_set_si) (operands[0], operands[1]));
+  DONE;
+})
+
+(define_insn "stack_protect_set_<mode>"
+  [(set (match_operand:PTR 0 "memory_operand" "=m")
+	(unspec:PTR [(match_operand:PTR 1 "memory_operand" "m")]
+	 UNSPEC_SP_SET))
+   (set (match_scratch:PTR 2 "=&r") (const_int 0))]
+  ""
+  "ldr\\t%x2, %1\;str\\t%x2, %0\;mov\t%x2,0"
+  [(set_attr "length" "12")])
+
+(define_expand "stack_protect_test"
+  [(match_operand 0 "memory_operand")
+   (match_operand 1 "memory_operand")
+   (match_operand 2)]
+  ""
+{
+
+#ifdef TARGET_THREAD_SSP_OFFSET
+  rtx tlsreg = gen_reg_rtx (Pmode);
+  emit_insn (gen_aarch64_load_tp_hard (tlsreg));
+  rtx addr = gen_rtx_PLUS (Pmode, tlsreg, GEN_INT (TARGET_THREAD_SSP_OFFSET));
+  operands[1] = gen_rtx_MEM (Pmode, addr);
+#endif
+
+  rtx result = gen_reg_rtx (Pmode);
+
+  enum machine_mode mode = GET_MODE (operands[0]);
+
+  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:PTR 0 "register_operand")
+	(unspec:PTR [(match_operand:PTR 1 "memory_operand" "m")
+		     (match_operand:PTR 2 "memory_operand" "m")]
+	 UNSPEC_SP_TEST))
+   (clobber (match_scratch:PTR 3 "=&r"))]
+  ""
+  "ldr\t%x3, %x1\;ldr\t%x0, %x2\;eor\t%x0, %x3, %x0"
+  [(set_attr "length" "12")])
+
 ;; AdvSIMD Stuff
 (include "aarch64-simd.md")
 
Index: gcc/configure
===================================================================
--- gcc/configure	(revision 205378)
+++ gcc/configure	(working copy)
@@ -27127,6 +27127,35 @@ 
 
 fi
 
+# Test for tls based stack protector support in target C library.
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking TLS stack guard support in target C library" >&5
+$as_echo_n "checking TLS stack guard support in target C library... " >&6; }
+if test "${gcc_cv_libc_provides_tls_ssp+set}" = set; then :
+  $as_echo_n "(cached) " >&6
+else
+  gcc_cv_libc_provides_tls_ssp=no
+   case "$target" in
+      aarch64-*-linux* | aarch64-*-kfreebsd*-gnu | aarch64-*-knetbsd*-gnu)
+     # glibc 2.19 and later provides TLS access to stack guard canary
+     # currently set for Aarch64.
+
+if test $glibc_version_major -gt 2 \
+  || ( test $glibc_version_major -eq 2 && test $glibc_version_minor -ge 19 ); then :
+  gcc_cv_libc_provides_tls_ssp=yes
+fi
+	;;
+   *) gcc_cv_libc_provides_tls_ssp=no ;;
+   esac
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_libc_provides_tls_ssp" >&5
+$as_echo "$gcc_cv_libc_provides_tls_ssp" >&6; }
+
+if test x$gcc_cv_libc_provides_tls_ssp = xyes; then
+
+$as_echo "#define TARGET_LIBC_PROVIDES_TLS_SSP 1" >>confdefs.h
+
+fi
+
 # Test for <sys/sdt.h> on the target.
 
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking sys/sdt.h in the target C library" >&5
Index: gcc/testsuite/gcc.dg/fstack-protector-strong.c
===================================================================
--- gcc/testsuite/gcc.dg/fstack-protector-strong.c	(revision 205378)
+++ gcc/testsuite/gcc.dg/fstack-protector-strong.c	(working copy)
@@ -1,6 +1,6 @@ 
 /* Test that stack protection is done on chosen functions. */
 
-/* { dg-do compile { target i?86-*-* x86_64-*-* rs6000-*-* s390x-*-* } } */
+/* { dg-do compile { target i?86-*-* x86_64-*-* rs6000-*-* s390x-*-* aarch64-*-* } } */
 /* { dg-options "-O2 -fstack-protector-strong" } */
 
 #include<string.h>
Index: gcc/testsuite/g++.dg/fstack-protector-strong.C
===================================================================
--- gcc/testsuite/g++.dg/fstack-protector-strong.C	(revision 205378)
+++ gcc/testsuite/g++.dg/fstack-protector-strong.C	(working copy)
@@ -1,6 +1,6 @@ 
 /* Test that stack protection is done on chosen functions. */
 
-/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
+/* { dg-do compile { target i?86-*-* x86_64-*-* aarch64*-*-* } } */
 /* { dg-options "-O2 -fstack-protector-strong" } */
 
 class A
Index: gcc/config.in
===================================================================
--- gcc/config.in	(revision 205378)
+++ gcc/config.in	(working copy)
@@ -1828,6 +1828,12 @@ 
 #endif
 
 
+/* Define if your target C library provides TLS stack protector support. */
+#ifndef USED_FOR_TARGET
+#undef TARGET_LIBC_PROVIDES_TLS_SSP
+#endif
+
+
 /* Define to 1 if you can safely include both <sys/time.h> and <time.h>. */
 #ifndef USED_FOR_TARGET
 #undef TIME_WITH_SYS_TIME
Index: gcc/configure.ac
===================================================================
--- gcc/configure.ac	(revision 205378)
+++ gcc/configure.ac	(working copy)
@@ -4896,6 +4896,24 @@ 
 	    [Define if your target C library provides stack protector support])
 fi
 
+# Test for tls based stack protector support in target C library.
+AC_CACHE_CHECK(TLS stack guard support in target C library,
+   gcc_cv_libc_provides_tls_ssp,
+   [gcc_cv_libc_provides_tls_ssp=no
+   case "$target" in
+      aarch64-*-linux* | aarch64-*-kfreebsd*-gnu | aarch64-*-knetbsd*-gnu)
+     # glibc 2.19 and later provides TLS access to stack guard canary
+     # currently set for Aarch64.
+     GCC_GLIBC_VERSION_GTE_IFELSE([2], [19], [gcc_cv_libc_provides_tls_ssp=yes])
+	;;
+   *) gcc_cv_libc_provides_tls_ssp=no ;;
+   esac])
+
+if test x$gcc_cv_libc_provides_tls_ssp = xyes; then
+  AC_DEFINE(TARGET_LIBC_PROVIDES_TLS_SSP, 1,
+	    [Define if your target C library provides TLS stack protector support.])
+fi
+
 # Test for <sys/sdt.h> on the target.
 GCC_TARGET_TEMPLATE([HAVE_SYS_SDT_H])
 AC_MSG_CHECKING(sys/sdt.h in the target C library)