From 08007d0b25a8175bf6f663fd12b25e4e4eea4d17 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 14 Jun 2022 15:12:21 +1000 Subject: [PATCH] Makefile: Avoid using wildcard sources The passt/pasta Makefile makes fairly heavy use of GNU make's $(wildcard) function to locate the sources and headers to build. Using wildcards for the things to compile is usually a bad idea though: if somehow you end up with a .c or .h file in your tree you didn't expect it can misbuild in an exceedingly confusing way. In particular this can sometimes happen if switching between releases / branches where files have been added or removed without 100% cleaning the tree. It also makes life a bit complicated if building multiple different binaries in the same tree: we already have some rather awkward $(filter-out) constructions to avoid including qrap.c in the passt build. Replace use of $(wildcard) with the more idiomatic approach of defining variables listing all the relevant source files then using that throughout. In the rule for seccomp.h there was also a bare "*.c" which caused make to always rebuild that target. Fix that as well. Similarly, seccomp.sh uses a wildcard to locate the sources, which is unwise for the same reasons. Make it take the sources to examine on the command line instead, and have the Makefile pass them in from the same variables. Signed-off-by: David Gibson --- Makefile | 37 ++++++++++++++++++++++--------------- seccomp.sh | 5 +++-- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/Makefile b/Makefile index b0de1ec..e8ed7a9 100644 --- a/Makefile +++ b/Makefile @@ -31,6 +31,17 @@ CFLAGS += -DPASST_AUDIT_ARCH=AUDIT_ARCH_$(AUDIT_ARCH) CFLAGS += -DRLIMIT_STACK_VAL=$(RLIMIT_STACK_VAL) CFLAGS += -DARCH=\"$(TARGET_ARCH)\" +PASST_SRCS = arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c icmp.c igmp.c \ + mld.c ndp.c netlink.c packet.c passt.c pasta.c pcap.c siphash.c \ + tap.c tcp.c tcp_splice.c udp.c util.c +QRAP_SRCS = qrap.c +SRCS = $(PASST_SRCS) $(QRAP_SRCS) + +PASST_HEADERS = arch.h arp.h checksum.h conf.h dhcp.h dhcpv6.h icmp.h \ + ndp.h netlink.h packet.h passt.h pasta.h pcap.h siphash.h \ + tap.h tcp.h tcp_splice.h udp.h util.h +HEADERS = $(PASST_HEADERS) + # On gcc 11.2, with -O2 and -flto, tcp_hash() and siphash_20b(), if inlined, # seem to be hitting something similar to: # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78993 @@ -82,18 +93,15 @@ endif static: CFLAGS += -static -DGLIBC_NO_STATIC_NSS static: clean all -seccomp.h: *.c $(filter-out seccomp.h,$(wildcard *.h)) - @ EXTRA_SYSCALLS=$(EXTRA_SYSCALLS) ./seccomp.sh +seccomp.h: $(PASST_SRCS) $(PASST_HEADERS) + @ EXTRA_SYSCALLS=$(EXTRA_SYSCALLS) ./seccomp.sh $^ -passt: $(filter-out qrap.c,$(wildcard *.c)) \ - $(filter-out qrap.h,$(wildcard *.h)) seccomp.h - $(CC) $(CFLAGS) $(filter-out qrap.c,$(wildcard *.c)) -o passt +passt: $(PASST_SRCS) $(PASST_HEADERS) seccomp.h + $(CC) $(CFLAGS) $(PASST_SRCS) -o passt passt.avx2: CFLAGS += -Ofast -mavx2 -ftree-vectorize -funroll-loops -passt.avx2: $(filter-out qrap.c,$(wildcard *.c)) \ - $(filter-out qrap.h,$(wildcard *.h)) seccomp.h - $(CC) $(filter-out -O2,$(CFLAGS)) $(filter-out qrap.c,$(wildcard *.c)) \ - -o passt.avx2 +passt.avx2: $(PASST_SRCS) $(PASST_HEADERS) seccomp.h + $(CC) $(filter-out -O2,$(CFLAGS)) $(PASST_SRCS) -o passt.avx2 passt.avx2: passt @@ -104,9 +112,8 @@ pasta: passt ln -s passt pasta ln -s passt.1 pasta.1 -qrap: qrap.c passt.h - $(CC) $(CFLAGS) \ - qrap.c -o qrap +qrap: $(QRAP_SRCS) passt.h + $(CC) $(CFLAGS) $(QRAP_SRCS) -o qrap valgrind: EXTRA_SYSCALLS="rt_sigprocmask rt_sigtimedwait rt_sigaction \ getpid gettid kill clock_gettime mmap munmap open \ @@ -203,7 +210,7 @@ pkgs: static # - concurrency-mt-unsafe # TODO: check again if multithreading is implemented -clang-tidy: $(wildcard *.c) $(wildcard *.h) +clang-tidy: $(SRCS) $(HEADERS) clang-tidy -checks=*,-modernize-*,\ -clang-analyzer-valist.Uninitialized,\ -cppcoreguidelines-init-variables,\ @@ -227,7 +234,7 @@ clang-tidy: $(wildcard *.c) $(wildcard *.h) -altera-struct-pack-align,\ -concurrency-mt-unsafe \ -config='{CheckOptions: [{key: bugprone-suspicious-string-compare.WarnOnImplicitComparison, value: "false"}]}' \ - --warnings-as-errors=* $(wildcard *.c) -- $(filter-out -pie,$(CFLAGS)) + --warnings-as-errors=* $(SRCS) -- $(filter-out -pie,$(CFLAGS)) ifeq ($(shell $(CC) -v 2>&1 | grep -c "gcc version"),1) TARGET := $(shell ${CC} -v 2>&1 | sed -n 's/Target: \(.*\)/\1/p') @@ -237,7 +244,7 @@ EXTRA_INCLUDES_OPT := -I$(EXTRA_INCLUDES) else EXTRA_INCLUDES_OPT := endif -cppcheck: $(wildcard *.c) $(wildcard *.h) +cppcheck: $(SRCS) $(HEADERS) cppcheck --std=c99 --error-exitcode=1 --enable=all --force \ --inconclusive --library=posix \ -I/usr/include $(EXTRA_INCLUDES_OPT) \ diff --git a/seccomp.sh b/seccomp.sh index 74eeb4b..17def4d 100755 --- a/seccomp.sh +++ b/seccomp.sh @@ -14,6 +14,7 @@ # Author: Stefano Brivio TMP="$(mktemp)" +IN="$@" OUT="seccomp.h" HEADER="/* This file was automatically generated by $(basename ${0}) */ @@ -231,9 +232,9 @@ gen_profile() { } printf '%s\n' "${HEADER}" > "${OUT}" -__profiles="$(sed -n 's/[\t ]*\*[\t ]*#syscalls:\([^ ]*\).*/\1/p' *.[ch] | sort -u)" +__profiles="$(sed -n 's/[\t ]*\*[\t ]*#syscalls:\([^ ]*\).*/\1/p' ${IN} | sort -u)" for __p in ${__profiles}; do - __calls="$(sed -n 's/[\t ]*\*[\t ]*#syscalls\(:'"${__p}"'\|\)[\t ]\{1,\}\(.*\)/\2/p' *.[ch])" + __calls="$(sed -n 's/[\t ]*\*[\t ]*#syscalls\(:'"${__p}"'\|\)[\t ]\{1,\}\(.*\)/\2/p' ${IN})" __calls="${__calls} ${EXTRA_SYSCALLS:-}" __calls="$(filter ${__calls})" echo "seccomp profile ${__p} allows: ${__calls}" | tr '\n' ' ' | fmt -t