Makefile, libminijail: Invert symbol visibility to allow sane unittesting
libminijail.c contains many helpers that are marked static. For instance,
consumestr and consumebytes are both static yet eminently unittestable.
The options for testing are as follows:
1. Replace "static" with a "private" or "protected" macro which we
undefined during testing.
2. #include "libminijail.c" into the unittests to avoid visibility
challenges.
3. Change default visibility to internal for all functions and data
then invert it during unittesting.
I chose #3. It also has the benefit of creating an optimally stripped
binary and shared object. Using 'internal' visibility also let's the
linker perform more optimizations.
Feedback on this approach is very welcome. In the past, I've chosen
approach #2, but that seems wrong for at least a couple of reasons.
TEST=build, run readelf -s in all the output. .so should show LOCAL for
all internal functions and on executables, private functions should show
INTERNAL. Running strip --unneeded should remove all of the private
linkage which can be checked with readelf -s again
BUG=none
Change-Id: Ifb1f02b4505f2f25d824c067748054520c39d3bf
Reviewed-on: https://gerrit.chromium.org/gerrit/10540
Commit-Ready: Will Drewry <wad@chromium.org>
Tested-by: Will Drewry <wad@chromium.org>
Reviewed-by: Will Drewry <wad@chromium.org>
diff --git a/Makefile b/Makefile
index de56da1..eb94b86 100644
--- a/Makefile
+++ b/Makefile
@@ -5,17 +5,27 @@
LIBDIR = lib
PRELOADPATH = \"/$(LIBDIR)/libminijailpreload.so\"
CFLAGS += -fPIC -Wall -Wextra -Werror -DPRELOADPATH="$(PRELOADPATH)"
+CFLAGS += -fvisibility=internal
all : minijail0 libminijail.so libminijailpreload.so
+tests : libminijail_unittest.wrapper
+
minijail0 : libsyscalls.gen.o libminijail.o minijail0.c
$(CC) $(CFLAGS) -o $@ $^ -lcap
libminijail.so : libminijail.o libsyscalls.gen.o
$(CC) $(CFLAGS) -shared -o $@ $^ -lcap
+# Allow unittests to access what are normally internal symbols.
+libminijail_unittest.wrapper :
+ $(MAKE) $(MAKEARGS) test-clean
+ $(MAKE) $(MAKEARGS) libminijail_unittest
+ $(MAKE) $(MAKEARGS) test-clean
+
+libminijail_unittest : CFLAGS := $(filter-out -fvisibility=%,$(CFLAGS))
libminijail_unittest : libminijail_unittest.o libminijail.o libsyscalls.gen.o
- $(CC) $(CFLAGS) $(LDFLAGS) -o $@ $^ -lcap
+ $(CC) $(CFLAGS) $(LDFLAGS) -o $@ $(filter-out $(CFLAGS_FILE),$^) -lcap
libminijailpreload.so : libminijailpreload.c libsyscalls.gen.o libminijail.o
$(CC) $(CFLAGS) -shared -o $@ $^ -ldl -lcap
@@ -66,7 +76,12 @@
@$(call gen_syscalls,$@)
@printf "done.\n"
-clean :
+# Only clean up files affected by the CFLAGS change for testing.
+test-clean :
+ @rm -f libminijail.o libminijail_unittest.o libsyscalls.gen.o
+
+clean : test-clean
@rm -f libminijail.o libminijailpreload.so minijail0
- @rm -f libminijail_unittest libminijail_unittest.o
- @rm -f libsyscalls.gen.c libsyscalls.gen.o
+ @rm -f libminijail.so
+ @rm -f libminijail_unittest
+ @rm -f libsyscalls.gen.c