Re: [Qemu-devel] [PATCH 1/3] configure: add option to disable -fstack-protector flags

["Noonan, Steven" <snoonan@amazon.com>]

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

On Thu, Jan 09, 2014 at 06:18:07PM -0500, Brad Smith wrote:
> On 09/01/14 5:40 PM, Paolo Bonzini wrote:
> >Il 09/01/2014 22:55, Steven Noonan ha scritto:
> >>From: Steven Noonan <snoonan@amazon.com>
> >>
> >>The -fstack-protector flag family is useful for ensuring safety and for
> >>debugging, but has a performance impact. Here's a boot time comparison between
> >>a QEMU build of qemu-system-arm with and without the -fstack-protector-all
> >>flag:
> >>
> >>     # WITHOUT -fstack-protector-all
> >>     [root@localhost ~]# systemd-analyze
> >>     Startup finished in 1.744s (kernel) + 11.345s (initrd) + 47.164s (userspace) = 1min 255ms
> >>
> >>     # WITH -fstack-protector-all
> >>     [root@localhost ~]# systemd-analyze
> >>     Startup finished in 1.843s (kernel) + 12.262s (initrd) + 1min 3.480s (userspace) = 1min 17.587s
> >
> >Can you try -fstack-protector-strong?
> >
> >Probably the right thing to do is to pick in order
> >-fstack-protector-strong, -fstack-protector, and nothing.
> 
> +1
> 

OK, in order to get -fstack-protector-strong I had to get a Fedora box
up and running, so the compiler and build/execution environment are
different. I took three samples from each build, applying a clean 
QCOW2 snapshot before each run to avoid cross-boot confounding
variables:

    # -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

There are a few points to note:

  - stack-protector-all doesn't cost nearly as much in this build, for
	reasons unclear to me. It looks like the kernel/initrd timings are
	rather closely reproduced, but the userspace timing varies quite a
	bit.

  - stack-protector-strong's performance hit is indeed much less than
	stack-protector-all.

  - The builds with the plain old -fstack-protector flag and without any
	stack protector are tied for the lead in terms of performance.

Amended patch attached.

[-- Attachment #2: 0001-configure-add-option-to-disable-fstack-protector-fla.patch --]
[-- Type: text/x-diff, Size: 3726 bytes --]

>From cad2b5990debede2bc8dc8552904b2ef9e14af22 Mon Sep 17 00:00:00 2001
From: Steven Noonan <snoonan@amazon.com>
Date: Mon, 6 Jan 2014 13:39:20 -0800
Subject: [PATCH] configure: add option to disable -fstack-protector flags

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..a485e94 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="yes"
 
 # 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" = "yes" ; 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
-- 
1.8.5.2