Skip to content

Commit

Permalink
baby-step towards solving flexArray issue facebook#3785
Browse files Browse the repository at this point in the history
the flexArray in structure FSE_DecompressWksp
is just a way to derive a pointer easily,
without risk/complexity of calculating it manually.

Not sure if this change is good enough to avoid ubsan warnings though.
  • Loading branch information
Cyan4973 authored and hswong3i committed Mar 27, 2024
1 parent 5c0be81 commit 1970af0
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 20 deletions.
28 changes: 14 additions & 14 deletions lib/common/fse_decompress.c
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ static size_t FSE_buildDTable_internal(FSE_DTable* dt, const short* normalizedCo
symbolNext[s] = 1;
} else {
if (normalizedCounter[s] >= largeLimit) DTableH.fastMode=0;
symbolNext[s] = normalizedCounter[s];
symbolNext[s] = (U16)normalizedCounter[s];
} } }
ZSTD_memcpy(dt, &DTableH, sizeof(DTableH));
}
Expand All @@ -99,8 +99,7 @@ static size_t FSE_buildDTable_internal(FSE_DTable* dt, const short* normalizedCo
* all symbols have counts <= 8. We ensure we have 8 bytes at the end of
* our buffer to handle the over-write.
*/
{
U64 const add = 0x0101010101010101ull;
{ U64 const add = 0x0101010101010101ull;
size_t pos = 0;
U64 sv = 0;
U32 s;
Expand All @@ -111,9 +110,8 @@ static size_t FSE_buildDTable_internal(FSE_DTable* dt, const short* normalizedCo
for (i = 8; i < n; i += 8) {
MEM_write64(spread + pos + i, sv);
}
pos += n;
}
}
pos += (size_t)n;
} }
/* Now we spread those positions across the table.
* The benefit of doing it in two stages is that we avoid the
* variable size inner loop, which caused lots of branch misses.
Expand Down Expand Up @@ -232,12 +230,13 @@ FORCE_INLINE_TEMPLATE size_t FSE_decompress_usingDTable_generic(
break;
} }

return op-ostart;
assert(op >= ostart);
return (size_t)(op-ostart);
}

typedef struct {
short ncount[FSE_MAX_SYMBOL_VALUE + 1];
FSE_DTable dtable[1]; /* Dynamically sized */
FSE_DTable dtable[1]; /* actual size is dynamic - member just helps get a pointer easily */
} FSE_DecompressWksp;


Expand All @@ -252,13 +251,14 @@ FORCE_INLINE_TEMPLATE size_t FSE_decompress_wksp_body(
unsigned tableLog;
unsigned maxSymbolValue = FSE_MAX_SYMBOL_VALUE;
FSE_DecompressWksp* const wksp = (FSE_DecompressWksp*)workSpace;
FSE_DTable* const dtable = wksp->dtable;

DEBUG_STATIC_ASSERT((FSE_MAX_SYMBOL_VALUE + 1) % 2 == 0);
if (wkspSize < sizeof(*wksp)) return ERROR(GENERIC);

/* normal FSE decoding mode */
{
size_t const NCountLength = FSE_readNCount_bmi2(wksp->ncount, &maxSymbolValue, &tableLog, istart, cSrcSize, bmi2);
{ size_t const NCountLength =
FSE_readNCount_bmi2(wksp->ncount, &maxSymbolValue, &tableLog, istart, cSrcSize, bmi2);
if (FSE_isError(NCountLength)) return NCountLength;
if (tableLog > maxLog) return ERROR(tableLog_tooLarge);
assert(NCountLength <= cSrcSize);
Expand All @@ -271,16 +271,16 @@ FORCE_INLINE_TEMPLATE size_t FSE_decompress_wksp_body(
workSpace = (BYTE*)workSpace + sizeof(*wksp) + FSE_DTABLE_SIZE(tableLog);
wkspSize -= sizeof(*wksp) + FSE_DTABLE_SIZE(tableLog);

CHECK_F( FSE_buildDTable_internal(wksp->dtable, wksp->ncount, maxSymbolValue, tableLog, workSpace, wkspSize) );
CHECK_F( FSE_buildDTable_internal(dtable, wksp->ncount, maxSymbolValue, tableLog, workSpace, wkspSize) );

{
const void* ptr = wksp->dtable;
const void* ptr = dtable;
const FSE_DTableHeader* DTableH = (const FSE_DTableHeader*)ptr;
const U32 fastMode = DTableH->fastMode;

/* select fast mode (static) */
if (fastMode) return FSE_decompress_usingDTable_generic(dst, dstCapacity, ip, cSrcSize, wksp->dtable, 1);
return FSE_decompress_usingDTable_generic(dst, dstCapacity, ip, cSrcSize, wksp->dtable, 0);
if (fastMode) return FSE_decompress_usingDTable_generic(dst, dstCapacity, ip, cSrcSize, dtable, 1);
return FSE_decompress_usingDTable_generic(dst, dstCapacity, ip, cSrcSize, dtable, 0);
}
}

Expand Down
13 changes: 7 additions & 6 deletions lib/compress/fse_compress.c
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,8 @@ size_t FSE_NCountWriteBound(unsigned maxSymbolValue, unsigned tableLog)
size_t const maxHeaderSize = (((maxSymbolValue+1) * tableLog
+ 4 /* bitCount initialized at 4 */
+ 2 /* first two symbols may use one additional bit each */) / 8)
+ 1 /* round up to whole nb bytes */
+ 2 /* additional two bytes for bitstream flush */;
+ 1 /* round up to whole nb bytes */
+ 2 /* additional two bytes for bitstream flush */;
return maxSymbolValue ? maxHeaderSize : FSE_NCOUNTBOUND; /* maxSymbolValue==0 ? use default */
}

Expand Down Expand Up @@ -255,7 +255,7 @@ FSE_writeNCount_generic (void* header, size_t headerBufferSize,
/* Init */
remaining = tableSize+1; /* +1 for extra accuracy */
threshold = tableSize;
nbBits = tableLog+1;
nbBits = (int)tableLog+1;

while ((symbol < alphabetSize) && (remaining>1)) { /* stops at 1 */
if (previousIs0) {
Expand All @@ -274,7 +274,7 @@ FSE_writeNCount_generic (void* header, size_t headerBufferSize,
}
while (symbol >= start+3) {
start+=3;
bitStream += 3 << bitCount;
bitStream += 3U << bitCount;
bitCount += 2;
}
bitStream += (symbol-start) << bitCount;
Expand All @@ -294,7 +294,7 @@ FSE_writeNCount_generic (void* header, size_t headerBufferSize,
count++; /* +1 for extra accuracy */
if (count>=threshold)
count += max; /* [0..max[ [max..threshold[ (...) [threshold+max 2*threshold[ */
bitStream += count << bitCount;
bitStream += (U32)count << bitCount;
bitCount += nbBits;
bitCount -= (count<max);
previousIs0 = (count==1);
Expand Down Expand Up @@ -322,7 +322,8 @@ FSE_writeNCount_generic (void* header, size_t headerBufferSize,
out[1] = (BYTE)(bitStream>>8);
out+= (bitCount+7) /8;

return (out-ostart);
assert(out >= ostart);
return (size_t)(out-ostart);
}


Expand Down

0 comments on commit 1970af0

Please sign in to comment.