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)
This commit is contained in:
Yann Collet 2018-08-14 12:56:21 -07:00
parent a25d8000ec
commit 6e66bbf5dd
10 changed files with 67 additions and 56 deletions

View File

@ -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;

View File

@ -83,7 +83,9 @@
* wkspSize should be sized to handle worst case situation, which is `1<<max_tableLog * sizeof(FSE_FUNCTION_TYPE)`
* workSpace must also be properly aligned with FSE_FUNCTION_TYPE requirements
*/
size_t FSE_buildCTable_wksp(FSE_CTable* ct, const short* normalizedCounter, unsigned maxSymbolValue, unsigned tableLog, void* workSpace, size_t wkspSize)
size_t FSE_buildCTable_wksp(FSE_CTable* ct,
const short* normalizedCounter, unsigned maxSymbolValue, unsigned tableLog,
void* workSpace, size_t wkspSize)
{
U32 const tableSize = 1 << tableLog;
U32 const tableMask = tableSize - 1;
@ -101,10 +103,14 @@ size_t FSE_buildCTable_wksp(FSE_CTable* ct, const short* normalizedCounter, unsi
if (((size_t)1 << tableLog) * sizeof(FSE_FUNCTION_TYPE) > 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<normalizedCounter[symbol]; nbOccurences++) {
int const freq = normalizedCounter[symbol];
for (nbOccurences=0; nbOccurences<freq; nbOccurences++) {
tableSymbol[position] = (FSE_FUNCTION_TYPE)symbol;
position = (position + step) & tableMask;
while (position > 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<max);
previous0 = (count==1);
previousIs0 = (count==1);
if (remaining<1) return ERROR(GENERIC);
while (remaining<threshold) { nbBits--; threshold>>=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 */);
}

View File

@ -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,

View File

@ -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);

View File

@ -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));

View File

@ -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;

View File

@ -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++)
{

View File

@ -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) {

View File

@ -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 */
}

View File

@ -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;
}