Search code examples
cmisraparasoft

MISRA2004-12_8-3 rule has static analysis violation for RHS operand (32u - n) even with limits check

I am currently attempting to use parasoft software to fix static analysis violations for my code using MISRA C coding standards. My code initially had this function:

static inline uint32_t rotate_right(uint32_t val, uint32_t n)
{
    return (val >> n) | (val << (32 - n));
}

This causes a static analysis violation under the rule MISRA2004-12_8-3. The rule says

The right-hand operand of a shift operator shall lie between zero and one less than the width in bits of the underlying type of the left-hand operand

The rule documentation states that this particular 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

As I am not using a constant for the right-hand operand, MISRA-C rules dictate that I surround this statement with limit checks. MISRA-C also states that

Use of an unsigned integer type will ensure that the operand is non-negative, so then only the upper limit needs to be checked (dynamically at run-time or by review). Otherwise both limits will need to be checked."

Since I am using an unsigned type, uint32_t, I only need to check the upper limits of the right-hand operand. However, for val << (32u - n), I cannot have the value of n as 0u. Therefore, I tried to resolve this violation by adding the following checks:

static inline uint32_t rotate_right(uint32_t val, uint32_t n)
{
    if (n == 0u)
    {
        return val;
    }
    else if ((n > 0u) && (n <= 31u))
    {
        return (val >> n) | (val << (32u - n)); 
    }
    else
    {
        return 0u;
    }
}

Doing so resolves the static analysis violation for (val >> n), but the same violation is still reported for (val << (32u - n)).

Hence, my questions are:

  1. The if statement clearly restricts the value of n to be less than 32u. Consequently, (32u - n) will also have a value less than or equal to 32u. Why is parasoft software still reporting an error for the right-hand operand being (32u - n) despite the limit check?

  2. What is the correct way to resolve this violation?

Solution

  • As the MISRA-C document says, it is only sensible to check this dynamically at run-time or during code review. Which of course won't stop dysfunctional static analysers from trying to do it during static analysis anyway...

    Why is parasoft software still reporting an error for the right-hand operand being (32u - n) despite the limit check?

    Likely because it is bugged and reports a false positive.

    Unless of course it spotted that some caller-side code is providing an n which is larger than 32. Some static analysers are able to give somewhat smarter warnings when they check the project as whole rather than individual translation units, one at a time.

    What is the correct way to resolve this violation?

    Your original code is fine and apart from 32 -> 32u, it is MISRA compliant. Don't clutter it up with defensive programming just to silence the potentially broken tool. Only add such checks if you have reason to believe that n will actually be zero or larger than 32.

    For MISRA-C compliance, it should be sufficient to state that the case of a variable n will be covered by code review.

    (To be picky, MISRA-C:2004 doesn't allow inline since it doesn't allow C99, you need MISRA-C:2012 for that.)