How to make (1 << 9) pass MISRA?

Question

We are using Parasoft Static Analysis with MISRA C 2004 checker turned on.

The software is an embedded system. We like to describe constants as follows:

[1]    #define MOTOR_ON (1 << 9)

This would show that the 9th bit in the register should be a 1 to turn on the motor.

The expression is failing MISRA, so we changed it:

[2]    #define MOTOR_ON (1U << 9U)

The changes convert to unsigned integer constants because shifting is best done with unsigned integers.

The expression in statement 2, is still failing because of the right hand operator (9U) needs checking. According to MISRA, if the right hand operator is larger than the bit width of the underlying type of the left hand operator, there is a problem.

The base of the problem is that 1U has an underlying type of unsigned char or 8-bits.
The register we are writing to is 16-bits, so theoretically there is not an issue.

How can I change the expression in [2] so that it passes MISRA C 2004, preferring not to use casts?

I'm using IAR Embedded Workbench with an ARM7TDMI processor in 8/32 bit mode.

Edit 1: Example code.

void turn_on_motor(void);
#define MOTOR_ON (1U << 9U)
void turn_on_motor(void)
{
    uint16_t * const p_motor_control = (uint16_t *)(0x01234567U);
    *p_motor_control = MOTOR_ON;
}

Error text: Constant used as the right-hand operand of a shift operator shall be limited.

From the MISRA Rule documentation provided by Parasoft:

Rule reports a violation if:

- the right-hand operand is a constant with negative value or with value that
  exceeds the length (in bits) of the left-hand operand

- the right-hand operand is not a constant and is not checked by specific
  pattern

Note that `1u << 9` is the same as `1u << 9u`. The type of the RHS does not affect the result. — Dietrich Epp, Feb 03 '14 at 20:29
maybe you have to declare the bit length of an unsigned int in your static code analysis software ? — Benoît, Feb 03 '14 at 20:31
"The base of the problem is that 1U has an underlying type of unsigned char or 8-bits." That's not standard C. — K-ballo, Feb 03 '14 at 20:33
@K-ballo: My understanding is that the type of a constant is the smallest type that the constant will fit into. — Thomas Matthews, Feb 03 '14 at 20:37
@ThomasMatthews Then your understanding is wrong. The minimum size is that of an `int`. — glglgl, Feb 03 '14 at 20:46
@ThomasMatthews: Before evaluating `<<`, the "integer promotions" are performed on both operands, turning them into possibly signed `int`s. An `int` is at least 16 bits. (That's standard C, not MISRA C. Apparently MISRA wants you to write `(unsigned)1U` ) — rici, Feb 03 '14 at 20:47
§6.4.4.1 shows that an integer constant with a `U` suffix is an `unsigned int` (or larger). It can't be an `unsigned char`. This has nothing to do with promotions. — Adrian McCarthy, Feb 03 '14 at 20:51
@AdrianMcCarthy: Not a duplicate, this one involves constants only which should have no problems passing MISRA. I wrote the other question. — Thomas Matthews, Feb 03 '14 at 20:57
Neither _#define_ should cause any problem. Please show the code that uses `MOTOR_ON`. — chux - Reinstate Monica, Feb 03 '14 at 21:13
@chux: See Edit 1, I added an small example that recreates the issue, the error message and the quote from the Parasoft Documentation. — Thomas Matthews, Feb 03 '14 at 21:38
This is not an answer because I don't know if it will work (having no actual experience with MISRA) but I would try `#define MOTOR_ON (((uint16_t)1) << 9)`, choosing `uint16_t` because that is the width of the hardware register in question. If *that* doesn't work, maybe it's time to give up on shifts and write `#define MOTOR_ON 0x0100u` or perhaps `#define MOTOR_ON UINT16_C(0x0100)`. — zwol, Feb 03 '14 at 21:43
The code in your question does not violate the quoted rule. Even if you're using a non-standard compiler that gives `1U` a type other than `unsigned int`, your MISRA tool would have to do the same thing. The typo "righ-hand" rather than "right-hand" suggests that you didn't copy-and-paste the exact error message; please update your question to do so. If nothing else, you could `#define MOTOR_ON 0x200 /* 1<<9 */` — Keith Thompson, Feb 03 '14 at 21:47
@KeithThompson The MISRA standard have different rules, it basically says that the value 1U has an "underlying type"(a MISRA concept) of the smallest type the constant can fit into - for the OPs platform this is probably an 8 bit type, while the underlying type of 9U is bigger. (This doesn't really make sense to me, the compiler is anyhow likely following the C standard, but that's what MISRA says). The easy way out seems to just be a #define for 0x200. — nos, Feb 03 '14 at 22:16
Sure this isn't simple an `int` to `uint16_t` issue? I'd suggest something like `#define MOTOR_ON ( (uint16_t) ((1u) << 9) )` This should fix "... value that exceeds the length (in bits) of the left-hand operand ..." — chux - Reinstate Monica, Feb 03 '14 at 22:20
@DietrichEpp and nos Then I must have misunderstood MISRA. I thought about it as a kind of coding standard, sitting on top of Standard C and extending C's rules. Obviously I am wrong here. — glglgl, Feb 03 '14 at 22:56
@KeithThompson: Corrected typo. Although reformatting to hex or decimal is an opportunity, I was hoping to save the shift for readability. — Thomas Matthews, Feb 04 '14 at 01:45
The concept of underlying type is just there to make you actually consider what types you are using. It is not really something you have to dwell on, because what they want in the end is type safety and protection from implicit type promotions. So simply cast the result of the shift to the intended type and it will be MISRA compliant. For example `(uint32_t)(1u << 9u)`. You might have to do this by rule 10.5 anyhow. — Lundin, Feb 04 '14 at 10:19
To cite MISRA: "Notice that underlying type is an artificial concept. It does not in any way influence the type of evaluation that is actually performed. The concept has been developed simply as a way of definiting a safe framwork in which to construct arithmetic expressions." Maybe Parasoft and/or missed that part. I don't believe they should write their tools so that they implement underlying type for integer literals, they should follow the C standard. A large integer type which is shifted 9 bits cannot "exceed the length". — Lundin, Feb 04 '14 at 10:27
Furthermore, the true danger here is left shifting a signed integer value which holds a negative value: this is _undefined behavior_. And it really doesn't make any sense whatsoever to use shift on signed integer types: code that does so suggest that the programmer is unaware of what types they are using. This is perhaps what MISRA is trying to say between the lines. — Lundin, Feb 04 '14 at 10:31
@Lundin: On many compilers for two's-complement architectures, if `x` is type `int`, `x< — supercat, May 20 '15 at 15:41
@supercat Left shift is not the same thing as multiplication: bitwise operators work on bits only, they don't care the slightest of what representation the variable is supposed to be in. Apart from that, never assume that the standard is logical, with a sound rationale for every single language mechanism! The C standard is rather the opposite: a vast collection of design mistakes, poorly-defined behavior and irrational nonsense. One such example of nonsense is that C allows other forms of signedness than two's completement, which is the root of many an evil. — Lundin, May 21 '15 at 06:33
@Lundin: On a two's-complement architecture, integer types behave mathematically as an infinite stream of bits were the sign bits and all bits to its left are required to always hold the same value (the infinite summation formula for 1+2+4+8+16+... yields -1). Shifting such a string of bits left will, assuming the required constraint is not violated, be *mathematically*, be equivalent to multiplication by a power of two. — supercat, May 21 '15 at 15:16
@supercat Rather: on any two's completement architecture, the Logical Shift Left CPU instruction doesn't care what's in the msb before performing the shift, whatever was there is merely dumped into a carry bit, at best. What will happen with that carry bit is no business of the LSL instruction. And C behaves just as the underlying machine code in this case. — Lundin, May 21 '15 at 15:25
@Lundin: I'm well aware that C is a bunch of ugly hacks stacked on top of each other, but there are a number of places where the behavior resulting from "straightforward" implementation of the code on typical platforms would be more useful than inferences which could be drawn by making such behaviors illegal. I would think the logical thing to do would be to specify predefined macros which would indicate what such behaviors would be supported, and say that define a term to describe the compliance of code which would require optional features, but not on features not described by the standard. — supercat, May 21 '15 at 19:38
@Lundin: Historically, two's-complement compilers have generally shifted bits left without regard for whether a value is signed or unsigned; I would posit the standard should specify that on two's-complement machines, in cases where all bits that shift into or out of the sign bit match, defining behavior in that fashion would be more useful than any inferences which could be drawn by making such behavior undefined. In some kinds of audio/video code it's necessary to scale things by variable powers of two, and I would posit using `<<` directly is the most readable way of doing so. — supercat, May 21 '15 at 19:48

steveha · Answer 1 · 2014-02-04T19:21:17.040

My advice is to define a macro that hides the ugly casting, but then go ahead and do the ugly casting to make MISRA happy.

Something like:

#define LSHIFT(x, n) \
    (((unsigned int)(x)) << ((unsigned int)(n)))

Then in your actual code:

#define MOTOR_ON LSHIFT(1, 9)

EDIT: In a comment below, @Lundin says that MISRA will complain about function-like macros. I've never used MISRA so I didn't know that.

A quick Google search finds that MISRA has special comments you can add to your code to disable warnings. This suggests two possibilities:

In your header file where you define LSHIFT() and RSHIFT() and any other bit-manipulating macros, wrap the macro definitions in the MISRA warning disable comments.
In your source file where you want to put the bit-shifting, add MISRA warning disable comments and just put your code as you had it before.

http://www.gimpel.com/Discussion.cfm?ThreadMode=Prev&ThreadID=2261

If I'm understanding correctly, MISRA has global enable/disable but does not have disable followed by "put it back the way it was". So, the comments to disable and then enable will always globally enable the check, so ideally these magic comments shouldn't be in a header file.

So I guess my suggestion now is to put your original bit shifting code int a .C source file, and put the magic comments to disable/enable the MISRA warnings around the bit shifting code.

But function-like macros are frowned upon by MISRA. There's advisory rules that suggest you ban them entirely, where a function would have been more suitable. I believe this is such a case. Furthermore, MISRA still won't be happy about the signed integer literals, nor by the use of `unsigned int` rather than `uintnn_t`. — Lundin, Feb 04 '14 at 10:18
@Lundin, thanks for the information. I have never used MISRA. I have updated my answer in response to your comment. — steveha, Feb 04 '14 at 19:21

Jens · Accepted Answer · 2014-02-04T09:27:44.953

1

You could also simply circumvent the shifting issue by using

 #define MOTOR_ON ((uint16_t)512U)   /* 1 << 9 = 2^9 */

edited Feb 04 '14 at 09:27

answered Feb 04 '14 at 09:06

Jens

69,818
15
125
179

In embedded systems, hexadecimal is usually preferred over decimal constants in these scenarios. – Thomas Matthews Feb 04 '14 at 14:24
@OP Is it the `(uint16_t)` that solved this issue? IOW `#define MOTOR_ON ((uint16_t) (1u << 9))` should work. Or is it the `512U` ((IOW `#define MOTOR_ON (512U)` should work. or were both needed? – chux - Reinstate Monica Feb 04 '14 at 21:54

score 0 · Answer 3 · edited May 23 '17 at 12:30

0

The value of expression is being assigned to an object with a narrower type. “1U << 9U” results in only preserving the low-order bits.

“Making MISRA happy” with ugly casting won’t change this fact, though a poor tool might be gamed, it shouldn’t.

The straightforward solution is to use an explicit cast :

#define MOTOR_ON ((uint16_t)0x200) /* 9th bit on turns on motor */

but if you believe the “shift is better for readability” then simply turn off the rule for this case and document the reasoning. The deviation process is perfectly fine (and encouraged) in MISRA compliant development, if fact complete conformance is impossible. You have to have a deviation management process to be fully compliant. Evidence of awareness is the goal, not evidence of conformance.

BTW, @Thomas, you are right, this is not an exact duplicate of MISRA C:2004, error with bit shifting which you also wrote. Though it is a Rule 10.3 violation, the concepts of “underlying type” described does little to help understand the intent of this warning for this specific question. My suggestion to all is taking a look at the description for Rule 10.3 in the latest version of MISRA-C:2012 (found here: http://misra.org.uk ) which makes the concepts and intent much more clear for engineers and tool makers.

edited May 23 '17 at 12:30

Community

1
1

answered Feb 04 '14 at 02:29

Eddie Jones

11
2

"this could have been a case of integral promotion": what? From this statement on, the answer seems to make no sense. – Potatoswatter Feb 04 '14 at 03:34
FIne I took out the tangent. – Eddie Jones Feb 04 '14 at 08:09
This should be posted as comments and not an answer, since it does not answer the question (which is also about MISRA-C:2004). – Lundin Feb 04 '14 at 10:15
Uh no, the answer is deviation, not obfuscation, which is perfectly fine in MISRA 2004 as well. I simply referred to MISRA 2012 because it provides more clarity to the intent of the rules. – Eddie Jones Feb 04 '14 at 11:02
@Lundin, I agree, I tried, but couldn't post comments as a new contributor, so gimme a break. – Eddie Jones Feb 05 '14 at 22:55
And I don't see anything wrong with the answer... IMHO use of the hex constant is clearer than shifting a bit?! – Andrew Nov 06 '14 at 12:22

How to make (1 << 9) pass MISRA?

3 Answers3

Linked