From e89e84782058f6785bdb5e5d3a1c7111f55e152d Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 1 Dec 2021 17:16:36 -0800 Subject: [PATCH 1/6] added alignment test and fix an incorrect alignment check in cwksp which was failing on m68k --- lib/common/mem.h | 20 ++++++++++++++++++++ lib/compress/zstd_cwksp.h | 4 ++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/lib/common/mem.h b/lib/common/mem.h index 1c61b7e5..4803897c 100644 --- a/lib/common/mem.h +++ b/lib/common/mem.h @@ -80,6 +80,26 @@ extern "C" { #endif +/*-************************************************************** +* Alignment check +*****************************************************************/ + +/* C11 support */ +#if defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 201112L) + +#include +# define MEM_ALIGN_COND(T) alignof(T) + +#elif defined(__GNUC__) || defined(_MSC_VER) + +# define MEM_ALIGN_COND(T) __alignof(T) + +#else + +# define MEM_ALIGN_COND(T) 1 /* most likely incorrect, just to pass tests */ + +#endif + /*-************************************************************** * Memory I/O API *****************************************************************/ diff --git a/lib/compress/zstd_cwksp.h b/lib/compress/zstd_cwksp.h index 7ba90262..30566760 100644 --- a/lib/compress/zstd_cwksp.h +++ b/lib/compress/zstd_cwksp.h @@ -422,8 +422,8 @@ MEM_STATIC void* ZSTD_cwksp_reserve_object(ZSTD_cwksp* ws, size_t bytes) { DEBUGLOG(5, "cwksp: reserving %p object %zd bytes (rounded to %zd), %zd bytes remaining", alloc, bytes, roundedBytes, ZSTD_cwksp_available_space(ws) - roundedBytes); - assert(((size_t)alloc & (sizeof(void*)-1)) == 0); - assert((bytes & (sizeof(void*)-1)) == 0); + assert((size_t)alloc % MEM_ALIGN_COND(void*) == 0); + assert(bytes % MEM_ALIGN_COND(void*) == 0); ZSTD_cwksp_assert_internal_consistency(ws); /* we must be in the first phase, no advance is possible */ if (ws->phase != ZSTD_cwksp_alloc_objects || end > ws->workspaceEnd) { From 39dced092e7ba82ea40f1e0ca0eb2037b8eb86c0 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 1 Dec 2021 23:02:00 -0800 Subject: [PATCH 2/6] fix align conditions for huf_compress --- lib/compress/huf_compress.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/compress/huf_compress.c b/lib/compress/huf_compress.c index 41a414b5..ceedce72 100644 --- a/lib/compress/huf_compress.c +++ b/lib/compress/huf_compress.c @@ -97,7 +97,7 @@ static size_t HUF_compressWeights(void* dst, size_t dstSize, const void* weightT unsigned maxSymbolValue = HUF_TABLELOG_MAX; U32 tableLog = MAX_FSE_TABLELOG_FOR_HUFF_HEADER; - HUF_CompressWeightsWksp* wksp = (HUF_CompressWeightsWksp*)HUF_alignUpWorkspace(workspace, &workspaceSize, sizeof(U32)); + HUF_CompressWeightsWksp* wksp = (HUF_CompressWeightsWksp*)HUF_alignUpWorkspace(workspace, &workspaceSize, MEM_ALIGN_COND(U32)); if (workspaceSize < sizeof(HUF_CompressWeightsWksp)) return ERROR(GENERIC); @@ -176,7 +176,7 @@ size_t HUF_writeCTable_wksp(void* dst, size_t maxDstSize, HUF_CElt const* const ct = CTable + 1; BYTE* op = (BYTE*)dst; U32 n; - HUF_WriteCTableWksp* wksp = (HUF_WriteCTableWksp*)HUF_alignUpWorkspace(workspace, &workspaceSize, sizeof(U32)); + HUF_WriteCTableWksp* wksp = (HUF_WriteCTableWksp*)HUF_alignUpWorkspace(workspace, &workspaceSize, MEM_ALIGN_COND(U32)); /* check conditions */ if (workspaceSize < sizeof(HUF_WriteCTableWksp)) return ERROR(GENERIC); @@ -679,7 +679,7 @@ static void HUF_buildCTableFromTree(HUF_CElt* CTable, nodeElt const* huffNode, i size_t HUF_buildCTable_wksp (HUF_CElt* CTable, const unsigned* count, U32 maxSymbolValue, U32 maxNbBits, void* workSpace, size_t wkspSize) { - HUF_buildCTable_wksp_tables* const wksp_tables = (HUF_buildCTable_wksp_tables*)HUF_alignUpWorkspace(workSpace, &wkspSize, sizeof(U32)); + HUF_buildCTable_wksp_tables* const wksp_tables = (HUF_buildCTable_wksp_tables*)HUF_alignUpWorkspace(workSpace, &wkspSize, MEM_ALIGN_COND(U32)); nodeElt* const huffNode0 = wksp_tables->huffNodeTbl; nodeElt* const huffNode = huffNode0+1; int nonNullRank; @@ -1183,7 +1183,7 @@ HUF_compress_internal (void* dst, size_t dstSize, HUF_CElt* oldHufTable, HUF_repeat* repeat, int preferRepeat, const int bmi2, unsigned suspectUncompressible) { - HUF_compress_tables_t* const table = (HUF_compress_tables_t*)HUF_alignUpWorkspace(workSpace, &wkspSize, sizeof(size_t)); + HUF_compress_tables_t* const table = (HUF_compress_tables_t*)HUF_alignUpWorkspace(workSpace, &wkspSize, MEM_ALIGN_COND(size_t)); BYTE* const ostart = (BYTE*)dst; BYTE* const oend = ostart + dstSize; BYTE* op = ostart; From ffbdc8ac575652d3609f317e93d1604f568d92f2 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 2 Dec 2021 10:30:04 -0800 Subject: [PATCH 3/6] m68k CI tests on GA are now compulsory --- .github/workflows/dev-short-tests.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/dev-short-tests.yml b/.github/workflows/dev-short-tests.yml index ae9d13c1..c8e85cd3 100644 --- a/.github/workflows/dev-short-tests.yml +++ b/.github/workflows/dev-short-tests.yml @@ -294,7 +294,6 @@ jobs: LDFLAGS="-static" CC=$XCC QEMU_SYS=$XEMU make clean check - name: M68K if: ${{ matrix.name == 'M68K' }} - continue-on-error: true # disable reporting errors (alignment issues) run: | LDFLAGS="-static" CC=$XCC QEMU_SYS=$XEMU make clean check From 80a13fd64555e2be571a0436a3fae0dd867ce626 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 2 Dec 2021 11:20:01 -0800 Subject: [PATCH 4/6] move the alignment macro to compiler.h because mem.h is dropped in the Linux kernel. Changed macro definition order (gcc/clang/msvc before c11) due to a limitation in the kernel source builder. Changed the backup to sizeof(), reverting to previous behavior when no support of alignof() is detected. --- lib/common/compiler.h | 33 +++++++++++++++++++++++++++++++++ lib/common/mem.h | 20 -------------------- 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/lib/common/compiler.h b/lib/common/compiler.h index ea5fe2f4..907b6bfa 100644 --- a/lib/common/compiler.h +++ b/lib/common/compiler.h @@ -282,6 +282,39 @@ # endif #endif +/*-************************************************************** +* Alignment check +*****************************************************************/ + +/* this test was initially positioned in mem.h, + * but this file is removed (or replaced) for linux kernel + * so it's now hosted in compiler.h, + * which remains valid for both user & kernel spaces. + */ + +#ifndef MEM_ALIGN_COND +# if defined(__GNUC__) || defined(_MSC_VER) +/* covers gcc, clang & MSVC */ +/* note : this section must come first, before C11, + * due to a limitation in the kernel source generator */ +# define MEM_ALIGN_COND(T) __alignof(T) + +# elif defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 201112L) +/* C11 support */ +# include +# define MEM_ALIGN_COND(T) alignof(T) + +# else +/* No known support for alignof() - imperfect backup */ +# define MEM_ALIGN_COND(T) sizeof(T) + +# endif +#endif /* MEM_ALIGN_COND */ + +/*-************************************************************** +* Sanitizer +*****************************************************************/ + #if ZSTD_MEMORY_SANITIZER /* Not all platforms that support msan provide sanitizers/msan_interface.h. * We therefore declare the functions we need ourselves, rather than trying to diff --git a/lib/common/mem.h b/lib/common/mem.h index 4803897c..1c61b7e5 100644 --- a/lib/common/mem.h +++ b/lib/common/mem.h @@ -80,26 +80,6 @@ extern "C" { #endif -/*-************************************************************** -* Alignment check -*****************************************************************/ - -/* C11 support */ -#if defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 201112L) - -#include -# define MEM_ALIGN_COND(T) alignof(T) - -#elif defined(__GNUC__) || defined(_MSC_VER) - -# define MEM_ALIGN_COND(T) __alignof(T) - -#else - -# define MEM_ALIGN_COND(T) 1 /* most likely incorrect, just to pass tests */ - -#endif - /*-************************************************************** * Memory I/O API *****************************************************************/ From 1d025d871bf0891aceac437685f33cd6e638f4c7 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 2 Dec 2021 11:30:03 -0800 Subject: [PATCH 5/6] bound alignment backup to sizeof(void*) --- lib/common/compiler.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/common/compiler.h b/lib/common/compiler.h index 907b6bfa..98590ce6 100644 --- a/lib/common/compiler.h +++ b/lib/common/compiler.h @@ -306,7 +306,7 @@ # else /* No known support for alignof() - imperfect backup */ -# define MEM_ALIGN_COND(T) sizeof(T) +# define MEM_ALIGN_COND(T) (sizeof(void*) < sizeof(T) ? sizeof(void*) : sizeof(T)) # endif #endif /* MEM_ALIGN_COND */ From 30b9db8ae45834fd4b654e6c4e0310cdaa5ab3ae Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 2 Dec 2021 12:57:42 -0800 Subject: [PATCH 6/6] changed macro name to ZSTD_ALIGNOF for better consistency --- lib/common/compiler.h | 10 +++++----- lib/compress/huf_compress.c | 8 ++++---- lib/compress/zstd_cwksp.h | 4 ++-- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/common/compiler.h b/lib/common/compiler.h index 98590ce6..17c049bf 100644 --- a/lib/common/compiler.h +++ b/lib/common/compiler.h @@ -292,24 +292,24 @@ * which remains valid for both user & kernel spaces. */ -#ifndef MEM_ALIGN_COND +#ifndef ZSTD_ALIGNOF # if defined(__GNUC__) || defined(_MSC_VER) /* covers gcc, clang & MSVC */ /* note : this section must come first, before C11, * due to a limitation in the kernel source generator */ -# define MEM_ALIGN_COND(T) __alignof(T) +# define ZSTD_ALIGNOF(T) __alignof(T) # elif defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 201112L) /* C11 support */ # include -# define MEM_ALIGN_COND(T) alignof(T) +# define ZSTD_ALIGNOF(T) alignof(T) # else /* No known support for alignof() - imperfect backup */ -# define MEM_ALIGN_COND(T) (sizeof(void*) < sizeof(T) ? sizeof(void*) : sizeof(T)) +# define ZSTD_ALIGNOF(T) (sizeof(void*) < sizeof(T) ? sizeof(void*) : sizeof(T)) # endif -#endif /* MEM_ALIGN_COND */ +#endif /* ZSTD_ALIGNOF */ /*-************************************************************** * Sanitizer diff --git a/lib/compress/huf_compress.c b/lib/compress/huf_compress.c index ceedce72..2b3d6adc 100644 --- a/lib/compress/huf_compress.c +++ b/lib/compress/huf_compress.c @@ -97,7 +97,7 @@ static size_t HUF_compressWeights(void* dst, size_t dstSize, const void* weightT unsigned maxSymbolValue = HUF_TABLELOG_MAX; U32 tableLog = MAX_FSE_TABLELOG_FOR_HUFF_HEADER; - HUF_CompressWeightsWksp* wksp = (HUF_CompressWeightsWksp*)HUF_alignUpWorkspace(workspace, &workspaceSize, MEM_ALIGN_COND(U32)); + HUF_CompressWeightsWksp* wksp = (HUF_CompressWeightsWksp*)HUF_alignUpWorkspace(workspace, &workspaceSize, ZSTD_ALIGNOF(U32)); if (workspaceSize < sizeof(HUF_CompressWeightsWksp)) return ERROR(GENERIC); @@ -176,7 +176,7 @@ size_t HUF_writeCTable_wksp(void* dst, size_t maxDstSize, HUF_CElt const* const ct = CTable + 1; BYTE* op = (BYTE*)dst; U32 n; - HUF_WriteCTableWksp* wksp = (HUF_WriteCTableWksp*)HUF_alignUpWorkspace(workspace, &workspaceSize, MEM_ALIGN_COND(U32)); + HUF_WriteCTableWksp* wksp = (HUF_WriteCTableWksp*)HUF_alignUpWorkspace(workspace, &workspaceSize, ZSTD_ALIGNOF(U32)); /* check conditions */ if (workspaceSize < sizeof(HUF_WriteCTableWksp)) return ERROR(GENERIC); @@ -679,7 +679,7 @@ static void HUF_buildCTableFromTree(HUF_CElt* CTable, nodeElt const* huffNode, i size_t HUF_buildCTable_wksp (HUF_CElt* CTable, const unsigned* count, U32 maxSymbolValue, U32 maxNbBits, void* workSpace, size_t wkspSize) { - HUF_buildCTable_wksp_tables* const wksp_tables = (HUF_buildCTable_wksp_tables*)HUF_alignUpWorkspace(workSpace, &wkspSize, MEM_ALIGN_COND(U32)); + HUF_buildCTable_wksp_tables* const wksp_tables = (HUF_buildCTable_wksp_tables*)HUF_alignUpWorkspace(workSpace, &wkspSize, ZSTD_ALIGNOF(U32)); nodeElt* const huffNode0 = wksp_tables->huffNodeTbl; nodeElt* const huffNode = huffNode0+1; int nonNullRank; @@ -1183,7 +1183,7 @@ HUF_compress_internal (void* dst, size_t dstSize, HUF_CElt* oldHufTable, HUF_repeat* repeat, int preferRepeat, const int bmi2, unsigned suspectUncompressible) { - HUF_compress_tables_t* const table = (HUF_compress_tables_t*)HUF_alignUpWorkspace(workSpace, &wkspSize, MEM_ALIGN_COND(size_t)); + HUF_compress_tables_t* const table = (HUF_compress_tables_t*)HUF_alignUpWorkspace(workSpace, &wkspSize, ZSTD_ALIGNOF(size_t)); BYTE* const ostart = (BYTE*)dst; BYTE* const oend = ostart + dstSize; BYTE* op = ostart; diff --git a/lib/compress/zstd_cwksp.h b/lib/compress/zstd_cwksp.h index 30566760..29d027e9 100644 --- a/lib/compress/zstd_cwksp.h +++ b/lib/compress/zstd_cwksp.h @@ -422,8 +422,8 @@ MEM_STATIC void* ZSTD_cwksp_reserve_object(ZSTD_cwksp* ws, size_t bytes) { DEBUGLOG(5, "cwksp: reserving %p object %zd bytes (rounded to %zd), %zd bytes remaining", alloc, bytes, roundedBytes, ZSTD_cwksp_available_space(ws) - roundedBytes); - assert((size_t)alloc % MEM_ALIGN_COND(void*) == 0); - assert(bytes % MEM_ALIGN_COND(void*) == 0); + assert((size_t)alloc % ZSTD_ALIGNOF(void*) == 0); + assert(bytes % ZSTD_ALIGNOF(void*) == 0); ZSTD_cwksp_assert_internal_consistency(ws); /* we must be in the first phase, no advance is possible */ if (ws->phase != ZSTD_cwksp_alloc_objects || end > ws->workspaceEnd) {