tests: Fix function mocking for clang
clang seems to like to do some aggressive optimizations that break our
approach of mocking functions for test by using objcopy to turn them
weak after the fact on individual compiled object files. For example, in
CB:56601 the function cbfs_get_boot_device() is mocked this way. When
compiling the cbfs_boot_lookup() function in src/lib/cbfs.c with clang,
it will generate a normal callq instruction to a relocation for
cbfs_boot_lookup(), which can then later be pointed to the mocked
version of that function. However, it will also somehow infer that the
version of cbfs_boot_lookup() in that file can only ever return a
pointer to the static local `ro` variable (because CONFIG_VBOOT is
disabled in the environment for that particular test), and instead
generate instructions that directly load the address of a relocation for
that variable into %rdi for the following call to cbfs_lookup(), rather
than using the real function return value. (Why it would do that is
anyone's guess because this seems unlikely to be faster than just moving
the function return value from %rax into %rdi like a normal compiler.)
Long story short, this optimization breaks our tests because
cbfs_lookup() will be called with the wrong pointer. clang doesn't
provide many options to disable individual optimizations, so the only
solution seems to be to make clang aware that the function is weak
during the compilation stage already, so it can be aware that it may get
replaced. This patch implements that by marking the mocked functions
weak via #pragma weak lines in the per-test autogenerated config header.
Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: I1f9011f444248544de7a71bbefc54edc006ae0cd
Reviewed-on: https://review.coreboot.org/c/coreboot/+/57009
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
Reviewed-by: Jakub Czapiga <jacz@semihalf.com>
diff --git a/tests/Makefile.inc b/tests/Makefile.inc
index f45638d..2f73cb3 100644
--- a/tests/Makefile.inc
+++ b/tests/Makefile.inc
@@ -108,7 +108,13 @@
# $1 - test name
define TEST_CC_template
-# Generate custom config.h redefining given symbols
+# Generate custom config.h redefining given config symbols, and declaring mocked
+# functions weak. It is important that the compiler already sees that they are
+# weak (and they aren't just turned weak at a later stage) to prevent certain
+# optimizations that would break if the function gets replaced. (For clang this
+# file needs to be marked `system_header` to prevent it from warning about
+# #pragma weak entries without a matching function declaration, since there's no
+# -Wno-xxx command line option for that.)
$(1)-config-file := $(testobj)/$(1)/config.h
$$($(1)-config-file): $(TEST_KCONFIG_AUTOHEADER)
mkdir -p $$(dir $$@)
@@ -120,12 +126,20 @@
printf '#undef %s\n' "$$$$key" >> $$@; \
printf '#define %s %s\n\n' "$$$$key" "$$$$value" >> $$@; \
done
+ printf '#ifdef __clang__\n' >> $$@;
+ printf '#pragma clang system_header\n' >> $$@;
+ printf '#endif\n' >> $$@;
+ printf '#ifdef __TEST_SRCOBJ__\n' >> $$@;
+ for m in $$($(1)-mocks); do \
+ printf '#pragma weak %s\n' "$$$$m" >> $$@; \
+ done
+ printf '#endif\n' >> $$@;
$($(1)-objs): TEST_CFLAGS += -I$$(dir $$($(1)-config-file)) \
-D__$$(shell echo $$($(1)-stage) | tr '[:lower:]' '[:upper:]')__
-# Weaken symbols listed as mocks to enable overriding in the code
-$($(1)-srcobjs): OBJCOPY_FLAGS += $$(foreach mock,$$($(1)-mocks),--globalize-symbol=$$(mock) --weaken-symbol=$$(mock))
+# Give us a way to distinguish between coreboot source files and test files in code.
+$($(1)-srcobjs): TEST_CFLAGS += -D__TEST_SRCOBJ__
# Compile sources and apply mocking/wrapping of selected symbols.
# For each listed mock add new symbol with prefix `__real_`,
@@ -134,17 +148,16 @@
mkdir -p $$(dir $$@)
$(HOSTCC) $(HOSTCFLAGS) $$(TEST_CFLAGS) $($(1)-cflags) -MMD \
-MF $$(basename $$@).d -MT $$@ -c $$< -o $$@.orig
- $(OBJCOPY) $$@.orig $$(OBJCOPY_FLAGS) $$@.orig2
objcopy_wrap_flags=''; \
for sym in $$($(1)-mocks); do \
- sym_line="$$$$($(OBJDUMP) -t $$@.orig2 | grep -E "[0-9a-fA-F]+\\s+w\\s+F\\s+.*\\s$$$$sym$$$$")"; \
+ sym_line="$$$$($(OBJDUMP) -t $$@.orig | grep -E "[0-9a-fA-F]+\\s+w\\s+F\\s+.*\\s$$$$sym$$$$")"; \
if [ ! -z "$$$$sym_line" ] ; then \
addr="$$$$(echo \"$$$$sym_line\" | awk '{ print $$$$1 }')"; \
section="$$$$(echo \"$$$$sym_line\" | awk '{ print $$$$(NF - 2) }')"; \
objcopy_wrap_flags="$$$$objcopy_wrap_flags --add-symbol __real_$$$${sym}=$$$${section}:0x$$$${addr},function,global"; \
fi \
done ; \
- $(OBJCOPY) $$@.orig2 $$$$objcopy_wrap_flags $$@
+ $(OBJCOPY) $$@.orig $$$$objcopy_wrap_flags $$@
$($(1)-bin): $($(1)-objs) $(CMOCKA_LIB)
$(HOSTCC) $$^ $($(1)-cflags) $$(TEST_LDFLAGS) -o $$@