Fix #8192: FN condition always false in for loop condition

[francois-berder]

When a for loop's condition is impossible given the initial value (e.g. for (int i = 0; i > 10; i++)), cppcheck was not emitting a knownConditionTrueFalse warning.

Fix by populating memory1, memory2 and memoryAfter with the init state when the condition is immediately false (and no error occured). We can then set the value for the condition token and thus emit a knownConditionTrueFalse warning.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[danmar]

tweaking the valueflow can have effects in various checkers..

I think it would be interesting if you run cppcheck/tools/test-my-pr.py script on 1000 or so packages and see what the changes are. Will you see more warnings, fewer warnings, which warnings..

A command like python3 tools/test-my-pr.py -p 1000 from inside your branch should work..

[francois-berder]

tweaking the valueflow can have effects in various checkers..

I think it would be interesting if you run cppcheck/tools/test-my-pr.py script on 1000 or so packages and see what the changes are. Will you see more warnings, fewer warnings, which warnings..

A command like python3 tools/test-my-pr.py -p 1000 from inside your branch should work..

Unfortunately, I am having trouble testing my PR in this way. While running tools/test-my-pr.py, I keep getting connection refused error:

Cppcheck 2.21 dev
Connecting to server to get count of packages..
ConnectionRefusedError in __get_packages_count: [Errno 111] Connection refused
Trying __get_packages_count again in 5.0 seconds
Connecting to server to get count of packages..
ConnectionRefusedError in __get_packages_count: [Errno 111] Connection refused
Trying __get_packages_count again in 10.0 seconds
Connecting to server to get count of packages..
ConnectionRefusedError in __get_packages_count: [Errno 111] Connection refused
Trying __get_packages_count again in 20.0 seconds
Connecting to server to get count of packages..
ConnectionRefusedError in __get_packages_count: [Errno 111] Connection refused
Trying __get_packages_count again in 40.0 seconds
Connecting to server to get count of packages..
Maximum number of tries reached for __get_packages_count

I also tried in a different way by downloading the DACA2 packages using daca2-download.py script and then running ./tools/test-my-pr.py --packages-path ~/daca2-packages/ but it then failed at some point with the same error:

Package: /home/francois/daca2-packages/aha_0.5.1.orig.tar.xz
Removing existing temporary data...
Unpacking..
extracted 1 of 3 files (skipped 0)
Detecting library usage...
Found libraries: ['posix', 'gnu', 'bsd']
Analyze..
nice /home/francois/cppcheck-test-my-pr-workfolder/tree-main/cppcheck --library=posix --library=gnu --library=bsd  --inconclusive --enable=style,information --inline-suppr --template=daca2 --disable=missingInclude --suppress=unmatchedSuppression --check-library --debug-warnings --suppress=autoNoType --suppress=valueFlowBailout --suppress=bailoutUninitVar --suppress=symbolDatabaseWarning --suppress=normalCheckLevelConditionExpressions -D__GNUC__ --platform=unix64 -rp=/home/francois/cppcheck-test-my-pr-workfolder/temp -j1 /home/francois/cppcheck-test-my-pr-workfolder/temp
Number of issues: 12
cppcheck finished with 0 in 1.7s
Analyze..
nice /home/francois/projects/cppcheck/cppcheck --library=posix --library=gnu --library=bsd  --inconclusive --enable=style,information --inline-suppr --template=daca2 --disable=missingInclude --suppress=unmatchedSuppression --check-library --debug-warnings --suppress=autoNoType --suppress=valueFlowBailout --suppress=bailoutUninitVar --suppress=symbolDatabaseWarning --suppress=normalCheckLevelConditionExpressions -D__GNUC__ --platform=unix64 -rp=/home/francois/cppcheck-test-my-pr-workfolder/temp -j1 /home/francois/cppcheck-test-my-pr-workfolder/temp
Number of issues: 12
cppcheck finished with 0 in 1.8s
Diff results..
148 of 323 packages processed

Connecting to server to get assigned work..
ConnectionRefusedError in __get_package: [Errno 111] Connection refused
Trying __get_package again in 30.0 seconds
Connecting to server to get assigned work..
ConnectionRefusedError in __get_package: [Errno 111] Connection refused
Trying __get_package again in 30.0 seconds
Connecting to server to get assigned work..
Maximum number of tries reached for __get_package
Traceback (most recent call last):
  File "/home/francois/projects/cppcheck/tools/test-my-pr.py", line 129, in <module>
    package = lib.get_package(packages_idxs.pop())
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/francois/projects/cppcheck/tools/donate_cpu_lib.py", line 292, in get_package
    return try_retry(__get_package, fargs=(package_index,), max_tries=3, sleep_duration=30.0, sleep_factor=1.0)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/francois/projects/cppcheck/tools/donate_cpu_lib.py", line 100, in try_retry
    raise e
  File "/home/francois/projects/cppcheck/tools/donate_cpu_lib.py", line 87, in try_retry
    return fun(*fargs)
           ^^^^^^^^^^^
  File "/home/francois/projects/cppcheck/tools/donate_cpu_lib.py", line 280, in __get_package
    sock.connect(__server_address)
ConnectionRefusedError: [Errno 111] Connection refused

[danmar]

Unfortunately, I am having trouble testing my PR in this way. While running tools/test-my-pr.py, I keep getting connection refused error:

I can reproduce that.. will fix it..

[danmar]

When #8491 has been merged, I suggest you update your main branch and rebase this PR.. and then you should be able to test it out properly.

When I tested your changes I got some crashes:

crashes
https://ftp.debian.org/debian/pool/main/m/microprofile/microprofile_4.0+dfsg.orig.tar.xz
https://ftp.debian.org/debian/pool/main/liba/libapache2-mod-authnz-external/libapache2-mod-authnz-external_3.3.2.orig.tar.gz
https://ftp.debian.org/debian/pool/main/p/pyside6/pyside6_6.10.3.orig.tar.xz
https://ftp.debian.org/debian/pool/main/f/festival/festival_2.5.0.orig.tar.gz
https://ftp.debian.org/debian/pool/main/l/lrslib/lrslib_0.73.orig.tar.gz
https://ftp.debian.org/debian/pool/main/a/akonadi-contacts/akonadi-contacts_25.12.3.orig.tar.xz
https://ftp.debian.org/debian/pool/main/j/jpy/jpy_1.4.0.orig.tar.xz
https://ftp.debian.org/debian/pool/main/libv/libvirt/libvirt_12.2.0.orig.tar.xz

I don't know for sure if these crashes was caused by your changes. But I have the feeling that main branch without your changes didn't crash, and my local branch with your changes did crash.
So I suggest an extra look on these packages.

[francois-berder]

I rebased on top of 950567e and I ran ./tools/test-my-pr.py -p 1000. I got the following result: my_check_diff.log
I did not get any crashes but a few timeouts.

[danmar]

The idea is that you will look at the warnings also and decide if they are correct. If there had been 100's of warnings you could have looked at a random subset but I only see 2 warnings in your my_check_diff.log:

https://ftp.debian.org/debian/pool/main/p/psautohint/psautohint_2.4.0.orig.tar.gz
libraries:posix,gnu,bsd,python
diff:
your psautohint-2.4.0/libpsautohint/src/eval.c:237:20: style: Condition 'minlen==overlaplen' is always false [knownConditionTrueFalse]
psautohint-2.4.0/libpsautohint/src/eval.c:236:22: note: minlen is assigned '(ltop-lbot)<=(rtop-rbot)?(ltop-lbot):(rtop-rbot)' here.
psautohint-2.4.0/libpsautohint/src/eval.c:235:26: note: overlaplen is assigned '((ltop)<=(rtop)?(ltop):(rtop))-((lbot)>=(rbot)?(lbot):(rbot))' here.
psautohint-2.4.0/libpsautohint/src/eval.c:237:20: note: Condition 'minlen==overlaplen' is always false

https://ftp.debian.org/debian/pool/main/v/vala-panel/vala-panel_24.05.orig.tar.bz2
libraries:posix,gnu,bsd,gtk,motif
diff:
your vala-panel-24.05/applets/core/cpu/cpu.c:161:51: style: Condition 'new_pixmap_height>0' is always true [knownConditionTrueFalse]
vala-panel-24.05/applets/core/cpu/cpu.c:161:24: note: Assuming that condition 'new_pixmap_width>0' is not redundant
vala-panel-24.05/applets/core/cpu/cpu.c:159:25: note: new_pixmap_width is assigned '(uint)(((allocation.width-22)>0)?(allocation.width-22):0)' here.
vala-panel-24.05/applets/core/cpu/cpu.c:160:27: note: Assignment 'new_pixmap_height=(uint)(((allocation.height-22)>0)?(allocation.height-22):0)', assigned value is symbolic=new_pixmap_width
vala-panel-24.05/applets/core/cpu/cpu.c:161:51: note: Condition 'new_pixmap_height>0' is always true

[danmar]

For information you should be able to look at such warnings with this tool: cppcheck/tools/triage
if you have qt you can compile and run that.
but right now it doesn't understand "https" well but I think it works if you replace "https://" with "ftp://" in the log file and then open it with "open log file" button.

[danmar]

About this:

https://ftp.debian.org/debian/pool/main/p/psautohint/psautohint_2.4.0.orig.tar.gz
your psautohint-2.4.0/libpsautohint/src/eval.c:237:20: style: Condition 'minlen==overlaplen' is always false [knownConditionTrueFalse]
psautohint-2.4.0/libpsautohint/src/eval.c:236:22: note: minlen is assigned '(ltop-lbot)<=(rtop-rbot)?(ltop-lbot):(rtop-rbot)' here.
psautohint-2.4.0/libpsautohint/src/eval.c:235:26: note: overlaplen is assigned '((ltop)<=(rtop)?(ltop):(rtop))-((lbot)>=(rbot)?(lbot):(rbot))' here.
psautohint-2.4.0/libpsautohint/src/eval.c:237:20: note: Condition 'minlen==overlaplen' is always false

The code is:

235:        Fixed overlaplen = NUMMIN(ltop, rtop) - NUMMAX(lbot, rbot);
236:        Fixed minlen = NUMMIN(ltop - lbot, rtop - rbot);
237:        if (minlen == overlaplen)

I don't immediately see if it's correct

[danmar]

About this:

https://ftp.debian.org/debian/pool/main/v/vala-panel/vala-panel_24.05.orig.tar.bz2
your vala-panel-24.05/applets/core/cpu/cpu.c:161:51: style: Condition 'new_pixmap_height>0' is always true [knownConditionTrueFalse]
vala-panel-24.05/applets/core/cpu/cpu.c:161:24: note: Assuming that condition 'new_pixmap_width>0' is not redundant
vala-panel-24.05/applets/core/cpu/cpu.c:159:25: note: new_pixmap_width is assigned '(uint)(((allocation.width-22)>0)?(allocation.width-22):0)' here.
vala-panel-24.05/applets/core/cpu/cpu.c:160:27: note: Assignment 'new_pixmap_height=(uint)(((allocation.height-22)>0)?(allocation.height-22):0)', assigned value is symbolic=new_pixmap_width
vala-panel-24.05/applets/core/cpu/cpu.c:161:51: note: Condition 'new_pixmap_height>0' is always true

The code is:

	uint new_pixmap_width  = (uint)MAX(allocation.width - BORDER_SIZE * 2, 0);
	uint new_pixmap_height = (uint)MAX(allocation.height - BORDER_SIZE * 2, 0);
	if ((new_pixmap_width > 0) && (new_pixmap_height > 0))

Is this a false positive? new_pixmap_height can be 0 right?

EDIT: spontanously it seems to me that "assigned value is symbolic=new_pixmap_width" is where ValueFlow made a mistake. My guess is that it should not claim that the assigned value is that.