[Qemu-devel] [PATCH] pass $($*.o-cflags) first to gcc/g++

[Qemu-devel] [PATCH] pass $($*.o-cflags) first to gcc/g++

[Stefano Stabellini]

rules.mak adds cflags specific to the target source file ($($@-cflags))
for last on the compiler command line.

As a consequence when compiling arm-a64.o, g++ might end up picking the
wrong utils.h header file, because it looks for utils.h on all the other
include paths first.

Fix the issue by passing the source file specific cflags first.
Do it consisently for *.c, *.cc, etc.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

diff --git a/rules.mak b/rules.mak
index ba2f4c1..da825e7 100644
--- a/rules.mak
+++ b/rules.mak
@@ -28,7 +28,7 @@ expand-objs = $(strip $(sort $(filter %.o,$1)) \
                   $(filter-out %.o %.mo,$1))
 
 %.o: %.c
-	$(call quiet-command,$(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) $($@-cflags) -c -o $@ $<,"  CC    $(TARGET_DIR)$@")
+	$(call quiet-command,$(CC)  $($@-cflags) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) -c -o $@ $<,"  CC    $(TARGET_DIR)$@")
 %.o: %.rc
 	$(call quiet-command,$(WINDRES) -I. -o $@ $<,"  RC    $(TARGET_DIR)$@")
 
@@ -42,7 +42,7 @@ LINK = $(call quiet-command, $(LINKPROG) $(QEMU_CFLAGS) $(CFLAGS) $(LDFLAGS) -o
 else
 LIBTOOL += $(if $(V),,--quiet)
 %.lo: %.c
-	$(call quiet-command,$(LIBTOOL) --mode=compile --tag=CC $(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) $($*.o-cflags) -c -o $@ $<,"  lt CC $@")
+	$(call quiet-command,$(LIBTOOL) --mode=compile --tag=CC $(CC) $($*.o-cflags) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) -c -o $@ $<,"  lt CC $@")
 %.lo: %.rc
 	$(call quiet-command,$(LIBTOOL) --mode=compile --tag=RC $(WINDRES) -I. -o $@ $<,"lt RC   $(TARGET_DIR)$@")
 %.lo: %.dtrace
@@ -63,13 +63,13 @@ endif
 	$(call quiet-command,$(AS) $(ASFLAGS) -o $@ $<,"  AS    $(TARGET_DIR)$@")
 
 %.o: %.cc
-	$(call quiet-command,$(CXX) $(QEMU_INCLUDES) $(QEMU_CXXFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) $($@-cflags) -c -o $@ $<,"  CXX   $(TARGET_DIR)$@")
+	$(call quiet-command,$(CXX) $($@-cflags) $(QEMU_INCLUDES) $(QEMU_CXXFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) -c -o $@ $<,"  CXX   $(TARGET_DIR)$@")
 
 %.o: %.cpp
-	$(call quiet-command,$(CXX) $(QEMU_INCLUDES) $(QEMU_CXXFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) $($@-cflags) -c -o $@ $<,"  CXX   $(TARGET_DIR)$@")
+	$(call quiet-command,$(CXX) $($@-cflags) $(QEMU_INCLUDES) $(QEMU_CXXFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) -c -o $@ $<,"  CXX   $(TARGET_DIR)$@")
 
 %.o: %.m
-	$(call quiet-command,$(OBJCC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) $($@-cflags) -c -o $@ $<,"  OBJC  $(TARGET_DIR)$@")
+	$(call quiet-command,$(OBJCC) $($@-cflags) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) -c -o $@ $<,"  OBJC  $(TARGET_DIR)$@")
 
 %.o: %.dtrace
 	$(call quiet-command,dtrace -o $@ -G -s $<, "  GEN   $(TARGET_DIR)$@")

Re: [Qemu-devel] [PATCH] pass $($*.o-cflags) first to gcc/g++

[Paolo Bonzini]

Il 09/07/2014 22:34, Stefano Stabellini ha scritto:
> rules.mak adds cflags specific to the target source file ($($@-cflags))
> for last on the compiler command line.
>
> As a consequence when compiling arm-a64.o, g++ might end up picking the
> wrong utils.h header file, because it looks for utils.h on all the other
> include paths first.
>
> Fix the issue by passing the source file specific cflags first.
> Do it consisently for *.c, *.cc, etc.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

You can just as well have a bug that requires you to put the option last 
(for example adding -Wno-something or -O0), which is why 
$($@-cflags)/$($*.o-cflags) comes last.

What package is it that has the conflicting utils.h file?  Any chance to 
get it fixed in your distro?  Here I get:

$ find /usr/include/ -name utils.h
/usr/include/libnl3/netlink/utils.h
/usr/include/libnl3/netlink/cli/utils.h
/usr/include/id3/utils.h
/usr/include/octave-3.6.4/octave/utils.h

but none of them have the path in -I.

Paolo

Re: [Qemu-devel] [PATCH] pass $($*.o-cflags) first to gcc/g++

[Peter Maydell]

On 9 July 2014 22:11, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 09/07/2014 22:34, Stefano Stabellini ha scritto:
>
>> rules.mak adds cflags specific to the target source file ($($@-cflags))
>> for last on the compiler command line.
>>
>> As a consequence when compiling arm-a64.o, g++ might end up picking the
>> wrong utils.h header file, because it looks for utils.h on all the other
>> include paths first.
>>
>> Fix the issue by passing the source file specific cflags first.
>> Do it consisently for *.c, *.cc, etc.
>>
>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>
>
> You can just as well have a bug that requires you to put the option last
> (for example adding -Wno-something or -O0), which is why
> $($@-cflags)/$($*.o-cflags) comes last.
>
> What package is it that has the conflicting utils.h file?  Any chance to get
> it fixed in your distro?

Yeah, at this point we run beyond what's reasonable to fix
in QEMU, and you need to fix your Xen build process.
As I recall from IRC, this is purely because the way Xen
builds QEMU involves them manually adding a directory
to the include path which is actually the source tree of
some Xen tool or other. That should be fixed so that the
relevant Xen headers get installed somewhere and then
you point the QEMU build at the installed headers.

(I've also suggested that libvixl makes these rather generic headers
live in a subdir to avoid the clash from that end, but I don't
expect that will make it through to a released libvixl and then
into QEMU for a while yet.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH] pass $($*.o-cflags) first to gcc/g++

[Stefano Stabellini]

On Wed, 9 Jul 2014, Paolo Bonzini wrote:
> Il 09/07/2014 22:34, Stefano Stabellini ha scritto:
> > rules.mak adds cflags specific to the target source file ($($@-cflags))
> > for last on the compiler command line.
> > 
> > As a consequence when compiling arm-a64.o, g++ might end up picking the
> > wrong utils.h header file, because it looks for utils.h on all the other
> > include paths first.
> > 
> > Fix the issue by passing the source file specific cflags first.
> > Do it consisently for *.c, *.cc, etc.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
> You can just as well have a bug that requires you to put the option last (for
> example adding -Wno-something or -O0), which is why
> $($@-cflags)/$($*.o-cflags) comes last.

I see. I can understand the reasoning, especially given the way
$@-cflags is used in block/Makefile.objs.  But in that case I would
think that there must be a better way to add -I$(libvixldir) from
disas/Makefile.objs. Maybe adding it to QEMU_CFLAGS is not so bad?

$(obj)/arm-a64.o: QEMU_CFLAGS := -I$(libvixldir) $(QEMU_CFLAGS)


> What package is it that has the conflicting utils.h file?  Any chance to get
> it fixed in your distro?  Here I get:
> 
> $ find /usr/include/ -name utils.h
> /usr/include/libnl3/netlink/utils.h
> /usr/include/libnl3/netlink/cli/utils.h
> /usr/include/id3/utils.h
> /usr/include/octave-3.6.4/octave/utils.h
> 
> but none of them have the path in -I.

It's Xen: when building QEMU as part of the Xen build process,
tools/Makefile uses --extra-cflags to add the local Xen directories to
the QEMU include path. However one of the Xen header files is named
utils.h, conflicting with utils.h from disas/libvixl.
It seems to be that --extra-cflags should come after the QEMU include
paths.

Re: [Qemu-devel] [PATCH] pass $($*.o-cflags) first to gcc/g++

[Paolo Bonzini]

Il 09/07/2014 23:59, Stefano Stabellini ha scritto:
> On Wed, 9 Jul 2014, Paolo Bonzini wrote:
>> What package is it that has the conflicting utils.h file?  Any chance to get
>> it fixed in your distro?  Here I get:
>>
>> $ find /usr/include/ -name utils.h
>> /usr/include/libnl3/netlink/utils.h
>> /usr/include/libnl3/netlink/cli/utils.h
>> /usr/include/id3/utils.h
>> /usr/include/octave-3.6.4/octave/utils.h
>>
>> but none of them have the path in -I.
>
> It's Xen: when building QEMU as part of the Xen build process,
> tools/Makefile uses --extra-cflags to add the local Xen directories to
> the QEMU include path. However one of the Xen header files is named
> utils.h, conflicting with utils.h from disas/libvixl.
> It seems to be that --extra-cflags should come after the QEMU include
> paths.

I think the bug is in Xen then.  I have an oldish checkout and I see:

$ find . -name utils.h
./tools/console/daemon/utils.h
./tools/xenstore/utils.h

Could you move more headers to tools/include/ and avoid polluting 
QEMU_CFLAGS?

That said, for this particular case not using foo.o-cflags can be a 
stopgap solution too.

Paolo

Re: [Qemu-devel] [PATCH] pass $($*.o-cflags) first to gcc/g++

[Stefano Stabellini]

On Thu, 10 Jul 2014, Paolo Bonzini wrote:
> Il 09/07/2014 23:59, Stefano Stabellini ha scritto:
> > On Wed, 9 Jul 2014, Paolo Bonzini wrote:
> > > What package is it that has the conflicting utils.h file?  Any chance to
> > > get
> > > it fixed in your distro?  Here I get:
> > > 
> > > $ find /usr/include/ -name utils.h
> > > /usr/include/libnl3/netlink/utils.h
> > > /usr/include/libnl3/netlink/cli/utils.h
> > > /usr/include/id3/utils.h
> > > /usr/include/octave-3.6.4/octave/utils.h
> > > 
> > > but none of them have the path in -I.
> > 
> > It's Xen: when building QEMU as part of the Xen build process,
> > tools/Makefile uses --extra-cflags to add the local Xen directories to
> > the QEMU include path. However one of the Xen header files is named
> > utils.h, conflicting with utils.h from disas/libvixl.
> > It seems to be that --extra-cflags should come after the QEMU include
> > paths.
> 
> I think the bug is in Xen then.  I have an oldish checkout and I see:
> 
> $ find . -name utils.h
> ./tools/console/daemon/utils.h
> ./tools/xenstore/utils.h

this is the one


> Could you move more headers to tools/include/ and avoid polluting QEMU_CFLAGS?

I'll see what I can do



> That said, for this particular case not using foo.o-cflags can be a stopgap
> solution too.

Thanks, I'll try to work on the Xen side to fix the problem. I'll come
back to this patch only if strictly necessary :-)