[Qemu-devel] [PATCH v3 0/4] configure: clang 3.5.0 build fixes

[Qemu-devel] [PATCH v3 0/4] configure: clang 3.5.0 build fixes

[John Snow]

QEMU does not compile cleanly under clang 3.5.0.  These patches eliminate the
avalanche of warnings and make the build usable.

v3:
- Drop tricore patch, it's already being worked on elsewhere.
- Factor out flag check test.

v2:
 - Stole the series from Stefan
 - No changes to the -nopie patch, which I think was fine.
 - Fixed the -Wno-unknown-attributes patch.
 - Added a tricore fix for -Wabsolute-value
 - Added a workaround to help suppress ccache warnings

The result is that you *should* be able to use clang 3.5.0 *with* ccache and
-Werror and produce all targets.

John Snow (3):
  configure: factor out supported flag check
  configure: silence glib unknown attribute __alloc_size__
  configure: Add workaround for ccache and clang

Stefan Hajnoczi (1):
  configure: handle clang -nopie argument warning

 configure | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 66 insertions(+), 15 deletions(-)

-- 
2.1.0

[Qemu-devel] [PATCH v3 1/4] configure: handle clang -nopie argument warning

[John Snow]

From: Stefan Hajnoczi <stefanha@redhat.com>

gcc 4.9.2 treats -nopie as an error:

  cc: error: unrecognized command line option ‘-nopie’

clang 3.5.0 treats -nopie as a warning:

  clang: warning: argument unused during compilation: '-nopie'

The causes ./configure to fail with clang:

  ERROR: configure test passed without -Werror but failed with -Werror.

Make the -nopie test use -Werror so that compile_prog works for both gcc
and clang.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index f74a6fd..062df84 100755
--- a/configure
+++ b/configure
@@ -1589,7 +1589,7 @@ EOF
     fi
   fi
 
-  if compile_prog "-fno-pie" "-nopie"; then
+  if compile_prog "-Werror -fno-pie" "-nopie"; then
     CFLAGS_NOPIE="-fno-pie"
     LDFLAGS_NOPIE="-nopie"
   fi
-- 
2.1.0

[Qemu-devel] [PATCH v3 2/4] configure: factor out supported flag check

[John Snow]

Factor out the function that checks if a compiler
flag is supported or not.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 configure | 36 ++++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/configure b/configure
index 062df84..9a0e4ef 100755
--- a/configure
+++ b/configure
@@ -435,6 +435,12 @@ EOF
   compile_object
 }
 
+write_c_skeleton() {
+    cat > $TMPC <<EOF
+int main(void) { return 0; }
+EOF
+}
+
 if check_define __linux__ ; then
   targetos="Linux"
 elif check_define _WIN32 ; then
@@ -704,9 +710,7 @@ if test "$mingw32" = "yes" ; then
   # enable C99/POSIX format strings (needs mingw32-runtime 3.15 or later)
   QEMU_CFLAGS="-D__USE_MINGW_ANSI_STDIO=1 $QEMU_CFLAGS"
   LIBS="-lwinmm -lws2_32 -liphlpapi $LIBS"
-cat > $TMPC << EOF
-int main(void) { return 0; }
-EOF
+  write_c_skeleton;
   if compile_prog "" "-liberty" ; then
     LIBS="-liberty $LIBS"
   fi
@@ -1438,10 +1442,7 @@ if test -z "$werror" ; then
 fi
 
 # check that the C compiler works.
-cat > $TMPC <<EOF
-int main(void) { return 0; }
-EOF
-
+write_c_skeleton;
 if compile_object ; then
   : C compiler works ok
 else
@@ -1489,16 +1490,23 @@ gcc_flags="-Wno-string-plus-int $gcc_flags"
 # enable it for all configure tests. If a configure test failed due
 # to -Werror this would just silently disable some features,
 # so it's too error prone.
-cat > $TMPC << EOF
-int main(void) { return 0; }
-EOF
-for flag in $gcc_flags; do
+
+cc_has_warning_flag() {
+    if [ "$2" != "--keep-tmpc" ]; then
+        write_c_skeleton;
+    fi
+
     # Use the positive sense of the flag when testing for -Wno-wombat
     # support (gcc will happily accept the -Wno- form of unknown
     # warning options).
-    optflag="$(echo $flag | sed -e 's/^-Wno-/-W/')"
-    if compile_prog "-Werror $optflag" "" ; then
-	QEMU_CFLAGS="$QEMU_CFLAGS $flag"
+    optflag="$(echo $1 | sed -e 's/^-Wno-/-W/')"
+    compile_prog "-Werror $optflag" ""
+}
+
+write_c_skeleton;
+for flag in $gcc_flags; do
+    if cc_has_warning_flag $flag --keep-tmpc; then
+        QEMU_CFLAGS="$QEMU_CFLAGS $flag"
     fi
 done
 
-- 
2.1.0

Re: [Qemu-devel] [PATCH v3 2/4] configure: factor out supported flag check

[Stefan Hajnoczi]

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

On Thu, Mar 19, 2015 at 03:01:35PM -0400, John Snow wrote:
> +cc_has_warning_flag() {
> +    if [ "$2" != "--keep-tmpc" ]; then
> +        write_c_skeleton;
> +    fi
> +
>      # Use the positive sense of the flag when testing for -Wno-wombat
>      # support (gcc will happily accept the -Wno- form of unknown
>      # warning options).
> -    optflag="$(echo $flag | sed -e 's/^-Wno-/-W/')"
> -    if compile_prog "-Werror $optflag" "" ; then
> -	QEMU_CFLAGS="$QEMU_CFLAGS $flag"
> +    optflag="$(echo $1 | sed -e 's/^-Wno-/-W/')"
> +    compile_prog "-Werror $optflag" ""
> +}
> +
> +write_c_skeleton;
> +for flag in $gcc_flags; do
> +    if cc_has_warning_flag $flag --keep-tmpc; then

The only caller that uses --keep-tmpc wants TMPC to be the skeleton
program anyway.  The option can be dropped and the write_c_skeleton in
cc_has_warning_flag can be unconditional.

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH v3 2/4] configure: factor out supported flag check

[John Snow]

On 03/23/2015 09:03 AM, Stefan Hajnoczi wrote:
> On Thu, Mar 19, 2015 at 03:01:35PM -0400, John Snow wrote:
>> +cc_has_warning_flag() {
>> +    if [ "$2" != "--keep-tmpc" ]; then
>> +        write_c_skeleton;
>> +    fi
>> +
>>       # Use the positive sense of the flag when testing for -Wno-wombat
>>       # support (gcc will happily accept the -Wno- form of unknown
>>       # warning options).
>> -    optflag="$(echo $flag | sed -e 's/^-Wno-/-W/')"
>> -    if compile_prog "-Werror $optflag" "" ; then
>> -	QEMU_CFLAGS="$QEMU_CFLAGS $flag"
>> +    optflag="$(echo $1 | sed -e 's/^-Wno-/-W/')"
>> +    compile_prog "-Werror $optflag" ""
>> +}
>> +
>> +write_c_skeleton;
>> +for flag in $gcc_flags; do
>> +    if cc_has_warning_flag $flag --keep-tmpc; then
>
> The only caller that uses --keep-tmpc wants TMPC to be the skeleton
> program anyway.  The option can be dropped and the write_c_skeleton in
> cc_has_warning_flag can be unconditional.
>
> Stefan
>

I just figured there was no use in rewriting the same skeleton file 
unconditionally in a loop.

The helper is used later where we do want to regenerate the skeleton.

If you still want me to just make it unconditional, I can, but I don't 
see the point.

--js

Re: [Qemu-devel] [PATCH v3 2/4] configure: factor out supported flag check

[Paolo Bonzini]

On 23/03/2015 15:44, John Snow wrote:
>>
> 
> I just figured there was no use in rewriting the same skeleton file
> unconditionally in a loop.
> 
> The helper is used later where we do want to regenerate the skeleton.
> 
> If you still want me to just make it unconditional, I can, but I don't
> see the point.

configure doesn't really look like the best place to microoptimize. :)

Paolo

Re: [Qemu-devel] [PATCH v3 2/4] configure: factor out supported flag check

[Stefan Hajnoczi]

On Mon, Mar 23, 2015 at 2:44 PM, John Snow <jsnow@redhat.com> wrote:
> On 03/23/2015 09:03 AM, Stefan Hajnoczi wrote:
>>
>> On Thu, Mar 19, 2015 at 03:01:35PM -0400, John Snow wrote:
>>>
>>> +cc_has_warning_flag() {
>>> +    if [ "$2" != "--keep-tmpc" ]; then
>>> +        write_c_skeleton;
>>> +    fi
>>> +
>>>       # Use the positive sense of the flag when testing for -Wno-wombat
>>>       # support (gcc will happily accept the -Wno- form of unknown
>>>       # warning options).
>>> -    optflag="$(echo $flag | sed -e 's/^-Wno-/-W/')"
>>> -    if compile_prog "-Werror $optflag" "" ; then
>>> -       QEMU_CFLAGS="$QEMU_CFLAGS $flag"
>>> +    optflag="$(echo $1 | sed -e 's/^-Wno-/-W/')"
>>> +    compile_prog "-Werror $optflag" ""
>>> +}
>>> +
>>> +write_c_skeleton;
>>> +for flag in $gcc_flags; do
>>> +    if cc_has_warning_flag $flag --keep-tmpc; then
>>
>>
>> The only caller that uses --keep-tmpc wants TMPC to be the skeleton
>> program anyway.  The option can be dropped and the write_c_skeleton in
>> cc_has_warning_flag can be unconditional.
>>
>> Stefan
>>
>
> I just figured there was no use in rewriting the same skeleton file
> unconditionally in a loop.
>
> The helper is used later where we do want to regenerate the skeleton.
>
> If you still want me to just make it unconditional, I can, but I don't see
> the point.

Dropping it makes the code cleaner and simpler.  The option is not
needed for functionality or correctness.

IMO it's premature optimization.

Stefan

[Qemu-devel] [PATCH v3 3/4] configure: silence glib unknown attribute __alloc_size__

[John Snow]

The glib headers use GCC attributes.  Unfortunately the __GNUC__ and
__GNUC_MINOR__ version macros are also defined by clang, but clang
doesn't support the same attributes as GCC.

clang 3.5.0 does not support the __alloc_size__ attribute:

  https://github.com/llvm-mirror/clang/commit/c047507a9a79e89fc8339e074fa72822a7e7ea73

The following warning is produced:

  gstrfuncs.h:257:44: warning: unknown attribute '__alloc_size__' ignored [-Wunknown-attributes]
        G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(2);
          gmacros.h:67:45: note: expanded from macro 'G_GNUC_ALLOC_SIZE'
                #define G_GNUC_ALLOC_SIZE(x) __attribute__((__alloc_size__(x)))

This patch checks whether glib headers cause warnings and disables
-Wunknown-attributes if it is able to.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
 configure | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/configure b/configure
index 9a0e4ef..fbcbf50 100755
--- a/configure
+++ b/configure
@@ -2792,6 +2792,18 @@ if ! $pkg_config --atleast-version=2.38 glib-2.0; then
     glib_subprocess=no
 fi
 
+# Silence clang 3.5.0 warnings about glib attribute __alloc_size__ usage
+cat > $TMPC << EOF
+#include <glib.h>
+int main(void) { return 0; }
+EOF
+if ! compile_prog "$glib_cflags -Werror" "$glib_libs" ; then
+    if cc_has_warning_flag "-Wno-unknown-attributes"; then
+        glib_cflags="-Wno-unknown-attributes $glib_cflags"
+        CFLAGS="-Wno-unknown-attributes $CFLAGS"
+    fi
+fi
+
 ##########################################
 # SHA command probe for modules
 if test "$modules" = yes; then
-- 
2.1.0

[Qemu-devel] [PATCH v3 4/4] configure: Add workaround for ccache and clang

[John Snow]

Test if ccache is interfering with our life, and
disable its habit of trying to compile already pre-processed
versions of code if so.

In particular, clang has different semantic warnings based on
if the warning arose from a macro or not. By trying to build
preprocessed versions of code, we get more errors than we should.

ccache allows us to disable this feature, opting instead to
compile the original version instead of its preprocessed version.

This makes ccache much slower for cache misses, but at least it
becomes usable with QEMU/clang.

Thanks to Peter Eisentraut for his writeup on the issue:
http://peter.eisentraut.org/blog/2014/12/01/ccache-and-clang-part-3/

Signed-off-by: John Snow <jsnow@redhat.com>
---
 configure | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/configure b/configure
index fbcbf50..75219a9 100755
--- a/configure
+++ b/configure
@@ -4172,6 +4172,33 @@ if compile_prog "" "" ; then
     getauxval=yes
 fi
 
+########################################
+# check if ccache is interfering with
+# semantic analysis of macros
+
+ccache=no
+cat > $TMPC << EOF
+static const int Z = 1;
+#define fn() ({ Z; })
+#define TAUT(X) ((X) == Z)
+#define PAREN(X, Y) (X == Y)
+#define ID(X) (X)
+int main(int argc, char *argv[])
+{
+    int x = 0, y = 0;
+    x = ID(x);
+    x = fn();
+    fn();
+    if (PAREN(x, y)) return 0;
+    if (TAUT(Z)) return 0;
+    return 0;
+}
+EOF
+
+if ! compile_object; then
+    ccache=yes
+fi
+
 ##########################################
 # End of CC checks
 # After here, no more $cc or $ld runs
@@ -5462,6 +5489,10 @@ if test "$numa" = "yes"; then
   echo "CONFIG_NUMA=y" >> $config_host_mak
 fi
 
+if test "$ccache" = "yes"; then
+  echo "export CCACHE_CPP2=y" >> $config_host_mak
+fi
+
 # build tree in object directory in case the source is not in the current directory
 DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests"
 DIRS="$DIRS fsdev"
-- 
2.1.0

Re: [Qemu-devel] [PATCH v3 4/4] configure: Add workaround for ccache and clang

[Stefan Hajnoczi]

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

On Thu, Mar 19, 2015 at 03:01:37PM -0400, John Snow wrote:
> +########################################
> +# check if ccache is interfering with
> +# semantic analysis of macros
> +
> +ccache=no
> +cat > $TMPC << EOF
> +static const int Z = 1;
> +#define fn() ({ Z; })
> +#define TAUT(X) ((X) == Z)
> +#define PAREN(X, Y) (X == Y)
> +#define ID(X) (X)
> +int main(int argc, char *argv[])
> +{
> +    int x = 0, y = 0;
> +    x = ID(x);
> +    x = fn();
> +    fn();
> +    if (PAREN(x, y)) return 0;
> +    if (TAUT(Z)) return 0;
> +    return 0;
> +}
> +EOF
> +
> +if ! compile_object; then
> +    ccache=yes
> +fi

This is not checking whether ccache is enabled, so the variable name
"ccache" is misleading.

The name "ccache_cpp2" would be more descriptive.

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH v3 4/4] configure: Add workaround for ccache and clang

[Peter Maydell]

On 19 March 2015 at 19:01, John Snow <jsnow@redhat.com> wrote:
> Test if ccache is interfering with our life, and
> disable its habit of trying to compile already pre-processed
> versions of code if so.
>
> In particular, clang has different semantic warnings based on
> if the warning arose from a macro or not. By trying to build
> preprocessed versions of code, we get more errors than we should.
>
> ccache allows us to disable this feature, opting instead to
> compile the original version instead of its preprocessed version.
>
> This makes ccache much slower for cache misses, but at least it
> becomes usable with QEMU/clang.
>
> Thanks to Peter Eisentraut for his writeup on the issue:
> http://peter.eisentraut.org/blog/2014/12/01/ccache-and-clang-part-3/

This is really working around a bug in either ccache or
in the way Fedora has configured ccache, so I kind of
feel it ought to be dealt with there. However I don't
object too much to our including the workaround in our
configure...

-- PMM

Re: [Qemu-devel] [PATCH v3 4/4] configure: Add workaround for ccache and clang

[John Snow]

On 03/23/2015 09:11 AM, Peter Maydell wrote:
> On 19 March 2015 at 19:01, John Snow <jsnow@redhat.com> wrote:
>> Test if ccache is interfering with our life, and
>> disable its habit of trying to compile already pre-processed
>> versions of code if so.
>>
>> In particular, clang has different semantic warnings based on
>> if the warning arose from a macro or not. By trying to build
>> preprocessed versions of code, we get more errors than we should.
>>
>> ccache allows us to disable this feature, opting instead to
>> compile the original version instead of its preprocessed version.
>>
>> This makes ccache much slower for cache misses, but at least it
>> becomes usable with QEMU/clang.
>>
>> Thanks to Peter Eisentraut for his writeup on the issue:
>> http://peter.eisentraut.org/blog/2014/12/01/ccache-and-clang-part-3/
>
> This is really working around a bug in either ccache or
> in the way Fedora has configured ccache, so I kind of
> feel it ought to be dealt with there. However I don't
> object too much to our including the workaround in our
> configure...
>
> -- PMM
>

I feel like it might be an inescapable consequence of using both ccache 
and clang together on any system, not just Fedora.

This at least ONLY turns on the workaround if it is observed to 
interfering, so it should improve the "ease of build" for clang + ccache 
in general, and not just on Fedora.

Of course, even if it is a workaround that only benefits out of the box 
configurations on Fedora, It's still useful to be able to build from 
source on Fedora, instead of waiting for a package maintainer to update ...

So unless there's a strong reason against this workaround in particular, 
I think it is harmless enough for inclusion. It certainly won't make the 
build worse for anyone.

For cases where you might be using ccache and gcc, for instance, this 
workaround won't even trigger. Further good news: We don't suppress any 
extra warnings we don't have to. Instead, we just tell ccache to 
re-process a file instead of trying to compile the preprocessed version.

--js

Re: [Qemu-devel] [PATCH v3 4/4] configure: Add workaround for ccache and clang

[Peter Maydell]

On 23 March 2015 at 14:52, John Snow <jsnow@redhat.com> wrote:
> On 03/23/2015 09:11 AM, Peter Maydell wrote:
>> This is really working around a bug in either ccache or
>> in the way Fedora has configured ccache, so I kind of
>> feel it ought to be dealt with there. However I don't
>> object too much to our including the workaround in our
>> configure...

> I feel like it might be an inescapable consequence of using both ccache and
> clang together on any system, not just Fedora.

Right, but it's not *QEMU* specific, so it would be better
if either (a) ccache automatically enabled this if it
noticed it was being run for clang or (b) Fedora automatically
enabled this in their aliases which make 'clang' automatically
be "clang run via ccache".

Is there a bug filed against Fedora for this?

-- PMM

Re: [Qemu-devel] [PATCH v3 4/4] configure: Add workaround for ccache and clang

[John Snow]

On 03/23/2015 11:14 AM, Peter Maydell wrote:
> On 23 March 2015 at 14:52, John Snow <jsnow@redhat.com> wrote:
>> On 03/23/2015 09:11 AM, Peter Maydell wrote:
>>> This is really working around a bug in either ccache or
>>> in the way Fedora has configured ccache, so I kind of
>>> feel it ought to be dealt with there. However I don't
>>> object too much to our including the workaround in our
>>> configure...
>
>> I feel like it might be an inescapable consequence of using both ccache and
>> clang together on any system, not just Fedora.
>
> Right, but it's not *QEMU* specific, so it would be better

OK, true.

> if either (a) ccache automatically enabled this if it
> noticed it was being run for clang or (b) Fedora automatically
> enabled this in their aliases which make 'clang' automatically
> be "clang run via ccache".
>
> Is there a bug filed against Fedora for this?
>
> -- PMM
>

Not that I can see on the RH or ccache/LLVM bugzillas at a quick glance. 
I can try to file something for ccache after I look a little more carefully.

In the meantime, until things improve, I think this workaround is sane 
if we want to deal with clang failures less.

I'll clean up the other minor comments and resubmit.

--js