| Message ID | 20220429183935.1094599-8-dmatlack@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: selftests: Add nested support to dirty_log_perf_test | expand |
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@google.com> > --- > 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@redhat.com> Thanks,
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) +$(TEST_GEN_PROGS): $(LIBKVM_OBJS) cscope: include_paths = $(LINUX_TOOL_INCLUDE) $(LINUX_HDR_PATH) include lib .. cscope:
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@google.com> --- tools/testing/selftests/kvm/Makefile | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)