From c6ae2e83bcf9da8869364921db39ebd317faf7d4 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 28 Apr 2020 18:29:20 -0700 Subject: [PATCH 1/4] fix libzstd-mt underlinking issue fix #2045 When compiling `libzstd` in multithreading mode, the `libzstd-mt` recipe would not include `-pthread`, resulting in an underlinked dynamic library. Added a test on Travis to check that the library is fully linked. This makes it possible, in some future release, to build a multi-threaded `libzstd` dynamic library by default as it would no longer impact the build script of user programs. --- .travis.yml | 4 +++- lib/Makefile | 18 +++++++++--------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/.travis.yml b/.travis.yml index e4e45560..31fc4257 100644 --- a/.travis.yml +++ b/.travis.yml @@ -56,12 +56,14 @@ matrix: # DEVNULLRIGHTS : will request sudo rights to test permissions on /dev/null - DEVNULLRIGHTS=test make test - - name: gcc-6 + gcc-7 compilation # ~ 3mn + - name: gcc-6 + gcc-7 + libzstdmt compilation # ~ 6mn script: - make gcc6install gcc7install - CC=gcc-6 CFLAGS=-Werror make -j all - make clean - CC=gcc-7 CFLAGS=-Werror make -j all + - make clean + - LDFLAGS=-Wl,--no-undefined make -C lib libzstd-mt - name: gcc-8 + ASan + UBSan + Test Zstd # ~6.5mn script: diff --git a/lib/Makefile b/lib/Makefile index fc7daf08..7a05b158 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -154,9 +154,6 @@ libzstd.a: $(ZSTD_OBJ) @echo compiling static library @$(AR) $(ARFLAGS) $@ $^ -libzstd.a-mt: CPPFLAGS += -DZSTD_MULTITHREAD -libzstd.a-mt: libzstd.a - ifneq (,$(filter Windows%,$(OS))) LIBZSTD = dll\libzstd.dll @@ -180,17 +177,20 @@ endif libzstd : $(LIBZSTD) -libzstd-mt : CPPFLAGS += -DZSTD_MULTITHREAD +lib : libzstd.a libzstd + +%-mt : CPPFLAGS += -DZSTD_MULTITHREAD +%-mt : LDFLAGS += -pthread + libzstd-mt : libzstd -lib: libzstd.a libzstd +libzstd.a-mt: libzstd.a -lib-mt: CPPFLAGS += -DZSTD_MULTITHREAD -lib-mt: lib +lib-mt : lib -lib-release lib-release-mt: DEBUGFLAGS := +%-release : DEBUGFLAGS := lib-release: lib -lib-release-mt: lib-mt +lib-mt-release : lib-mt # Special case : building library in single-thread mode _and_ without zstdmt_compress.c ZSTDMT_FILES = compress/zstdmt_compress.c From f77fd5ced0042bcccdb68f387c725e3ee142589c Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 28 Apr 2020 18:43:55 -0700 Subject: [PATCH 2/4] generalized pattern rules --- lib/Makefile | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/Makefile b/lib/Makefile index 7a05b158..ac38867a 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -181,16 +181,13 @@ lib : libzstd.a libzstd %-mt : CPPFLAGS += -DZSTD_MULTITHREAD %-mt : LDFLAGS += -pthread - -libzstd-mt : libzstd - -libzstd.a-mt: libzstd.a - -lib-mt : lib +%-mt : % + @echo multi-threading build completed %-release : DEBUGFLAGS := -lib-release: lib -lib-mt-release : lib-mt +%-release : % + @echo release build completed + # Special case : building library in single-thread mode _and_ without zstdmt_compress.c ZSTDMT_FILES = compress/zstdmt_compress.c From 7ea2ae66495854346ae7cd26ab926859f410d492 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 28 Apr 2020 21:18:29 -0700 Subject: [PATCH 3/4] added test linking user program to multi-threaded libzstd --- .travis.yml | 3 +++ programs/Makefile | 22 +++++++++++++++++++--- tests/Makefile | 22 ++++++++++------------ 3 files changed, 32 insertions(+), 15 deletions(-) diff --git a/.travis.yml b/.travis.yml index 31fc4257..aeaae729 100644 --- a/.travis.yml +++ b/.travis.yml @@ -64,6 +64,9 @@ matrix: - CC=gcc-7 CFLAGS=-Werror make -j all - make clean - LDFLAGS=-Wl,--no-undefined make -C lib libzstd-mt + - make -C tests zbufftest-dll + # LDFLAGS=-Wl,--no-undefined : will make the linker fail if dll is underlinked + # zbufftest-dll : test that a user program can link to multi-threaded libzstd without specifying -pthread - name: gcc-8 + ASan + UBSan + Test Zstd # ~6.5mn script: diff --git a/programs/Makefile b/programs/Makefile index 8146732b..3366c093 100644 --- a/programs/Makefile +++ b/programs/Makefile @@ -79,6 +79,9 @@ endif # Sort files in alphabetical order for reproducible builds ZSTDLIB_FILES := $(sort $(wildcard $(ZSTD_FILES)) $(wildcard $(ZSTDLEGACY_FILES)) $(wildcard $(ZDICT_FILES))) +ZSTD_CLI_FILES := $(wildcard *.c) +ZSTD_CLI_OBJ := $(patsubst %.c,%.o,$(ZSTD_CLI_FILES)) + # Define *.exe as extension for Windows systems ifneq (,$(filter Windows%,$(OS))) EXT =.exe @@ -172,7 +175,7 @@ zstd : CPPFLAGS += -DZSTD_LEGACY_SUPPORT=$(ZSTD_LEGACY_SUPPORT) ifneq (,$(filter Windows%,$(OS))) zstd : $(RES_FILE) endif -zstd : $(ZSTDLIB_FILES) zstdcli.o util.o timefn.o fileio.o benchfn.o benchzstd.o datagen.o dibio.o +zstd : $(ZSTDLIB_FILES) $(ZSTD_CLI_OBJ) @echo "$(THREAD_MSG)" @echo "$(ZLIB_MSG)" @echo "$(LZMA_MSG)" @@ -190,10 +193,10 @@ zstd32 : CPPFLAGS += -DZSTD_LEGACY_SUPPORT=$(ZSTD_LEGACY_SUPPORT) ifneq (,$(filter Windows%,$(OS))) zstd32 : $(RES32_FILE) endif -zstd32 : $(ZSTDLIB_FILES) zstdcli.c util.c timefn.c fileio.c benchfn.c benchzstd.c datagen.c dibio.c +zstd32 : $(ZSTDLIB_FILES) $(ZSTD_CLI_FILES) $(CC) -m32 $(FLAGS) $^ -o $@$(EXT) -zstd-nolegacy : $(ZSTD_FILES) $(ZDICT_FILES) zstdcli.o util.o fileio.c benchfn.o benchzstd.o timefn.o datagen.o dibio.o +zstd-nolegacy : $(ZSTD_FILES) $(ZDICT_FILES) $(ZSTD_CLI_OBJ) $(CC) $(FLAGS) $^ -o $@$(EXT) $(LDFLAGS) zstd-nomt : THREAD_CPP := @@ -211,6 +214,19 @@ zstd-noxz : LZMALD := zstd-noxz : LZMA_MSG := - xz/lzma support is disabled zstd-noxz : zstd +# note : the following target doesn't build +# because zstd uses non-public symbols from libzstd +# such as XXH64 (for benchmark), +# ZDICT_trainFromBuffer_unsafe_legacy for dictionary builder +# and ZSTD_cycleLog (likely for --patch-from) +# It's unclear at this stage if this is a scenario we want to support +## zstd-dll: zstd executable linked to dynamic library libzstd (must already exist) +.PHONY: zstd-dll +zstd-dll : LDFLAGS+= -L$(ZSTDDIR) -lzstd +zstd-dll : ZSTDLIB_FILES = +zstd-dll : $(ZSTD_CLI_OBJ) + $(CC) $(FLAGS) $^ -o $@$(EXT) $(LDFLAGS) + zstd-pgo : $(MAKE) clean diff --git a/tests/Makefile b/tests/Makefile index 3d007471..d347a948 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -91,6 +91,7 @@ allnothread: MULTITHREAD_CPP= allnothread: MULTITHREAD_LD= allnothread: fullbench fuzzer paramgrill datagen decodecorpus +# note : broken : requires symbols unavailable from dynamic library dll: fuzzer-dll zstreamtest-dll PHONY: zstd zstd32 zstd-nolegacy # must be phony, only external makefile knows how to build them, or if they need an update @@ -98,12 +99,15 @@ zstd zstd32 zstd-nolegacy: $(MAKE) -C $(PRGDIR) $@ MOREFLAGS+="$(DEBUGFLAGS)" gzstd: - $(MAKE) -C $(PRGDIR) zstd HAVE_ZLIB=1 MOREFLAGS+="$(DEBUGFLAGS)" + $(MAKE) -C $(PRGDIR) $@ HAVE_ZLIB=1 MOREFLAGS+="$(DEBUGFLAGS)" -.PHONY: zstd-dll -zstd-dll : +.PHONY: libzstd +libzstd : $(MAKE) -C $(ZSTDDIR) libzstd +%-dll : libzstd +%-dll : LDFLAGS+= -L$(ZSTDDIR) -lzstd + .PHONY: zstd-staticLib zstd-staticLib : $(MAKE) -C $(ZSTDDIR) libzstd.a @@ -141,9 +145,7 @@ fullbench-lib : zstd-staticLib fullbench-lib : $(PRGDIR)/datagen.c $(PRGDIR)/util.c $(PRGDIR)/timefn.c $(PRGDIR)/benchfn.c fullbench.c $(CC) $(FLAGS) $(filter %.c,$^) -o $@$(EXT) $(ZSTDDIR)/libzstd.a -# note : broken : requires unavailable symbols -fullbench-dll : zstd-dll -fullbench-dll : LDFLAGS+= -L$(ZSTDDIR) -lzstd +# note : broken : requires symbols unavailable from dynamic library fullbench-dll: $(PRGDIR)/datagen.c $(PRGDIR)/util.c $(PRGDIR)/benchfn.c $(PRGDIR)/timefn.c fullbench.c # $(CC) $(FLAGS) $(filter %.c,$^) -o $@$(EXT) -DZSTD_DLL_IMPORT=1 $(ZSTDDIR)/dll/libzstd.dll $(CC) $(FLAGS) $(filter %.c,$^) -o $@$(EXT) @@ -156,8 +158,7 @@ fuzzer32: $(ZSTD_FILES) fuzzer fuzzer32 : $(ZDICT_FILES) $(PRGDIR)/util.c $(PRGDIR)/timefn.c $(PRGDIR)/datagen.c fuzzer.c $(CC) $(FLAGS) $^ -o $@$(EXT) -fuzzer-dll : zstd-dll -fuzzer-dll : LDFLAGS+= -L$(ZSTDDIR) -lzstd +# note : broken : requires symbols unavailable from dynamic library fuzzer-dll : $(ZSTDDIR)/common/xxhash.c $(PRGDIR)/util.c $(PRGDIR)/timefn.c $(PRGDIR)/datagen.c fuzzer.c $(CC) $(CPPFLAGS) $(CFLAGS) $(filter %.c,$^) $(LDFLAGS) -o $@$(EXT) @@ -167,8 +168,6 @@ zbufftest32 : CFLAGS += -m32 zbufftest zbufftest32 : $(ZSTD_OBJECTS) $(ZBUFF_FILES) $(PRGDIR)/util.c $(PRGDIR)/timefn.c $(PRGDIR)/datagen.c zbufftest.c $(CC) $(FLAGS) $^ -o $@$(EXT) -zbufftest-dll : zstd-dll -zbufftest-dll : LDFLAGS+= -L$(ZSTDDIR) -lzstd zbufftest-dll : $(ZSTDDIR)/common/xxhash.c $(PRGDIR)/util.c $(PRGDIR)/timefn.c $(PRGDIR)/datagen.c zbufftest.c $(CC) $(CPPFLAGS) $(CFLAGS) $(filter %.c,$^) $(LDFLAGS) -o $@$(EXT) @@ -191,8 +190,7 @@ zstreamtest_tsan : CFLAGS += -fsanitize=thread zstreamtest_tsan : $(ZSTREAMFILES) $(CC) $(FLAGS) $(MULTITHREAD) $^ -o $@$(EXT) -zstreamtest-dll : zstd-dll -zstreamtest-dll : LDFLAGS+= -L$(ZSTDDIR) -lzstd +# note : broken : requires symbols unavailable from dynamic library zstreamtest-dll : $(ZSTDDIR)/common/xxhash.c # xxh symbols not exposed from dll zstreamtest-dll : $(ZSTREAM_LOCAL_FILES) $(CC) $(CPPFLAGS) $(CFLAGS) $(filter %.c,$^) $(LDFLAGS) -o $@$(EXT) From 6f62a9caaa8fa2d7c877c1357b537ee9f37ccc12 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 29 Apr 2020 11:56:21 -0700 Subject: [PATCH 4/4] fixed zstd-nolegacy target when compiled as part of allVariants (though I'm unsure why it was working before ...) --- programs/Makefile | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/programs/Makefile b/programs/Makefile index 3366c093..abad6da6 100644 --- a/programs/Makefile +++ b/programs/Makefile @@ -196,6 +196,8 @@ endif zstd32 : $(ZSTDLIB_FILES) $(ZSTD_CLI_FILES) $(CC) -m32 $(FLAGS) $^ -o $@$(EXT) +## zstd-nolegacy: same scope as zstd, with just support of legacy formats removed +zstd-nolegacy : LDFLAGS += $(THREAD_LD) $(ZLIBLD) $(LZMALD) $(LZ4LD) $(DEBUGFLAGS_LD) zstd-nolegacy : $(ZSTD_FILES) $(ZDICT_FILES) $(ZSTD_CLI_OBJ) $(CC) $(FLAGS) $^ -o $@$(EXT) $(LDFLAGS) @@ -240,7 +242,7 @@ zstd-pgo : $(RM) zstd *.o $(ZSTDDECOMP_O) $(ZSTDDIR)/compress/*.o $(MAKE) zstd MOREFLAGS=-fprofile-use -# minimal target, with only zstd compression and decompression. no bench. no legacy. +## zstd-small: minimal target, supporting only zstd compression and decompression. no bench. no legacy. no other format. zstd-small: CFLAGS = -Os -s zstd-frugal zstd-small: $(ZSTD_FILES) zstdcli.c util.c timefn.c fileio.c $(CC) $(FLAGS) -DZSTD_NOBENCH -DZSTD_NODICT $^ -o $@$(EXT) @@ -251,6 +253,7 @@ zstd-decompress: $(ZSTDCOMMON_FILES) $(ZSTDDECOMP_FILES) zstdcli.c util.c timefn zstd-compress: $(ZSTDCOMMON_FILES) $(ZSTDCOMP_FILES) zstdcli.c util.c timefn.c fileio.c $(CC) $(FLAGS) -DZSTD_NOBENCH -DZSTD_NODICT -DZSTD_NODECOMPRESS $^ -o $@$(EXT) +## zstd-dictBuilder: executable supporting dictionary creation and compression (only) zstd-dictBuilder: CPPFLAGS += -DZSTD_NOBENCH -DZSTD_NODECOMPRESS zstd-dictBuilder: $(ZSTDCOMMON_FILES) $(ZSTDCOMP_FILES) $(ZDICT_FILES) zstdcli.c util.c timefn.c fileio.c dibio.c $(CC) $(FLAGS) $^ -o $@$(EXT) @@ -318,6 +321,7 @@ ifeq ($HAVE_COLORNEVER, 1) EGREP_OPTIONS += --color=never endif EGREP = egrep $(EGREP_OPTIONS) +AWK = awk # Print a two column output of targets and their description. To add a target description, put a # comment in the Makefile with the format "## : ". For example: @@ -326,14 +330,14 @@ EGREP = egrep $(EGREP_OPTIONS) .PHONY: list list: @TARGETS=$$($(MAKE) -pRrq -f $(lastword $(MAKEFILE_LIST)) : 2>/dev/null \ - | awk -v RS= -F: '/^# File/,/^# Finished Make data base/ {if ($$1 !~ "^[#.]") {print $$1}}' \ + | $(AWK) -v RS= -F: '/^# File/,/^# Finished Make data base/ {if ($$1 !~ "^[#.]") {print $$1}}' \ | $(EGREP) -v -e '^[^[:alnum:]]' | sort); \ { \ printf "Target Name\tDescription\n"; \ printf "%0.s-" {1..16}; printf "\t"; printf "%0.s-" {1..40}; printf "\n"; \ for target in $$TARGETS; do \ line=$$($(EGREP) "^##[[:space:]]+$$target:" $(lastword $(MAKEFILE_LIST))); \ - description=$$(echo $$line | awk '{i=index($$0,":"); print substr($$0,i+1)}' | xargs); \ + description=$$(echo $$line | $(AWK) '{i=index($$0,":"); print substr($$0,i+1)}' | xargs); \ printf "$$target\t$$description\n"; \ done \ } | column -t -s $$'\t'