diff mbox

configure: add option to disable -fstack-protector flags

Message ID 1389643202-1890-1-git-send-email-snoonan@amazon.com
State New
Headers show

Commit Message

Steven Noonan Jan. 13, 2014, 8 p.m. UTC
The -fstack-protector flag family is useful for ensuring safety and for
debugging, but has a performance impact. Here are some boot time comparisons of
the various versions of -fstack-protector using qemu-system-arm on an x86_64
host:

    # -fstack-protector-all
    Startup finished in 1.810s (kernel) + 12.331s (initrd) + 49.016s (userspace) = 1min 3.159s
    Startup finished in 1.801s (kernel) + 12.287s (initrd) + 47.925s (userspace) = 1min 2.013s
    Startup finished in 1.812s (kernel) + 12.302s (initrd) + 47.995s (userspace) = 1min 2.111s

    # -fstack-protector-strong
    Startup finished in 1.744s (kernel) + 11.223s (initrd) + 44.688s (userspace) = 57.657s
    Startup finished in 1.721s (kernel) + 11.222s (initrd) + 44.194s (userspace) = 57.138s
    Startup finished in 1.693s (kernel) + 11.250s (initrd) + 44.426s (userspace) = 57.370s

    # -fstack-protector
    Startup finished in 1.705s (kernel) + 11.409s (initrd) + 43.563s (userspace) = 56.677s
    Startup finished in 1.877s (kernel) + 11.137s (initrd) + 43.719s (userspace) = 56.734s
    Startup finished in 1.708s (kernel) + 11.141s (initrd) + 43.628s (userspace) = 56.478s

    # no stack protector
    Startup finished in 1.743s (kernel) + 11.190s (initrd) + 43.709s (userspace) = 56.643s
    Startup finished in 1.763s (kernel) + 11.216s (initrd) + 43.767s (userspace) = 56.747s
    Startup finished in 1.711s (kernel) + 11.283s (initrd) + 43.878s (userspace) = 56.873s

This patch introduces a configure option to disable the stack protector
entirely, and conditional stack protector flag selection (in order, based on
availability): -fstack-protector-strong, -fstack-protector, no stack protector.

Signed-off-by: Steven Noonan <snoonan@amazon.com>
---
 configure | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

Comments

Stefan Weil Jan. 13, 2014, 8:27 p.m. UTC | #1
Hi,

please see comments below.

Am 13.01.2014 21:00, schrieb Steven Noonan:
> The -fstack-protector flag family is useful for ensuring safety and for
> debugging, but has a performance impact. Here are some boot time comparisons of
> the various versions of -fstack-protector using qemu-system-arm on an x86_64
> host:
> 
>     # -fstack-protector-all
>     Startup finished in 1.810s (kernel) + 12.331s (initrd) + 49.016s (userspace) = 1min 3.159s
>     Startup finished in 1.801s (kernel) + 12.287s (initrd) + 47.925s (userspace) = 1min 2.013s
>     Startup finished in 1.812s (kernel) + 12.302s (initrd) + 47.995s (userspace) = 1min 2.111s
> 
>     # -fstack-protector-strong
>     Startup finished in 1.744s (kernel) + 11.223s (initrd) + 44.688s (userspace) = 57.657s
>     Startup finished in 1.721s (kernel) + 11.222s (initrd) + 44.194s (userspace) = 57.138s
>     Startup finished in 1.693s (kernel) + 11.250s (initrd) + 44.426s (userspace) = 57.370s
> 
>     # -fstack-protector
>     Startup finished in 1.705s (kernel) + 11.409s (initrd) + 43.563s (userspace) = 56.677s
>     Startup finished in 1.877s (kernel) + 11.137s (initrd) + 43.719s (userspace) = 56.734s
>     Startup finished in 1.708s (kernel) + 11.141s (initrd) + 43.628s (userspace) = 56.478s
> 
>     # no stack protector
>     Startup finished in 1.743s (kernel) + 11.190s (initrd) + 43.709s (userspace) = 56.643s
>     Startup finished in 1.763s (kernel) + 11.216s (initrd) + 43.767s (userspace) = 56.747s
>     Startup finished in 1.711s (kernel) + 11.283s (initrd) + 43.878s (userspace) = 56.873s
> 
> This patch introduces a configure option to disable the stack protector
> entirely, and conditional stack protector flag selection (in order, based on
> availability): -fstack-protector-strong, -fstack-protector, no stack protector.
> 
> Signed-off-by: Steven Noonan <snoonan@amazon.com>
> ---
>  configure | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/configure b/configure
> index 07b6be3..c9c0b2c 100755
> --- a/configure
> +++ b/configure
> @@ -147,6 +147,7 @@ audio_win_int=""
>  cc_i386=i386-pc-linux-gnu-gcc
>  libs_qga=""
>  debug_info="yes"
> +stack_protector=""
>  
>  # Don't accept a target_list environment variable.
>  unset target_list
> @@ -879,6 +880,10 @@ for opt do
>    ;;
>    --disable-werror) werror="no"
>    ;;
> +  --enable-stack-protector) stack_protector="yes"
> +  ;;
> +  --disable-stack-protector) stack_protector="no"
> +  ;;
>    --disable-curses) curses="no"
>    ;;
>    --enable-curses) curses="yes"
> @@ -1117,6 +1122,7 @@ echo "  --enable-sparse          enable sparse checker"
>  echo "  --disable-sparse         disable sparse checker (default)"
>  echo "  --disable-strip          disable stripping binaries"
>  echo "  --disable-werror         disable compilation abort on warning"
> +echo "  --disable-stack-protector disable GCC-provided stack protection"


Clang also supports stack protection AFAIK, so "GCC-provided" can be
removed here (or replaced by "compiler").


>  echo "  --disable-sdl            disable SDL"
>  echo "  --enable-sdl             enable SDL"
>  echo "  --disable-gtk            disable gtk UI"
> @@ -1298,9 +1304,15 @@ for flag in $gcc_flags; do
>      fi
>  done
>  
> -if compile_prog "-Werror -fstack-protector-all" "" ; then
> -    QEMU_CFLAGS="$QEMU_CFLAGS -fstack-protector-all"
> -    LIBTOOLFLAGS="$LIBTOOLFLAGS -Wc,-fstack-protector-all"
> +if test "$stack_protector" != "no" ; then
> +  gcc_flags="-fstack-protector-strong -fstack-protector"
> +  for flag in $gcc_flags; do
> +    if compile_prog "-Werror $flag" "" ; then
> +      QEMU_CFLAGS="$QEMU_CFLAGS $flag"
> +      LIBTOOLFLAGS="$LIBTOOLFLAGS -Wc,$flag"
> +      break
> +    fi
> +  done
>  fi
>  
>  # Workaround for http://gcc.gnu.org/PR55489.  Happens with -fPIE/-fPIC and
> 


Reviewed-by: Stefan Weil <sw@weilnetz.de>

I think this patch can be used as a base for further improvements (MinGW
specific settings, error handling when user's choice does not work).

Maybe you will have to resend the patch as a top level patch (don't use
the reply function of your mailer). As Paolo said, Anthony might
overwise be unable to pick it up with his scripts.

Regards
Stefan
Noonan, Steven Jan. 13, 2014, 8:38 p.m. UTC | #2
On Mon, Jan 13, 2014 at 09:27:42PM +0100, Stefan Weil wrote:
> Hi,
> 
> please see comments below.
> 
> Am 13.01.2014 21:00, schrieb Steven Noonan:
> > The -fstack-protector flag family is useful for ensuring safety and for
> > debugging, but has a performance impact. Here are some boot time comparisons of
> > the various versions of -fstack-protector using qemu-system-arm on an x86_64
> > host:
> > 
> >     # -fstack-protector-all
> >     Startup finished in 1.810s (kernel) + 12.331s (initrd) + 49.016s (userspace) = 1min 3.159s
> >     Startup finished in 1.801s (kernel) + 12.287s (initrd) + 47.925s (userspace) = 1min 2.013s
> >     Startup finished in 1.812s (kernel) + 12.302s (initrd) + 47.995s (userspace) = 1min 2.111s
> > 
> >     # -fstack-protector-strong
> >     Startup finished in 1.744s (kernel) + 11.223s (initrd) + 44.688s (userspace) = 57.657s
> >     Startup finished in 1.721s (kernel) + 11.222s (initrd) + 44.194s (userspace) = 57.138s
> >     Startup finished in 1.693s (kernel) + 11.250s (initrd) + 44.426s (userspace) = 57.370s
> > 
> >     # -fstack-protector
> >     Startup finished in 1.705s (kernel) + 11.409s (initrd) + 43.563s (userspace) = 56.677s
> >     Startup finished in 1.877s (kernel) + 11.137s (initrd) + 43.719s (userspace) = 56.734s
> >     Startup finished in 1.708s (kernel) + 11.141s (initrd) + 43.628s (userspace) = 56.478s
> > 
> >     # no stack protector
> >     Startup finished in 1.743s (kernel) + 11.190s (initrd) + 43.709s (userspace) = 56.643s
> >     Startup finished in 1.763s (kernel) + 11.216s (initrd) + 43.767s (userspace) = 56.747s
> >     Startup finished in 1.711s (kernel) + 11.283s (initrd) + 43.878s (userspace) = 56.873s
> > 
> > This patch introduces a configure option to disable the stack protector
> > entirely, and conditional stack protector flag selection (in order, based on
> > availability): -fstack-protector-strong, -fstack-protector, no stack protector.
> > 
> > Signed-off-by: Steven Noonan <snoonan@amazon.com>
> > ---
> >  configure | 18 +++++++++++++++---
> >  1 file changed, 15 insertions(+), 3 deletions(-)
> > 
> > diff --git a/configure b/configure
> > index 07b6be3..c9c0b2c 100755
> > --- a/configure
> > +++ b/configure
> > @@ -147,6 +147,7 @@ audio_win_int=""
> >  cc_i386=i386-pc-linux-gnu-gcc
> >  libs_qga=""
> >  debug_info="yes"
> > +stack_protector=""
> >  
> >  # Don't accept a target_list environment variable.
> >  unset target_list
> > @@ -879,6 +880,10 @@ for opt do
> >    ;;
> >    --disable-werror) werror="no"
> >    ;;
> > +  --enable-stack-protector) stack_protector="yes"
> > +  ;;
> > +  --disable-stack-protector) stack_protector="no"
> > +  ;;
> >    --disable-curses) curses="no"
> >    ;;
> >    --enable-curses) curses="yes"
> > @@ -1117,6 +1122,7 @@ echo "  --enable-sparse          enable sparse checker"
> >  echo "  --disable-sparse         disable sparse checker (default)"
> >  echo "  --disable-strip          disable stripping binaries"
> >  echo "  --disable-werror         disable compilation abort on warning"
> > +echo "  --disable-stack-protector disable GCC-provided stack protection"
> 
> 
> Clang also supports stack protection AFAIK, so "GCC-provided" can be
> removed here (or replaced by "compiler").
> 

Fixed in v3 of the patch.

> 
> >  echo "  --disable-sdl            disable SDL"
> >  echo "  --enable-sdl             enable SDL"
> >  echo "  --disable-gtk            disable gtk UI"
> > @@ -1298,9 +1304,15 @@ for flag in $gcc_flags; do
> >      fi
> >  done
> >  
> > -if compile_prog "-Werror -fstack-protector-all" "" ; then
> > -    QEMU_CFLAGS="$QEMU_CFLAGS -fstack-protector-all"
> > -    LIBTOOLFLAGS="$LIBTOOLFLAGS -Wc,-fstack-protector-all"
> > +if test "$stack_protector" != "no" ; then
> > +  gcc_flags="-fstack-protector-strong -fstack-protector"
> > +  for flag in $gcc_flags; do
> > +    if compile_prog "-Werror $flag" "" ; then
> > +      QEMU_CFLAGS="$QEMU_CFLAGS $flag"
> > +      LIBTOOLFLAGS="$LIBTOOLFLAGS -Wc,$flag"
> > +      break
> > +    fi
> > +  done
> >  fi
> >  
> >  # Workaround for http://gcc.gnu.org/PR55489.  Happens with -fPIE/-fPIC and
> > 
> 
> 
> Reviewed-by: Stefan Weil <sw@weilnetz.de>
> 
> I think this patch can be used as a base for further improvements (MinGW
> specific settings, error handling when user's choice does not work).
> 
> Maybe you will have to resend the patch as a top level patch (don't use
> the reply function of your mailer). As Paolo said, Anthony might
> overwise be unable to pick it up with his scripts.

Thanks, I've done that now.

- Steven
diff mbox

Patch

diff --git a/configure b/configure
index 07b6be3..c9c0b2c 100755
--- a/configure
+++ b/configure
@@ -147,6 +147,7 @@  audio_win_int=""
 cc_i386=i386-pc-linux-gnu-gcc
 libs_qga=""
 debug_info="yes"
+stack_protector=""
 
 # Don't accept a target_list environment variable.
 unset target_list
@@ -879,6 +880,10 @@  for opt do
   ;;
   --disable-werror) werror="no"
   ;;
+  --enable-stack-protector) stack_protector="yes"
+  ;;
+  --disable-stack-protector) stack_protector="no"
+  ;;
   --disable-curses) curses="no"
   ;;
   --enable-curses) curses="yes"
@@ -1117,6 +1122,7 @@  echo "  --enable-sparse          enable sparse checker"
 echo "  --disable-sparse         disable sparse checker (default)"
 echo "  --disable-strip          disable stripping binaries"
 echo "  --disable-werror         disable compilation abort on warning"
+echo "  --disable-stack-protector disable GCC-provided stack protection"
 echo "  --disable-sdl            disable SDL"
 echo "  --enable-sdl             enable SDL"
 echo "  --disable-gtk            disable gtk UI"
@@ -1298,9 +1304,15 @@  for flag in $gcc_flags; do
     fi
 done
 
-if compile_prog "-Werror -fstack-protector-all" "" ; then
-    QEMU_CFLAGS="$QEMU_CFLAGS -fstack-protector-all"
-    LIBTOOLFLAGS="$LIBTOOLFLAGS -Wc,-fstack-protector-all"
+if test "$stack_protector" != "no" ; then
+  gcc_flags="-fstack-protector-strong -fstack-protector"
+  for flag in $gcc_flags; do
+    if compile_prog "-Werror $flag" "" ; then
+      QEMU_CFLAGS="$QEMU_CFLAGS $flag"
+      LIBTOOLFLAGS="$LIBTOOLFLAGS -Wc,$flag"
+      break
+    fi
+  done
 fi
 
 # Workaround for http://gcc.gnu.org/PR55489.  Happens with -fPIE/-fPIC and