[Qemu-devel] [PATCH] rules.mak: Fix DSO build by pulling in archive symbols

[Qemu-devel] [PATCH] rules.mak: Fix DSO build by pulling in archive symbols

[Fam Zheng]

This fixes an issue with module build system. block/iscsi.so is
currently broken:

    $ ~/build/last/qemu-img
    Failed to open module: /home/fam/build/master/block-iscsi.so:
    undefined symbol: qmp_query_uuid
    qemu-img: Not enough arguments
    Try 'qemu-img --help' for more information

To fix this, we should (at least) let qemu-img link qmp_query_uuid from
libqemustub.a. (There are a few other symbols missing, as well.)

This patch changes the linking rules to:

1) Build ".mo" with "ld -r -o $@ $^" for each ".so", and later build .so
   with it.

2) Always build all the .mo before linking the executables. This is
   achieved by adding those .mo files to the executables' "-y"
   variables.

3) When linking an executable, those .mo files in its "-y" variables are
   filtered out, and replaced by one or more -Wl,-u,$symbol flags. This
   is done in the added macro "process-archive-undefs".

   These "-Wl,-u,$symbol" flags will force ld to pull in the function
   definition from the archives when linking.

   Note that the .mo objects, that are actually meant to be linked in
   the executables, are already expanded in unnest-vars, before the
   linking command. So we are safe to simply filter out .mo for the
   purpose of pulling undefined symbols.

   process-archive-undefs works as this: For each ".mo", find all the
   undefined symbols in it, filter ones that are defined in the
   archives. For each of these symbols, generate a "-Wl,-u,$symbol" in
   the link command, and put them before archive names in the command
   line.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 rules.mak | 45 +++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 41 insertions(+), 4 deletions(-)

diff --git a/rules.mak b/rules.mak
index ba2f4c1..2bb2446 100644
--- a/rules.mak
+++ b/rules.mak
@@ -22,6 +22,32 @@ QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(*D)/$(*F).d
 # Same as -I$(SRC_PATH) -I., but for the nested source/object directories
 QEMU_INCLUDES += -I$(<D) -I$(@D)
 
+WL_U := -Wl,-u,
+find-symbols = $(if $1, $(sort $(shell nm -f posix $1 | awk '$$2=="$2"{print $$1}' | sort -u)))
+defined-symbols = $(call find-symbols,$1,T)
+undefined-symbols = $(call find-symbols,$1,U)
+
+# All the .mo objects in -m variables are also added into corresponding -y
+# variable in unnest-vars, but filtered out here, when LINK is called.
+#
+# The .mo objects are supposed to be linked as a DSO, for module build. So here
+# they are only used as a placeholders to generate those "archive undefined"
+# symbol options (-Wl,-u,$symbol_name), which are the archive functions
+# referenced by the code in the DSO.
+#
+# Also the presence in -y variables will also guarantee they are built before
+# linking executables that will load them. So we can look up symbol reference
+# in LINK.
+#
+# This is necessary because the exectuable itself may not use the function, in
+# which case the function would not be linked in. Then the DSO loading will
+# fail because of the missing symbol.
+process-archive-undefs = $(filter-out %.a %.mo,$1) \
+                $(addprefix $(WL_U),\
+                     $(filter $(call defined-symbols,$(filter %.a, $1)), \
+                              $(call undefined-symbols,$(filter %.mo,$1)))) \
+                $(filter %.a,$1)
+
 extract-libs = $(strip $(foreach o,$1,$($o-libs)))
 expand-objs = $(strip $(sort $(filter %.o,$1)) \
                   $(foreach o,$(filter %.mo,$1),$($o-objs)) \
@@ -38,7 +64,8 @@ LINKPROG = $(or $(CXX),$(CC))
 
 ifeq ($(LIBTOOL),)
 LINK = $(call quiet-command, $(LINKPROG) $(QEMU_CFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ \
-       $1 $(version-obj-y) $(call extract-libs,$1) $(LIBS),"  LINK  $(TARGET_DIR)$@")
+       $(call process-archive-undefs, $1)\
+       $(version-obj-y) $(call extract-libs,$1) $(LIBS),"  LINK  $(TARGET_DIR)$@")
 else
 LIBTOOL += $(if $(V),,--quiet)
 %.lo: %.c
@@ -49,8 +76,9 @@ LIBTOOL += $(if $(V),,--quiet)
 	$(call quiet-command,$(LIBTOOL) --mode=compile --tag=CC dtrace -o $@ -G -s $<, " lt GEN $(TARGET_DIR)$@")
 
 LINK = $(call quiet-command,\
-       $(if $(filter %.lo %.la,$1),$(LIBTOOL) --mode=link --tag=CC \
-       )$(LINKPROG) $(QEMU_CFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ $1 \
+       $(if $(filter %.lo %.la,$1),$(LIBTOOL) --mode=link --tag=CC) \
+       $(LINKPROG) $(QEMU_CFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ \
+       $(call process-archive-undefs, $1)\
        $(if $(filter %.lo %.la,$1),$(version-lobj-y),$(version-obj-y)) \
        $(if $(filter %.lo %.la,$1),$(LIBTOOLFLAGS)) \
        $(call extract-libs,$(1:.lo=.o)) $(LIBS),$(if $(filter %.lo %.la,$1),"lt LINK ", "  LINK  ")"$(TARGET_DIR)$@")
@@ -76,11 +104,17 @@ endif
 
 %$(DSOSUF): CFLAGS += -fPIC -DBUILD_DSO
 %$(DSOSUF): LDFLAGS += $(LDFLAGS_SHARED)
-%$(DSOSUF):
+%$(DSOSUF): %.mo
 	$(call LINK,$^)
 	@# Copy to build root so modules can be loaded when program started without install
 	$(if $(findstring /,$@),$(call quiet-command,cp $@ $(subst /,-,$@), "  CP    $(subst /,-,$@)"))
 
+
+LD_REL := $(CC) -nostdlib -Wl,-r
+
+%.mo:
+	$(call quiet-command,$(LD_REL) -o $@ $^,"  LD -r $(TARGET_DIR)$@")
+
 .PHONY: modules
 modules:
 
@@ -306,6 +340,9 @@ define unnest-vars
         # For module build, build shared libraries during "make modules"
         # For non-module build, add -m to -y
         $(if $(CONFIG_MODULES),
+             $(foreach o,$($v),
+                   $(eval $o: $($o-objs)))
+             $(eval $(patsubst %-m,%-y,$v) += $($v))
              $(eval modules: $($v:%.mo=%$(DSOSUF))),
              $(eval $(patsubst %-m,%-y,$v) += $(call expand-objs, $($v)))))
 
-- 
2.1.0

Re: [Qemu-devel] [PATCH] rules.mak: Fix DSO build by pulling in archive symbols

[Paolo Bonzini]

Il 29/08/2014 05:00, Fam Zheng ha scritto:
> Signed-off-by: Fam Zheng <famz@redhat.com>

Perhaps add Suggested-by: H.J. Lu <hongjiu.lu@intel.com>?

> +find-symbols = $(if $1, $(sort $(shell nm -f posix $1 | awk '$$2=="$2"{print $$1}' | sort -u)))

No need to use both $(sort) and sort -u.

Also please use -P instead of "-f posix", it's more portable.

> +defined-symbols = $(call find-symbols,$1,T)

What about data symbols?  Those use other types like B, D or R.  Perhaps
use "nm -g" and remove U?

> +undefined-symbols = $(call find-symbols,$1,U)

"nm -g" would work here too (filtering U only here, of course).

The other comments are aesthetic only.

> +# All the .mo objects in -m variables are also added into corresponding -y
> +# variable in unnest-vars, but filtered out here, when LINK is called.
> +#
> +# The .mo objects are supposed to be linked as a DSO, for module build. So here
> +# they are only used as a placeholders to generate those "archive undefined"
> +# symbol options (-Wl,-u,$symbol_name), which are the archive functions
> +# referenced by the code in the DSO.
> +#
> +# Also the presence in -y variables will also guarantee they are built before
> +# linking executables that will load them. So we can look up symbol reference
> +# in LINK.
> +#
> +# This is necessary because the exectuable itself may not use the function, in
> +# which case the function would not be linked in. Then the DSO loading will
> +# fail because of the missing symbol.
> +process-archive-undefs = $(filter-out %.a %.mo,$1) \
> +                $(addprefix $(WL_U),\
> +                     $(filter $(call defined-symbols,$(filter %.a, $1)), \
> +                              $(call undefined-symbols,$(filter %.mo,$1)))) \
> +                $(filter %.a,$1)
> +
>  extract-libs = $(strip $(foreach o,$1,$($o-libs)))
>  expand-objs = $(strip $(sort $(filter %.o,$1)) \
>                    $(foreach o,$(filter %.mo,$1),$($o-objs)) \
> @@ -38,7 +64,8 @@ LINKPROG = $(or $(CXX),$(CC))
>  
>  ifeq ($(LIBTOOL),)
>  LINK = $(call quiet-command, $(LINKPROG) $(QEMU_CFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ \
> -       $1 $(version-obj-y) $(call extract-libs,$1) $(LIBS),"  LINK  $(TARGET_DIR)$@")
> +       $(call process-archive-undefs, $1)\

Space before \

> +       $(version-obj-y) $(call extract-libs,$1) $(LIBS),"  LINK  $(TARGET_DIR)$@")
>  else
>  LIBTOOL += $(if $(V),,--quiet)
>  %.lo: %.c
> @@ -49,8 +76,9 @@ LIBTOOL += $(if $(V),,--quiet)
>  	$(call quiet-command,$(LIBTOOL) --mode=compile --tag=CC dtrace -o $@ -G -s $<, " lt GEN $(TARGET_DIR)$@")
>  
>  LINK = $(call quiet-command,\
> -       $(if $(filter %.lo %.la,$1),$(LIBTOOL) --mode=link --tag=CC \
> -       )$(LINKPROG) $(QEMU_CFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ $1 \
> +       $(if $(filter %.lo %.la,$1),$(LIBTOOL) --mode=link --tag=CC) \
> +       $(LINKPROG) $(QEMU_CFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ \

IIRC the parenthesis was there to make "V=1" a little prettier.

> +       $(call process-archive-undefs, $1)\

Space before \

>         $(if $(filter %.lo %.la,$1),$(version-lobj-y),$(version-obj-y)) \
>         $(if $(filter %.lo %.la,$1),$(LIBTOOLFLAGS)) \
>         $(call extract-libs,$(1:.lo=.o)) $(LIBS),$(if $(filter %.lo %.la,$1),"lt LINK ", "  LINK  ")"$(TARGET_DIR)$@")
> @@ -76,11 +104,17 @@ endif
>  
>  %$(DSOSUF): CFLAGS += -fPIC -DBUILD_DSO
>  %$(DSOSUF): LDFLAGS += $(LDFLAGS_SHARED)
> -%$(DSOSUF):
> +%$(DSOSUF): %.mo
>  	$(call LINK,$^)
>  	@# Copy to build root so modules can be loaded when program started without install
>  	$(if $(findstring /,$@),$(call quiet-command,cp $@ $(subst /,-,$@), "  CP    $(subst /,-,$@)"))
>  
> +
> +LD_REL := $(CC) -nostdlib -Wl,-r
> +
> +%.mo:
> +	$(call quiet-command,$(LD_REL) -o $@ $^,"  LD -r $(TARGET_DIR)$@")
> +
>  .PHONY: modules
>  modules:
>  
> @@ -306,6 +340,9 @@ define unnest-vars
>          # For module build, build shared libraries during "make modules"
>          # For non-module build, add -m to -y
>          $(if $(CONFIG_MODULES),
> +             $(foreach o,$($v),
> +                   $(eval $o: $($o-objs)))
> +             $(eval $(patsubst %-m,%-y,$v) += $($v))
>               $(eval modules: $($v:%.mo=%$(DSOSUF))),
>               $(eval $(patsubst %-m,%-y,$v) += $(call expand-objs, $($v)))))
>  
>