MISRA 2008 C++ violations

[Daniel Poirot]

[forking the discussion to its own thread]

Static analysis tools vendors are between a rock and a hard place when
it comes to MISRA.

Folks often think 'more is better' when it comes to analysis rules but
MISRA is a whole different story.

With foundations in robustness and high availability, MISRA defines
rules with the aim of avoiding run-time failures in safety-critical,
deeply-embedded applications. Think aircraft avionics control,
robotics, medical systems and anti-lock brakes.

The definition document, 'MISRA C++ Guidelines for the use of the C++
language in critical systems', is a 220 page guide to building high
quality C++ code.

At Synopsys, we divide these rules into seven levels of MISRA
compliance or maturity.

Using my common analysis options on NTPsec, I get 27 Coverity issues,
18 of which are RESOURCE_LEAK. 17 of the Coverity findings are
disabled by comments in the source code - leaving 10 open issues. An
example finding of OVERRUN in 'ntp_control.c' is listed below.

'Level 1' of the MISRA C++ rules identifies an additional 2422
findings, 'Level 5' (the level at which Rule 3-9-2 is first added)
adds a staggering 13539 findings with 2773 of them places where the
'wrong' type is used.

Turning the dial up to 'Level 7' identifies a mind-breaking 18,273
violations of the MISRA 2008 C++ guidelines in just the set of 168
source files. Kind of a "can't see the forest for the trees" kind of
problem!

- dan



1109 /*
1110 * ctl_putunqstr - write a tagged string into the response packet
1111 *   in the form:
1112 *
1113 *   tag=data
1114 *
1115 * len is the data length excluding the NUL terminator.
1116 * data must not contain a comma or whitespace.
1117 */
1118 static void
1119 ctl_putunqstr(
1120 const char * tag,
1121 const char * data,
1122 size_t len
1123 )
1124 {
1125 char buffer[512];
1126 char *cp;
1127 size_t tl;
1128
1129 tl = strlen(tag);

(1) Event cond_false: Condition "tl >= 512UL /* sizeof (buffer) */",
taking false branch.
(3) Event cond_at_most: Checking "tl >= 512UL" implies that "tl" may
be up to 511 on the false branch.
Also see events: [alias][ptr_incr][overrun-local]

1130 if (tl >= sizeof(buffer))

(2) Event if_end: End of if statement.

1131    return;
1132 memcpy(buffer, tag, tl);

(4) Event alias: Assigning: "cp" = "&buffer[tl]". "cp" may now point
to as high as byte 511 of "buffer" (which consists of 512 bytes).
Also see events: [cond_at_most][ptr_incr][overrun-local]

1133 cp = buffer + tl;

(5) Event cond_true: Condition "len > 0", taking true branch.

1134 if (len > 0) {

(6) Event cond_true: Condition "tl + 1 + len <= 512UL /* sizeof
(buffer) */", taking true branch.

1135 NTP_INSIST(tl + 1 + len <= sizeof(buffer));

(7) Event ptr_incr: Incrementing "cp". "cp" may now point to as high
as byte 512 of "buffer" (which consists of 512 bytes).
Also see events: [cond_at_most][alias][overrun-local]

1136 *cp++ = '=';

(8) Event overrun-local: Overrunning array of 512 bytes at byte offset
512 by dereferencing pointer "cp". [Note: The source code
implementation of the function has been overridden by a builtin
model.]
Also see events: [cond_at_most][alias][ptr_incr]

1137 memcpy(cp, data, len);
1138 cp += len;
1139 }
1140 ctl_putdata(buffer, (u_int)(cp - buffer), false);
1141 }