From a06574fc9701b056529825e9079421c29fd4a031 Mon Sep 17 00:00:00 2001 From: Casey McGinty Date: Thu, 6 Sep 2018 18:46:52 -0700 Subject: [PATCH 1/6] Print a stack trace on unexpected term signal (e.g. SIGABRT) For OSX and Linux, add a signal handler to SIGABRT, SGIFPE, SIGILL, SIGSEGV, and SIGBUS. When the program terminates unexpectedly the handler will print the current stack to the terminal to help determine the location of the failure. On OSX the output will look like: ``` Stack trace: 4 zstd 0x000000010927ed96 main + 16886 5 libdyld.dylib 0x00007fff767d1015 start + 1 6 ??? 0x0000000000000001 0x0 + 1 ``` On Linux the output will look like: ``` Stack trace: ./zstd() [0x4b8e1b] ./zstd() [0x4b928a] ./zstd() [0x403dc2] /lib64/libc.so.6(__libc_start_main+0xf5) [0x7f5e0fbb0445] ./zstd() [0x405754] ``` As is, the code does not function on WIN32. See also: https://oroboro.com/stack-trace-on-crash/ --- lib/common/zstd_common.c | 54 ++++++++++++++++++++++++++++++++++++++ lib/common/zstd_internal.h | 3 +++ programs/fileio.c | 2 +- programs/zstdcli.c | 4 +++ 4 files changed, 62 insertions(+), 1 deletion(-) diff --git a/lib/common/zstd_common.c b/lib/common/zstd_common.c index 6f05d240e..d3ad05730 100644 --- a/lib/common/zstd_common.c +++ b/lib/common/zstd_common.c @@ -15,10 +15,21 @@ ***************************************/ #include /* malloc, calloc, free */ #include /* memset */ +#include /* fprintf(), stderr */ +#include /* signal() */ +#ifndef _WIN32 +#include /* backtrace, backtrace_symbols, symbollist */ +#endif #include "error_private.h" #include "zstd_internal.h" +/*-************************************ +* Display Macros +**************************************/ +#define DISPLAY(...) fprintf(stderr, __VA_ARGS__) + + /*-**************************************** * Version ******************************************/ @@ -79,3 +90,46 @@ void ZSTD_free(void* ptr, ZSTD_customMem customMem) free(ptr); } } + + +/*-********************************************************* +* Termination signal trapping (Print debug stack trace) +***********************************************************/ +#define MAX_STACK_FRAMES 50 + +#ifndef _WIN32 +static void ABRThandler(int sig) +{ + void* addrlist[MAX_STACK_FRAMES + 1]; + char** symbollist; + U32 addrlen, i; + + (void)sig; + + DISPLAY("Stack trace:\n"); + // Retrieve current stack addresses. + addrlen = backtrace(addrlist, sizeof(addrlist) / sizeof(void*)); + if (addrlen == 0) { + DISPLAY("\n"); + return; + } + // Create readable strings to each frame. + symbollist = backtrace_symbols(addrlist, addrlen); + // Print the stack trace, excluding calls handling the signal. + for (i = 4; i < addrlen; i++) { + DISPLAY("%s\n", symbollist[i]); + } + free(symbollist); +} +#endif + +void ZSTD_addAbortHandler() +{ +#ifndef _WIN32 + signal(SIGABRT, ABRThandler); + signal(SIGFPE, ABRThandler); + signal(SIGILL, ABRThandler); + signal(SIGSEGV, ABRThandler); + signal(SIGBUS, ABRThandler); +#endif +} diff --git a/lib/common/zstd_internal.h b/lib/common/zstd_internal.h index e75adfa61..b555dd828 100644 --- a/lib/common/zstd_internal.h +++ b/lib/common/zstd_internal.h @@ -206,6 +206,9 @@ void* ZSTD_malloc(size_t size, ZSTD_customMem customMem); void* ZSTD_calloc(size_t size, ZSTD_customMem customMem); void ZSTD_free(void* ptr, ZSTD_customMem customMem); +/* custom crash signal handler */ +void ZSTD_addAbortHandler(void); + MEM_STATIC U32 ZSTD_highbit32(U32 val) /* compress, dictBuilder, decodeCorpus */ { diff --git a/programs/fileio.c b/programs/fileio.c index 5f10958d7..7e9c31cb2 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -157,7 +157,7 @@ static void clearHandler(void) } -/* ************************************************************ +/*-************************************************************ * Avoid fseek()'s 2GiB barrier with MSVC, macOS, *BSD, MinGW ***************************************************************/ #if defined(_MSC_VER) && _MSC_VER >= 1400 diff --git a/programs/zstdcli.c b/programs/zstdcli.c index 0fccd34f9..0688d44bc 100644 --- a/programs/zstdcli.c +++ b/programs/zstdcli.c @@ -39,6 +39,7 @@ #endif #define ZSTD_STATIC_LINKING_ONLY /* ZSTD_maxCLevel */ #include "zstd.h" /* ZSTD_VERSION_STRING */ +#include "zstd_internal.h" /* ZSTD_addAbortHandler */ /*-************************************ @@ -511,6 +512,9 @@ int main(int argCount, const char* argv[]) if (exeNameMatch(programName, ZSTD_UNLZ4)) { operation=zom_decompress; FIO_setCompressionType(FIO_lz4Compression); } /* behave like unlz4, also supports multiple formats */ memset(&compressionParams, 0, sizeof(compressionParams)); + /* init crash handler */ + ZSTD_addAbortHandler(); + /* command switches */ for (argNb=1; argNb Date: Tue, 11 Sep 2018 10:40:45 -0700 Subject: [PATCH 2/6] Refactor abort signal handling - Print signal name to term - Add -rdynamic option to generate Linux symbol names in backtrace - Raise default signal after handler to ensure program termination --- lib/common/zstd_common.c | 24 +++++++++++++++++++++--- programs/Makefile | 1 + 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/lib/common/zstd_common.c b/lib/common/zstd_common.c index d3ad05730..8377cf0fd 100644 --- a/lib/common/zstd_common.c +++ b/lib/common/zstd_common.c @@ -98,15 +98,30 @@ void ZSTD_free(void* ptr, ZSTD_customMem customMem) #define MAX_STACK_FRAMES 50 #ifndef _WIN32 + +#ifdef __linux__ +#define START_STACK_FRAME 2 +#elif defined __APPLE__ +#define START_STACK_FRAME 4 +#endif + static void ABRThandler(int sig) { + const char* name; void* addrlist[MAX_STACK_FRAMES + 1]; char** symbollist; U32 addrlen, i; - (void)sig; + switch (sig) { + case SIGABRT: name = "SIGABRT"; break; + case SIGFPE: name = "SIGFPE"; break; + case SIGILL: name = "SIGILL"; break; + case SIGINT: name = "SIGINT"; break; + case SIGSEGV: name = "SIGSEGV"; break; + default: name = "UNKNOWN"; break; + } - DISPLAY("Stack trace:\n"); + DISPLAY("Caught %s signal, printing stack:\n", name); // Retrieve current stack addresses. addrlen = backtrace(addrlist, sizeof(addrlist) / sizeof(void*)); if (addrlen == 0) { @@ -116,10 +131,13 @@ static void ABRThandler(int sig) // Create readable strings to each frame. symbollist = backtrace_symbols(addrlist, addrlen); // Print the stack trace, excluding calls handling the signal. - for (i = 4; i < addrlen; i++) { + for (i = START_STACK_FRAME; i < addrlen; i++) { DISPLAY("%s\n", symbollist[i]); } free(symbollist); + // Reset and raise the signal so default handler runs. + signal(sig, SIG_DFL); + raise(sig); } #endif diff --git a/programs/Makefile b/programs/Makefile index 912f9eff0..49319484c 100644 --- a/programs/Makefile +++ b/programs/Makefile @@ -48,6 +48,7 @@ DEBUGFLAGS+=-Wall -Wextra -Wcast-qual -Wcast-align -Wshadow \ -Wvla -Wformat=2 -Winit-self -Wfloat-equal -Wwrite-strings \ -Wredundant-decls CFLAGS += $(DEBUGFLAGS) $(MOREFLAGS) +LDFLAGS += -rdynamic FLAGS = $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) From d4337b6f1dd5fb5411c9a705ac90a3d1811fc1e7 Mon Sep 17 00:00:00 2001 From: Casey McGinty Date: Tue, 11 Sep 2018 11:39:49 -0700 Subject: [PATCH 3/6] Move ABRThandler func out of internal lib --- lib/common/zstd_common.c | 65 -------------------------------------- lib/common/zstd_internal.h | 3 -- programs/fileio.c | 59 ++++++++++++++++++++++++++++++++-- programs/fileio.h | 3 ++ programs/platform.h | 11 +++++++ programs/zstdcli.c | 1 - 6 files changed, 71 insertions(+), 71 deletions(-) diff --git a/lib/common/zstd_common.c b/lib/common/zstd_common.c index 8377cf0fd..85e9c2d42 100644 --- a/lib/common/zstd_common.c +++ b/lib/common/zstd_common.c @@ -16,10 +16,6 @@ #include /* malloc, calloc, free */ #include /* memset */ #include /* fprintf(), stderr */ -#include /* signal() */ -#ifndef _WIN32 -#include /* backtrace, backtrace_symbols, symbollist */ -#endif #include "error_private.h" #include "zstd_internal.h" @@ -90,64 +86,3 @@ void ZSTD_free(void* ptr, ZSTD_customMem customMem) free(ptr); } } - - -/*-********************************************************* -* Termination signal trapping (Print debug stack trace) -***********************************************************/ -#define MAX_STACK_FRAMES 50 - -#ifndef _WIN32 - -#ifdef __linux__ -#define START_STACK_FRAME 2 -#elif defined __APPLE__ -#define START_STACK_FRAME 4 -#endif - -static void ABRThandler(int sig) -{ - const char* name; - void* addrlist[MAX_STACK_FRAMES + 1]; - char** symbollist; - U32 addrlen, i; - - switch (sig) { - case SIGABRT: name = "SIGABRT"; break; - case SIGFPE: name = "SIGFPE"; break; - case SIGILL: name = "SIGILL"; break; - case SIGINT: name = "SIGINT"; break; - case SIGSEGV: name = "SIGSEGV"; break; - default: name = "UNKNOWN"; break; - } - - DISPLAY("Caught %s signal, printing stack:\n", name); - // Retrieve current stack addresses. - addrlen = backtrace(addrlist, sizeof(addrlist) / sizeof(void*)); - if (addrlen == 0) { - DISPLAY("\n"); - return; - } - // Create readable strings to each frame. - symbollist = backtrace_symbols(addrlist, addrlen); - // Print the stack trace, excluding calls handling the signal. - for (i = START_STACK_FRAME; i < addrlen; i++) { - DISPLAY("%s\n", symbollist[i]); - } - free(symbollist); - // Reset and raise the signal so default handler runs. - signal(sig, SIG_DFL); - raise(sig); -} -#endif - -void ZSTD_addAbortHandler() -{ -#ifndef _WIN32 - signal(SIGABRT, ABRThandler); - signal(SIGFPE, ABRThandler); - signal(SIGILL, ABRThandler); - signal(SIGSEGV, ABRThandler); - signal(SIGBUS, ABRThandler); -#endif -} diff --git a/lib/common/zstd_internal.h b/lib/common/zstd_internal.h index b555dd828..e75adfa61 100644 --- a/lib/common/zstd_internal.h +++ b/lib/common/zstd_internal.h @@ -206,9 +206,6 @@ void* ZSTD_malloc(size_t size, ZSTD_customMem customMem); void* ZSTD_calloc(size_t size, ZSTD_customMem customMem); void ZSTD_free(void* ptr, ZSTD_customMem customMem); -/* custom crash signal handler */ -void ZSTD_addAbortHandler(void); - MEM_STATIC U32 ZSTD_highbit32(U32 val) /* compress, dictBuilder, decodeCorpus */ { diff --git a/programs/fileio.c b/programs/fileio.c index 7e9c31cb2..678143d9d 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -30,6 +30,10 @@ #include /* malloc, free */ #include /* strcmp, strlen */ #include /* errno */ +#include +#ifndef _WIN32 +#include /* backtrace, backtrace_symbols */ +#endif #if defined (_MSC_VER) # include @@ -124,8 +128,6 @@ static UTIL_time_t g_displayClock = UTIL_TIME_INITIALIZER; /*-************************************ * Signal (Ctrl-C trapping) **************************************/ -#include - static const char* g_artefact = NULL; static void INThandler(int sig) { @@ -157,6 +159,59 @@ static void clearHandler(void) } +/*-********************************************************* +* Termination signal trapping (Print debug stack trace) +***********************************************************/ +#define MAX_STACK_FRAMES 50 + +#ifndef _WIN32 +static void ABRThandler(int sig) { + const char* name; + void* addrlist[MAX_STACK_FRAMES + 1]; + char** symbollist; + U32 addrlen, i; + + switch (sig) { + case SIGABRT: name = "SIGABRT"; break; + case SIGFPE: name = "SIGFPE"; break; + case SIGILL: name = "SIGILL"; break; + case SIGINT: name = "SIGINT"; break; + case SIGSEGV: name = "SIGSEGV"; break; + default: name = "UNKNOWN"; + } + + DISPLAY("Caught %s signal, printing stack:\n", name); + // Retrieve current stack addresses. + addrlen = backtrace(addrlist, sizeof(addrlist) / sizeof(void*)); + if (addrlen == 0) { + DISPLAY("\n"); + return; + } + // Create readable strings to each frame. + symbollist = backtrace_symbols(addrlist, addrlen); + // Print the stack trace, excluding calls handling the signal. + for (i = ZSTD_START_SYMBOLLIST_FRAME; i < addrlen; i++) { + DISPLAY("%s\n", symbollist[i]); + } + free(symbollist); + // Reset and raise the signal so default handler runs. + signal(sig, SIG_DFL); + raise(sig); +} +#endif + +void FIO_addAbortHandler() +{ +#ifndef _WIN32 + signal(SIGABRT, ABRThandler); + signal(SIGFPE, ABRThandler); + signal(SIGILL, ABRThandler); + signal(SIGSEGV, ABRThandler); + signal(SIGBUS, ABRThandler); +#endif +} + + /*-************************************************************ * Avoid fseek()'s 2GiB barrier with MSVC, macOS, *BSD, MinGW ***************************************************************/ diff --git a/programs/fileio.h b/programs/fileio.h index 69c83f71d..d25aee8dd 100644 --- a/programs/fileio.h +++ b/programs/fileio.h @@ -95,6 +95,9 @@ int FIO_decompressMultipleFilenames(const char** srcNamesTable, unsigned nbFiles const char* dictFileName); +/* custom crash signal handler */ +void FIO_addAbortHandler(void); + #if defined (__cplusplus) } #endif diff --git a/programs/platform.h b/programs/platform.h index a550eb1a3..89eba37ec 100644 --- a/programs/platform.h +++ b/programs/platform.h @@ -148,6 +148,17 @@ static __inline int IS_CONSOLE(FILE* stdStream) { #endif +#ifndef ZSTD_START_SYMBOLLIST_FRAME +# ifdef __linux__ +# define ZSTD_START_SYMBOLLIST_FRAME 2 +# elif defined __APPLE__ +# define ZSTD_START_SYMBOLLIST_FRAME 4 +# else +# define ZSTD_START_SYMBOLLIST_FRAME 0 +# endif +#endif + + #if defined (__cplusplus) } #endif diff --git a/programs/zstdcli.c b/programs/zstdcli.c index 0688d44bc..0fb127cde 100644 --- a/programs/zstdcli.c +++ b/programs/zstdcli.c @@ -39,7 +39,6 @@ #endif #define ZSTD_STATIC_LINKING_ONLY /* ZSTD_maxCLevel */ #include "zstd.h" /* ZSTD_VERSION_STRING */ -#include "zstd_internal.h" /* ZSTD_addAbortHandler */ /*-************************************ From 2224ecd718b4daff9a3d063aa26d5da3dc67e749 Mon Sep 17 00:00:00 2001 From: Casey McGinty Date: Tue, 11 Sep 2018 11:56:50 -0700 Subject: [PATCH 4/6] Remove dead code and method name typo --- lib/common/zstd_common.c | 7 ------- programs/zstdcli.c | 2 +- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/lib/common/zstd_common.c b/lib/common/zstd_common.c index 85e9c2d42..6f05d240e 100644 --- a/lib/common/zstd_common.c +++ b/lib/common/zstd_common.c @@ -15,17 +15,10 @@ ***************************************/ #include /* malloc, calloc, free */ #include /* memset */ -#include /* fprintf(), stderr */ #include "error_private.h" #include "zstd_internal.h" -/*-************************************ -* Display Macros -**************************************/ -#define DISPLAY(...) fprintf(stderr, __VA_ARGS__) - - /*-**************************************** * Version ******************************************/ diff --git a/programs/zstdcli.c b/programs/zstdcli.c index 0fb127cde..3d4548a4b 100644 --- a/programs/zstdcli.c +++ b/programs/zstdcli.c @@ -512,7 +512,7 @@ int main(int argCount, const char* argv[]) memset(&compressionParams, 0, sizeof(compressionParams)); /* init crash handler */ - ZSTD_addAbortHandler(); + FIO_addAbortHandler(); /* command switches */ for (argNb=1; argNb Date: Tue, 11 Sep 2018 14:49:47 -0700 Subject: [PATCH 5/6] Update comments, and LD flag usage in Make --- programs/Makefile | 5 ++-- programs/fileio.c | 58 +++++++++++++++++++++++------------------------ 2 files changed, 32 insertions(+), 31 deletions(-) diff --git a/programs/Makefile b/programs/Makefile index 49319484c..5c1ac33eb 100644 --- a/programs/Makefile +++ b/programs/Makefile @@ -47,8 +47,8 @@ DEBUGFLAGS+=-Wall -Wextra -Wcast-qual -Wcast-align -Wshadow \ -Wstrict-prototypes -Wundef -Wpointer-arith -Wformat-security \ -Wvla -Wformat=2 -Winit-self -Wfloat-equal -Wwrite-strings \ -Wredundant-decls +DEBUGFLAGS_LD+=-rdynamic CFLAGS += $(DEBUGFLAGS) $(MOREFLAGS) -LDFLAGS += -rdynamic FLAGS = $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) @@ -145,7 +145,7 @@ allVariants: zstd zstd-compress zstd-decompress zstd-small zstd-nolegacy $(ZSTDDECOMP_O): CFLAGS += $(ALIGN_LOOP) zstd : CPPFLAGS += $(THREAD_CPP) $(ZLIBCPP) $(LZMACPP) $(LZ4CPP) -zstd : LDFLAGS += $(THREAD_LD) $(ZLIBLD) $(LZMALD) $(LZ4LD) +zstd : LDFLAGS += $(THREAD_LD) $(ZLIBLD) $(LZMALD) $(LZ4LD) $(DEBUGFLAGS_LD) zstd : CPPFLAGS += -DZSTD_LEGACY_SUPPORT=$(ZSTD_LEGACY_SUPPORT) zstd : $(ZSTDLIB_FILES) zstdcli.o fileio.o bench.o datagen.o dibio.o @echo "$(THREAD_MSG)" @@ -159,6 +159,7 @@ endif .PHONY: zstd-release zstd-release: DEBUGFLAGS := +zstd-release: DEBUGFLAGS_LD := zstd-release: zstd zstd32 : CPPFLAGS += $(THREAD_CPP) diff --git a/programs/fileio.c b/programs/fileio.c index 678143d9d..5f6b12e51 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -166,37 +166,37 @@ static void clearHandler(void) #ifndef _WIN32 static void ABRThandler(int sig) { - const char* name; - void* addrlist[MAX_STACK_FRAMES + 1]; - char** symbollist; - U32 addrlen, i; + const char* name; + void* addrlist[MAX_STACK_FRAMES]; + char** symbollist; + U32 addrlen, i; - switch (sig) { - case SIGABRT: name = "SIGABRT"; break; - case SIGFPE: name = "SIGFPE"; break; - case SIGILL: name = "SIGILL"; break; - case SIGINT: name = "SIGINT"; break; - case SIGSEGV: name = "SIGSEGV"; break; - default: name = "UNKNOWN"; - } + switch (sig) { + case SIGABRT: name = "SIGABRT"; break; + case SIGFPE: name = "SIGFPE"; break; + case SIGILL: name = "SIGILL"; break; + case SIGINT: name = "SIGINT"; break; + case SIGSEGV: name = "SIGSEGV"; break; + default: name = "UNKNOWN"; + } - DISPLAY("Caught %s signal, printing stack:\n", name); - // Retrieve current stack addresses. - addrlen = backtrace(addrlist, sizeof(addrlist) / sizeof(void*)); - if (addrlen == 0) { - DISPLAY("\n"); - return; - } - // Create readable strings to each frame. - symbollist = backtrace_symbols(addrlist, addrlen); - // Print the stack trace, excluding calls handling the signal. - for (i = ZSTD_START_SYMBOLLIST_FRAME; i < addrlen; i++) { - DISPLAY("%s\n", symbollist[i]); - } - free(symbollist); - // Reset and raise the signal so default handler runs. - signal(sig, SIG_DFL); - raise(sig); + DISPLAY("Caught %s signal, printing stack:\n", name); + /* Retrieve current stack addresses. */ + addrlen = backtrace(addrlist, MAX_STACK_FRAMES); + if (addrlen == 0) { + DISPLAY("\n"); + return; + } + /* Create readable strings to each frame. */ + symbollist = backtrace_symbols(addrlist, addrlen); + /* Print the stack trace, excluding calls handling the signal. */ + for (i = ZSTD_START_SYMBOLLIST_FRAME; i < addrlen; i++) { + DISPLAY("%s\n", symbollist[i]); + } + free(symbollist); + /* Reset and raise the signal so default handler runs. */ + signal(sig, SIG_DFL); + raise(sig); } #endif From b703181f93d0a6ebeebf0d540b05b66fca44e6c9 Mon Sep 17 00:00:00 2001 From: Casey McGinty Date: Tue, 11 Sep 2018 16:19:34 -0700 Subject: [PATCH 6/6] Disable -rdynamic LD option on Windows --- programs/Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/programs/Makefile b/programs/Makefile index 5c1ac33eb..6c39c9830 100644 --- a/programs/Makefile +++ b/programs/Makefile @@ -40,6 +40,8 @@ CPPFLAGS+= -I$(ZSTDDIR) -I$(ZSTDDIR)/common -I$(ZSTDDIR)/compress \ -DXXH_NAMESPACE=ZSTD_ ifeq ($(OS),Windows_NT) # MinGW assumed CPPFLAGS += -D__USE_MINGW_ANSI_STDIO # compatibility with %zu formatting +else +DEBUGFLAGS_LD+= -rdynamic # Enable backtrace symbol names for Linux/Darwin endif CFLAGS ?= -O3 DEBUGFLAGS+=-Wall -Wextra -Wcast-qual -Wcast-align -Wshadow \ @@ -47,7 +49,6 @@ DEBUGFLAGS+=-Wall -Wextra -Wcast-qual -Wcast-align -Wshadow \ -Wstrict-prototypes -Wundef -Wpointer-arith -Wformat-security \ -Wvla -Wformat=2 -Winit-self -Wfloat-equal -Wwrite-strings \ -Wredundant-decls -DEBUGFLAGS_LD+=-rdynamic CFLAGS += $(DEBUGFLAGS) $(MOREFLAGS) FLAGS = $(CPPFLAGS) $(CFLAGS) $(LDFLAGS)