Skip to content

blow up errors on Intel Compiler on O2: _BitScanReverse return code not checked #1951

@audioMirror

Description

@audioMirror

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.

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions