On Fri, Apr 29, 2022 at 06:39:33PM +0000, David Matlack wrote:
> The linker does obey strong/weak symbols when linking static libraries,
> it simply resolves an undefined symbol to the first-encountered symbol.
> This means that defining __weak arch-generic functions and then defining
> arch-specific strong functions to override them in libkvm will not
> always work.
>
> More specifically, if we have:
>
> lib/generic.c:
>
> void __weak foo(void)
> {
> pr_info("weak\n");
> }
>
> void bar(void)
> {
> foo();
> }
>
> lib/x86_64/arch.c:
>
> void foo(void)
> {
> pr_info("strong\n");
> }
>
> And a selftest that calls bar(), it will print "weak". Now if you make
> generic.o explicitly depend on arch.o (e.g. add function to arch.c that
> is called directly from generic.c) it will print "strong". In other
> words, it seems that the linker is free to throw out arch.o when linking
> because generic.o does not explicitly depend on it, which causes the
> linker to lose the strong symbol.
>
> One solution is to link libkvm.a with --whole-archive so that the linker
> doesn't throw away object files it thinks are unnecessary. However that
> is a bit difficult to plumb since we are using the common selftests
> makefile rules. An easier solution is to drop libkvm.a just link
> selftests with all the .o files that were originally in libkvm.a.
>
> Signed-off-by: David Matlack <dmatlack@xxxxxxxxxx>
> ---
> tools/testing/selftests/kvm/Makefile | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> index af582d168621..c1eb6acb30de 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -172,12 +172,13 @@ LDFLAGS += -pthread $(no-pie-option) $(pgste-option)
> # $(TEST_GEN_PROGS) starts with $(OUTPUT)/
> include ../lib.mk
>
> -STATIC_LIBS := $(OUTPUT)/libkvm.a
> LIBKVM_C := $(filter %.c,$(LIBKVM))
> LIBKVM_S := $(filter %.S,$(LIBKVM))
> LIBKVM_C_OBJ := $(patsubst %.c, $(OUTPUT)/%.o, $(LIBKVM_C))
> LIBKVM_S_OBJ := $(patsubst %.S, $(OUTPUT)/%.o, $(LIBKVM_S))
> -EXTRA_CLEAN += $(LIBKVM_C_OBJ) $(LIBKVM_S_OBJ) $(STATIC_LIBS) cscope.*
> +LIBKVM_OBJS = $(LIBKVM_C_OBJ) $(LIBKVM_S_OBJ)
> +
> +EXTRA_CLEAN += $(LIBKVM_OBJS) cscope.*
>
> x := $(shell mkdir -p $(sort $(dir $(LIBKVM_C_OBJ) $(LIBKVM_S_OBJ))))
> $(LIBKVM_C_OBJ): $(OUTPUT)/%.o: %.c
> @@ -186,13 +187,9 @@ $(LIBKVM_C_OBJ): $(OUTPUT)/%.o: %.c
> $(LIBKVM_S_OBJ): $(OUTPUT)/%.o: %.S
> $(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c $< -o $@
>
> -LIBKVM_OBJS = $(LIBKVM_C_OBJ) $(LIBKVM_S_OBJ)
> -$(OUTPUT)/libkvm.a: $(LIBKVM_OBJS)
> - $(AR) crs $@ $^
> -
> x := $(shell mkdir -p $(sort $(dir $(TEST_GEN_PROGS))))
> -all: $(STATIC_LIBS)
> -$(TEST_GEN_PROGS): $(STATIC_LIBS)
> +all: $(LIBKVM_OBJS)
Can this line be dropped alongside? Default targets should have already
been set in ../lib.mk anyway iiuc, and they all depend on the objs.
> +$(TEST_GEN_PROGS): $(LIBKVM_OBJS)
Never know such a difference, but it does seem true to happen.. Good to
learn this.
Reviewed-by: Peter Xu <peterx@xxxxxxxxxx>
Thanks,
--
Peter Xu
