Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

generate ctu-info from FileSettings for whole project analysis #6499

Closed
wants to merge 2 commits into from

Conversation

dzid26
Copy link
Contributor

@dzid26 dzid26 commented Jun 9, 2024

Partially fixes https://sourceforge.net/p/cppcheck/discussion/general/thread/1911a624d8/ - for multiple configurations only the last one is analyzed. There still needs to be some way of generating and handling multiple ctu for the same file.

@firewave
Copy link
Collaborator

There can only be either the files or the FileSettings. They will never be provided both. This is a pending refactoring.

We also need tests for this. Right now I cannot remember if I already started adding those.

@dzid26
Copy link
Contributor Author

dzid26 commented Jun 10, 2024

There can only be either the files or the FileSettings

Which is why only one of those for loops would execute.


I couldn't figure out how ctus can be combined from multiple file configuration analyses. In any case, I am looking forward to see whole system addons to work with --project .

@dzid26 dzid26 closed this Jun 10, 2024
@firewave
Copy link
Collaborator

The change is fine. I did introduce the TODO but didn't fix it yet because I did't (actually don't) understand the whole program analysis fully. And we were most likely missing tests for.

@firewave
Copy link
Collaborator

Now that I have written a test for this I filed https://trac.cppcheck.net/ticket/12839 for tracking this.

@dzid26
Copy link
Contributor Author

dzid26 commented Jun 14, 2024

My fix doesn't work great with different configurations though. But neither the current cppcheck with --force in some scenarios, e.g. unusedStructMember .

With my fix swapping the order of the .json file entries changes the outcome. This is because ctu-infos are deleted between runs, so addon doesn't have whole project information.

E.g.

typedef struct {
    int x;
} X;
static void func(void) {};

int main(void) {
    #ifdef A
    return 0;
    #endif
    
    #ifdef B
    X x = {5};
    func();
    return x.x;
    #endif
}
}
[
    {
        "command": "arm-none-eabi-gcc   1.c -DA",
        "directory": "/home/dzid_/tests/misra",
        "output": "1_DA.o",
        "file": "1.c"
    },
    {
        "command": "arm-none-eabi-gcc   1.c -DB",
        "directory": "/home/dzid_/tests/misra",
        "output": "1_DB.o",
        "file": "1.c"
    }
]

produces:

/cppcheck --enable=all --addon=misra --project=cp.json
Checking 1.c ...
Checking... 1.c: A=1
1.c:3:9: style: struct member 'X::x' is never used. [unusedStructMember]
    int x;
        ^
1/2 files checked 50% done
Checking 1.c ...
Checking... 1.c: B=1
2/2 files checked 100% done
Checking... whole program
nofile:0:0: information: Active checkers: 295/1002 (use --checkers-report=<filename> to see details) [checkersReport]

But if I swap the order of json:

[
    
    {
        "command": "arm-none-eabi-gcc   1.c -DB",
        "directory": "/home/dzid_/openpilot/panda/tests/misra",
        "output": "1_DB.o",
        "file": "1.c"
    },
    {
        "command": "arm-none-eabi-gcc   1.c -DA",
        "directory": "/home/dzid_/openpilot/panda/tests/misra",
        "output": "1_DA.o",
        "file": "1.c"
    }
]

I get errors reported:

cppcheck --enable=all --addon=misra --project=cp.json
Checking 1.c ...
Checking... 1.c: B=1
1/2 files checked 50% done
Checking 1.c ...
Checking... 1.c: A=1
1.c:3:9: style: struct member 'X::x' is never used. [unusedStructMember]
    int x;
        ^
2/2 files checked 100% done
Checking... whole program
1.c:2:1: style: misra violation (use --rule-texts=<file> to get proper output) [misra-c2012-2.3]
typedef struct {
^
nofile:0:0: information: Active checkers: 295/1002 (use --checkers-report=<filename> to see details) [checkersReport]

So really the last configuration is what is being reported for the "whole program".

Link to testscases covering the issue: #6551

@dzid26 dzid26 reopened this Jun 14, 2024
@firewave
Copy link
Collaborator

Yes, configurations do not work properly. But let's take this one step at a time and use a test-driven approach. I am pretty sure there might be more shortcomings with configurations. I already have a few more tests for the whole program analysis which I will publish after the current PR and the fix for the initial issues have been merged.

@firewave
Copy link
Collaborator

I published a PR incorporating part of your changes with #6525.

@dzid26 dzid26 closed this Jun 14, 2024
@firewave
Copy link
Collaborator

It would be great if you could turn the example you provided earlier into a Python test. Same for any other shortcomings you might be aware of.

I still have another PR upcoming after this so maybe wait for that to be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants