From: Yann Collet Date: Tue, 14 Aug 2018 19:56:21 +0000 (-0700) Subject: fixed several minor issues detected by scan-build X-Git-Tag: v0.0.29~36^2~7 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6e66bbf5dde09f555a2adb1dd763808c0de3c6c6;p=thirdparty%2Fzstd.git fixed several minor issues detected by scan-build only notable one : writeNCount() resists better vs invalid distributions (though it should never happen within zstd anyway) --- diff --git a/lib/common/zstd_internal.h b/lib/common/zstd_internal.h index b4c1af53f..b36a2fbd0 100644 --- a/lib/common/zstd_internal.h +++ b/lib/common/zstd_internal.h @@ -79,8 +79,7 @@ static const U32 repStartValue[ZSTD_REP_NUM] = { 1, 4, 8 }; static const size_t ZSTD_fcs_fieldSize[4] = { 0, 2, 4, 8 }; static const size_t ZSTD_did_fieldSize[4] = { 0, 1, 2, 4 }; -#define ZSTD_FRAMEIDSIZE 4 -static const size_t ZSTD_frameIdSize = ZSTD_FRAMEIDSIZE; /* magic number size */ +#define ZSTD_FRAMEIDSIZE 4 /* magic number size */ #define ZSTD_BLOCKHEADERSIZE 3 /* C standard doesn't allow `static const` variable to be init using another `static const` variable */ static const size_t ZSTD_blockHeaderSize = ZSTD_BLOCKHEADERSIZE; diff --git a/lib/compress/fse_compress.c b/lib/compress/fse_compress.c index 70daae3bc..caa90edec 100644 --- a/lib/compress/fse_compress.c +++ b/lib/compress/fse_compress.c @@ -83,7 +83,9 @@ * wkspSize should be sized to handle worst case situation, which is `1< wkspSize) return ERROR(tableLog_tooLarge); tableU16[-2] = (U16) tableLog; tableU16[-1] = (U16) maxSymbolValue; - assert(tableLog < 16); /* required for the threshold strategy to work */ + assert(tableLog < 16); /* required for threshold strategy to work */ /* For explanations on how to distribute symbol values over the table : - * http://fastcompression.blogspot.fr/2014/02/fse-distributing-symbol-values.html */ + * http://fastcompression.blogspot.fr/2014/02/fse-distributing-symbol-values.html */ + + #ifdef __clang_analyzer__ + memset(tableSymbol, 0, sizeof(*tableSymbol) * tableSize); /* useless initialization, just to keep scan-build happy */ + #endif /* symbol start positions */ { U32 u; @@ -124,13 +130,15 @@ size_t FSE_buildCTable_wksp(FSE_CTable* ct, const short* normalizedCounter, unsi U32 symbol; for (symbol=0; symbol<=maxSymbolValue; symbol++) { int nbOccurences; - for (nbOccurences=0; nbOccurences highThreshold) position = (position + step) & tableMask; /* Low proba area */ + while (position > highThreshold) + position = (position + step) & tableMask; /* Low proba area */ } } - if (position!=0) return ERROR(GENERIC); /* Must have gone through all positions */ + assert(position==0); /* Must have initialized all positions */ } /* Build table */ @@ -201,9 +209,10 @@ size_t FSE_NCountWriteBound(unsigned maxSymbolValue, unsigned tableLog) return maxSymbolValue ? maxHeaderSize : FSE_NCOUNTBOUND; /* maxSymbolValue==0 ? use default */ } -static size_t FSE_writeNCount_generic (void* header, size_t headerBufferSize, - const short* normalizedCounter, unsigned maxSymbolValue, unsigned tableLog, - unsigned writeIsSafe) +static size_t +FSE_writeNCount_generic (void* header, size_t headerBufferSize, + const short* normalizedCounter, unsigned maxSymbolValue, unsigned tableLog, + unsigned writeIsSafe) { BYTE* const ostart = (BYTE*) header; BYTE* out = ostart; @@ -212,13 +221,12 @@ static size_t FSE_writeNCount_generic (void* header, size_t headerBufferSize, const int tableSize = 1 << tableLog; int remaining; int threshold; - U32 bitStream; - int bitCount; - unsigned charnum = 0; - int previous0 = 0; + U32 bitStream = 0; + int bitCount = 0; + unsigned symbol = 0; + unsigned const alphabetSize = maxSymbolValue + 1; + int previousIs0 = 0; - bitStream = 0; - bitCount = 0; /* Table Size */ bitStream += (tableLog-FSE_MIN_TABLELOG) << bitCount; bitCount += 4; @@ -228,11 +236,11 @@ static size_t FSE_writeNCount_generic (void* header, size_t headerBufferSize, threshold = tableSize; nbBits = tableLog+1; - while (remaining>1) { /* stops at 1 */ - if (previous0) { - unsigned start = charnum; - while (!normalizedCounter[charnum]) charnum++; - while (charnum >= start+24) { + while ((symbol < alphabetSize) && (remaining>1)) { /* stops at 1 */ + if (previousIs0) { + unsigned start = symbol; + while ((symbol < alphabetSize) && !normalizedCounter[symbol]) symbol++; + while (symbol >= start+24) { start+=24; bitStream += 0xFFFFU << bitCount; if ((!writeIsSafe) && (out > oend-2)) return ERROR(dstSize_tooSmall); /* Buffer overflow */ @@ -241,12 +249,12 @@ static size_t FSE_writeNCount_generic (void* header, size_t headerBufferSize, out+=2; bitStream>>=16; } - while (charnum >= start+3) { + while (symbol >= start+3) { start+=3; bitStream += 3 << bitCount; bitCount += 2; } - bitStream += (charnum-start) << bitCount; + bitStream += (symbol-start) << bitCount; bitCount += 2; if (bitCount>16) { if ((!writeIsSafe) && (out > oend - 2)) return ERROR(dstSize_tooSmall); /* Buffer overflow */ @@ -256,15 +264,15 @@ static size_t FSE_writeNCount_generic (void* header, size_t headerBufferSize, bitStream >>= 16; bitCount -= 16; } } - { int count = normalizedCounter[charnum++]; - int const max = (2*threshold-1)-remaining; + { int count = normalizedCounter[symbol++]; + int const max = (2*threshold-1) - remaining; remaining -= count < 0 ? -count : count; count++; /* +1 for extra accuracy */ if (count>=threshold) count += max; /* [0..max[ [max..threshold[ (...) [threshold+max 2*threshold[ */ bitStream += count << bitCount; bitCount += nbBits; bitCount -= (count>=1; } } @@ -277,14 +285,15 @@ static size_t FSE_writeNCount_generic (void* header, size_t headerBufferSize, bitCount -= 16; } } + if (remaining != 1) return ERROR(GENERIC); /* incorrect normalized distribution */ + assert(symbol <= alphabetSize); + /* flush remaining bitStream */ if ((!writeIsSafe) && (out > oend - 2)) return ERROR(dstSize_tooSmall); /* Buffer overflow */ out[0] = (BYTE)bitStream; out[1] = (BYTE)(bitStream>>8); out+= (bitCount+7) /8; - if (charnum > maxSymbolValue + 1) return ERROR(GENERIC); - return (out-ostart); } @@ -297,7 +306,7 @@ size_t FSE_writeNCount (void* buffer, size_t bufferSize, const short* normalized if (bufferSize < FSE_NCountWriteBound(maxSymbolValue, tableLog)) return FSE_writeNCount_generic(buffer, bufferSize, normalizedCounter, maxSymbolValue, tableLog, 0); - return FSE_writeNCount_generic(buffer, bufferSize, normalizedCounter, maxSymbolValue, tableLog, 1); + return FSE_writeNCount_generic(buffer, bufferSize, normalizedCounter, maxSymbolValue, tableLog, 1 /* write in buffer is safe */); } diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index ed3aab871..29dce1250 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -1147,7 +1147,6 @@ static size_t ZSTD_resetCCtx_internal(ZSTD_CCtx* zc, if (zc->workSpace == NULL) return ERROR(memory_allocation); zc->workSpaceSize = neededSpace; zc->workSpaceOversizedDuration = 0; - ptr = zc->workSpace; /* Statically sized space. * entropyWorkspace never moves, diff --git a/lib/compress/zstd_opt.c b/lib/compress/zstd_opt.c index 476cdc148..c4b9bb13b 100644 --- a/lib/compress/zstd_opt.c +++ b/lib/compress/zstd_opt.c @@ -970,7 +970,7 @@ _shortestPath: /* cur, last_pos, best_mlen, best_off have to be set */ U32 seqPos = cur; DEBUGLOG(6, "start reverse traversal (last_pos:%u, cur:%u)", - last_pos, cur); + last_pos, cur); (void)last_pos; assert(storeEnd < ZSTD_OPT_NUM); DEBUGLOG(6, "last sequence copied into pos=%u (llen=%u,mlen=%u,ofc=%u)", storeEnd, lastSequence.litlen, lastSequence.mlen, lastSequence.off); diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index 8f4589d13..16a08fb6b 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -185,7 +185,7 @@ size_t ZSTD_estimateDCtxSize(void) { return sizeof(ZSTD_DCtx); } static size_t ZSTD_startingInputLength(ZSTD_format_e format) { size_t const startingInputLength = (format==ZSTD_f_zstd1_magicless) ? - ZSTD_frameHeaderSize_prefix - ZSTD_frameIdSize : + ZSTD_frameHeaderSize_prefix - ZSTD_FRAMEIDSIZE : ZSTD_frameHeaderSize_prefix; ZSTD_STATIC_ASSERT(ZSTD_FRAMEHEADERSIZE_PREFIX >= ZSTD_FRAMEIDSIZE); /* only supports formats ZSTD_f_zstd1 and ZSTD_f_zstd1_magicless */ @@ -278,7 +278,7 @@ void ZSTD_copyDCtx(ZSTD_DCtx* dstDCtx, const ZSTD_DCtx* srcDCtx) * Note 3 : Skippable Frame Identifiers are considered valid. */ unsigned ZSTD_isFrame(const void* buffer, size_t size) { - if (size < ZSTD_frameIdSize) return 0; + if (size < ZSTD_FRAMEIDSIZE) return 0; { U32 const magic = MEM_readLE32(buffer); if (magic == ZSTD_MAGICNUMBER) return 1; if ((magic & 0xFFFFFFF0U) == ZSTD_MAGIC_SKIPPABLE_START) return 1; @@ -331,6 +331,7 @@ size_t ZSTD_getFrameHeader_advanced(ZSTD_frameHeader* zfhPtr, const void* src, s size_t const minInputSize = ZSTD_startingInputLength(format); if (srcSize < minInputSize) return minInputSize; + if (src==NULL) return ERROR(GENERIC); /* invalid parameter */ if ( (format != ZSTD_f_zstd1_magicless) && (MEM_readLE32(src) != ZSTD_MAGICNUMBER) ) { @@ -339,7 +340,7 @@ size_t ZSTD_getFrameHeader_advanced(ZSTD_frameHeader* zfhPtr, const void* src, s if (srcSize < ZSTD_skippableHeaderSize) return ZSTD_skippableHeaderSize; /* magic number + frame length */ memset(zfhPtr, 0, sizeof(*zfhPtr)); - zfhPtr->frameContentSize = MEM_readLE32((const char *)src + ZSTD_frameIdSize); + zfhPtr->frameContentSize = MEM_readLE32((const char *)src + ZSTD_FRAMEIDSIZE); zfhPtr->frameType = ZSTD_skippableFrame; return 0; } @@ -451,7 +452,7 @@ unsigned long long ZSTD_findDecompressedSize(const void* src, size_t srcSize) size_t skippableSize; if (srcSize < ZSTD_skippableHeaderSize) return ERROR(srcSize_wrong); - skippableSize = MEM_readLE32((const BYTE *)src + ZSTD_frameIdSize) + skippableSize = MEM_readLE32((const BYTE *)src + ZSTD_FRAMEIDSIZE) + ZSTD_skippableHeaderSize; if (srcSize < skippableSize) { return ZSTD_CONTENTSIZE_ERROR; @@ -1763,7 +1764,7 @@ size_t ZSTD_findFrameCompressedSize(const void *src, size_t srcSize) #endif if ( (srcSize >= ZSTD_skippableHeaderSize) && (MEM_readLE32(src) & 0xFFFFFFF0U) == ZSTD_MAGIC_SKIPPABLE_START ) { - return ZSTD_skippableHeaderSize + MEM_readLE32((const BYTE*)src + ZSTD_frameIdSize); + return ZSTD_skippableHeaderSize + MEM_readLE32((const BYTE*)src + ZSTD_FRAMEIDSIZE); } else { const BYTE* ip = (const BYTE*)src; const BYTE* const ipstart = ip; @@ -1797,7 +1798,6 @@ size_t ZSTD_findFrameCompressedSize(const void *src, size_t srcSize) if (zfh.checksumFlag) { /* Final frame content checksum */ if (remainingSize < 4) return ERROR(srcSize_wrong); ip += 4; - remainingSize -= 4; } return ip - ipstart; @@ -1932,7 +1932,7 @@ static size_t ZSTD_decompressMultiFrame(ZSTD_DCtx* dctx, size_t skippableSize; if (srcSize < ZSTD_skippableHeaderSize) return ERROR(srcSize_wrong); - skippableSize = MEM_readLE32((const BYTE*)src + ZSTD_frameIdSize) + skippableSize = MEM_readLE32((const BYTE*)src + ZSTD_FRAMEIDSIZE) + ZSTD_skippableHeaderSize; if (srcSize < skippableSize) return ERROR(srcSize_wrong); @@ -2057,7 +2057,7 @@ size_t ZSTD_decompressContinue(ZSTD_DCtx* dctx, void* dst, size_t dstCapacity, c case ZSTDds_getFrameHeaderSize : assert(src != NULL); if (dctx->format == ZSTD_f_zstd1) { /* allows header */ - assert(srcSize >= ZSTD_frameIdSize); /* to read skippable magic number */ + assert(srcSize >= ZSTD_FRAMEIDSIZE); /* to read skippable magic number */ if ((MEM_readLE32(src) & 0xFFFFFFF0U) == ZSTD_MAGIC_SKIPPABLE_START) { /* skippable frame */ memcpy(dctx->headerBuffer, src, srcSize); dctx->expected = ZSTD_skippableHeaderSize - srcSize; /* remaining to load to get full skippable frame header */ @@ -2167,7 +2167,7 @@ size_t ZSTD_decompressContinue(ZSTD_DCtx* dctx, void* dst, size_t dstCapacity, c assert(src != NULL); assert(srcSize <= ZSTD_skippableHeaderSize); memcpy(dctx->headerBuffer + (ZSTD_skippableHeaderSize - srcSize), src, srcSize); /* complete skippable header */ - dctx->expected = MEM_readLE32(dctx->headerBuffer + ZSTD_frameIdSize); /* note : dctx->expected can grow seriously large, beyond local buffer size */ + dctx->expected = MEM_readLE32(dctx->headerBuffer + ZSTD_FRAMEIDSIZE); /* note : dctx->expected can grow seriously large, beyond local buffer size */ dctx->stage = ZSTDds_skipFrame; return 0; @@ -2268,7 +2268,7 @@ static size_t ZSTD_decompress_insertDictionary(ZSTD_DCtx* dctx, const void* dict if (magic != ZSTD_MAGIC_DICTIONARY) { return ZSTD_refDictContent(dctx, dict, dictSize); /* pure content mode */ } } - dctx->dictID = MEM_readLE32((const char*)dict + ZSTD_frameIdSize); + dctx->dictID = MEM_readLE32((const char*)dict + ZSTD_FRAMEIDSIZE); /* load entropy tables */ { size_t const eSize = ZSTD_loadEntropy(&dctx->entropy, dict, dictSize); @@ -2381,7 +2381,7 @@ static size_t ZSTD_loadEntropy_inDDict(ZSTD_DDict* ddict, ZSTD_dictContentType_e return 0; /* pure content mode */ } } - ddict->dictID = MEM_readLE32((const char*)ddict->dictContent + ZSTD_frameIdSize); + ddict->dictID = MEM_readLE32((const char*)ddict->dictContent + ZSTD_FRAMEIDSIZE); /* load entropy tables */ CHECK_E( ZSTD_loadEntropy(&ddict->entropy, ddict->dictContent, ddict->dictSize), dictionary_corrupted ); @@ -2510,7 +2510,7 @@ unsigned ZSTD_getDictID_fromDict(const void* dict, size_t dictSize) { if (dictSize < 8) return 0; if (MEM_readLE32(dict) != ZSTD_MAGIC_DICTIONARY) return 0; - return MEM_readLE32((const char*)dict + ZSTD_frameIdSize); + return MEM_readLE32((const char*)dict + ZSTD_FRAMEIDSIZE); } /*! ZSTD_getDictID_fromDDict() : @@ -2855,7 +2855,7 @@ size_t ZSTD_decompressStream(ZSTD_DStream* zds, ZSTD_outBuffer* output, ZSTD_inB CHECK_F(ZSTD_decompressBegin_usingDDict(zds, zds->ddict)); if ((MEM_readLE32(zds->headerBuffer) & 0xFFFFFFF0U) == ZSTD_MAGIC_SKIPPABLE_START) { /* skippable frame */ - zds->expected = MEM_readLE32(zds->headerBuffer + ZSTD_frameIdSize); + zds->expected = MEM_readLE32(zds->headerBuffer + ZSTD_FRAMEIDSIZE); zds->stage = ZSTDds_skipFrame; } else { CHECK_F(ZSTD_decodeFrameHeader(zds, zds->headerBuffer, zds->lhSize)); diff --git a/lib/dictBuilder/zdict.c b/lib/dictBuilder/zdict.c index b8a51789a..a4d0a4481 100644 --- a/lib/dictBuilder/zdict.c +++ b/lib/dictBuilder/zdict.c @@ -698,7 +698,7 @@ static size_t ZDICT_analyzeEntropy(void* dstBuffer, size_t maxDstSize, short litLengthNCount[MaxLL+1]; U32 repOffset[MAXREPOFFSET]; offsetCount_t bestRepOffset[ZSTD_REP_NUM+1]; - EStats_ress_t esr; + EStats_ress_t esr = { NULL, NULL, NULL }; ZSTD_parameters params; U32 u, huffLog = 11, Offlog = OffFSELog, mlLog = MLFSELog, llLog = LLFSELog, total; size_t pos = 0, errorCode; diff --git a/lib/legacy/zstd_v04.c b/lib/legacy/zstd_v04.c index a2e2cfa80..15000db6d 100644 --- a/lib/legacy/zstd_v04.c +++ b/lib/legacy/zstd_v04.c @@ -1093,6 +1093,7 @@ static size_t FSE_buildDTable(FSE_DTable* dt, const short* normalizedCounter, un if (tableLog > FSE_MAX_TABLELOG) return ERROR(tableLog_tooLarge); /* Init, lay down lowprob symbols */ + memset(tableDecode, 0, sizeof(FSE_DECODE_TYPE) * (maxSymbolValue+1) ); /* useless init, but keep static analyzer happy, and we don't need to performance optimize legacy decoders */ DTableH.tableLog = (U16)tableLog; for (s=0; s<=maxSymbolValue; s++) { diff --git a/lib/legacy/zstd_v05.c b/lib/legacy/zstd_v05.c index a5e1b1ffc..89a5fe7e6 100644 --- a/lib/legacy/zstd_v05.c +++ b/lib/legacy/zstd_v05.c @@ -1224,6 +1224,7 @@ size_t FSEv05_buildDTable(FSEv05_DTable* dt, const short* normalizedCounter, uns if (tableLog > FSEv05_MAX_TABLELOG) return ERROR(tableLog_tooLarge); /* Init, lay down lowprob symbols */ + memset(tableDecode, 0, sizeof(FSEv05_FUNCTION_TYPE) * (maxSymbolValue+1) ); /* useless init, but keep static analyzer happy, and we don't need to performance optimize legacy decoders */ DTableH.tableLog = (U16)tableLog; for (s=0; s<=maxSymbolValue; s++) { if (normalizedCounter[s]==-1) { diff --git a/lib/legacy/zstd_v06.c b/lib/legacy/zstd_v06.c index 8b068b3e5..d8de77ca9 100644 --- a/lib/legacy/zstd_v06.c +++ b/lib/legacy/zstd_v06.c @@ -4006,7 +4006,7 @@ size_t ZBUFFv06_decompressContinue(ZBUFFv06_DCtx* zbd, if (ZSTDv06_isError(hSize)) return hSize; if (toLoad > (size_t)(iend-ip)) { /* not enough input to load full header */ memcpy(zbd->headerBuffer + zbd->lhSize, ip, iend-ip); - zbd->lhSize += iend-ip; ip = iend; notDone = 0; + zbd->lhSize += iend-ip; *dstCapacityPtr = 0; return (hSize - zbd->lhSize) + ZSTDv06_blockHeaderSize; /* remaining header bytes + next block header */ } diff --git a/lib/legacy/zstd_v07.c b/lib/legacy/zstd_v07.c index 70b170f0f..9f95f62ad 100644 --- a/lib/legacy/zstd_v07.c +++ b/lib/legacy/zstd_v07.c @@ -3150,10 +3150,10 @@ size_t ZSTDv07_getFrameParams(ZSTDv07_frameParams* fparamsPtr, const void* src, const BYTE* ip = (const BYTE*)src; if (srcSize < ZSTDv07_frameHeaderSize_min) return ZSTDv07_frameHeaderSize_min; + memset(fparamsPtr, 0, sizeof(*fparamsPtr)); if (MEM_readLE32(src) != ZSTDv07_MAGICNUMBER) { if ((MEM_readLE32(src) & 0xFFFFFFF0U) == ZSTDv07_MAGIC_SKIPPABLE_START) { if (srcSize < ZSTDv07_skippableHeaderSize) return ZSTDv07_skippableHeaderSize; /* magic number + skippable frame length */ - memset(fparamsPtr, 0, sizeof(*fparamsPtr)); fparamsPtr->frameContentSize = MEM_readLE32((const char *)src + 4); fparamsPtr->windowSize = 0; /* windowSize==0 means a frame is skippable */ return 0; @@ -3175,11 +3175,13 @@ size_t ZSTDv07_getFrameParams(ZSTDv07_frameParams* fparamsPtr, const void* src, U32 windowSize = 0; U32 dictID = 0; U64 frameContentSize = 0; - if ((fhdByte & 0x08) != 0) return ERROR(frameParameter_unsupported); /* reserved bits, which must be zero */ + if ((fhdByte & 0x08) != 0) /* reserved bits, which must be zero */ + return ERROR(frameParameter_unsupported); if (!directMode) { BYTE const wlByte = ip[pos++]; U32 const windowLog = (wlByte >> 3) + ZSTDv07_WINDOWLOG_ABSOLUTEMIN; - if (windowLog > ZSTDv07_WINDOWLOG_MAX) return ERROR(frameParameter_unsupported); + if (windowLog > ZSTDv07_WINDOWLOG_MAX) + return ERROR(frameParameter_unsupported); windowSize = (1U << windowLog); windowSize += (windowSize >> 3) * (wlByte&7); } @@ -3201,7 +3203,8 @@ size_t ZSTDv07_getFrameParams(ZSTDv07_frameParams* fparamsPtr, const void* src, case 3 : frameContentSize = MEM_readLE64(ip+pos); break; } if (!windowSize) windowSize = (U32)frameContentSize; - if (windowSize > windowSizeMax) return ERROR(frameParameter_unsupported); + if (windowSize > windowSizeMax) + return ERROR(frameParameter_unsupported); fparamsPtr->frameContentSize = frameContentSize; fparamsPtr->windowSize = windowSize; fparamsPtr->dictID = dictID; @@ -3220,11 +3223,10 @@ size_t ZSTDv07_getFrameParams(ZSTDv07_frameParams* fparamsPtr, const void* src, - frame header not completely provided (`srcSize` too small) */ unsigned long long ZSTDv07_getDecompressedSize(const void* src, size_t srcSize) { - { ZSTDv07_frameParams fparams; - size_t const frResult = ZSTDv07_getFrameParams(&fparams, src, srcSize); - if (frResult!=0) return 0; - return fparams.frameContentSize; - } + ZSTDv07_frameParams fparams; + size_t const frResult = ZSTDv07_getFrameParams(&fparams, src, srcSize); + if (frResult!=0) return 0; + return fparams.frameContentSize; }