[Qemu-devel] [PATCH, build fix] osdep: Work around MinGW assert

[Qemu-devel] [PATCH, build fix] osdep: Work around MinGW assert

[Richard Henderson]

In several places we use assert(FEATURE), and assume that if FEATURE
is disabled, all following code is removed as unreachable.  Which allows
us to compile-out functions that are only present with FEATURE, and
have a link-time failure if the functions remain used.

MinGW does not mark its internal function _assert() as noreturn, so the
compiler cannot see when code is unreachable, which leads to link errors
for this host that are not present elsewhere.

The current build-time failure concerns 62823083b8a2, but I remember
having seen this same error before.  Fix it once and for all for MinGW.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/qemu/osdep.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 4f8559e550..0c1e335a43 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -122,6 +122,18 @@ extern int daemon(int, int);
 #include "glib-compat.h"
 #include "qemu/typedefs.h"
 
+/*
+ * For mingw, as of v6.0.0, the function implementing the assert macro is
+ * not marked a noreturn, so the compiler cannot delete code following an
+ * assert(false) as unused.  We rely on this within the code base to delete
+ * code that is unreachable when features are disabled.
+ * All supported versions of Glib's g_assert() satisfy this requirement.
+ */
+#ifdef __MINGW32__
+#undef assert
+#define assert(x)  g_assert(x)
+#endif
+
 /*
  * According to waitpid man page:
  * WCOREDUMP
-- 
2.17.2

Re: [Qemu-devel] [PATCH, build fix] osdep: Work around MinGW assert

[Richard Henderson]

On 10/22/18 7:16 PM, Richard Henderson wrote:
> + * not marked a noreturn, so the compiler cannot delete code following an

Bah.  Peter, if you apply this directly, can you please fix the grammar around
"marked a return" (either s/a/as/ or s/a// sound equally plausible for me).


r~

Re: [Qemu-devel] [PATCH, build fix] osdep: Work around MinGW assert

[Philippe Mathieu-Daudé]

On 22/10/18 20:16, Richard Henderson wrote:
> In several places we use assert(FEATURE), and assume that if FEATURE
> is disabled, all following code is removed as unreachable.  Which allows
> us to compile-out functions that are only present with FEATURE, and
> have a link-time failure if the functions remain used.
> 
> MinGW does not mark its internal function _assert() as noreturn, so the
> compiler cannot see when code is unreachable, which leads to link errors
> for this host that are not present elsewhere.
> 
> The current build-time failure concerns 62823083b8a2, but I remember
> having seen this same error before.  Fix it once and for all for MinGW.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>   include/qemu/osdep.h | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 4f8559e550..0c1e335a43 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -122,6 +122,18 @@ extern int daemon(int, int);
>   #include "glib-compat.h"
>   #include "qemu/typedefs.h"
>   
> +/*
> + * For mingw, as of v6.0.0, the function implementing the assert macro is
> + * not marked a noreturn, so the compiler cannot delete code following an
> + * assert(false) as unused.  We rely on this within the code base to delete
> + * code that is unreachable when features are disabled.
> + * All supported versions of Glib's g_assert() satisfy this requirement.
> + */
> +#ifdef __MINGW32__
> +#undef assert
> +#define assert(x)  g_assert(x)
> +#endif
> +
>   /*
>    * According to waitpid man page:
>    * WCOREDUMP
> 

[Qemu-devel] [PATCH, build fix] osdep: Work around MinGW assert

[Richard Henderson]

In several places we use assert(FEATURE), and assume that if FEATURE
is disabled, all following code is removed as unreachable.  Which allows
us to compile-out functions that are only present with FEATURE, and
have a link-time failure if the functions remain used.

MinGW does not mark its internal function _assert() as noreturn, so the
compiler cannot see when code is unreachable, which leads to link errors
for this host that are not present elsewhere.

The current build-time failure concerns 62823083b8a2, but I remember
having seen this same error before.  Fix it once and for all for MinGW.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/qemu/osdep.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 4f8559e550..0c1e335a43 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -122,6 +122,18 @@ extern int daemon(int, int);
 #include "glib-compat.h"
 #include "qemu/typedefs.h"
 
+/*
+ * For mingw, as of v6.0.0, the function implementing the assert macro is
+ * not marked a noreturn, so the compiler cannot delete code following an
+ * assert(false) as unused.  We rely on this within the code base to delete
+ * code that is unreachable when features are disabled.
+ * All supported versions of Glib's g_assert() satisfy this requirement.
+ */
+#ifdef __MINGW32__
+#undef assert
+#define assert(x)  g_assert(x)
+#endif
+
 /*
  * According to waitpid man page:
  * WCOREDUMP
-- 
2.17.2

Re: [Qemu-devel] [PATCH, build fix] osdep: Work around MinGW assert

[Philippe Mathieu-Daudé]

On 23/10/18 9:02, Richard Henderson wrote:
> In several places we use assert(FEATURE), and assume that if FEATURE
> is disabled, all following code is removed as unreachable.  Which allows
> us to compile-out functions that are only present with FEATURE, and
> have a link-time failure if the functions remain used.
> 
> MinGW does not mark its internal function _assert() as noreturn, so the
> compiler cannot see when code is unreachable, which leads to link errors
> for this host that are not present elsewhere.
> 
> The current build-time failure concerns 62823083b8a2, but I remember
> having seen this same error before.  Fix it once and for all for MinGW.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   include/qemu/osdep.h | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 4f8559e550..0c1e335a43 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -122,6 +122,18 @@ extern int daemon(int, int);
>   #include "glib-compat.h"
>   #include "qemu/typedefs.h"
>   
> +/*
> + * For mingw, as of v6.0.0, the function implementing the assert macro is
> + * not marked a noreturn, so the compiler cannot delete code following an
> + * assert(false) as unused.  We rely on this within the code base to delete
> + * code that is unreachable when features are disabled.
> + * All supported versions of Glib's g_assert() satisfy this requirement.
> + */
> +#ifdef __MINGW32__
> +#undef assert
> +#define assert(x)  g_assert(x)
> +#endif
> +
>   /*
>    * According to waitpid man page:
>    * WCOREDUMP
> 

Re: [Qemu-devel] [PATCH, build fix] osdep: Work around MinGW assert

[Peter Maydell]

On 22 October 2018 at 19:16, Richard Henderson
<richard.henderson@linaro.org> wrote:
> In several places we use assert(FEATURE), and assume that if FEATURE
> is disabled, all following code is removed as unreachable.  Which allows
> us to compile-out functions that are only present with FEATURE, and
> have a link-time failure if the functions remain used.
>
> MinGW does not mark its internal function _assert() as noreturn, so the
> compiler cannot see when code is unreachable, which leads to link errors
> for this host that are not present elsewhere.
>
> The current build-time failure concerns 62823083b8a2, but I remember
> having seen this same error before.  Fix it once and for all for MinGW.
>

Applied to master (with the typo fixed), thanks.

-- PMM