how to detect size mismatch between variable and mask with bitwise operators

68 Views Asked by At

I want to check that a bitwise operator mask being used with a variable in a C source file is within the minimum range of the variable. I do get a warning if I cast the too large mask however I would prefer to not require a cast to trigger the warning.

In other words, I want a mask value used in a bitwise operation that is larger than can be contained in an unsigned short, e.g. 0x80000, would generate a warning or error when that mask value is used with an unsigned short variable, e,g, usVal & MY_MASK where MY_MASK is defined as #define MY_MASK 0x80000.

I realize that unsigned short may be larger than the minimum range 0 to 65,535 however I'm using Visual Studio with x86, 32 bit compilation which appears to treat unsigned short as a 16 bit value.

There may be alternatives that I could use and suggestions would be appreciated as long as it would work with Visual Studio 2019. It may be that other C compilers that can be used with Visual Studio will generate such a warning or a lint type of utility. There may also be a source analyzer setting in Visual Studio 2019 that will also flag such an error. Or Visual Studio 2022 may issue such a warning.

It appears gcc and clang have a -Wconversion flag, The Best and Worst GCC Compiler Flags For Embedded: -Wconversion, to warn of implicit conversions so is there something similar for Visual Studio 2019? Would that catch this type of problem?

In Visual Studio 2019 I can have the following C source:

typedef unsigned long ULONG;
typedef unsigned short USHORT;

#define KPS_MASK16_ALT_MAN   (ULONG)0x80000
#define KPS_MASK16_ALT_AUTO  0x4000

#define MASK_CHECK(t,v,op,m)  ((v) op (t)(m))

int    KpsIsAlt1(USHORT usOutPrinterInfo) {
    return  (usOutPrinterInfo & (USHORT)KPS_MASK16_ALT_MAN) || (usOutPrinterInfo & (USHORT)KPS_MASK16_ALT_AUTO);
}


int    KpsIsAlt2(USHORT usOutPrinterInfo) {
    return  (usOutPrinterInfo & KPS_MASK16_ALT_MAN) || (usOutPrinterInfo & KPS_MASK16_ALT_AUTO);
}

int    KpsIsAlt3(USHORT usOutPrinterInfo) {
    return  (MASK_CHECK(USHORT,usOutPrinterInfo, &, KPS_MASK16_ALT_MAN) || (usOutPrinterInfo & KPS_MASK16_ALT_AUTO));
}

which will generate the following compiler warnings using warning level 3 or 4 where line 136 is the mask operation in function KpsIsAlt1() and line 145 is the mask operation in function KpsIsAlt3() using the define MASK_CHECK:

1>Source2.c
1>D:\Users\rickc\Documents\vs2019repos\ConsoleApplicationClosure\Source2.c(136,38): warning C4310: cast truncates constant value
1>D:\Users\rickc\Documents\vs2019repos\ConsoleApplicationClosure\Source2.c(145,11): warning C4310: cast truncates constant value
1>ConsoleApplicationClosure.vcxproj -> D:\Users\rickc\Documents\vs2019repos\ConsoleApplicationClosure\Debug\ConsoleApplicationClosure.exe
1>Done building project "ConsoleApplicationClosure.vcxproj".

I would expect to see some kind of warning for the masking at line 141 in function KpsIsAlt2() as well. However there is no warning that the value of the mask exceeds the range of the variable.

It appears that Visual Studio will not issue a warning should a mask be used with a variable whose type is too small to be used with that mask. It is as if the compiler is silently promoting the unsigned short variable to unsigned long, and doing the mask.

What I would like to see is a warning along the lines of "bitwise operation out of range" or some other kind of a hint that the bitwise operation is using a mask that will not test any of the bits in the variable.

I tried the MASK_CHECK() define as a test of a possible solution but it really makes no sense to do that rather than just using the cast as used in the function KpsIsAlt1(). And unless there is some way to cause the Preprocessor to discern the variable type, it's not really practical.

I also did a quick test replacing defines with:

//#define KPS_MASK16_ALT_MAN   (ULONG)0x80000
//#define KPS_MASK16_ALT_AUTO  0x4000

ULONG const KPS_MASK16_ALT_MAN = 0x80000;
USHORT const KPS_MASK16_ALT_AUTO = 0x4000;

and saw no warnings at all.

If I change to enum I get the same warnings as if I were using the defines.

//#define KPS_MASK16_ALT_MAN   (ULONG)0x80000
//#define KPS_MASK16_ALT_AUTO  0x4000

//ULONG const KPS_MASK16_ALT_MAN = 0x80000;
//USHORT const KPS_MASK16_ALT_AUTO = 0x4000;

enum { KPS_MASK16_ALT_MAN = 0x80000, KPS_MASK16_ALT_AUTO = 0x4000 };
1

There are 1 best solutions below

1
dbush On

It is as if the compiler is silently promoting the unsigned short variable to unsigned long, and doing the mask.

Almost. It promotes the unsigned short value to int, then converts it to unsigned long before the & operator is applied.

What you are seeing is a result of integer promotions and the usual arithmetic conversions.

For the former, generally speaking, anyplace an integer variable or expression smaller that int is used in an expression, it is first promoted to either int or unsigned int.

This is spelled out in section 6.3.1.1p2 of the C standard:

The following may be used in an expression wherever an int or unsigned int may be used:

  • An object or expression with an integer type (other than int or unsigned int) whose integer conversion rank is less than or equal to the rank of int and unsigned int.
  • A bit-field of type _Bool, int, signed int, or unsigned int.

If an int can represent all values of the original type (as restricted by the width, for a bit-field), the value is converted to an int; otherwise, it is converted to an unsigned int. These are called the integer promotions. All other types are unchanged by the integer promotions.

In the latter case, when an operator has operands of two different types with one smaller than the other, the value with the smaller type is converted to the larger type. This is specified in section 6.3.1.8p1 (see the portion in bold):

... the integer promotions are performed on both operands. Then the following rules are applied to the promoted operands:

  • If both operands have the same type, then no further conversion is needed.
  • Otherwise, if both operands have signed integer types or both have unsigned integer types, the operand with the type of lesser integer conversion rank is converted to the type of the operand with greater rank.
  • Otherwise, if the operand that has unsigned integer type has rank greater or equal to the rank of the type of the other operand, then the operand with signed integer type is converted to the type of the operand with unsigned integer type.
  • Otherwise, if the type of the operand with signed integer type can represent all of the values of the type of the operand with unsigned integer type, then the operand with unsigned integer type is converted to the type of the operand with signed integer type.
  • Otherwise, both operands are converted to the unsigned integer type corresponding to the type of the operand with signed integer type

So in the case of this expression:

usOutPrinterInfo & KPS_MASK16_ALT_MAN

The value of usOutPrinterInfo is first promoted to int, then converted to unsigned long (because that is the type of KPS_MASK16_ALT_MAN), and the result has type unsigned long.

Which means this behavior is expected. For what you're doing, really your only option is to apply a cast, preferably at the point where the constant is defined so you only have to do it in one place.