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 <address@hidden>

Perhaps add Suggested-by: H.J. Lu <address@hidden>?

> +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)))))
>  
>