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

[PATCH RESEND] 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>
---

Re-sent due to previous bad patch submission

 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 18b940db75..475a1c62ff 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 5139db2ff7..b12ead85f6 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 RESEND] osdep: set _FORTIFY_SOURCE=2 when optimization is enabled

[Richard Henderson]

On 10/3/23 02:15, Daniel P. Berrangé wrote:
> 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>
> ---
> 
> Re-sent due to previous bad patch submission
> 
>   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(-)

I agree this is better than the meson fragment.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

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

[Stefan Hajnoczi]

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

On Tue, Oct 03, 2023 at 10:15:49AM +0100, Daniel P. Berrangé wrote:
> 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>
> ---
> 
> Re-sent due to previous bad patch submission
> 
>  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(-)

Thanks, applied to my block tree:
https://gitlab.com/stefanha/qemu/commits/block

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]