From 87406548f070cb89ecd27af34766b748f142d436 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 6 Sep 2018 17:07:53 -0700 Subject: [PATCH 1/3] reduced DDict size, by -2KB corresponding to the removal of workspace which is needed while building huffman table and is now either present in DCtx, or temporarily borrowed from available FSE table space. --- lib/decompress/huf_decompress.c | 6 +-- lib/decompress/zstd_decompress.c | 66 ++++++++++++++++++-------------- 2 files changed, 41 insertions(+), 31 deletions(-) diff --git a/lib/decompress/huf_decompress.c b/lib/decompress/huf_decompress.c index a696261b..83ecaff0 100644 --- a/lib/decompress/huf_decompress.c +++ b/lib/decompress/huf_decompress.c @@ -533,9 +533,9 @@ static void HUF_fillDTableX2(HUF_DEltX2* DTable, const U32 targetLog, } } -size_t HUF_readDTableX2_wksp(HUF_DTable* DTable, const void* src, - size_t srcSize, void* workSpace, - size_t wkspSize) +size_t HUF_readDTableX2_wksp(HUF_DTable* DTable, + const void* src, size_t srcSize, + void* workSpace, size_t wkspSize) { U32 tableLog, maxW, sizeOfSort, nbSymbols; DTableDesc dtd = HUF_getDTableDesc(DTable); diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index 7c4769cd..e9fc3633 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -114,7 +114,6 @@ typedef struct { ZSTD_seqSymbol OFTable[SEQSYMBOL_TABLE_SIZE(OffFSELog)]; ZSTD_seqSymbol MLTable[SEQSYMBOL_TABLE_SIZE(MLFSELog)]; HUF_DTable hufTable[HUF_DTABLE_SIZE(HufLog)]; /* can accommodate HUF_decompress4X */ - U32 workspace[HUF_DECOMPRESS_WORKSPACE_SIZE_U32]; U32 rep[ZSTD_REP_NUM]; } ZSTD_entropyDTables_t; @@ -125,6 +124,7 @@ struct ZSTD_DCtx_s const ZSTD_seqSymbol* OFTptr; const HUF_DTable* HUFptr; ZSTD_entropyDTables_t entropy; + U32 workspace[HUF_DECOMPRESS_WORKSPACE_SIZE_U32]; /* space needed when building huffman tables */ const void* previousDstEnd; /* detect continuity */ const void* prefixStart; /* start of current segment */ const void* virtualStart; /* virtual start of previous segment if it was just before current one */ @@ -612,9 +612,9 @@ size_t ZSTD_decodeLiteralsBlock(ZSTD_DCtx* dctx, HUF_decompress4X_usingDTable_bmi2(dctx->litBuffer, litSize, istart+lhSize, litCSize, dctx->HUFptr, dctx->bmi2) ) : ( singleStream ? HUF_decompress1X1_DCtx_wksp_bmi2(dctx->entropy.hufTable, dctx->litBuffer, litSize, istart+lhSize, litCSize, - dctx->entropy.workspace, sizeof(dctx->entropy.workspace), dctx->bmi2) : + dctx->workspace, sizeof(dctx->workspace), dctx->bmi2) : HUF_decompress4X_hufOnly_wksp_bmi2(dctx->entropy.hufTable, dctx->litBuffer, litSize, istart+lhSize, litCSize, - dctx->entropy.workspace, sizeof(dctx->entropy.workspace), dctx->bmi2)))) + dctx->workspace, sizeof(dctx->workspace), dctx->bmi2)))) return ERROR(corruption_detected); dctx->litPtr = dctx->litBuffer; @@ -2196,7 +2196,8 @@ static size_t ZSTD_refDictContent(ZSTD_DCtx* dctx, const void* dict, size_t dict /* ZSTD_loadEntropy() : * dict : must point at beginning of a valid zstd dictionary * @return : size of entropy tables read */ -static size_t ZSTD_loadEntropy(ZSTD_entropyDTables_t* entropy, const void* const dict, size_t const dictSize) +static size_t ZSTD_loadEntropy(ZSTD_entropyDTables_t* entropy, + const void* const dict, size_t const dictSize) { const BYTE* dictPtr = (const BYTE*)dict; const BYTE* const dictEnd = dictPtr + dictSize; @@ -2204,10 +2205,12 @@ static size_t ZSTD_loadEntropy(ZSTD_entropyDTables_t* entropy, const void* const if (dictSize <= 8) return ERROR(dictionary_corrupted); dictPtr += 8; /* skip header = magic + dictID */ - - { size_t const hSize = HUF_readDTableX2_wksp( - entropy->hufTable, dictPtr, dictEnd - dictPtr, - entropy->workspace, sizeof(entropy->workspace)); + ZSTD_STATIC_ASSERT(offsetof(ZSTD_entropyDTables_t, hufTable) >= HUF_DECOMPRESS_WORKSPACE_SIZE); + { void* const workspace = entropy; /* use fse tables as temporary workspace; implies fse table precede huffTable at beginning of entropy */ + size_t const workspaceSize = offsetof(ZSTD_entropyDTables_t, hufTable); + size_t const hSize = HUF_readDTableX2_wksp(entropy->hufTable, + dictPtr, dictEnd - dictPtr, + workspace, workspaceSize); if (HUF_isError(hSize)) return ERROR(dictionary_corrupted); dictPtr += hSize; } @@ -2218,7 +2221,7 @@ static size_t ZSTD_loadEntropy(ZSTD_entropyDTables_t* entropy, const void* const if (FSE_isError(offcodeHeaderSize)) return ERROR(dictionary_corrupted); if (offcodeMaxValue > MaxOff) return ERROR(dictionary_corrupted); if (offcodeLog > OffFSELog) return ERROR(dictionary_corrupted); - ZSTD_buildFSETable(entropy->OFTable, + ZSTD_buildFSETable( entropy->OFTable, offcodeNCount, offcodeMaxValue, OF_base, OF_bits, offcodeLog); @@ -2231,7 +2234,7 @@ static size_t ZSTD_loadEntropy(ZSTD_entropyDTables_t* entropy, const void* const if (FSE_isError(matchlengthHeaderSize)) return ERROR(dictionary_corrupted); if (matchlengthMaxValue > MaxML) return ERROR(dictionary_corrupted); if (matchlengthLog > MLFSELog) return ERROR(dictionary_corrupted); - ZSTD_buildFSETable(entropy->MLTable, + ZSTD_buildFSETable( entropy->MLTable, matchlengthNCount, matchlengthMaxValue, ML_base, ML_bits, matchlengthLog); @@ -2244,7 +2247,7 @@ static size_t ZSTD_loadEntropy(ZSTD_entropyDTables_t* entropy, const void* const if (FSE_isError(litlengthHeaderSize)) return ERROR(dictionary_corrupted); if (litlengthMaxValue > MaxLL) return ERROR(dictionary_corrupted); if (litlengthLog > LLFSELog) return ERROR(dictionary_corrupted); - ZSTD_buildFSETable(entropy->LLTable, + ZSTD_buildFSETable( entropy->LLTable, litlengthNCount, litlengthMaxValue, LL_base, LL_bits, litlengthLog); @@ -2365,7 +2368,9 @@ size_t ZSTD_decompressBegin_usingDDict(ZSTD_DCtx* dstDCtx, const ZSTD_DDict* ddi return 0; } -static size_t ZSTD_loadEntropy_inDDict(ZSTD_DDict* ddict, ZSTD_dictContentType_e dictContentType) +static size_t +ZSTD_loadEntropy_inDDict(ZSTD_DDict* ddict, + ZSTD_dictContentType_e dictContentType) { ddict->dictID = 0; ddict->entropyPresent = 0; @@ -2386,7 +2391,9 @@ static size_t ZSTD_loadEntropy_inDDict(ZSTD_DDict* ddict, ZSTD_dictContentType_e 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 ); + CHECK_E( ZSTD_loadEntropy(&ddict->entropy, + ddict->dictContent, ddict->dictSize), + dictionary_corrupted ); ddict->entropyPresent = 1; return 0; } @@ -2425,14 +2432,15 @@ ZSTD_DDict* ZSTD_createDDict_advanced(const void* dict, size_t dictSize, if (!customMem.customAlloc ^ !customMem.customFree) return NULL; { ZSTD_DDict* const ddict = (ZSTD_DDict*) ZSTD_malloc(sizeof(ZSTD_DDict), customMem); - if (!ddict) return NULL; + if (ddict == NULL) return NULL; ddict->cMem = customMem; - - if (ZSTD_isError( ZSTD_initDDict_internal(ddict, dict, dictSize, dictLoadMethod, dictContentType) )) { - ZSTD_freeDDict(ddict); - return NULL; - } - + { size_t const initResult = ZSTD_initDDict_internal(ddict, + dict, dictSize, + dictLoadMethod, dictContentType); + if (ZSTD_isError(initResult)) { + ZSTD_freeDDict(ddict); + return NULL; + } } return ddict; } } @@ -2459,23 +2467,25 @@ ZSTD_DDict* ZSTD_createDDict_byReference(const void* dictBuffer, size_t dictSize const ZSTD_DDict* ZSTD_initStaticDDict( - void* workspace, size_t workspaceSize, + void* sBuffer, size_t sBufferSize, const void* dict, size_t dictSize, ZSTD_dictLoadMethod_e dictLoadMethod, ZSTD_dictContentType_e dictContentType) { - size_t const neededSpace = - sizeof(ZSTD_DDict) + (dictLoadMethod == ZSTD_dlm_byRef ? 0 : dictSize); - ZSTD_DDict* const ddict = (ZSTD_DDict*)workspace; - assert(workspace != NULL); + size_t const neededSpace = sizeof(ZSTD_DDict) + + (dictLoadMethod == ZSTD_dlm_byRef ? 0 : dictSize); + ZSTD_DDict* const ddict = (ZSTD_DDict*)sBuffer; + assert(sBuffer != NULL); assert(dict != NULL); - if ((size_t)workspace & 7) return NULL; /* 8-aligned */ - if (workspaceSize < neededSpace) return NULL; + if ((size_t)sBuffer & 7) return NULL; /* 8-aligned */ + if (sBufferSize < neededSpace) return NULL; if (dictLoadMethod == ZSTD_dlm_byCopy) { memcpy(ddict+1, dict, dictSize); /* local copy */ dict = ddict+1; } - if (ZSTD_isError( ZSTD_initDDict_internal(ddict, dict, dictSize, ZSTD_dlm_byRef, dictContentType) )) + if (ZSTD_isError( ZSTD_initDDict_internal(ddict, + dict, dictSize, + ZSTD_dlm_byRef, dictContentType) )) return NULL; return ddict; } From f97ca36eab4e3aadf2666a6130d37680a4f6a7d3 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 6 Sep 2018 17:54:13 -0700 Subject: [PATCH 2/3] strengthened conditions for using workplace into fse table space ensure that the structure layout is as expected. will trigger an error if it changes in the future. Another solution would be to use a union, this would be cleaner and get rid of these static asserts. However, in order to keep the current code unmodified, it would be necessary to use an un-named unions. And apparently, un-named unions are only possible on "recent" compilers (C99+). --- lib/decompress/zstd_decompress.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index e9fc3633..35ed7c59 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -2203,8 +2203,13 @@ static size_t ZSTD_loadEntropy(ZSTD_entropyDTables_t* entropy, const BYTE* const dictEnd = dictPtr + dictSize; if (dictSize <= 8) return ERROR(dictionary_corrupted); + assert(MEM_readLE32(dict) == ZSTD_MAGIC_DICTIONARY); /* dict must be valid */ dictPtr += 8; /* skip header = magic + dictID */ + ZSTD_STATIC_ASSERT(offsetof(ZSTD_entropyDTables_t, LLTable) == 0); + ZSTD_STATIC_ASSERT(offsetof(ZSTD_entropyDTables_t, OFTable) == sizeof(entropy->LLTable)); + ZSTD_STATIC_ASSERT(offsetof(ZSTD_entropyDTables_t, MLTable) == sizeof(entropy->LLTable) + sizeof(entropy->OFTable)); + ZSTD_STATIC_ASSERT(offsetof(ZSTD_entropyDTables_t, hufTable) == sizeof(entropy->LLTable) + sizeof(entropy->OFTable) + sizeof(entropy->MLTable)); ZSTD_STATIC_ASSERT(offsetof(ZSTD_entropyDTables_t, hufTable) >= HUF_DECOMPRESS_WORKSPACE_SIZE); { void* const workspace = entropy; /* use fse tables as temporary workspace; implies fse table precede huffTable at beginning of entropy */ size_t const workspaceSize = offsetof(ZSTD_entropyDTables_t, hufTable); From 3675ef476248b3c1f3ebbb9fb9853094792d3fcd Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 10 Sep 2018 11:24:17 -0700 Subject: [PATCH 3/3] added comment about minimum size of FSE tables required for DDict creation, which use this space as workspace during Hufman table building stage. --- lib/decompress/zstd_decompress.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index 35ed7c59..d78a286f 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -110,9 +110,9 @@ typedef struct { #define SEQSYMBOL_TABLE_SIZE(log) (1 + (1 << (log))) typedef struct { - ZSTD_seqSymbol LLTable[SEQSYMBOL_TABLE_SIZE(LLFSELog)]; - ZSTD_seqSymbol OFTable[SEQSYMBOL_TABLE_SIZE(OffFSELog)]; - ZSTD_seqSymbol MLTable[SEQSYMBOL_TABLE_SIZE(MLFSELog)]; + ZSTD_seqSymbol LLTable[SEQSYMBOL_TABLE_SIZE(LLFSELog)]; /* Note : Space reserved for FSE Tables */ + ZSTD_seqSymbol OFTable[SEQSYMBOL_TABLE_SIZE(OffFSELog)]; /* is also used as temporary workspace while building hufTable during DDict creation */ + ZSTD_seqSymbol MLTable[SEQSYMBOL_TABLE_SIZE(MLFSELog)]; /* and therefore must be at least HUF_DECOMPRESS_WORKSPACE_SIZE large */ HUF_DTable hufTable[HUF_DTABLE_SIZE(HufLog)]; /* can accommodate HUF_decompress4X */ U32 rep[ZSTD_REP_NUM]; } ZSTD_entropyDTables_t;