Ytukyg

I have written a C Macro to set/unset Bits in a uint32 variable. Here are the definitions of the macros:

extern uint32_t error_field, error_field2;

    #define SET_ERROR_BIT(x) do{

                                if(x < 0 || x >63){

                                    break;

                                }

                                if(((uint32_t)x)<32U){

                                    (error_field |= ((uint32_t)1U << ((uint32_t)x)));

                                    break;

                                } else if(((uint32_t)x)<64U){

                                    (error_field2 |= ((uint32_t)1U<<(((uint32_t)x)-32U)));

                                }

                            }while(0)



    #define RESET_ERROR_BIT(x) do{

                                if(((uint32_t)x)<32U){

                                    (error_field &= ~((uint32_t)1U<<((uint32_t)x)));

                                    break;

                                } else if(((uint32_t)x) < 64U){

                                    (error_field2 &= ~((uint32_t)1U<<(((uint32_t)x)-32U)));

                                }

                             } while(0)

I am passing a field of an enumeration, that looks like this:

enum error_bits {

    error_chamber01_data = 0,

    error_port21_data,

    error_port22_data,

    error_port23_data,

    error_port24_data,

/*this goes on until 47*/

};

This warning is produced:

left shift count >= width of type [-Wshift-count-overflow]

I am calling the Macros like this:

USART2->CR1 |= USART_CR1_RXNEIE;

SET_ERROR_BIT(error_usart2);

/*error_usart2 is 47 in the enum*/

return -1;

I get this warning with every macro, even with those where the left shift count is < 31.

If I use the definition of the macro without the macro, it produces no warning. The behaviour is the same with a 64 bit variable. I am programming a STM32F7 with AC6 STM32 MCU GCC compiler.
I can't figure out why this happens. Can anyone help me?

Probably a problem with the compiler not being able to diagnose correctly, as stated by M Oehm. A workaround could be, instead of using the minus operation, use the remainder operation:

#define _SET_BIT(x, bit) (x) |= 1U<<((bit) % 32U)

#define SET_BIT(x, bit) _SET_BIT(x, (uint32_t)(bit))

#define _SET_ERROR_BIT(x) do{

                            if((x)<32U){

                                SET_BIT(error_field, x);

                            } else if((x)<64U){

                                SET_BIT(error_field2, x);

                            }

                        }while(0)

#define SET_ERROR_BIT(x) _SET_ERROR_BIT((uint32_t)(x))

This way the compiler is finally smart enough to know that the value of x will never exceed 32.

The call to the "_" macro is used in order to force x to always be an uint32_t, inconditionally of the macro call, avoiding the UB of a call with a negative value of x.

Tested in coliru

Problem:

In the macros, you distinguish two cases, which, on their own, are okay. The warning comes from the branch that isn't executed, where the shift is out of range. (Apparently these diagnostics are issued before the dead branch is eliminated.)
@M Oehm

Solution

Insure shifts are in range 0-31 in both paths regardless of the x value and type of x.

x & 31 is a stronger insurance than x%32 or x%32u. % can result in negative remainders when x < 0 and with a wide enough type.

   #define SET_ERROR_BIT(x) do{

                                if((x) < 0 || (x) >63){

                                    break;

                                }

                                if(((uint32_t)x)<32U){

                                    (error_field |= ((uint32_t)1U << ( (x)&31 )));

                                    break;

                                } else if(((uint32_t)x)<64U){

                                    (error_field2 |= ((uint32_t)1U<<( (x)&31 )));

                                }

                            }while(0)

As a general rule: good to use () around each usage of x.

Seeing the thread I wanted to indicate a nice (and perhaps cleaner) way to set, reset and toggle the status of a bit in the case of the two unsigned integers as in thread. This code should be OT because uses x that shall be an unsigned int (or an int) and not a enum value.

I've written the line of code at the end of this answer.

The code receives as input a number of parameter couples. Each couple of parameter is a letter and a number. The letter may be:

• S to set a bit


• R to reset a bit


• T to toggle a bit

The number has to be a bit value from 0 to 63. The macros in the code discard each number greater than 63 and nothing is modified into the variables. The negative values haven't been evalued because we suppose a bit value is an unsigned value.

For Example (if we name the program bitman):

Executing: bitman S 0 S 1 T 7 S 64 T 7 S 2 T 80 R 1 S 63 S 32 R 63 T 62

The output will be:

S 0 00000000-00000001

S 1 00000000-00000003

T 7 00000000-00000083

S 64 00000000-00000083

T 7 00000000-00000003

S 2 00000000-00000007

T 80 00000000-00000007

R 1 00000000-00000005

S 63 80000000-00000005

S 32 80000001-00000005

R 63 00000001-00000005

T 62 40000001-00000005

#include <unistd.h>

#include <stdio.h>

#include <stdint.h>

#include <string.h>



static uint32_t err1 = 0;

static uint32_t err2 = 0;



#define SET_ERROR_BIT(x) (

    ((unsigned)(x)>63)?err1=err1:((x)<32)?

    (err1 |= (1U<<(x))):

    (err2 |= (1U<<((x)-32)))

    )



#define RESET_ERROR_BIT(x) (

    ((unsigned)(x)>63)?err1=err1:((x)<32)?

    (err1 &= ~(1U<<(x))):

    (err2 &= ~(1U<<((x)-32)))

    )



#define TOGGLE_ERROR_BIT(x) (

    ((unsigned)(x)>63)?err1=err1:((x)<32)?

    (err1 ^= (1U<<(x))):

    (err2 ^= (1U<<((x)-32)))

    )



int main(int argc, char *argv)

{

    int i;

    unsigned int x;



    for(i=1;i<argc;i+=2) {

        x=strtoul(argv[i+1],NULL,0);



        switch (argv[i][0]) {

        case 'S':

            SET_ERROR_BIT(x);

            break;

        case 'T':

            TOGGLE_ERROR_BIT(x);

            break;

        case 'R':

            RESET_ERROR_BIT(x);

            break;

        default:

            break;

        }



        printf("%c %2d %08X-%08Xn",argv[i][0], x, err2, err1);

    }



    return 0;

}

The macros are splitted in more then one line, but they are each a one-line code.

The code main has no error control then if the parameters are not correctly specified the program might be undefined behaviour.