Cppcheck-2.4

[Daniel Marjamäki]

Let's prepare for cppcheck-2.4 release. We need to stabilize Cppcheck.

Two observations;
* There are many crashes.
* The number of open defects have increased significantly. It seems partly because people have been extra active to report issues.

My goal right now is to release cppcheck-2.4 in ~ 2-4 weeks.

If you feel that there are some extra important issues that should be solved then please mention it here in this forum.

[Robert Reif]

I need some time to fix all the hangs and crashes in simplifyUsing that I introduced when changing ScopeInfo3 from a list item to a tree item. I hope I have all the crashes fixed now. There are multiple cyclic graph hangs now. One is due to template class derived from itself and I have a fix that I will present shortly. Another is related to multiple namespaces with the same name. I have no fix for that yet. There may be others.

[Daniel Marjamäki]

Thanks! There is no big hurry. I feel I want to fix many more defects..

[Daniel Marjamäki]

what do you say are we getting ready to release?

I am afraid I find very little time for cppcheck development lately and I don't think there will be much time soon neither.

[Daniel Marjamäki]

Unless somebody will complain I will release cppcheck-2.4 on saturday or sunday.

[Richard Smith]

MISRA rule 9.2 is crashing on my codebase.

Traceback (most recent call last):
  File "share/Cppcheck/addons/misra.py", line 3581, in <module>
    main()
  File "share/Cppcheck/addons/misra.py", line 3525, in main
    checker.parseDump(item)
  File "share/Cppcheck/addons/misra.py", line 3357, in parseDump
    self.executeCheck(902, self.misra_9_2, cfg)
  File "share/Cppcheck/addons/misra.py", line 3307, in executeCheck
    check_function(*args)
  File "share/Cppcheck/addons/misra.py", line 1732, in misra_9_2
    checkArrayInitializer(eq.astOperand2, dimensions, valueType)
  File "share/Cppcheck/addons/misra.py", line 1563, in checkArrayInitializer
    elements = getRecordElements(valueType) if valueType.type == 'record' else None
AttributeError: 'NoneType' object has no attribute 'type'

This was introduced with commit 4d3f76b63 which added the rule check.

[Georgiy Komarov]

Can you reproduce this crash with the addon from the main branch (commit b1b7fbb)?

[Richard Smith]

Heh.. I was doing exactly that as your message came in. Yes. As of 8fffb84aad7456bb37cd3add1419f17e61ec1601 the crash exists.

[Richard Smith]

The problem exists in current HEAD as well although the messages are different. Note I've modified the path output a bit to remove some stuff specific to our repository

File "share/Cppcheck/addons/misra.py", line 3327, in <module>
    main()
  File "share/Cppcheck/addons/misra.py", line 3271, in main
    checker.parseDump(item)
  File "share/Cppcheck/addons/misra.py", line 3102, in parseDump
    self.executeCheck(902, self.misra_9_2, cfg)
  File "share/Cppcheck/addons/misra.py", line 3053, in executeCheck
    check_function(*args)
  File "share/Cppcheck/addons/misra.py", line 1499, in misra_9_2
    misra_9.misra_9_x(self, data, 902)
  File "share/Cppcheck/addons/misra_9.py", line 404, in misra_9_x
    parser.parseInitializer(ed, eq.astOperand2)
  File "share/Cppcheck/addons/misra_9.py", line 255, in parseInitializer
    nextChild = self.root.getNextChild()
AttributeError: 'NoneType' object has no attribute 'getNextChild'

[Georgiy Komarov]

I think this can be easily corrected by adding an additional None check. But, ideally, I would like to have a regression test to avoid the same errors in further modifications of misra.py. Maybe you could tell us which input code is causing this crash?

[Richard Smith]

I'll see what I can do. Its challenging since this is not a public codebase so I can't just post up the source file.

[Daniel Marjamäki]

I think a code example would be really good to have.. how did you try to reproduce this?

maybe you tried that .. but I recommend that you start with your original code and then remove various stuff from it piece by piece. includes, types, functions, blocks of code, ...
there is probably some detail in your code that confuses Cppcheck and is very hard to guess.

[Richard Smith]

I have been trying to reproduce this but when I start to cut down the code to a smaller example it seems to go away and it is not consistent. I can re-add the code back that I removed and it continues to work. It is very confusing.

I think that additional debugging output in the rule code will be required to detect when the the conditions that end up with a NONE happen so I can run it on my codebase and have a better idea of what things are causing it to be messed up.

[Daniel Marjamäki]

I can re-add the code back that I removed and it continues to work. It is very confusing.

Strange! Maybe we'll have to solve it without a regression test :-(

[Richard Smith]

I figured it out. The following will duplicate my previous assertion.

    undefined_struct_t undefined =
    {
        .member1 = {0},
        .member2 = {0}
    }

However, that may not be the only one. I'm also getting this in what seems to be the same area but with more of the actual code and not the faked code.

Traceback (most recent call last):
  File "/home/rsmith/miniconda3/envs/<snip>/share/Cppcheck/addons/misra.py", line 3421, in <module>
    main()
  File "/home/rsmith/miniconda3/envs/<snip>/share/Cppcheck/addons/misra.py", line 3365, in main
    checker.parseDump(item)
  File "/home/rsmith/miniconda3/envs/<snip>/share/Cppcheck/addons/misra.py", line 3200, in parseDump
    self.executeCheck(905, self.misra_9_5, cfg, data.rawTokens)
  File "/home/rsmith/miniconda3/envs/<snip>/share/Cppcheck/addons/misra.py", line 3126, in executeCheck
    check_function(*args)
  File "/home/rsmith/miniconda3/envs/<snip>/share/Cppcheck/addons/misra.py", line 1581, in misra_9_5
    misra_9.misra_9_x(self, data, 905, rawTokens)
  File "/home/rsmith/miniconda3/envs/<snip>/share/Cppcheck/addons/misra_9.py", line 405, in misra_9_x
    ed = getElementDef(nameToken, rawTokens)
  File "/home/rsmith/miniconda3/envs/<snip>/share/Cppcheck/addons/misra_9.py", line 420, in getElementDef
    createArrayChildrenDefs(ed, nameToken.astParent, rawTokens)
  File "/home/rsmith/miniconda3/envs/<snip>/share/Cppcheck/addons/misra_9.py", line 431, in createArrayChildrenDefs
    foundToken = next(rawToken for rawToken in rawTokens if rawToken.file == token.file and rawToken.linenr == token.linenr and rawToken.column == token.column)
StopIteration

[Richard Smith]

The following patch to the test suite will duplicate the issue.

diff --git a/addons/test/misra/misra-test.c b/addons/test/misra/misra-test.c
index a0092160c..d15ebe805 100644
--- a/addons/test/misra/misra-test.c
+++ b/addons/test/misra/misra-test.c
@@ -480,6 +480,7 @@ void misra_9_struct_initializers(void) {
     dummy_struct dsd[2][2] = { 1, 2, 3, 4 };                  // 9.2
     dummy_struct dse[3]    = { {1,2}, {3,4}, [1] = {5,6} };   // 9.3 9.4
     dummy_struct dsd[]     = { [0] = 1 };                     // 9.5
+    dummy_struct dsg       = { .a = {0}, .b = {0} };

     // Obsolete initialization syntax for GCC
     struct1 os1 = { i1: 1, i2: 2 }; // 10.4 13.4

This problem only seems to happen when the structure is undefined. Since it was happening in my code base that suggests that cppcheck was unable to resolve the definition of one of our structures. :(

With current head and my codebase I'm not getting the NoneType exception but I am getting the StopIteration exception. The StopIteration is new from my original report. Not sure if the StopIteration is preventing the NoneType from occuring or not.

[Richard Smith]

The following patch works around the StopIteration exception and allows cppcheck to run on our codebase. However, I have no idea what effect this has on the validity of this check.

@Georgiy thoughts?

diff --git a/addons/misra_9.py b/addons/misra_9.py
index 653245322..f005fce62 100644
--- a/addons/misra_9.py
+++ b/addons/misra_9.py
@@ -428,7 +428,10 @@ def getElementDef(nameToken, rawTokens = None):
 def createArrayChildrenDefs(ed, token, rawTokens = None):
     if token.str == '[':
         if rawTokens is not None:
-            foundToken = next(rawToken for rawToken in rawTokens if rawToken.file == token.file and rawToken.linenr == token.linenr and rawToken.column == token.column)
+            try:
+                foundToken = next(rawToken for rawToken in rawTokens if rawToken.file == token.file and rawToken.linenr == token.linenr and rawToken.column == token.column)
+            except StopIteration:
+                foundToken = None

             if foundToken and foundToken.next and foundToken.next.str == ']':
                 ed.markAsFlexibleArray(token)

[Richard Smith]

Is there a way you can add something to the exception handler that shows what part of the code its working on ? Maybe show what the last parsed expression was or some representation of what part of the AST tree its in? I can examine the .dump file and see if I can figure out what source is causing it.

[Richard Smith]

I wrapped the call in a try statement and used the print output that was there plus some other prints of variables and a tweak to getInitDump(). Here is the modifications I made:

           try:
                parser.parseInitializer(ed, eq.astOperand2)
            except:
                print(rule, nameToken.str + '= ', ed.getInitDump())
                print(ed)
                print(nameToken)
                sys.exit(-1)

Output below. The { No Children } is my addition it means that len(self.children) was zero.

The structure in question 'hello_pkt_info' is a structure local to a function that is initialized with designated initializers (ie struct bla = { .bla1, .bla2 };) and has missing members that are left to the default. The structure contains some other structures and some of those structures have enum values but it does not seem overly complex to me (ie no tags for forward declarations)

I tried to re-create a similar struct in misra-test.c but I was unable to create something that triggers the crash. It may be due to a lot of the values for the struct init coming from header files or other static variables in the source code module.

I can look into the dump file and try to pull out some info if you have any questions but I'm afraid I can't send you the .dump

902 hello_pkt_info=  P{ No children }
ED(hello_pkt_info, P, childIndex=0, elementType='record', valueType=None)
Token(Id='0x560f21c6f920', str='hello_pkt_info', scopeId='0x560f21ca4130', isName=True, isUnsigned=False, isSigned=False, isNumber=False, isInt=False, isFloat=False, isString=False, strlen=None, isChar=False, isOp=False, isArithmeticalOp=False, isComparisonOp=False, isLogicalOp=False, isExpandedMacro=False, isSplittedVarDeclComma=False, isSplittedVarDeclEq=False, linkId=None, varId=505, variableId='0x560f21cce8b0', functionId=None, valuesId=None, valueType=None, typeScopeId=None, astParentId='0x560f21c6fa10', astOperand1Id=None, file='<removed>'

[Georgiy Komarov]

Thanks for this input. So far, I have not been able to reproduce this crash, but I have found two more crashes related to the initialization of the structs.

A few more questions about hello_pkt_info... Does it have anonymous structs/enums/unions as members? Do you use bit fields? May be you are using obsolete initialization syntax for GCC (e.g. struct { int x, y; } s = { y: 42, x: 19 };)?

[Richard Smith]

Nope. We use C99 so no anonymous strucrs. all struct declarations are from typedefs.
ill work more on recreating a similar struct example that I can post up.

[Richard Smith]

And no bit feilds. However they are packed with the packed attribute. They are for network access so no padding is allowed.

[Richard Smith]

Here is an example of the code. The struct is larger than this but most of the other members are just a uint8_t or uint32_t.

All of the typedefs live in header files. I've tried this code in misra-test.c both with the defs in the source file and with them included from a header file and also with init in the source file but not inlcuding the header file and I have not had any luck re-creating the exception.

typedef enum
{
    ENUM_1 = 0x01u,
    ENUM_2 = 0x02u,
    ENUM_MAX
} enum_type_t;

#define TEST_REV_LEVEL  1
#define PACKED_STRUCT struct __attribute__((packed))

typedef PACKED_STRUCT
{
    enum_type_t     enum_val;
    uint8_t         u1;
    uint8_t         u2;
} header_struct_t;

typedef PACKED_STRUCT
{
    header_struct_t     test_hs;
    uint8_t             rev;
    uint8_t             str1[20];
} crash_test_t;

crash_test_t crash_test =
 {
        .rev        = TEST_REV_LEVEL,
        .str1       = {0},
  };

[Robert Reif]

I think simplifyUsing is working OK now. I'm working on a template hang in #10190 but I think a good fix will be weeks away. #10190 is a really weird case so it's not a release blocker. I have a bad hang fix and the instantiated code is wrong so it's not ready for release yet. My time is limited for a while so don't wait for a fix from me.

I'm surprised no one is working on the other hangs in value flow and elsewhere. It looks like they were there in the last release too. There are crashes that are being hidden by timeouts when daca is run with -j1. I have a fix for one that I will submit tomorrow.

[Paul Fultz]

I think the hangs in ValueFlow is because of problems in the AST. I have a PR to at least turn the hangs into an error. That should help diagnosing these issue in the future.