use valueflow from cppcheck-1.88 for "normal" check level?

[danmar]

Is it a good idea to use valueflow from cppcheck-1.88 for "normal" check level?

PRO: There is a significant speedup.

There has been many improvements in valueflow since 1.88 that slowed down the analysis and for me those can be part of "exhaustive" checking.

There has been many fixes for false positives. We will need to fix those in the normal valueflow also somehow.

What is your opinions?

I envision that we will move duplicated heuristics from "valueflow.cpp" to "valueflownormal.cpp" and call the ValueFlowNormal function from the exhaustive pipeline..

Checking "lib" folder with this branch:

real    0m18,366s
user    0m18,319s
sys     0m0,044s

Checking "lib" folder with main branch:

real    2m49,526s
user    2m49,481s
sys     0m0,028s

[danmar]

I would not be against that we improve valueflow in ValueFlowNormal but my idea is that we will be extra careful..

[danmar]

This is a temporary hack! We will check if "normal" check level is enabled and then call ValueFlowNormal::setValues

[danmar]

@firewave @pfultz2 @orbitcowboy @chrchr-github it would be interesting with some opinions. This PR is far from complete so don't bother too much looking at the code.

[pfultz2]

I think using valueflow from 1.88 will introduce a lot of FPs(and FNs). I dont think its a good idea to make this the normal analysis. We could have a fast mode that ran this type of analysis. However, there is still a lot of code duplication. It would be better to prune back our current analysis in a way that can run faster and minimize FPs for such a fast mode.

[pfultz2]

Also, when analyzing lib/, most of the time is spent for symboldatabase.cpp file:

real    1m28.936s
user    1m28.844s
sys     0m0.066s

If we limit the max iterations to 1 and remove valueFlowSubFunction the time jumps down to:

real    0m3.242s
user    0m3.180s
sys     0m0.064s

valueFlowSubFunction does a cross-product of all parameter, and this can grow to 512 different sets of arguments in symboldatabase.cpp. If we limit the size to 8 then it still runs pretty fast:

real    0m4.539s
user    0m4.476s
sys     0m0.063s

So #4950 and #4951 address this issue.

[danmar]

@pfultz2 ok thanks for your inputs.. that makes sense. I close this.

[firewave]

I would really like to look into and comment on this but I have been out sick for the week and I really need to step back from being so involved.
I know I have been saying this multiple times but I finally have to actually do it.

This means I will try to finish up all my open stuff and add lots of tickets, TODOs and (unfortunately) also some draft PRs. I will also no longer participate in discussions outside of my tickets/PRs and also no longer keep an eye on the CI - I keep getting sidetracked all the time I cannot do this anymore.

Sorry about that.

[danmar]

@firewave please take care.