MISRA: Bitwise operation on signed integer

Question

I have this error according to MISRA rules : bitwise operation may not be performed on signed integers.

    typedef unsigned __int8   gUBYTE;

    typedef gUBYTE gBORDER;
    enum {
        gbrLEFT     = 0x01,
        gbrTOP      = 0x02,
        gbrRIGHT    = 0x04,
        gbrBOTTOM   = 0x08,
        gbrALL      = gbrLEFT | gbrTOP | gbrRIGHT | gbrBOTTOM
    };

how could I solve this error ?

Answer 1

Change:

gbrALL      = gbrLEFT | gbrTOP | gbrRIGHT | gbrBOTTOM

to:

gbrALL      = gbrLEFT + gbrTOP + gbrRIGHT + gbrBOTTOM

This solves the problem in this particular expression. Later in the code, you could have expressions such as x & gbrLEFT, and this might also be flagged as a bitwise operation on a signed integer. However, if x is of type gUBYTE, it is unsigned. On the other hand, it will be promoted to an int, and then x & gbrLEFT is technically an operation on signed integers. On the other other hand, it would seem this would be a problem for a MISRA-analyzer; code might perform a completely safe bitwise operation on two unsigned objects which are promoted to signed int, thus triggering a warning from the analyzer. So it seems a good analyzer should recognize that the underlying objects are safe for the bitwise operation. (This applies to many bitwise operations. Some, such as ~ might be unsafe if applied to an unsigned object that has been promoted to int, depending on the C implementation and other context.)

Is this so? If so, then fixing up the one expression shown above might suffice.

Answer 2

Apparently the integers defined in an enum end up being "signed". Your compiler is deciding that for you. See: Are C++ enums signed or unsigned?

My tests indicate that adam's suggestion there with "enum Y : unsigned int { ...." is only acceptable in C++.

I've tested my compiler (gcc), and it already chooses an unsigned int as the data type for my enum. You could try to assign a value of 0xc0000000 to one of the items in your enum. In that case the compiler is forced to chose either a long long, or an unsigned int. Still it is implementation defined what it will chose for you.

Answer 3

What are you gaining here from using an enum? Use a bit field, constant or a #define. Enums should not be used for bit fields because you are going to end up with values that are not valid enum values. e.g. LEFT|TOP is 0x3 and you don't have that as an enum value.

There are many other ways of doing this. Some people have a preference as to which.

#define GBR_LEFT            (1U << 0U)
#define GBR_TOP             (1U << 1U)
#define GBR_RIGHT           (1U << 2U)
#define GBR_BOTTOM          (1U << 3U)
#define GBR_MASK_ALL        (GBR_LEFT | GBR_TOP | GBR_RIGHT | GBR_BOTTOM)

or

static const unsigned int GBR_LEFT          = (1U << 0U);
static const unsigned int GBR_TOP           = (1U << 1U);
static const unsigned int GBR_RIGHT         = (1U << 2U);
static const unsigned int GBR_BOTTOM        = (1U << 3U);
static const unsigned int GBR_MASK_ALL      = (GBR_LEFT | GBR_TOP | GBR_RIGHT | GBR_BOTTOM);

or preferably use bit-fields (although never do this if you are defining actual hardware registers or something that should never be able to be mangled by the compiler)

struct gbrLocation
{
    unsigned left :1;
    unsigned top :1;
    unsigned right :1;
    unsigned bottom :1;
};

Often it is also wise to set up clear masks as well so you don't have to have bit inversions nested within a lot of brackets. This helps improve code readability.

Answer 4

Make your integers unsigned by adding u to them 0x01u.

Answer 5

Make your values a set of constants, instead of having them in an enum, so that you can explicitly assign the type unsigned int to each of them.

Later you could create an enumeration using the resulting values. The problem is that you are combining a calculation and your enum definition together. If you separate them, the warning will go away.

Answer 6

An enumeration constant such as those in this example are always equivalent to int. Since int is always signed, you cannot use enumeration constants in bitwise expressions, unless you use an explicit typecast.

As has already been mentioned, replacing | with + will do the trick. Alternatively (though less readable), you could have written

gbrALL = (int)( (uint16_t)gbrLEFT  | 
                (uint16_t)gbrTOP   | 
                (uint16_t)gbrRIGHT | 
                (uint16_t)gbrBOTTOM)

It would have been MISRA-C compilant but very ugly code...

The best and easiest way to write MISRA-C conformant code is to avoid enums entirely. Not only are enumeration constants always equal to int. But an enumerated type (a variable with enum type) has implementation-defined size and signedness (C11 6.7.2.2). Perhaps you thought that C was a rational language and that an enumerated type would be of a type always suitable to hold an enumeration constant. Not so. It could be unsigned, it could be a small integer type or a large one. This is one of many big flaws in the C language, a flaw that MISRA-C will painfully expose if you weren't already aware.

Even more painfully, MISRA-C enforces you to document all implementation-defined behavior relied upon. Meaning that you can't just pass static analysis to make the code containing enums MISRA-C compliant. You'll also have to write an essay of all the oddities and flaws of the enum type and state how enum is implemented on your specific compiler, by citing compiler documentation. (And if the compiler lacks such documentation, it isn't C compliant and therefore causes further MISRA issues).

Therefore, the best way to achieve what you want is to #define all constants as unsigned integer literals:

#define gbrLEFT  0x01u

and so on.