| Message ID | 1396023542-19667-1-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Mar 28, 2014 at 5:19 PM, Paolo Bonzini <pbonzini@redhat.com> wrote: > From: Steven Noonan <steven@uplinklabs.net> > > 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-all, > no stack protector. > > Signed-off-by: Steven Noonan <snoonan@amazon.com> > Cc: Anthony Liguori <aliguori@amazon.com> > Reviewed-by: Stefan Weil <sw@weilnetz.de> > [Prefer -fstack-protector-all to -fstack-protector, suggested by > Laurent Desnogues. - Paolo] > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > configure | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/configure b/configure > index fb3bd05..eb0e7bb 100755 > --- a/configure > +++ b/configure > @@ -198,6 +198,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 > @@ -950,6 +951,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" > @@ -1219,6 +1224,7 @@ Advanced options (experts only): > --disable-sparse disable sparse checker (default) > --disable-strip disable stripping binaries > --disable-werror disable compilation abort on warning > + --disable-stack-protector disable compiler-provided stack protection > --disable-sdl disable SDL > --enable-sdl enable SDL > --with-sdlabi select preferred SDL ABI 1.2 or 2.0 > @@ -1439,9 +1445,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-all" > + 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 My understanding is that -fstack-protector, -fstack-protector-strong, and -fstack-protector-all are strictly ordered in terms of the number of functions that are checked, so you have changed the default behavior to check less functions for compilers that support -fstack-protector-strong. Is that what you had in mind? Also aren't there some versions of gcc that have -fstack-protector but not the other two options? Thanks, Laurent > # Workaround for http://gcc.gnu.org/PR55489. Happens with -fPIE/-fPIC and > -- > 1.8.5.3 >
Il 28/03/2014 18:41, Laurent Desnogues ha scritto: >> > + gcc_flags="-fstack-protector-strong -fstack-protector-all" >> > + 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 > My understanding is that -fstack-protector, -fstack-protector-strong, > and -fstack-protector-all are strictly ordered in terms of the number > of functions that are checked, so you have changed the default > behavior to check less functions for compilers that support > -fstack-protector-strong. Is that what you had in mind? Yes. -fstack-protector-all adds protection in places where it doesn't really matter, and that's why it has such a high cost. Paolo
On 3/28/14, 10:51 AM, "Paolo Bonzini" <pbonzini@redhat.com> wrote: >Il 28/03/2014 18:41, Laurent Desnogues ha scritto: >>> > + gcc_flags="-fstack-protector-strong -fstack-protector-all" >>> > + 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 >> My understanding is that -fstack-protector, -fstack-protector-strong, >> and -fstack-protector-all are strictly ordered in terms of the number >> of functions that are checked, so you have changed the default >> behavior to check less functions for compilers that support >> -fstack-protector-strong. Is that what you had in mind? > >Yes. -fstack-protector-all adds protection in places where it doesn't >really matter, and that's why it has such a high cost. Correct, -fstack-protector-all was too high impact. Sadly -fstack-protector-strong seems to only exist in RedHat-provided compilers, which I don't always use -- thus the new default this change provides doesn't really help, so I'd need to just do 'configure --disable-stack-protector' to avoid the performance penalty.
On 28/03/14 2:04 PM, Noonan, Steven wrote: > On 3/28/14, 10:51 AM, "Paolo Bonzini" <pbonzini@redhat.com> wrote: > >> Il 28/03/2014 18:41, Laurent Desnogues ha scritto: >>>>> + gcc_flags="-fstack-protector-strong -fstack-protector-all" >>>>> + 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 >>> My understanding is that -fstack-protector, -fstack-protector-strong, >>> and -fstack-protector-all are strictly ordered in terms of the number >>> of functions that are checked, so you have changed the default >>> behavior to check less functions for compilers that support >>> -fstack-protector-strong. Is that what you had in mind? >> >> Yes. -fstack-protector-all adds protection in places where it doesn't >> really matter, and that's why it has such a high cost. > > Correct, -fstack-protector-all was too high impact. Sadly > -fstack-protector-strong seems to only exist in RedHat-provided compilers, > which I don't always use -- thus the new default this change provides > doesn't really help, so I'd need to just do 'configure > --disable-stack-protector' to avoid the performance penalty. -fstack-protector-strong exists in OpenBSD's GCC and now LLVM too. I'd very much be interested in seeing this go in as we're already using -strong in our own package.
On 28 March 2014 16:19, Paolo Bonzini <pbonzini@redhat.com> wrote: > From: Steven Noonan <steven@uplinklabs.net> > > 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: Applied to master, thanks. -- PMM
diff --git a/configure b/configure index fb3bd05..eb0e7bb 100755 --- a/configure +++ b/configure @@ -198,6 +198,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 @@ -950,6 +951,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" @@ -1219,6 +1224,7 @@ Advanced options (experts only): --disable-sparse disable sparse checker (default) --disable-strip disable stripping binaries --disable-werror disable compilation abort on warning + --disable-stack-protector disable compiler-provided stack protection --disable-sdl disable SDL --enable-sdl enable SDL --with-sdlabi select preferred SDL ABI 1.2 or 2.0 @@ -1439,9 +1445,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-all" + 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