From f17c1df1ac86475f16d5d577b27a74271960e837 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 8 Oct 2018 17:03:06 -0700 Subject: [PATCH 1/5] backtrace support compiled with more conditions following #1356, only enable backtrace compilation on linux+glibc. Also, disable backtrace by default from "release" compilation, so that less platforms get impacted by the new requirements. Can be manually enabled/disabled using BACKTRACE=1/0. --- programs/Makefile | 20 +++++++++++--------- programs/README.md | 6 +++--- programs/fileio.c | 13 +++++++------ 3 files changed, 21 insertions(+), 18 deletions(-) diff --git a/programs/Makefile b/programs/Makefile index 8faa5111b..53f2f280a 100644 --- a/programs/Makefile +++ b/programs/Makefile @@ -134,11 +134,13 @@ else LZ4_MSG := $(NO_LZ4_MSG) endif -# enable backtrace symbol names for Linux/Darwin -ALL_SYMBOLS := 0 +# enable backtrace symbol names for Linux & Darwin +BACKTRACE ?= 0 +DEBUGFLAGS = -DBACKTRACE_ENABLE=$(BACKTRACE) ifeq (,$(filter Windows%, $(OS))) -ifeq ($(ALL_SYMBOLS), 1) -DEBUGFLAGS_LD+=-rdynamic +ifeq ($(BACKTRACE), 1) +DEBUGFLAGS += -DBACKTRACE_ENABLE=1 +DEBUGFLAGS_LD += -rdynamic endif endif @@ -168,12 +170,12 @@ endif $(CC) $(FLAGS) $^ $(RES_FILE) -o $@$(EXT) $(LDFLAGS) .PHONY: zstd-release -zstd-release: DEBUGFLAGS := +zstd-release: DEBUGFLAGS := -DBACKTRACE_ENABLE=0 zstd-release: DEBUGFLAGS_LD := zstd-release: zstd zstd32 : CPPFLAGS += $(THREAD_CPP) -zstd32 : LDFLAGS += $(THREAD_LD) +zstd32 : LDFLAGS += $(THREAD_LD) zstd32 : CPPFLAGS += -DZSTD_LEGACY_SUPPORT=$(ZSTD_LEGACY_SUPPORT) zstd32 : $(ZSTDLIB_FILES) zstdcli.c fileio.c bench.c datagen.c dibio.c ifneq (,$(filter Windows%,$(OS))) @@ -185,17 +187,17 @@ zstd-nolegacy : $(ZSTD_FILES) $(ZDICT_FILES) zstdcli.o fileio.c bench.o datagen. $(CC) $(FLAGS) $^ -o $@$(EXT) $(LDFLAGS) zstd-nomt : THREAD_CPP := -zstd-nomt : THREAD_LD := +zstd-nomt : THREAD_LD := zstd-nomt : THREAD_MSG := - multi-threading disabled zstd-nomt : zstd zstd-nogz : ZLIBCPP := -zstd-nogz : ZLIBLD := +zstd-nogz : ZLIBLD := zstd-nogz : ZLIB_MSG := - gzip support is disabled zstd-nogz : zstd zstd-noxz : LZMACPP := -zstd-noxz : LZMALD := +zstd-noxz : LZMALD := zstd-noxz : LZMA_MSG := - xz/lzma support is disabled zstd-noxz : zstd diff --git a/programs/README.md b/programs/README.md index 804cb8b0a..ca9056eaa 100644 --- a/programs/README.md +++ b/programs/README.md @@ -61,12 +61,12 @@ There are however other Makefile targets that create different variations of CLI In which case, linking stage will fail if `lz4` library cannot be found. This is useful to prevent silent feature disabling. -- __ALL_SYMBOLS__ : `zstd` can display a stack backtrace if the execution +- __BACKTRACE__ : `zstd` can display a stack backtrace when execution generates a runtime exception. By default, this feature may be degraded/disabled on some platforms unless additional compiler directives are - applied. When triaging a runtime issue, enabling this feature can provided + applied. When triaging a runtime issue, enabling this feature can provide more context to determine the location of the fault. - Example : `make zstd ALL_SYMBOLS=1` + Example : `make zstd BACKTRACE=1` #### Aggregation of parameters diff --git a/programs/fileio.c b/programs/fileio.c index 1a73a75e4..aff1b512c 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -20,9 +20,10 @@ # define _POSIX_SOURCE 1 /* disable %llu warnings with MinGW on Windows */ #endif -#if !defined(BACKTRACES_ENABLE) && \ - (defined(__GLIBC__) || (defined(__APPLE__) && defined(__MACH__)) ) -# define BACKTRACES_ENABLE 1 +#if !defined(BACKTRACE_ENABLE) \ + && ((defined(__linux__) && defined(__GLIBC__)) \ + || (defined(__APPLE__) && defined(__MACH__)) ) +# define BACKTRACE_ENABLE 1 #endif @@ -37,7 +38,7 @@ #include #include /* errno */ #include -#ifdef BACKTRACES_ENABLE +#ifdef BACKTRACE_ENABLE # include /* backtrace, backtrace_symbols */ #endif @@ -167,7 +168,7 @@ static void clearHandler(void) /*-********************************************************* * Termination signal trapping (Print debug stack trace) ***********************************************************/ -#ifdef BACKTRACES_ENABLE +#ifdef BACKTRACE_ENABLE #define MAX_STACK_FRAMES 50 @@ -208,7 +209,7 @@ static void ABRThandler(int sig) { void FIO_addAbortHandler() { -#ifdef BACKTRACES_ENABLE +#ifdef BACKTRACE_ENABLE signal(SIGABRT, ABRThandler); signal(SIGFPE, ABRThandler); signal(SIGILL, ABRThandler); From 1e0c5466c5a2cd52210270f56f61ac5ca2b1ec4a Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 9 Oct 2018 12:57:42 -0700 Subject: [PATCH 2/5] fixed BACKTRACE_ENABLE macro test --- programs/Makefile | 1 - programs/fileio.c | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/programs/Makefile b/programs/Makefile index 53f2f280a..9e241357b 100644 --- a/programs/Makefile +++ b/programs/Makefile @@ -136,7 +136,6 @@ endif # enable backtrace symbol names for Linux & Darwin BACKTRACE ?= 0 -DEBUGFLAGS = -DBACKTRACE_ENABLE=$(BACKTRACE) ifeq (,$(filter Windows%, $(OS))) ifeq ($(BACKTRACE), 1) DEBUGFLAGS += -DBACKTRACE_ENABLE=1 diff --git a/programs/fileio.c b/programs/fileio.c index aff1b512c..8b1e9f2f2 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -38,7 +38,7 @@ #include #include /* errno */ #include -#ifdef BACKTRACE_ENABLE +#if defined(BACKTRACE_ENABLE) && (BACKTRACE_ENABLE >= 1) # include /* backtrace, backtrace_symbols */ #endif @@ -168,7 +168,7 @@ static void clearHandler(void) /*-********************************************************* * Termination signal trapping (Print debug stack trace) ***********************************************************/ -#ifdef BACKTRACE_ENABLE +#if defined(BACKTRACE_ENABLE) && (BACKTRACE_ENABLE>=1) #define MAX_STACK_FRAMES 50 @@ -209,7 +209,7 @@ static void ABRThandler(int sig) { void FIO_addAbortHandler() { -#ifdef BACKTRACE_ENABLE +#if defined(BACKTRACE_ENABLE) && (BACKTRACE_ENABLE>=1) signal(SIGABRT, ABRThandler); signal(SIGFPE, ABRThandler); signal(SIGILL, ABRThandler); From e0ab6b61b7df61172bb9f2ce7eb9d76106d5e556 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 9 Oct 2018 17:12:21 -0700 Subject: [PATCH 3/5] fixed explicit BACKTRACE order and automatic linux backtrace detection : __GLIBC__ must be tested after #include --- programs/Makefile | 6 ++++-- programs/fileio.c | 18 +++++++++--------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/programs/Makefile b/programs/Makefile index 9e241357b..ac17caee8 100644 --- a/programs/Makefile +++ b/programs/Makefile @@ -134,8 +134,10 @@ else LZ4_MSG := $(NO_LZ4_MSG) endif -# enable backtrace symbol names for Linux & Darwin -BACKTRACE ?= 0 +# explicit backtrace enable/disable for Linux & Darwin +ifeq ($(BACKTRACE), 0) +DEBUGFLAGS += -DBACKTRACE_ENABLE=0 +endif ifeq (,$(filter Windows%, $(OS))) ifeq ($(BACKTRACE), 1) DEBUGFLAGS += -DBACKTRACE_ENABLE=1 diff --git a/programs/fileio.c b/programs/fileio.c index 8b1e9f2f2..16bfd530d 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -20,12 +20,6 @@ # define _POSIX_SOURCE 1 /* disable %llu warnings with MinGW on Windows */ #endif -#if !defined(BACKTRACE_ENABLE) \ - && ((defined(__linux__) && defined(__GLIBC__)) \ - || (defined(__APPLE__) && defined(__MACH__)) ) -# define BACKTRACE_ENABLE 1 -#endif - /*-************************************* * Includes @@ -38,9 +32,6 @@ #include #include /* errno */ #include -#if defined(BACKTRACE_ENABLE) && (BACKTRACE_ENABLE >= 1) -# include /* backtrace, backtrace_symbols */ -#endif #if defined (_MSC_VER) # include @@ -168,8 +159,17 @@ static void clearHandler(void) /*-********************************************************* * Termination signal trapping (Print debug stack trace) ***********************************************************/ +#if !defined(BACKTRACE_ENABLE) \ + && ((defined(__linux__) && defined(__GLIBC__)) \ + || (defined(__APPLE__) && defined(__MACH__)) ) +# define BACKTRACE_ENABLE 1 +#endif + + #if defined(BACKTRACE_ENABLE) && (BACKTRACE_ENABLE>=1) +#include /* backtrace, backtrace_symbols */ + #define MAX_STACK_FRAMES 50 static void ABRThandler(int sig) { From 70d8c2a0313deb2a64b549a80fad4eb63596dec2 Mon Sep 17 00:00:00 2001 From: Julian Fessard Date: Tue, 9 Oct 2018 17:14:57 -0700 Subject: [PATCH 4/5] fileio.c: Disable backtrace when built with address sanitizer Covers clang and gcc's sanitizer flags. Can still be overridden through CFLAGS on commandline. --- programs/fileio.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/programs/fileio.c b/programs/fileio.c index 8b1e9f2f2..a2d5af2c4 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -20,6 +20,14 @@ # define _POSIX_SOURCE 1 /* disable %llu warnings with MinGW on Windows */ #endif +#if defined(__has_feature) && !defined(BACKTRACE_ENABLE) /* Clang compiler */ +# if (__has_feature(address_sanitizer)) +# define BACKTRACE_ENABLE 0 +# endif /* __has_feature(address_sanitizer) */ +#elif defined(__SANITIZE_ADDRESS__) && !defined(BACKTRACE_ENABLE) /* GCC compiler */ +# define BACKTRACE_ENABLE 0 +#endif + #if !defined(BACKTRACE_ENABLE) \ && ((defined(__linux__) && defined(__GLIBC__)) \ || (defined(__APPLE__) && defined(__MACH__)) ) From b304b679e50be863f4f532ff176617f06056ef62 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 9 Oct 2018 17:56:59 -0700 Subject: [PATCH 5/5] use #if BACKTRACE_ENABLE directly as suggested by @terrelln --- programs/fileio.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index 1eb9a1e0a..7cbaab7da 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -166,14 +166,20 @@ static void clearHandler(void) # define BACKTRACE_ENABLE 0 #endif -#if !defined(BACKTRACE_ENABLE) \ - && ((defined(__linux__) && defined(__GLIBC__)) \ - || (defined(__APPLE__) && defined(__MACH__)) ) -# define BACKTRACE_ENABLE 1 +#if !defined(BACKTRACE_ENABLE) +/* automatic detector : backtrace enabled by default on linux+glibc and osx */ +# if (defined(__linux__) && defined(__GLIBC__)) \ + || (defined(__APPLE__) && defined(__MACH__)) +# define BACKTRACE_ENABLE 1 +# else +# define BACKTRACE_ENABLE 0 +# endif #endif +/* note : after this point, BACKTRACE_ENABLE is necessarily defined */ -#if defined(BACKTRACE_ENABLE) && (BACKTRACE_ENABLE>=1) + +#if BACKTRACE_ENABLE #include /* backtrace, backtrace_symbols */ @@ -216,7 +222,7 @@ static void ABRThandler(int sig) { void FIO_addAbortHandler() { -#if defined(BACKTRACE_ENABLE) && (BACKTRACE_ENABLE>=1) +#if BACKTRACE_ENABLE signal(SIGABRT, ABRThandler); signal(SIGFPE, ABRThandler); signal(SIGILL, ABRThandler);