From e2f28fcce1c40c70fa710f607a6fcf3625fbcf2b Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 13 Dec 2024 06:52:09 +0000 Subject: [PATCH 1/3] Revert "build: clarify that the libs are tmp artifacts" This reverts commit e953c773a6ee92c3fd49e43593b47d2816064b74. Signed-off-by: Hanno Becker --- .github/workflows/ci.yml | 2 +- Makefile | 2 ++ mk/config.mk | 1 - mk/crypto.mk | 8 ++++---- mk/rules.mk | 6 +++--- mk/schemes.mk | 16 ++++++++-------- 6 files changed, 18 insertions(+), 17 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 569d549f7..ad09e5482 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -104,7 +104,7 @@ jobs: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - name: make lib run: | - make test/build/tmp/libtmp_mlkem.a + make lib build_kat: needs: [quickcheck, quickcheck-windows] diff --git a/Makefile b/Makefile index 1721b79ef..a24c40245 100644 --- a/Makefile +++ b/Makefile @@ -21,6 +21,8 @@ quickcheck: buildall ./scripts/acvp $(Q)echo " Functionality and ACVP tests passed!" +lib: $(BUILD_DIR)/libmlkem.a + mlkem: \ $(MLKEM512_DIR)/bin/test_mlkem512 \ $(MLKEM768_DIR)/bin/test_mlkem768 \ diff --git a/mk/config.mk b/mk/config.mk index 4aedd674f..cf54aecae 100644 --- a/mk/config.mk +++ b/mk/config.mk @@ -89,7 +89,6 @@ include mk/auto.mk endif BUILD_DIR ?= test/build -TMP_DIR ?= test/build/tmp MAKE_OBJS = $(2:%=$(1)/%.o) OBJS = $(call MAKE_OBJS,$(BUILD_DIR),$(1)) diff --git a/mk/crypto.mk b/mk/crypto.mk index 8a9043be9..a66e73ead 100644 --- a/mk/crypto.mk +++ b/mk/crypto.mk @@ -5,16 +5,16 @@ ifeq ($(OPT),1) FIPS202_SRCS += $(wildcard fips202/native/aarch64/*.S) $(wildcard fips202/native/x86_64/xkcp/*.c) endif -$(TMP_DIR)/libtmp_fips202.a: $(call OBJS, $(FIPS202_SRCS)) -$(TMP_DIR)/libtmp_mlkem.a: $(call OBJS, $(FIPS202_SRCS)) +$(BUILD_DIR)/libfips202.a: $(call OBJS, $(FIPS202_SRCS)) +$(BUILD_DIR)/libmlkem.a: $(call OBJS, $(FIPS202_SRCS)) # all lib.a depends on libfips202.a define ADD_FIPS202 -$(TMP_DIR)/libtmp_$(1).a: LDLIBS += -ltmp_fips202 +$(BUILD_DIR)/lib$(1).a: LDLIBS += -lfips202 # NOTE: # - Merging multiple .a files with ar is more complex than building a single library directly from all the object files (.o). Hence, all .o files are added as dependencies here. -$(TMP_DIR)/libtmp_$(1).a: $(TMP_DIR)/libtmp_fips202.a $(call OBJS, $(FIPS202_SRCS)) +$(BUILD_DIR)/lib$(1).a: $(BUILD_DIR)/libfips202.a $(call OBJS, $(FIPS202_SRCS)) endef $(foreach scheme,mlkem512 mlkem768 mlkem1024, \ diff --git a/mk/rules.mk b/mk/rules.mk index f957e3455..032431b3e 100644 --- a/mk/rules.mk +++ b/mk/rules.mk @@ -23,16 +23,16 @@ $(BUILD_DIR)/%.a: $(CONFIG) # NOTE: # $AR doesn't care about duplicated symbols, one can only find it out via actually linking. # The easiest one to do this that one can think of is to create a dummy C file with empty main function on the fly, pipe it to $CC and link with the built library - $(eval _LIB := $(subst $(shell dirname $@)/lib,,$(@:%.a=%))) + $(eval _LIB := $(subst $(BUILD_DIR)/lib,,$(@:%.a=%))) ifneq ($(findstring clang,$(shell $(CC) --version)),) # if CC is clang $(Q)echo "int main() {return 0;}" \ - | $(CC) -x c - -L$(shell dirname $@) \ + | $(CC) -x c - -L$(BUILD_DIR) \ -all_load -Wl,-undefined,dynamic_lookup -l$(_LIB) \ -Imlkem $(wildcard test/notrandombytes/*.c) $(Q)rm -f a.out else # if CC is not clang $(Q)echo "int main() {return 0;}" \ - | $(CC) -x c - -L$(shell dirname $@) \ + | $(CC) -x c - -L$(BUILD_DIR) \ -Wl,--whole-archive,--unresolved-symbols=ignore-in-object-files -l$(_LIB) \ -Wl,--no-whole-archive \ -Imlkem $(wildcard test/notrandombytes/*.c) diff --git a/mk/schemes.mk b/mk/schemes.mk index 6e894df7a..2253d35dc 100644 --- a/mk/schemes.mk +++ b/mk/schemes.mk @@ -15,18 +15,18 @@ MLKEM1024_DIR = $(BUILD_DIR)/mlkem1024 # build lib.a define BUILD_LIB -$(TMP_DIR)/libtmp_$(1).a: CFLAGS += -static -$(TMP_DIR)/libtmp_$(1).a: $(call MAKE_OBJS,$(BUILD_DIR)/$(1),$(SOURCES)) +$(BUILD_DIR)/lib$(1).a: CFLAGS += -static +$(BUILD_DIR)/lib$(1).a: $(call MAKE_OBJS,$(BUILD_DIR)/$(1),$(SOURCES)) # NOTE: # - The order matters, or else the `MLKEM_K` preprocessor won't be properly set # - Merging multiple .a files with ar is more complex than building a single library directly from all the object files (.o). Hence, all .o files are added as dependencies here. -$(TMP_DIR)/libtmp_mlkem.a: $(TMP_DIR)/libtmp_$(1).a $(call MAKE_OBJS,$(BUILD_DIR)/$(1),$(SOURCES)) +$(BUILD_DIR)/libmlkem.a: $(BUILD_DIR)/lib$(1).a $(call MAKE_OBJS,$(BUILD_DIR)/$(1),$(SOURCES)) endef -$(TMP_DIR)/libtmp_mlkem512.a: CPPFLAGS += -DMLKEM_K=2 -$(TMP_DIR)/libtmp_mlkem768.a: CPPFLAGS += -DMLKEM_K=3 -$(TMP_DIR)/libtmp_mlkem1024.a: CPPFLAGS += -DMLKEM_K=4 +$(BUILD_DIR)/libmlkem512.a: CPPFLAGS += -DMLKEM_K=2 +$(BUILD_DIR)/libmlkem768.a: CPPFLAGS += -DMLKEM_K=3 +$(BUILD_DIR)/libmlkem1024.a: CPPFLAGS += -DMLKEM_K=4 # build libmlkem512.a libmlkem768.a libmlkem1024.a $(foreach scheme,mlkem512 mlkem768 mlkem1024, \ @@ -34,8 +34,8 @@ $(foreach scheme,mlkem512 mlkem768 mlkem1024, \ # rules for compilation for all tests: mainly linking with mlkem static link library define ADD_SOURCE -$(BUILD_DIR)/$(1)/bin/$(2)$(shell echo $(1) | tr -d -c 0-9): LDLIBS += -L$(TMP_DIR) -ltmp_$(1) -$(BUILD_DIR)/$(1)/bin/$(2)$(shell echo $(1) | tr -d -c 0-9): $(BUILD_DIR)/$(1)/test/$(2).c.o $(TMP_DIR)/libtmp_$(1).a +$(BUILD_DIR)/$(1)/bin/$(2)$(shell echo $(1) | tr -d -c 0-9): LDLIBS += -L$(BUILD_DIR) -l$(1) +$(BUILD_DIR)/$(1)/bin/$(2)$(shell echo $(1) | tr -d -c 0-9): $(BUILD_DIR)/$(1)/test/$(2).c.o $(BUILD_DIR)/lib$(1).a endef $(MLKEM512_DIR)/bin/%: CPPFLAGS += -DMLKEM_K=2 From 1c0ac3b856d8ebb22a74e6120c3453e3387da9f7 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 13 Dec 2024 07:03:46 +0000 Subject: [PATCH 2/3] Build: Don't build a libfips202.a The FIPS-202 code in mlkem-native is not meant as a standalone library; instead, it only provides the minimal functionality needed to implement ML-KEM. As such, it should not be generated as a separate library, but directly linked into the libmlkem{XXX}.a. This commit adjusts the build accordingly. Signed-off-by: Hanno Becker --- mk/crypto.mk | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/mk/crypto.mk b/mk/crypto.mk index a66e73ead..faf0eb047 100644 --- a/mk/crypto.mk +++ b/mk/crypto.mk @@ -5,18 +5,7 @@ ifeq ($(OPT),1) FIPS202_SRCS += $(wildcard fips202/native/aarch64/*.S) $(wildcard fips202/native/x86_64/xkcp/*.c) endif -$(BUILD_DIR)/libfips202.a: $(call OBJS, $(FIPS202_SRCS)) $(BUILD_DIR)/libmlkem.a: $(call OBJS, $(FIPS202_SRCS)) - -# all lib.a depends on libfips202.a -define ADD_FIPS202 -$(BUILD_DIR)/lib$(1).a: LDLIBS += -lfips202 -# NOTE: -# - Merging multiple .a files with ar is more complex than building a single library directly from all the object files (.o). Hence, all .o files are added as dependencies here. - -$(BUILD_DIR)/lib$(1).a: $(BUILD_DIR)/libfips202.a $(call OBJS, $(FIPS202_SRCS)) -endef - -$(foreach scheme,mlkem512 mlkem768 mlkem1024, \ - $(eval $(call ADD_FIPS202,$(scheme))) \ -) +$(BUILD_DIR)/libmlkem512.a: $(call OBJS, $(FIPS202_SRCS)) +$(BUILD_DIR)/libmlkem768.a: $(call OBJS, $(FIPS202_SRCS)) +$(BUILD_DIR)/libmlkem1024.a: $(call OBJS, $(FIPS202_SRCS)) From 7e9bb9401ddd9b0c0e426eceb47b8472d82484da Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 13 Dec 2024 07:22:52 +0000 Subject: [PATCH 3/3] Make: Minor clarification of build rules for libmlkem.a Signed-off-by: Hanno Becker --- mk/schemes.mk | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/mk/schemes.mk b/mk/schemes.mk index 2253d35dc..deabcdd86 100644 --- a/mk/schemes.mk +++ b/mk/schemes.mk @@ -19,8 +19,9 @@ $(BUILD_DIR)/lib$(1).a: CFLAGS += -static $(BUILD_DIR)/lib$(1).a: $(call MAKE_OBJS,$(BUILD_DIR)/$(1),$(SOURCES)) # NOTE: -# - The order matters, or else the `MLKEM_K` preprocessor won't be properly set -# - Merging multiple .a files with ar is more complex than building a single library directly from all the object files (.o). Hence, all .o files are added as dependencies here. +# libmlkem.a does not link against libmlkem{512,768,1024}.a, but the underlying object files. +# Still, we currently need a dependency on libmlkem{512,768,1024}.a here as otherwise there is +# a hiccup with the setting of MLKEM_K (TODO: look at this more closely) $(BUILD_DIR)/libmlkem.a: $(BUILD_DIR)/lib$(1).a $(call MAKE_OBJS,$(BUILD_DIR)/$(1),$(SOURCES)) endef