No warning on uninitialized variable

[popy2007]

Hey guys.

Have found an code where cppcheck (current version 1.84) does'nt recognize an uninitialized variable.

Here is the code where it fails, InputReady could be uninitialized!

void main()
{
        unsigned char InputReady;
        unsigned char d = *RS232_Input_DataPtr;

        //LF?
        if (d == 10) InputReady = 1;

        //CR?
        if (d == 13) InputReady = 1;

        //Is input ready?
        if (InputReady == 1)
        {
            printf("input ready\n\r");
        }
}

Result (uninitvar is overseen):

cppcheck.exe --xml --enable=warning,style --error-exitcode=1 --template=vs cppcheck_uninitialized_var.c
<?xml version="1.0" encoding="UTF-8"?>
<results version="2">
    <cppcheck version="1.84"/>
    <errors>
Checking cppcheck_uninitialized_var.c ...
    </errors>
</results>

When i'll modify the code a little bit and put the crlf check into one if, it does'nt fail:

void main()
{
        unsigned char InputReady;
        unsigned char d = *RS232_Input_DataPtr;

        //LFCR?
        if ((d == 10) || (d == 13)) InputReady = 1;

        //Is input ready?
        if (InputReady == 1)
        {
            printf("input ready\n\r");
        }
}

The result is correct, uninit var is recognized by cppcheck:

cppcheck.exe --xml --enable=warning,style --error-exitcode=1 --template=vs cppcheck_uninitialized_var.c
<?xml version="1.0" encoding="UTF-8"?>
<results version="2">
    <cppcheck version="1.84"/>
    <errors>
Checking cppcheck_uninitialized_var.c ...
        <error id="uninitvar" severity="error" msg="Uninitialized variable: InputReady" verbose="Uninitialized variable: InputReady" cwe="908">
            <location file="cppcheck_uninitialized_var.c" line="10"/>
            <symbol>InputReady</symbol>
        </error>
    </errors>
</results>

Already tried --inconclusive cli option, which does'nt make a difference.
Any hints?
Is there a option i am missing?

Thanks a lot

[versat]

To me it looks like this should be fixed in Cppcheck. I guess you can not do much to get Cppcheck to report this issue.
Here is the output for the first example where the message is missing with some debug output:

$ ./cppcheck --enable=all --inconclusive --debug-normal uninitvar_fn.c
Checking uninitvar_fn.c ...

##file uninitvar_fn.c
1: void main ( )
2: {
3: unsigned char InputReady@1 ;
4: unsigned char d@2 ; d@2 = * RS232_Input_DataPtr ;
5:
6:
7: if ( d@2 == 10 ) { InputReady@1 = 1 ; }
8:
9:
10: if ( d@2 == 13 ) { InputReady@1 = 1 ; }
11:
12:
13: if ( InputReady@1 == 1 )
14: {
15: printf ( "input ready\n\r" ) ;
16: }
17: }

##Value flow
Line 7
  d possible {10,13}
  == possible {1,0}
  10 always 10
  1 always 1
Line 10
  d possible {13,10}
  == possible {1,0}
  13 always 13
  1 always 1
Line 13
  InputReady possible {1,Uninit}
  == possible 1
  1 always 1
Line 15
  "input ready\n\r" always "input ready\n\r"

Output for the second example where the message is shown with debug output:

$ ./cppcheck --enable=all --inconclusive --debug-normal uninitvar.c
Checking uninitvar.c ...

##file uninitvar.c
1: void main ( )
2: {
3: unsigned char InputReady@1 ;
4: unsigned char d@2 ; d@2 = * RS232_Input_DataPtr ;
5:
6:
7: if ( ( d@2 == 10 ) || ( d@2 == 13 ) ) { InputReady@1 = 1 ; }
8:
9:
10: if ( InputReady@1 == 1 )
11: {
12: printf ( "input ready\n\r" ) ;
13: }
14: }

##Value flow
Line 7
  d possible {10,13}
  == possible {1,0}
  10 always 10
  13 always 13
  1 always 1
Line 10
  InputReady possible {1,Uninit}
  == possible 1
  1 always 1
Line 12
  "input ready\n\r" always "input ready\n\r"
[uninitvar.c:10]: (error) Uninitialized variable: InputReady

In both cases Cppcheck knows that InputReady could be "Uninit", but it is only reported in one case.

Do you have a trac account to report it?
I have not found a ticket that reports this issue already, but there are some tickets related to uninitvar false negatives. I have not read all of them, there could be related tickets:
https://trac.cppcheck.net/search?q=false+negative+uninitvar

[popy2007]

thx, for the debug output.
Sadly i have no trac account to report it.
Can you please report it and link this discussion?

Thanks a lot

[versat]

I have created the ticket 8755 for this issue.
Thanks for your report.

[popy2007]

thx a lot

[Daniel Marjamäki]

Thanks!

hmm... it's not entirely conclusive if your code is unsafe or safe. it would be nice to see what the possible values of d is. If d is always 10 or 13 then there is no bug.

I have no idea how much noise it will produce to warn about code like this, I can only guess. I am just saying that we need to be careful about this.

[Daniel Marjamäki]

I do believe we can safely warn here:

void f(int d) {
    int inputReady;

    if (d == 10)
        inputReady = 1;
    if (d == 13)
        inputReady = 1;

    if (d == 'x') {}

    if (inputReady) {}
}

Does your real code also handle other values of d than 10 or 13?

[popy2007]

d is a char received over serial interface, so yes, it could be 0-255.
The code does not handle other values.
cppcheck should handle it, shoud'nt it?

[Daniel Marjamäki]

somehow we need to determine that it's an arbitrary value. We have some tickets about treating input values from files/etc as arbitrary..

[Daniel Marjamäki]

It might be ok to assume that all input variables have arbitrary values if --inconclusive is used.

I do think this code looks dangerous and can see that a warning would be interesting.

[Daniel Marjamäki]

It might be ok to assume that all variables have arbitrary values if --inconclusive is used.

If somebody writes a patch it would be very interesting to try it out to see how noisy it is.

[Set]

Hi, this topic also covers another false negative of missing warning on uninitialized var. Therefore I decided to add it there. Please, test the following piece of code:

class Elem {
public:
    Elem();
    ~Elem();
private:
    int i;
}

Elem::Elem()
//: i(0) // Initialization commented
{
    // Using 'this' in a constructor suppresses warnings about missing initialization of member variables!
    //pList.reset(new pList( this ));
}

Running:
"C:\Program Files\Cppcheck\cppcheck.exe" --enable=all C:\Users\x\Desktop\reportToCPPCheck\member-var-not-init-missing-warning.cpp

reports:

Checking C:\Users\x\Desktop\reportToCPPCheck\member-var-not-init-missing-warning.cpp ...
[C:\Users\x\Desktop\reportToCPPCheck\member-var-not-init-missing-warning.cpp:9]: (warning) Member variable 'Elem::i' is not initialized in the constructor.

What is wrong:
There is no warning about missing initialization after uncommenting line with keyword this what I suppose is wrong.

[Set]

Reminder, there is no reposnse for almost three weeks...
This issue persists in the newest version of CPPCheck, 1.86.

[Daniel Marjamäki]

ok sorry that we did not reply.

Let me explain why there is no warning when "this" is used. I believe it is because that in general we need to determine what exactly this code is doing:

pList.reset(new pList( this ));

If it indirectly writes i then there is no error.

In your code example the variable i can't be indirectly written through the public interface so I guess a warning is fine. But in general; if there is a setter or something then a warning should not be written unless we determine that the setter is not used.

[Daniel Marjamäki]

Example code:

class Elem {
public:
    Elem();
    ~Elem();
    void setI(int value) {
        i = value;
    }
private:
    int i;
}

Elem::Elem()
{
    pList.reset(new pList( this ));
}

We can't warn here unless we determine that pList constructor does not call setI.

[versat]

I was hoping that someone with more knowledge of the internals of Cppcheck writes something about this issue. For me it is not entirely clear if there is really no way that i could be initialized by pList().
Since pList is not known it might be hard for Cppcheck to tell what it does with the this pointer. On the other hand i is a private variable and there are no friend classes, so i guess it should be safe to assume that i is not initialized anywhere.

[Set]

Thanks for this discussion. My simplified example is based on a real code in which a developer really forgot to initialize a private variable and there was no setter. This hidden mystake caused serious problems! That is why I suppose it is rather important to think about the missing warning (and implement the warning somehow).

[popy2007]

The initial issue is solved in cppcheck v1.89!
It finds the issue and warns about it.
Thanks a lot for fixing this.