MISRA C 2012 Rule 11.4 violation

920 Views Asked by At

Facing Issue related to MISRA C 2012 Rule 11.4 violation. Using PC-Lint Plus for rule check. Keil uVision V5.38.0.0

Error:

conversion between object pointer type 'GPIO_Type *' and integer type 'unsigned int' [MISRA 2012 Rule 11.4, advisory]```
uint *L_uc_byte = (uint *)&GPIO->PIN[0];

Following are the details related to GPIO

#define     __IO    volatile             /*!< Defines 'read / write' permissions */
/** Peripheral GPIO base address */
#define GPIO_BASE                                (0x4008C000u)
/** Peripheral GPIO base pointer */
#define GPIO                                     ((GPIO_Type *)GPIO_BASE)

/** GPIO - Register Layout Typedef */
typedef struct {
  __IO uint8_t B[6][32];                           /**< Byte pin registers for all port 0 and 1 GPIO pins, array offset: 0x0, array step: index*0x20, index2*0x1 */
       uint8_t RESERVED_0[3904];
  __IO uint32_t W[6][32];                          /**< Word pin registers for all port 0 and 1 GPIO pins, array offset: 0x1000, array step: index*0x80, index2*0x4 */
       uint8_t RESERVED_1[3328];
  __IO uint32_t DIR[6];                            /**< Direction registers, array offset: 0x2000, array step: 0x4 */
       uint8_t RESERVED_2[104];
  __IO uint32_t MASK[6];                           /**< Mask register, array offset: 0x2080, array step: 0x4 */
       uint8_t RESERVED_3[104];
  __IO uint32_t PIN[6];                            /**< Port pin register, array offset: 0x2100, array step: 0x4 */
       uint8_t RESERVED_4[104];
  __IO uint32_t MPIN[6];                           /**< Masked port register, array offset: 0x2180, array step: 0x4 */
       uint8_t RESERVED_5[104];
  __IO uint32_t SET[6];                            /**< Write: Set register for port Read: output bits for port, array offset: 0x2200, array step: 0x4 */
       uint8_t RESERVED_6[104];
  __O  uint32_t CLR[6];                            /**< Clear port, array offset: 0x2280, array step: 0x4 */
       uint8_t RESERVED_7[104];
  __O  uint32_t NOT[6];                            /**< Toggle port, array offset: 0x2300, array step: 0x4 */
       uint8_t RESERVED_8[104];
  __O  uint32_t DIRSET[6];                         /**< Set pin direction bits for port, array offset: 0x2380, array step: 0x4 */
       uint8_t RESERVED_9[104];
  __O  uint32_t DIRCLR[6];                         /**< Clear pin direction bits for port, array offset: 0x2400, array step: 0x4 */
       uint8_t RESERVED_10[104];
  __O  uint32_t DIRNOT[6];                         /**< Toggle pin direction bits for port, array offset: 0x2480, array step: 0x4 */
} GPIO_Type;

I have tried multiple things

  1. conversion between object pointer type 'GPIO_Type *' and integer type 'unsigned int' [MISRA 2012 Rule 11.4, advisory]
    uint data = ( ( GPIO->PIN[0] >> 17U ) & 0x03U );
    
  2. conversion between object pointer type 'uint *' (aka 'unsigned int *') and integer type 'volatile uint32_t' (aka 'volatile unsigned int') [MISRA 2012 Rule 11.4, advisory]
    volatile uint L_uc_byte = *( uint * )(GPIO->PIN[0]);
    
  3. conversion between object pointer type 'GPIO_Type *' and integer type 'unsigned int' [MISRA 2012 Rule 11.4, advisory]
    uint L_uc_byte = GPIO->PIN[0];
    
  4. Using address literal as a pointer
    conversion between object pointer type 'uint *' (aka 'unsigned int *') and integer type 'unsigned int' [MISRA 2012 Rule 11.4, advisory]
    uint L_uc_byte = *( uint * )( 0x40002000U + 0x2100U);
    

Need Solution to resolve this rule violation.

3

There are 3 best solutions below

0
Andrew On BEST ANSWER

MISRA acknowledge that some Guidelines may cause problems for legitimate use cases... that is why we have created the Deviation process, as documented in MISRA Compliance.

Likewise for Advisory Guidelines (such as R.11.4), these can be disapplied.

R.11.4 even includes (within the Rationale) a statement to the effect that violating this Rule may be necessary when addressing memory mapped register.

So the correct approach (over and above Lundin's excellent suggestions) is not to ignore R.11.4, but to disapply it (with justification)

-- See profile for affiliation

3
Lundin On
  • You need to nuke that strange uint type from your codebase - don't invent strange non-standard, non-self-documenting integer types. Use standard C uint32_t.

  • Please note that it is best to avoid mixing volatile qualified expression such as reading/writing to registers with other operands or sub expressions (and also another MISRA violation). It's bad for performance purpose and clarity, and the extra side-effect can give unpredicted results. Try to make it so that register access expression only does a write or a read and nothing else.

  • Never perform pointer conversion casts unless you have in-depth knowledge of C. Casting an integer to a uint* which isn't volatile qualified is never correct. In particular it is almost certainly never the correct route to go in case you get warnings.

  • "Magic numbers" such as x >> 17 & 42 shouldn't be used since they make the code unreadable and forces the reader to sit with their nose constantly buried in the MCU manual's register descriptions. Instead use meaningful names. In case you are using a register map from a MCU vendor it typically comes with named constants which you can use.

Details here: How to access a hardware register from firmware?

Furthermore, MISRA-C have some advisory rules that are overly pedantic and not really applicable to embedded systems. One such rule forbids conversions between integers and pointers, but there is just no way to define a register map without performing such casts, so the rule has to be ignored.

The key to MISRA is understanding why they made the rule in the first place. The main issue in this case is alignment, something like uint32_t* data = (uint32_t*)0x0003; would cause a misaligned pointer. If you understand why that is a severe bug, then you understand why MISRA made the rule. Given that you aren't going to provide any home-made addresses like that, you are safe to ignore the rule.

The standard practice is that you need no formal deviation for skipping advisory rules. Though ideally your MISRA-based coding standard should list all advisory rules that are OK to ignore and then they should be disabled accordingly in the static analyser/MISRA checker.

Conclusions:

uint data = ( ( GPIO->PIN[0] >> 17U ) & 0x03U ); change to uint32_t data and name the constants something meaningful. Ideally and for MISRA compliance you should also split this up into several expressions, which is a bit overzealous but should be 100% MISRA compliant:

uint32_t data = GPIO->PIN[0]; // volatile expression as a stand-alone read/write

// just making up some names here, refer to the MCU manual for proper names:
data = (data >> GPIO_POS ) & GPIO_MASK; 

And if ignoring the integer to pointer advisory rule as we ought to:

#define SOME_REGISTER (*(volatile uint32_t*)(0x40002000U + 0x2100U))
...
uint32_t L_uc_byte = SOME_REGISTER;
0
kesselhaus On

If you plan to use GPIO on several places, you could at least have the occurence of the MISRA violation to a single place.

Create a peripherl.h file like:

extern GPIO_Type * const GPIO;

Create a peripheral.c file like:

#include "peripheral.h"

#define GPIO_BASE (0x4008C000u) // define as needed
#define ADC_BASE  (0x4008D000u) // define as needed

// add your MISRA tool comment/pragma here
// this should be the only place of violating the rule
GPIO_Type * const GPIO = (GPIO_Type*)GPIO_BASE;
ADC_Type  * const ADC  = (ADC_Type*)ADC_BASE;

Now you can use the GPIO pointer in your code without this violation;

main.c

#include "peripheral.h"

int main(void) {
    // since the GPIO is defined in header as GPIO_Type*
    // and initialized in peripheral.c, there should be
    // no violation here
    uint32_t x = (GPIO->PIN[0] >> 17u) & 0x3u;
    
    return 0;
}