[Qemu-devel] [RFC PATCH qemu] dtc: Allow compiling with old gcc

Alexey Kardashevskiy posted 1 patch 7 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170308124955.35623-1-aik@ozlabs.ru
Test checkpatch passed
Test docker passed
configure | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[Qemu-devel] [RFC PATCH qemu] dtc: Allow compiling with old gcc
Posted by Alexey Kardashevskiy 7 years, 1 month ago
After 6e85fce0225f "dtc: Update requirement to v1.4.2" QEMU stopped
compiling in CentOS7:

In file included from /home/aik/p/qemu/dtc/libfdt/libfdt.h:54:0,
                 from /home/aik/p/qemu/device_tree.c:30:
/home/aik/p/qemu/dtc/libfdt/libfdt_env.h:64:0: error: "__bitwise" redefined [-Werror]
 #define __bitwise
 ^
In file included from /usr/include/asm/ptrace.h:27:0,
                 from /usr/include/asm/sigcontext.h:11,
                 from /usr/include/bits/sigcontext.h:27,
                 from /usr/include/signal.h:340,
                 from /home/aik/p/qemu/include/qemu/osdep.h:86,
                 from /home/aik/p/qemu/device_tree.c:14:
/usr/include/linux/types.h:21:0: note: this is the location of the previous definition
 #define __bitwise __bitwise__
 ^
cc1: all warnings being treated as errors
make: *** [device_tree.o] Error 1
make: *** Waiting for unfinished jobs....

The reason is that CentOS7 comes with libfdt 1.4.0 so QEMU tries using
the internal one which does not compile as CentOS7 comes with gcc v4.8.5
which reports warnings which it would not if the OS's libfdt was used
(libfdt_env.h has not changed between 1.4.0 and 1.4.2).

gcc 6.2.0 from Ubuntu v16.10 handles this fine.

This replaces -I with -isystem to suppress the warning (which turns
to an error because of -Werror).

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index 6c21975f02..2c2a5df14a 100755
--- a/configure
+++ b/configure
@@ -3414,7 +3414,7 @@ EOF
        symlink "$source_path/dtc/Makefile" "dtc/Makefile"
        symlink "$source_path/dtc/scripts" "dtc/scripts"
     fi
-    fdt_cflags="-I\$(SRC_PATH)/dtc/libfdt"
+    fdt_cflags="-isystem\$(SRC_PATH)/dtc/libfdt"
     fdt_libs="-L\$(BUILD_DIR)/dtc/libfdt $fdt_libs"
   elif test "$fdt" = "yes" ; then
     # have neither and want - prompt for system/submodule install
-- 
2.11.0


Re: [Qemu-devel] [RFC PATCH qemu] dtc: Allow compiling with old gcc
Posted by Peter Maydell 7 years, 1 month ago
On 8 March 2017 at 13:49, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> After 6e85fce0225f "dtc: Update requirement to v1.4.2" QEMU stopped
> compiling in CentOS7:
>
> In file included from /home/aik/p/qemu/dtc/libfdt/libfdt.h:54:0,
>                  from /home/aik/p/qemu/device_tree.c:30:
> /home/aik/p/qemu/dtc/libfdt/libfdt_env.h:64:0: error: "__bitwise" redefined [-Werror]
>  #define __bitwise
>  ^
> In file included from /usr/include/asm/ptrace.h:27:0,
>                  from /usr/include/asm/sigcontext.h:11,
>                  from /usr/include/bits/sigcontext.h:27,
>                  from /usr/include/signal.h:340,
>                  from /home/aik/p/qemu/include/qemu/osdep.h:86,
>                  from /home/aik/p/qemu/device_tree.c:14:
> /usr/include/linux/types.h:21:0: note: this is the location of the previous definition
>  #define __bitwise __bitwise__
>  ^
> cc1: all warnings being treated as errors
> make: *** [device_tree.o] Error 1
> make: *** Waiting for unfinished jobs....
>
> The reason is that CentOS7 comes with libfdt 1.4.0 so QEMU tries using
> the internal one which does not compile as CentOS7 comes with gcc v4.8.5
> which reports warnings which it would not if the OS's libfdt was used
> (libfdt_env.h has not changed between 1.4.0 and 1.4.2).
>
> gcc 6.2.0 from Ubuntu v16.10 handles this fine.
>
> This replaces -I with -isystem to suppress the warning (which turns
> to an error because of -Werror).

Thanks for the bug report. I think it would be cleaner to fix this
by fixing the problem upstream in libfdt and then moving our
submodule forward to the fixed version. (libfdt should not be
defining __ prefixed symbols as these are reserved for the
system.)

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH qemu] dtc: Allow compiling with old gcc
Posted by Alexey Kardashevskiy 7 years, 1 month ago
On 09/03/17 01:21, Peter Maydell wrote:
> On 8 March 2017 at 13:49, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>> After 6e85fce0225f "dtc: Update requirement to v1.4.2" QEMU stopped
>> compiling in CentOS7:
>>
>> In file included from /home/aik/p/qemu/dtc/libfdt/libfdt.h:54:0,
>>                  from /home/aik/p/qemu/device_tree.c:30:
>> /home/aik/p/qemu/dtc/libfdt/libfdt_env.h:64:0: error: "__bitwise" redefined [-Werror]
>>  #define __bitwise
>>  ^
>> In file included from /usr/include/asm/ptrace.h:27:0,
>>                  from /usr/include/asm/sigcontext.h:11,
>>                  from /usr/include/bits/sigcontext.h:27,
>>                  from /usr/include/signal.h:340,
>>                  from /home/aik/p/qemu/include/qemu/osdep.h:86,
>>                  from /home/aik/p/qemu/device_tree.c:14:
>> /usr/include/linux/types.h:21:0: note: this is the location of the previous definition
>>  #define __bitwise __bitwise__
>>  ^
>> cc1: all warnings being treated as errors
>> make: *** [device_tree.o] Error 1
>> make: *** Waiting for unfinished jobs....
>>
>> The reason is that CentOS7 comes with libfdt 1.4.0 so QEMU tries using
>> the internal one which does not compile as CentOS7 comes with gcc v4.8.5
>> which reports warnings which it would not if the OS's libfdt was used
>> (libfdt_env.h has not changed between 1.4.0 and 1.4.2).
>>
>> gcc 6.2.0 from Ubuntu v16.10 handles this fine.
>>
>> This replaces -I with -isystem to suppress the warning (which turns
>> to an error because of -Werror).
> 
> Thanks for the bug report. I think it would be cleaner to fix this

Agree.

> by fixing the problem upstream in libfdt and then moving our
> submodule forward to the fixed version. (libfdt should not be
> defining __ prefixed symbols as these are reserved for the
> system.)

Also agree, regardless my proposal, libfdt should be fixed. However v2.9
won't compile on CentOS7 (and I suppose on RHEL7) which is a bit annoying.
And having "-isystem" makes some sense as these headers are normally system
ones and I'd think they should be treated the same way (i.e. path should be
included via -isystem vs. -I).



-- 
Alexey

Re: [Qemu-devel] [RFC PATCH qemu] dtc: Allow compiling with old gcc
Posted by David Gibson 7 years, 1 month ago
On Thu, Mar 09, 2017 at 11:52:17AM +1100, Alexey Kardashevskiy wrote:
> On 09/03/17 01:21, Peter Maydell wrote:
> > On 8 March 2017 at 13:49, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> >> After 6e85fce0225f "dtc: Update requirement to v1.4.2" QEMU stopped
> >> compiling in CentOS7:
> >>
> >> In file included from /home/aik/p/qemu/dtc/libfdt/libfdt.h:54:0,
> >>                  from /home/aik/p/qemu/device_tree.c:30:
> >> /home/aik/p/qemu/dtc/libfdt/libfdt_env.h:64:0: error: "__bitwise" redefined [-Werror]
> >>  #define __bitwise
> >>  ^
> >> In file included from /usr/include/asm/ptrace.h:27:0,
> >>                  from /usr/include/asm/sigcontext.h:11,
> >>                  from /usr/include/bits/sigcontext.h:27,
> >>                  from /usr/include/signal.h:340,
> >>                  from /home/aik/p/qemu/include/qemu/osdep.h:86,
> >>                  from /home/aik/p/qemu/device_tree.c:14:
> >> /usr/include/linux/types.h:21:0: note: this is the location of the previous definition
> >>  #define __bitwise __bitwise__
> >>  ^
> >> cc1: all warnings being treated as errors
> >> make: *** [device_tree.o] Error 1
> >> make: *** Waiting for unfinished jobs....
> >>
> >> The reason is that CentOS7 comes with libfdt 1.4.0 so QEMU tries using
> >> the internal one which does not compile as CentOS7 comes with gcc v4.8.5
> >> which reports warnings which it would not if the OS's libfdt was used
> >> (libfdt_env.h has not changed between 1.4.0 and 1.4.2).
> >>
> >> gcc 6.2.0 from Ubuntu v16.10 handles this fine.
> >>
> >> This replaces -I with -isystem to suppress the warning (which turns
> >> to an error because of -Werror).
> > 
> > Thanks for the bug report. I think it would be cleaner to fix this
> 
> Agree.
> 
> > by fixing the problem upstream in libfdt and then moving our
> > submodule forward to the fixed version. (libfdt should not be
> > defining __ prefixed symbols as these are reserved for the
> > system.)

Yes.  I've now fixed this in upstream libfdt, at Paolo's suggestion.
Unfortunately, it came just after the 1.4.3 release.  I was just leery
about making a new release and qemu pull so soon after the last one.
Should I do that anyway?

> Also agree, regardless my proposal, libfdt should be fixed. However v2.9
> won't compile on CentOS7 (and I suppose on RHEL7) which is a bit annoying.
> And having "-isystem" makes some sense as these headers are normally system
> ones and I'd think they should be treated the same way (i.e. path should be
> included via -isystem vs. -I).

I'm not really clear on why -isystem fixes this problem anyway.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Re: [Qemu-devel] [RFC PATCH qemu] dtc: Allow compiling with old gcc
Posted by Alexey Kardashevskiy 7 years, 1 month ago
On 09/03/17 12:31, David Gibson wrote:
> On Thu, Mar 09, 2017 at 11:52:17AM +1100, Alexey Kardashevskiy wrote:
>> On 09/03/17 01:21, Peter Maydell wrote:
>>> On 8 March 2017 at 13:49, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>> After 6e85fce0225f "dtc: Update requirement to v1.4.2" QEMU stopped
>>>> compiling in CentOS7:
>>>>
>>>> In file included from /home/aik/p/qemu/dtc/libfdt/libfdt.h:54:0,
>>>>                  from /home/aik/p/qemu/device_tree.c:30:
>>>> /home/aik/p/qemu/dtc/libfdt/libfdt_env.h:64:0: error: "__bitwise" redefined [-Werror]
>>>>  #define __bitwise
>>>>  ^
>>>> In file included from /usr/include/asm/ptrace.h:27:0,
>>>>                  from /usr/include/asm/sigcontext.h:11,
>>>>                  from /usr/include/bits/sigcontext.h:27,
>>>>                  from /usr/include/signal.h:340,
>>>>                  from /home/aik/p/qemu/include/qemu/osdep.h:86,
>>>>                  from /home/aik/p/qemu/device_tree.c:14:
>>>> /usr/include/linux/types.h:21:0: note: this is the location of the previous definition
>>>>  #define __bitwise __bitwise__
>>>>  ^
>>>> cc1: all warnings being treated as errors
>>>> make: *** [device_tree.o] Error 1
>>>> make: *** Waiting for unfinished jobs....
>>>>
>>>> The reason is that CentOS7 comes with libfdt 1.4.0 so QEMU tries using
>>>> the internal one which does not compile as CentOS7 comes with gcc v4.8.5
>>>> which reports warnings which it would not if the OS's libfdt was used
>>>> (libfdt_env.h has not changed between 1.4.0 and 1.4.2).
>>>>
>>>> gcc 6.2.0 from Ubuntu v16.10 handles this fine.
>>>>
>>>> This replaces -I with -isystem to suppress the warning (which turns
>>>> to an error because of -Werror).
>>>
>>> Thanks for the bug report. I think it would be cleaner to fix this
>>
>> Agree.
>>
>>> by fixing the problem upstream in libfdt and then moving our
>>> submodule forward to the fixed version. (libfdt should not be
>>> defining __ prefixed symbols as these are reserved for the
>>> system.)
> 
> Yes.  I've now fixed this in upstream libfdt, at Paolo's suggestion.
> Unfortunately, it came just after the 1.4.3 release.  I was just leery
> about making a new release and qemu pull so soon after the last one.
> Should I do that anyway?
>
>> Also agree, regardless my proposal, libfdt should be fixed. However v2.9
>> won't compile on CentOS7 (and I suppose on RHEL7) which is a bit annoying.
>> And having "-isystem" makes some sense as these headers are normally system
>> ones and I'd think they should be treated the same way (i.e. path should be
>> included via -isystem vs. -I).
> 
> I'm not really clear on why -isystem fixes this problem anyway.

I believe it is a workaround as if all warnings were enabled, than many
(many) system headers would have triggered warnings because many have not
changed for quite a while and do not keep up with the actual gcc version.
"-isystem" tells gcc where it can relax warnings.

Pretty much like moving from older Microsoft Visual Studio to a newer one
would produce thousands warnings in my files (but not in system ones),
mostly about "deprecated" stuff but they could afford this as they ship
both the compiler and the standard library in one package :)


-- 
Alexey

Re: [Qemu-devel] [RFC PATCH qemu] dtc: Allow compiling with old gcc
Posted by Peter Maydell 7 years, 1 month ago
On 9 March 2017 at 02:31, David Gibson <david@gibson.dropbear.id.au> wrote:
> Yes.  I've now fixed this in upstream libfdt, at Paolo's suggestion.
> Unfortunately, it came just after the 1.4.3 release.  I was just leery
> about making a new release and qemu pull so soon after the last one.
> Should I do that anyway?

Yes, I would go ahead and do that.

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH qemu] dtc: Allow compiling with old gcc
Posted by David Gibson 7 years, 1 month ago
On Thu, Mar 09, 2017 at 07:43:07AM +0100, Peter Maydell wrote:
> On 9 March 2017 at 02:31, David Gibson <david@gibson.dropbear.id.au> wrote:
> > Yes.  I've now fixed this in upstream libfdt, at Paolo's suggestion.
> > Unfortunately, it came just after the 1.4.3 release.  I was just leery
> > about making a new release and qemu pull so soon after the last one.
> > Should I do that anyway?
> 
> Yes, I would go ahead and do that.

Ok, release tagged.  I'll send the qemu update once it's mirrored to qemu.org.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Re: [Qemu-devel] [RFC PATCH qemu] dtc: Allow compiling with old gcc
Posted by David Gibson 7 years, 1 month ago
On Thu, Mar 09, 2017 at 07:43:07AM +0100, Peter Maydell wrote:
> On 9 March 2017 at 02:31, David Gibson <david@gibson.dropbear.id.au> wrote:
> > Yes.  I've now fixed this in upstream libfdt, at Paolo's suggestion.
> > Unfortunately, it came just after the 1.4.3 release.  I was just leery
> > about making a new release and qemu pull so soon after the last one.
> > Should I do that anyway?
> 
> Yes, I would go ahead and do that.

Ok, done.  The submodule update is in the ppc pull request I sent a
few minutes ago.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson