Messages in this thread Patch in this message | | | From | Nick Terrell <> | Subject | Re: [PATCH v2] lib: zstd: Fix -Wstringop-overflow warning | Date | Tue, 10 Jan 2023 23:10:08 +0000 |
| |
> On Jan 4, 2023, at 1:20 PM, Kees Cook <keescook@chromium.org> wrote: > > !-------------------------------------------------------------------| > This Message Is From an External Sender > > |-------------------------------------------------------------------! > > Fix the following -Wstringop-overflow warning when building with GCC 11+: > > lib/zstd/decompress/huf_decompress.c: In function ‘HUF_readDTableX2_wksp’: > lib/zstd/decompress/huf_decompress.c:700:5: warning: ‘HUF_fillDTableX2.constprop’ accessing 624 bytes in a region of size 52 [-Wstringop-overflow=] > 700 | HUF_fillDTableX2(dt, maxTableLog, > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > 701 | wksp->sortedSymbol, sizeOfSort, > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > 702 | wksp->rankStart0, wksp->rankVal, maxW, > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > 703 | tableLog+1, > | ~~~~~~~~~~~ > 704 | wksp->calleeWksp, sizeof(wksp->calleeWksp) / sizeof(U32)); > | > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > lib/zstd/decompress/huf_decompress.c:700:5: note: referencing argument 6 of type ‘U32 (*)[13]’ {aka ‘unsigned int (*)[13]’} > lib/zstd/decompress/huf_decompress.c:571:13: note: in a call to function ‘HUF_fillDTableX2.constprop’ > 571 | static void HUF_fillDTableX2(HUF_DEltX2* DTable, const U32 targetLog, > | ^~~~~~~~~~~~~~~~ > > by using pointer notation instead of array notation. > > This is one of the last remaining warnings to be fixed before globally > enabling -Wstringop-overflow.
The patch looks correct to me, thanks for reviving it. But, I was attempting to reproduce the issue, so I could better understand what's going on, and I wasn't able to reproduce it myself.
To attempt to reproduce, I applied this patch
--- diff --git a/lib/zstd/Makefile b/lib/zstd/Makefile index 20f08c644b71..190d3d5ab4be 100644 --- a/lib/zstd/Makefile +++ b/lib/zstd/Makefile @@ -12,6 +12,8 @@ obj-$(CONFIG_ZSTD_COMPRESS) += zstd_compress.o obj-$(CONFIG_ZSTD_DECOMPRESS) += zstd_decompress.o obj-$(CONFIG_ZSTD_COMMON) += zstd_common.o +ccflags-y := -Wstringop-overflow=4 -Werror + zstd_compress-y := \ zstd_compress_module.o \ compress/fse_compress.o \ --- Then compiled on x86-64 with gcc 12.2.0 on tag v6.2-rc3. I saw no errors. I also tried with just `-Wstringop-overflow`, and on upstream zstd. I tried to make a minimal reproducer on godbolt, so I could see if it was the gcc version, but wasn't able to make it fail with any of them https://gcc.godbolt.org/z/Exzq9arMr.
Could you please tell me how to reproduce this warning?
Best, Nick Terrell
> Co-developed-by: Gustavo A. R. Silva <gustavoars@kernel.org> > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > Cc: Nick Terrell <terrelln@fb.com> > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > v2: use "rankValCol_t *" instead of U32 > v1: https://lore.kernel.org/lkml/20220330193352.GA119296@embeddedor/ > --- > lib/zstd/decompress/huf_decompress.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/zstd/decompress/huf_decompress.c b/lib/zstd/decompress/huf_decompress.c > index 89b269a641c7..60958afebc41 100644 > --- a/lib/zstd/decompress/huf_decompress.c > +++ b/lib/zstd/decompress/huf_decompress.c > @@ -985,7 +985,7 @@ static void HUF_fillDTableX2Level2(HUF_DEltX2* DTable, U32 targetLog, const U32 > > static void HUF_fillDTableX2(HUF_DEltX2* DTable, const U32 targetLog, > const sortedSymbol_t* sortedList, > - const U32* rankStart, rankVal_t rankValOrigin, const U32 maxWeight, > + const U32* rankStart, rankValCol_t *rankValOrigin, const U32 maxWeight, > const U32 nbBitsBaseline) > { > U32* const rankVal = rankValOrigin[0]; > -- > 2.34.1 >
| |