[PATCH 0/2] Improve _FORTIFY_SOURCE robustness

[PATCH 0/2] Improve _FORTIFY_SOURCE robustness

[Daniel P. Berrangé]

This ensures that we don't turn on _FORTIFY_SOURCE when the user
turns off optimizations via CFLAGS env variable. See patch 2 for
the full details.

Daniel P. Berrangé (2):
  configure: support passthrough of -Dxxx args to meson
  osdep: set _FORTIFY_SOURCE=2 when optimization is enabled

 configure                    |  2 ++
 include/qemu/osdep.h         |  4 ++++
 meson.build                  | 10 ----------
 util/coroutine-sigaltstack.c |  4 ++--
 util/coroutine-ucontext.c    |  4 ++--
 5 files changed, 10 insertions(+), 14 deletions(-)

-- 
2.41.0

[PATCH 1/2] configure: support passthrough of -Dxxx args to meson

[Daniel P. Berrangé]

This can be useful for setting some meson global options, such as the
optimization level or debug state, which don't have an analogous
option explicitly defined in QEMU's configure wrapper script.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 configure | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configure b/configure
index e08127045d..cbd7e03e9f 100755
--- a/configure
+++ b/configure
@@ -931,6 +931,8 @@ cat << EOF
   bsd-user        all BSD usermode emulation targets
   pie             Position Independent Executables
 
+  -Dmesonoptname=val      passthrough option to meson unmodified
+
 NOTE: The object files are built at the place where configure is launched
 EOF
 exit 0
-- 
2.41.0

[PATCH 2/2] osdep: set _FORTIFY_SOURCE=2 when optimization is enabled

[Daniel P. Berrangé]

Currently we set _FORTIFY_SOURCE=2 as a compiler argument when the
meson 'optimization' setting is non-zero, the compiler is GCC and
the target is Linux.

While the default QEMU optimization level is 2, user could override
this by setting CFLAGS="-O0" or --extra-cflags="-O0" when running
configure and this won't be reflected in the meson 'optimization'
setting. As a result we try to enable _FORTIFY_SOURCE=2 and then the
user gets compile errors as it only works with optimization.

Rather than trying to improve detection in meson, it is simpler to
just check the __OPTIMIZE__ define from osdep.h.

The comment about being incompatible with clang appears to be
outdated, as compilation works fine without excluding clang.

In the coroutine code we must set _FORTIFY_SOURCE=0 to stop the
logic in osdep.h then enabling it.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/qemu/osdep.h         |  4 ++++
 meson.build                  | 10 ----------
 util/coroutine-sigaltstack.c |  4 ++--
 util/coroutine-ucontext.c    |  4 ++--
 4 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 2897720fac..b317649d13 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -27,6 +27,10 @@
 #ifndef QEMU_OSDEP_H
 #define QEMU_OSDEP_H
 
+#if !defined _FORTIFY_SOURCE && defined __OPTIMIZE__ && __OPTIMIZE__ && defined __linux__
+# define _FORTIFY_SOURCE 2
+#endif
+
 #include "config-host.h"
 #ifdef NEED_CPU_H
 #include CONFIG_TARGET
diff --git a/meson.build b/meson.build
index f426861d90..4947ae48b3 100644
--- a/meson.build
+++ b/meson.build
@@ -479,16 +479,6 @@ if 'cpp' in all_languages
   qemu_cxxflags = ['-D__STDC_LIMIT_MACROS', '-D__STDC_CONSTANT_MACROS', '-D__STDC_FORMAT_MACROS'] + qemu_cflags
 endif
 
-# clang does not support glibc + FORTIFY_SOURCE (is it still true?)
-if get_option('optimization') != '0' and targetos == 'linux'
-  if cc.get_id() == 'gcc'
-    qemu_cflags += ['-U_FORTIFY_SOURCE', '-D_FORTIFY_SOURCE=2']
-  endif
-  if 'cpp' in all_languages and cxx.get_id() == 'gcc'
-    qemu_cxxflags += ['-U_FORTIFY_SOURCE', '-D_FORTIFY_SOURCE=2']
-  endif
-endif
-
 add_project_arguments(qemu_cflags, native: false, language: 'c')
 add_project_arguments(cc.get_supported_arguments(warn_flags), native: false, language: 'c')
 if 'cpp' in all_languages
diff --git a/util/coroutine-sigaltstack.c b/util/coroutine-sigaltstack.c
index e2690c5f41..037d6416c4 100644
--- a/util/coroutine-sigaltstack.c
+++ b/util/coroutine-sigaltstack.c
@@ -22,9 +22,9 @@
  */
 
 /* XXX Is there a nicer way to disable glibc's stack check for longjmp? */
-#ifdef _FORTIFY_SOURCE
 #undef _FORTIFY_SOURCE
-#endif
+#define _FORTIFY_SOURCE 0
+
 #include "qemu/osdep.h"
 #include <pthread.h>
 #include "qemu/coroutine_int.h"
diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
index ddc98fb4f8..7b304c79d9 100644
--- a/util/coroutine-ucontext.c
+++ b/util/coroutine-ucontext.c
@@ -19,9 +19,9 @@
  */
 
 /* XXX Is there a nicer way to disable glibc's stack check for longjmp? */
-#ifdef _FORTIFY_SOURCE
 #undef _FORTIFY_SOURCE
-#endif
+#define _FORTIFY_SOURCE 0
+
 #include "qemu/osdep.h"
 #include <ucontext.h>
 #include "qemu/coroutine_int.h"
-- 
2.41.0

Re: [PATCH 1/2] configure: support passthrough of -Dxxx args to meson

[Peter Maydell]

On Fri, 22 Sept 2023 at 14:56, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> This can be useful for setting some meson global options, such as the
> optimization level or debug state, which don't have an analogous
> option explicitly defined in QEMU's configure wrapper script.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

The commit message says it's adding support for a new feature...

> ---
>  configure | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/configure b/configure
> index e08127045d..cbd7e03e9f 100755
> --- a/configure
> +++ b/configure
> @@ -931,6 +931,8 @@ cat << EOF
>    bsd-user        all BSD usermode emulation targets
>    pie             Position Independent Executables
>
> +  -Dmesonoptname=val      passthrough option to meson unmodified
> +
>  NOTE: The object files are built at the place where configure is launched
>  EOF
>  exit 0

...but the patch is only updating the --help text. Is there
a missing piece of code here ?

thanks
-- PMM

Re: [PATCH 1/2] configure: support passthrough of -Dxxx args to meson

[Thomas Huth]

On 22/09/2023 16.00, Peter Maydell wrote:
> On Fri, 22 Sept 2023 at 14:56, Daniel P. Berrangé <berrange@redhat.com> wrote:
>>
>> This can be useful for setting some meson global options, such as the
>> optimization level or debug state, which don't have an analogous
>> option explicitly defined in QEMU's configure wrapper script.
>>
>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> The commit message says it's adding support for a new feature...
> 
>> ---
>>   configure | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/configure b/configure
>> index e08127045d..cbd7e03e9f 100755
>> --- a/configure
>> +++ b/configure
>> @@ -931,6 +931,8 @@ cat << EOF
>>     bsd-user        all BSD usermode emulation targets
>>     pie             Position Independent Executables
>>
>> +  -Dmesonoptname=val      passthrough option to meson unmodified
>> +
>>   NOTE: The object files are built at the place where configure is launched
>>   EOF
>>   exit 0
> 
> ...but the patch is only updating the --help text. Is there
> a missing piece of code here ?

The patch has already been merged, see commit ff136d2a99253483f ... and IIRC 
I slightly modified it when picking it up (according to the patch 
description), so this here is likely a left-over of a rebase. Daniel, I 
think you can drop this patch here.

  Thomas

Re: [PATCH 1/2] configure: support passthrough of -Dxxx args to meson

[Daniel P. Berrangé]

On Fri, Sep 22, 2023 at 04:36:00PM +0200, Thomas Huth wrote:
> On 22/09/2023 16.00, Peter Maydell wrote:
> > On Fri, 22 Sept 2023 at 14:56, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > 
> > > This can be useful for setting some meson global options, such as the
> > > optimization level or debug state, which don't have an analogous
> > > option explicitly defined in QEMU's configure wrapper script.
> > > 
> > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > 
> > The commit message says it's adding support for a new feature...
> > 
> > > ---
> > >   configure | 2 ++
> > >   1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/configure b/configure
> > > index e08127045d..cbd7e03e9f 100755
> > > --- a/configure
> > > +++ b/configure
> > > @@ -931,6 +931,8 @@ cat << EOF
> > >     bsd-user        all BSD usermode emulation targets
> > >     pie             Position Independent Executables
> > > 
> > > +  -Dmesonoptname=val      passthrough option to meson unmodified
> > > +
> > >   NOTE: The object files are built at the place where configure is launched
> > >   EOF
> > >   exit 0
> > 
> > ...but the patch is only updating the --help text. Is there
> > a missing piece of code here ?
> 
> The patch has already been merged, see commit ff136d2a99253483f ... and IIRC
> I slightly modified it when picking it up (according to the patch
> description), so this here is likely a left-over of a rebase. Daniel, I
> think you can drop this patch here.

Opps, yes, a mistake updating an old branch.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|