-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Description
Platform:
Windows 10
Visual Studio 2010
Intel Compiler 2013
Result:
Some streams blow-up on decompress
Cause:
Incorrect results leading to a blow-up are created by bitstream.h when doing an 02 optimized compile:
MEM_STATIC unsigned BIT_highbit32 (U32 val)
{
assert(val != 0);
{
# if defined(_MSC_VER) /* Visual */
unsigned long r=0;
_BitScanReverse ( &r, val );
return (unsigned) r;
...
}
_BitScanReverse returns success (!=0) or failure (0). The Microsoft definition is:
__success(return!=0)
BOOLEAN
_BitScanReverse (
__out DWORD *Index,
__in DWORD Mask
);
The Zstd code assumed that 'r' will be left at zero when the success if false. However, I don't think the intrinsic is defined that way. The 'r' value is likely undefined on failure. Certainly, the Intel Compiler is making this assumption during its optimization at level O2 (O1 or below don't have the bug).
This could be an Intel Compiler bug, but it is more likely a bug in the zstd code assuming that 'r' has a valid value on failure.
This code stops giving the wrong value when changed to:
MEM_STATIC unsigned BIT_highbit32 (U32 val)
{
assert(val != 0);
{
# if defined(_MSC_VER) /* Visual */
unsigned long r;
return (_BitScanReverse ( &r, val )) ? (unsigned)r : 0;
...
}
The following Microsoft Intrinsics are used on zstd:
_BitScanForward
_BitScanReverse
_BitScanForward64
_BitScanReverse64
All of these calls will potentially be affected by this bug, and all cased should be fixed.