* [Qemu-devel] [PATCH] rules.mak: Force CFLAGS for all objects in DSO
@ 2015-05-06 13:46 Fam Zheng
2015-05-06 14:07 ` Paolo Bonzini
2015-05-06 23:25 ` Alexander Graf
0 siblings, 2 replies; 7+ messages in thread
From: Fam Zheng @ 2015-05-06 13:46 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Fam Zheng, Stefan Weil, Michael Tokarev, agraf,
Paolo Bonzini
Because of the trick of process-archive-undefs, all .mo objects, even
with --enable-modules, are dependencies of executables.
This breaks CFLAGS propogation because the compiling of module object
will happen too early before building for DSO.
With GCC 5, the linking would fail because .o doesn't have -fPIC. Also,
BUILD_DSO will be missed. (module-common.o will have it, so the stamp
symbol was still liked in .so).
Fix the problem by forcing the CFLAGS during unnest-vars.
Reported-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
rules.mak | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/rules.mak b/rules.mak
index 3a05627..6c0caf3 100644
--- a/rules.mak
+++ b/rules.mak
@@ -102,7 +102,6 @@ endif
%.o: %.dtrace
$(call quiet-command,dtrace -o $@ -G -s $<, " GEN $(TARGET_DIR)$@")
-%$(DSOSUF): CFLAGS += -fPIC -DBUILD_DSO
%$(DSOSUF): LDFLAGS += $(LDFLAGS_SHARED)
%$(DSOSUF): %.mo
$(call LINK,$^)
@@ -353,6 +352,7 @@ define unnest-vars
$(foreach o,$($v),
$(eval $o: $($o-objs)))
$(eval $(patsubst %-m,%-y,$v) += $($v))
+ $(eval $($v:%.mo=%$(DSOSUF)) $($v) $(foreach o,$($v),$($o-objs)) .PHONY: CFLAGS += -fPIC -DBUILD_DSO)
$(eval modules: $($v:%.mo=%$(DSOSUF))),
$(eval $(patsubst %-m,%-y,$v) += $(call expand-objs, $($v)))))
--
1.9.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH] rules.mak: Force CFLAGS for all objects in DSO
2015-05-06 13:46 [Qemu-devel] [PATCH] rules.mak: Force CFLAGS for all objects in DSO Fam Zheng
@ 2015-05-06 14:07 ` Paolo Bonzini
2015-05-06 14:23 ` Fam Zheng
2015-05-06 23:25 ` Alexander Graf
1 sibling, 1 reply; 7+ messages in thread
From: Paolo Bonzini @ 2015-05-06 14:07 UTC (permalink / raw)
To: Fam Zheng, qemu-devel; +Cc: Peter Maydell, Michael Tokarev, agraf, Stefan Weil
On 06/05/2015 15:46, Fam Zheng wrote:
> Because of the trick of process-archive-undefs, all .mo objects, even
> with --enable-modules, are dependencies of executables.
>
> This breaks CFLAGS propogation because the compiling of module object
> will happen too early before building for DSO.
>
> With GCC 5, the linking would fail because .o doesn't have -fPIC. Also,
> BUILD_DSO will be missed. (module-common.o will have it, so the stamp
> symbol was still liked in .so).
>
> Fix the problem by forcing the CFLAGS during unnest-vars.
>
> Reported-by: Alexander Graf <agraf@suse.de>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
> rules.mak | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/rules.mak b/rules.mak
> index 3a05627..6c0caf3 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -102,7 +102,6 @@ endif
> %.o: %.dtrace
> $(call quiet-command,dtrace -o $@ -G -s $<, " GEN $(TARGET_DIR)$@")
>
> -%$(DSOSUF): CFLAGS += -fPIC -DBUILD_DSO
> %$(DSOSUF): LDFLAGS += $(LDFLAGS_SHARED)
> %$(DSOSUF): %.mo
> $(call LINK,$^)
> @@ -353,6 +352,7 @@ define unnest-vars
> $(foreach o,$($v),
> $(eval $o: $($o-objs)))
> $(eval $(patsubst %-m,%-y,$v) += $($v))
> + $(eval $($v:%.mo=%$(DSOSUF)) $($v) $(foreach o,$($v),$($o-objs)) .PHONY: CFLAGS += -fPIC -DBUILD_DSO)
Can you explain the various parts to the left of the colon?
Thanks,
Paolo
> $(eval modules: $($v:%.mo=%$(DSOSUF))),
> $(eval $(patsubst %-m,%-y,$v) += $(call expand-objs, $($v)))))
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] rules.mak: Force CFLAGS for all objects in DSO
2015-05-06 14:07 ` Paolo Bonzini
@ 2015-05-06 14:23 ` Fam Zheng
2015-05-06 14:36 ` Paolo Bonzini
0 siblings, 1 reply; 7+ messages in thread
From: Fam Zheng @ 2015-05-06 14:23 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Peter Maydell, agraf, Michael Tokarev, qemu-devel, Stefan Weil
On Wed, 05/06 16:07, Paolo Bonzini wrote:
>
>
> On 06/05/2015 15:46, Fam Zheng wrote:
> > Because of the trick of process-archive-undefs, all .mo objects, even
> > with --enable-modules, are dependencies of executables.
> >
> > This breaks CFLAGS propogation because the compiling of module object
> > will happen too early before building for DSO.
> >
> > With GCC 5, the linking would fail because .o doesn't have -fPIC. Also,
> > BUILD_DSO will be missed. (module-common.o will have it, so the stamp
> > symbol was still liked in .so).
> >
> > Fix the problem by forcing the CFLAGS during unnest-vars.
> >
> > Reported-by: Alexander Graf <agraf@suse.de>
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> > rules.mak | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/rules.mak b/rules.mak
> > index 3a05627..6c0caf3 100644
> > --- a/rules.mak
> > +++ b/rules.mak
> > @@ -102,7 +102,6 @@ endif
> > %.o: %.dtrace
> > $(call quiet-command,dtrace -o $@ -G -s $<, " GEN $(TARGET_DIR)$@")
> >
> > -%$(DSOSUF): CFLAGS += -fPIC -DBUILD_DSO
> > %$(DSOSUF): LDFLAGS += $(LDFLAGS_SHARED)
> > %$(DSOSUF): %.mo
> > $(call LINK,$^)
> > @@ -353,6 +352,7 @@ define unnest-vars
> > $(foreach o,$($v),
> > $(eval $o: $($o-objs)))
> > $(eval $(patsubst %-m,%-y,$v) += $($v))
> > + $(eval $($v:%.mo=%$(DSOSUF)) $($v) $(foreach o,$($v),$($o-objs)) .PHONY: CFLAGS += -fPIC -DBUILD_DSO)
^ ^ ^ ^
| | | |
| | | `- In case all others are empty.
| | |
| | `- Expansion of all %.mo-objs so it's a %.o list.
| | For $v=block-obj-m, this will contain curl.o, iscsi.o, ...
| |
| `- %.mo list
| For $v=block-obj-m, this will contain curl.mo, iscsi.mo, ...
|
`- %.so list
For $v=block-obj-m, this will contain curl.so, iscsi.so, ...
>
> Can you explain the various parts to the left of the colon?
Context:
- The line is inside "$(if $(CONFIG_MODULES),...)"
- $v is one of "block-job-m common-obj-m ..."
- So, $($v) is the module object list "foo.mo bar.mo ..."
Do you want a respin to include the ascii art? :)
Fam
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH] rules.mak: Force CFLAGS for all objects in DSO
2015-05-06 14:23 ` Fam Zheng
@ 2015-05-06 14:36 ` Paolo Bonzini
2015-05-06 15:01 ` Fam Zheng
0 siblings, 1 reply; 7+ messages in thread
From: Paolo Bonzini @ 2015-05-06 14:36 UTC (permalink / raw)
To: Fam Zheng; +Cc: Peter Maydell, agraf, Michael Tokarev, qemu-devel, Stefan Weil
On 06/05/2015 16:23, Fam Zheng wrote:
>>> > > + $(eval $($v:%.mo=%$(DSOSUF)) $($v) $(foreach o,$($v),$($o-objs)) .PHONY: CFLAGS += -fPIC -DBUILD_DSO)
> ^ ^ ^ ^
> | | | |
> | | | `- In case all others are empty.
> | | |
> | | `- Expansion of all %.mo-objs so it's a %.o list.
> | | For $v=block-obj-m, this will contain curl.o, iscsi.o, ...
> | |
> | `- %.mo list
> | For $v=block-obj-m, this will contain curl.mo, iscsi.mo, ...
> |
> `- %.so list
> For $v=block-obj-m, this will contain curl.so, iscsi.so, .
Great. :) You should have used Unicode drawing characters to be
consistent with rules.mak!
But hopefully the ASCII art is not needed at all. Do we need all of those?
- foo.mo is a prerequisite of foo.so. You cannot use foo.so (this is
the bug you are fixing) but foo.so doesn't use CFLAGS in its rule, and
you should be able to remove it.
- can you use foo.mo without hitting the original bug? If so, could you
simply use:
-%$(DSOSUF): CFLAGS += -fPIC -DBUILD_DSO
+%.mo: CFLAGS += -fPIC -DBUILD_DSO
(and if so move the rule down, above "%.mo:")? If you cannot use
foo.mo, foo.mo also doesn't use CFLAGS in its rules, and you should be
able to remove foo.mo too.
- so the other possibility is to just use $(foreach o,$($v),$($o-objs)).
In this case there's already a convenient $(foreach) just above, and
you can just add another $(eval) inside it.
- and if that is true, you do not need .PHONY either because you have
$(foreach ... $(eval)) instead of $(eval $(foreach)).
Maybe I'm wrong, in which case the patch is ok. :)
Paolo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] rules.mak: Force CFLAGS for all objects in DSO
2015-05-06 14:36 ` Paolo Bonzini
@ 2015-05-06 15:01 ` Fam Zheng
2015-05-06 15:03 ` Paolo Bonzini
0 siblings, 1 reply; 7+ messages in thread
From: Fam Zheng @ 2015-05-06 15:01 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Peter Maydell, agraf, Michael Tokarev, qemu-devel, Stefan Weil
On Wed, 05/06 16:36, Paolo Bonzini wrote:
>
>
> On 06/05/2015 16:23, Fam Zheng wrote:
> >>> > > + $(eval $($v:%.mo=%$(DSOSUF)) $($v) $(foreach o,$($v),$($o-objs)) .PHONY: CFLAGS += -fPIC -DBUILD_DSO)
> > ^ ^ ^ ^
> > | | | |
> > | | | `- In case all others are empty.
> > | | |
> > | | `- Expansion of all %.mo-objs so it's a %.o list.
> > | | For $v=block-obj-m, this will contain curl.o, iscsi.o, ...
> > | |
> > | `- %.mo list
> > | For $v=block-obj-m, this will contain curl.mo, iscsi.mo, ...
> > |
> > `- %.so list
> > For $v=block-obj-m, this will contain curl.so, iscsi.so, .
>
> Great. :) You should have used Unicode drawing characters to be
> consistent with rules.mak!
Well, I'd have to find and read the vim plugin doc to do that again!
>
> But hopefully the ASCII art is not needed at all. Do we need all of those?
>
> - foo.mo is a prerequisite of foo.so. You cannot use foo.so (this is
> the bug you are fixing) but foo.so doesn't use CFLAGS in its rule, and
> you should be able to remove it.
>
> - can you use foo.mo without hitting the original bug? If so, could you
> simply use:
>
> -%$(DSOSUF): CFLAGS += -fPIC -DBUILD_DSO
> +%.mo: CFLAGS += -fPIC -DBUILD_DSO
I believe this will propagate the flags correctly. However that will affect
non-module build, so I didn't want to do it unconditionally.
>
> (and if so move the rule down, above "%.mo:")? If you cannot use
> foo.mo, foo.mo also doesn't use CFLAGS in its rules, and you should be
> able to remove foo.mo too.
>
> - so the other possibility is to just use $(foreach o,$($v),$($o-objs)).
> In this case there's already a convenient $(foreach) just above, and
> you can just add another $(eval) inside it.
>
> - and if that is true, you do not need .PHONY either because you have
> $(foreach ... $(eval)) instead of $(eval $(foreach)).
>
OK.
It's been a long day for me, so you can see that I'm going defensive with the
code I'm writing, but yes, your last "if" looks about right, except we still
need the "$(if $(CONFIG_MODULES),...)".
Sanity check: is -fPIC only ever needed in compiling, but not in (partial)
linking, right?
Fam
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] rules.mak: Force CFLAGS for all objects in DSO
2015-05-06 15:01 ` Fam Zheng
@ 2015-05-06 15:03 ` Paolo Bonzini
0 siblings, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2015-05-06 15:03 UTC (permalink / raw)
To: Fam Zheng; +Cc: Peter Maydell, agraf, Michael Tokarev, qemu-devel, Stefan Weil
On 06/05/2015 17:01, Fam Zheng wrote:
>> > -%$(DSOSUF): CFLAGS += -fPIC -DBUILD_DSO
>> > +%.mo: CFLAGS += -fPIC -DBUILD_DSO
> I believe this will propagate the flags correctly. However that will affect
> non-module build, so I didn't want to do it unconditionally.
You're right. You'd need something like
$(call lif $(CONFIG_MODULES), -fPIC -DBUILD_DSO) instead.
>> > (and if so move the rule down, above "%.mo:")? If you cannot use
>> > foo.mo, foo.mo also doesn't use CFLAGS in its rules, and you should be
>> > able to remove foo.mo too.
>> >
>> > - so the other possibility is to just use $(foreach o,$($v),$($o-objs)).
>> > In this case there's already a convenient $(foreach) just above, and
>> > you can just add another $(eval) inside it.
>> >
>> > - and if that is true, you do not need .PHONY either because you have
>> > $(foreach ... $(eval)) instead of $(eval $(foreach)).
>> >
> OK.
>
> It's been a long day for me, so you can see that I'm going defensive with the
> code I'm writing, but yes, your last "if" looks about right, except we still
> need the "$(if $(CONFIG_MODULES),...)".
Understood entirely. But there's no hurry, sleep over it and take your
time to test it tomorrow.
> Sanity check: is -fPIC only ever needed in compiling, but not in (partial)
> linking, right?
Yes. It's LDFLAGS_SHARED that is used in linking (and nothing in
partial linking).
Paolo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] rules.mak: Force CFLAGS for all objects in DSO
2015-05-06 13:46 [Qemu-devel] [PATCH] rules.mak: Force CFLAGS for all objects in DSO Fam Zheng
2015-05-06 14:07 ` Paolo Bonzini
@ 2015-05-06 23:25 ` Alexander Graf
1 sibling, 0 replies; 7+ messages in thread
From: Alexander Graf @ 2015-05-06 23:25 UTC (permalink / raw)
To: Fam Zheng, qemu-devel
Cc: Peter Maydell, Stefan Weil, Michael Tokarev, Dirk Müller,
Paolo Bonzini, Andreas Färber
On 06.05.15 15:46, Fam Zheng wrote:
> Because of the trick of process-archive-undefs, all .mo objects, even
> with --enable-modules, are dependencies of executables.
>
> This breaks CFLAGS propogation because the compiling of module object
> will happen too early before building for DSO.
>
> With GCC 5, the linking would fail because .o doesn't have -fPIC. Also,
> BUILD_DSO will be missed. (module-common.o will have it, so the stamp
> symbol was still liked in .so).
>
> Fix the problem by forcing the CFLAGS during unnest-vars.
>
> Reported-by: Alexander Graf <agraf@suse.de>
> Signed-off-by: Fam Zheng <famz@redhat.com>
As a heads-up I just verified that this patch does indeed fix
compilation with gcc5 for me. However looking at the mail thread I
assume there's a v2 coming, so I'll hold off my tested-by tag ;).
For the final patch, we will probably also want to have it in the
qemu-stable tree, so that people will be able to compile older versions
of qemu (and modules) with newer compilers.
Alex
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-05-06 23:25 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-06 13:46 [Qemu-devel] [PATCH] rules.mak: Force CFLAGS for all objects in DSO Fam Zheng
2015-05-06 14:07 ` Paolo Bonzini
2015-05-06 14:23 ` Fam Zheng
2015-05-06 14:36 ` Paolo Bonzini
2015-05-06 15:01 ` Fam Zheng
2015-05-06 15:03 ` Paolo Bonzini
2015-05-06 23:25 ` Alexander Graf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).