[C-safe-secure-studygroup] what constitutes a rule violation?

Martin Sebor msebor at gmail.com
Sat Jul 14 00:50:25 BST 2018


On 07/13/2018 09:15 AM, Aaron Ballman wrote:
> On Wed, Jul 11, 2018 at 3:22 PM, Martin Sebor <msebor at gmail.com> wrote:
>> I get the impression that we're spending large amounts of time
>> with little progress by rehashing the same issue because we're
>> not on the same page about a fundamental question: what
>> constitutes a rule violation.
>>
>> Take the following examples.  Assume what you see is the complete
>> program that's visible to the analyzer; what's not defined in it
>> is in some library that the analyzer doesn't see or have knowledge
>> of -- like in the vast majority of software.
>
> CodeSonar allows the user to configure this somewhat (it does not have
> fine granularity and is more of a project-wide setting), but by
> default it is generally pessimistic and assumes anything can happen
> within a function it cannot see. The user can go to the trouble of
> writing a model for that function if they'd like, but that's more of
> an extreme circumstance and doesn't seem like the scenario you're
> asking about (that's more useful for times when you know what the
> behavior of the function is but the definition will never be seen by
> the tool).
>
> I tried your examples out in three static analysis tools I have handy
> right now: clang (through clang-tidy), CodeSonar (with and without
> MISRA checks enabled), and Visual Studio's analyzer.
>
>>   extern int f (int*);
>>
>>   int main (void)
>>   {
>>     int i;   // uninitialized
>>     f (&);
>>     return i;
>>   }
>
> Clang, Visual Studio, and CodeSonar (in both modes) do not diagnose
> the possibly uninitialized variable by default (after fixing the
> typo). I was surprised that CodeSonar does not diagnose the return
> statement as a violation of MISRA C:2012 Rule 9.1 and found out you
> need to enable this manually due to a high false positive rate even
> within the MISRA checks.

Thanks, that's very helpful!

>
> A related question along these same lines is what should be reported
> here, if anything:
>
> int main(void) {
>   int *i = (int)malloc(sizeof(int));
>   f(i);
>   return *i; // Diagnose a use-after-free here? Or is this a null
> pointer dereference? Dereferencing an invalid pointer value?
> }

Right. There are many variations on the same theme.  I'm not
sure that applying the same answer to all of them would yield
an optimal solution but I am convinced that we need to at
a minimum decide what we want the baseline answer to be.
We can then consider tweaking it for individual rules.

>
>> Is it anyone's expectation that the read of i in main violate
>> MISRA Rule 9.1 or TS 17961 rule 5.35 (reading uninitialized
>> memory), and so requires a diagnostic?
>>
>> What about if I change main to
>>
>>   int main (void)
>>   {
>>     int i;
>>     extern int *p = &i;
>>     f (0);
>>     return i;
>>   }
>>
>> Does this require a diagnostic?  Should it?
>
> Yes and yes. 'p': cannot initialize extern variables with block scope  ;-)

Right, thanks for the correction.  (It would be nifty if email
spellcheckers also tested code snippets for syntax errors ;-)

>
> Switching the code to:
>
> extern int f(int*);
>
> int i;
> extern int *p = &i;
>
> int main(void)
> {
>   f(0);
>   return i;
> }
>
> Clang, Visual Studio, and CodeSonar (in both modes) do not diagnose
> the return of i by default, but that seems like reasonable behavior
> given that i is zero initialized due to being a file-scope static.

I meant something more like this:

   extern int f(int*);

   int main(void)
   {
     extern int *p;

     int i;

     p = &i;   // i's address escapes

     f(0);     // f() could assign to i by *p = 0;

     return i;
   }

>> Or:
>>
>>   int main (void)
>>   {
>>     int i;
>>     switch (f (0)) {
>>     case 0:
>>     case 1:
>>     case 2: i = 0; break;
>>     case -1: i = 1; break;
>>     }
>>     return i;
>>   }
>>
>> Does this require a diagnostic?
>
> CodeSonar (in both modes), Visual Studio, and Clang all diagnose the
> uninitialized variable on "return i".

GCC gives a -Wmaybe-uninitialized (I don't get a warning with
Clang but that's not important).  What I think is important
about the GCC warning is the phrasing: may be uninitialized.
GCC makes a distinction between -Wuninitialized (definitely
uninitialized) and -Wmaybe-uninitialized (could be uninitialized,
but not necessarily -- I don't know enough about the program to
tell fot sure).  The choice of having two warnings and two
command line options to control them was made in response to
users complaining about false positives in the latter warning.
My proposal to require analyzers to make the same distinction
is based on our successful experience with this and other
checkers like it.

Martin




More information about the C-safe-secure-studygroup mailing list