From c0bedafd070ce1da408e41a2717528edf318175b Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Sat, 7 Oct 2023 13:32:06 -0700 Subject: [PATCH] Replace trailing 1-element arrays with C99 flex-arrays Using a trailing 1-element array as a variable-length array is undefined behavior. Compilers have traditionally allowed this, as this is a traditional, pre-C99 convention. But it's being increasingly phased out in favor of C99 flexible arrays, which avoid ambiguity about whether the array is intended to be variable-length or not. gcc 13+ and clang 16+ support the -fstrict-flex-arrays=3 option, which Linux v6.5+ enables globally, resulting in UBSAN errors when these arrays are accessed. (See https://lore.kernel.org/r/00000000000049964e06041f2cbf@google.com) Fix this by making zstd use C99 flexible arrays instead of 1-element arrays. This only affects three places (that I could find): one in fse_decompress.c and two in zstdmt_compress.c. Note, this breaks the c89build CI job and thus might not be suitable for merging yet. I recommend that zstd drop support for C89/C90. --- lib/common/compiler.h | 5 +++++ lib/common/fse.h | 2 +- lib/common/fse_decompress.c | 2 +- lib/compress/zstdmt_compress.c | 12 ++++++------ 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/lib/common/compiler.h b/lib/common/compiler.h index df39d91c6e0..fa9758f5cb6 100644 --- a/lib/common/compiler.h +++ b/lib/common/compiler.h @@ -199,6 +199,11 @@ # include /* For Visual 2005 */ # pragma warning(disable : 4100) /* disable: C4100: unreferenced formal parameter */ # pragma warning(disable : 4127) /* disable: C4127: conditional expression is constant */ +/* + * Disable C4200: "nonstandard extension used: zero-sized array in struct/union" + * because it warns about standard C99 flexible arrays. + */ +# pragma warning(disable : 4200) # pragma warning(disable : 4204) /* disable: C4204: non-constant aggregate initializer */ # pragma warning(disable : 4214) /* disable: C4214: non-int bitfields */ # pragma warning(disable : 4324) /* disable: C4324: padded structure */ diff --git a/lib/common/fse.h b/lib/common/fse.h index 02a1f0bc530..4977ea3b4d8 100644 --- a/lib/common/fse.h +++ b/lib/common/fse.h @@ -277,7 +277,7 @@ size_t FSE_buildCTable_wksp(FSE_CTable* ct, const short* normalizedCounter, unsi FSE_PUBLIC_API size_t FSE_buildDTable_wksp(FSE_DTable* dt, const short* normalizedCounter, unsigned maxSymbolValue, unsigned tableLog, void* workSpace, size_t wkspSize); /**< Same as FSE_buildDTable(), using an externally allocated `workspace` produced with `FSE_BUILD_DTABLE_WKSP_SIZE_U32(maxSymbolValue)` */ -#define FSE_DECOMPRESS_WKSP_SIZE_U32(maxTableLog, maxSymbolValue) (FSE_DTABLE_SIZE_U32(maxTableLog) + 1 + FSE_BUILD_DTABLE_WKSP_SIZE_U32(maxTableLog, maxSymbolValue) + (FSE_MAX_SYMBOL_VALUE + 1) / 2 + 1) +#define FSE_DECOMPRESS_WKSP_SIZE_U32(maxTableLog, maxSymbolValue) (FSE_DTABLE_SIZE_U32(maxTableLog) + 1 + FSE_BUILD_DTABLE_WKSP_SIZE_U32(maxTableLog, maxSymbolValue) + (FSE_MAX_SYMBOL_VALUE + 1) / 2) #define FSE_DECOMPRESS_WKSP_SIZE(maxTableLog, maxSymbolValue) (FSE_DECOMPRESS_WKSP_SIZE_U32(maxTableLog, maxSymbolValue) * sizeof(unsigned)) size_t FSE_decompress_wksp_bmi2(void* dst, size_t dstCapacity, const void* cSrc, size_t cSrcSize, unsigned maxLog, void* workSpace, size_t wkspSize, int bmi2); /**< same as FSE_decompress(), using an externally allocated `workSpace` produced with `FSE_DECOMPRESS_WKSP_SIZE_U32(maxLog, maxSymbolValue)`. diff --git a/lib/common/fse_decompress.c b/lib/common/fse_decompress.c index 1e1c9f92d6b..d0781d5d7d8 100644 --- a/lib/common/fse_decompress.c +++ b/lib/common/fse_decompress.c @@ -237,7 +237,7 @@ FORCE_INLINE_TEMPLATE size_t FSE_decompress_usingDTable_generic( typedef struct { short ncount[FSE_MAX_SYMBOL_VALUE + 1]; - FSE_DTable dtable[1]; /* Dynamically sized */ + FSE_DTable dtable[]; /* Dynamically sized */ } FSE_DecompressWksp; diff --git a/lib/compress/zstdmt_compress.c b/lib/compress/zstdmt_compress.c index add99d769be..33757dd3bf7 100644 --- a/lib/compress/zstdmt_compress.c +++ b/lib/compress/zstdmt_compress.c @@ -100,13 +100,13 @@ typedef struct ZSTDMT_bufferPool_s { unsigned totalBuffers; unsigned nbBuffers; ZSTD_customMem cMem; - buffer_t bTable[1]; /* variable size */ + buffer_t bTable[]; /* variable size */ } ZSTDMT_bufferPool; static ZSTDMT_bufferPool* ZSTDMT_createBufferPool(unsigned maxNbBuffers, ZSTD_customMem cMem) { ZSTDMT_bufferPool* const bufPool = (ZSTDMT_bufferPool*)ZSTD_customCalloc( - sizeof(ZSTDMT_bufferPool) + (maxNbBuffers-1) * sizeof(buffer_t), cMem); + sizeof(ZSTDMT_bufferPool) + maxNbBuffers * sizeof(buffer_t), cMem); if (bufPool==NULL) return NULL; if (ZSTD_pthread_mutex_init(&bufPool->poolMutex, NULL)) { ZSTD_customFree(bufPool, cMem); @@ -136,7 +136,7 @@ static void ZSTDMT_freeBufferPool(ZSTDMT_bufferPool* bufPool) static size_t ZSTDMT_sizeof_bufferPool(ZSTDMT_bufferPool* bufPool) { size_t const poolSize = sizeof(*bufPool) - + (bufPool->totalBuffers - 1) * sizeof(buffer_t); + + bufPool->totalBuffers * sizeof(buffer_t); unsigned u; size_t totalBufferSize = 0; ZSTD_pthread_mutex_lock(&bufPool->poolMutex); @@ -350,7 +350,7 @@ typedef struct { int totalCCtx; int availCCtx; ZSTD_customMem cMem; - ZSTD_CCtx* cctx[1]; /* variable size */ + ZSTD_CCtx* cctx[]; /* variable size */ } ZSTDMT_CCtxPool; /* note : all CCtx borrowed from the pool should be released back to the pool _before_ freeing the pool */ @@ -369,7 +369,7 @@ static ZSTDMT_CCtxPool* ZSTDMT_createCCtxPool(int nbWorkers, ZSTD_customMem cMem) { ZSTDMT_CCtxPool* const cctxPool = (ZSTDMT_CCtxPool*) ZSTD_customCalloc( - sizeof(ZSTDMT_CCtxPool) + (nbWorkers-1)*sizeof(ZSTD_CCtx*), cMem); + sizeof(ZSTDMT_CCtxPool) + nbWorkers*sizeof(ZSTD_CCtx*), cMem); assert(nbWorkers > 0); if (!cctxPool) return NULL; if (ZSTD_pthread_mutex_init(&cctxPool->poolMutex, NULL)) { @@ -403,7 +403,7 @@ static size_t ZSTDMT_sizeof_CCtxPool(ZSTDMT_CCtxPool* cctxPool) ZSTD_pthread_mutex_lock(&cctxPool->poolMutex); { unsigned const nbWorkers = cctxPool->totalCCtx; size_t const poolSize = sizeof(*cctxPool) - + (nbWorkers-1) * sizeof(ZSTD_CCtx*); + + nbWorkers * sizeof(ZSTD_CCtx*); unsigned u; size_t totalCCtxSize = 0; for (u=0; u