[clang] [analyzer] Don't assume third iteration in loops (PR #119388)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 10 07:09:06 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Donát Nagy (NagyDonat)
<details>
<summary>Changes</summary>
This commit ensures that if the loop condition is opaque (the analyzer cannot determine whether it's true or false) and there were at least two iterations, then the analyzer doesn't make the unjustified assumption that it can enter yet another iteration.
Note that the presence of a loop suggests that the developer thought that two iterations can happen (otherwise an `if` would've been sufficient), but it does not imply that the developer expected three or four iterations -- and in fact there are many false positives where a loop iterates over a two-element (or three-element) data structure, but the analyzer cannot understand the loop condition and blindly assumes that there may be three or more iterations. (In particular, analyzing the FFMPEG project produces 100+ such false positives.)
Moreover, this provides some performance improvements in the sense that the analyzer won't waste time on traversing the execution paths with 3 or 4 iterations in a loop (which are very similar to the paths with 2 iterations) and therefore will be able to traverse more branches elsewhere on the `ExplodedGraph`.
This logic is disabled if the user enables the widen-loops analyzer option (which is disabled by default), because the "simulate one final iteration after the invalidation" execution path would be suppressed by the "exit the loop if the loop condition is opaque and there were at least two iterations" logic. If we want to support loop widening, we would need to create a follow-up commit which ensures that it "plays nicely" with this logic.
---
To evaluate the effects of this commit, I also implemented an intermediate version which doesn't change the traversal of the exploded graph and instead of discarding the "assumes a third iteration" code paths eagerly, it analyzes them (wasting some time) and then discards the results found on them.
I analyzed our usual set open source projects these three version (upstream, intermediate, this commit) and the checker groups core, cplusplus, deadcode, nullability, security, unix and valist, plus the checker `alpha.security.ArrayBoundV2`.
**Comparison between upstream (a few weeks old main revision) and the intermediate version:**
| Project | Only in intermediate | Only in upstream | Evaluation of eliminated reports |
|---------|-------------|------------------|-----|
| memcached | [0 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=memcached_1.6.8_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=memcached_1.6.8_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=New) | [0 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=memcached_1.6.8_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=memcached_1.6.8_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved) | |
| tmux | [0 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=tmux_2.6_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=tmux_2.6_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=New) | [0 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=tmux_2.6_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=tmux_2.6_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved) | |
| curl | [0 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=curl_curl-7_66_0_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=curl_curl-7_66_0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=New) | [7 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=curl_curl-7_66_0_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=curl_curl-7_66_0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved) | all ArrayBoundV2: 6 clear FPs and 1 [arguably true positive](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=curl_curl-7_66_0_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=curl_curl-7_66_0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&report-id=6806776&report-hash=1d1647635de913f59604a0d657e32dd6&report-filepath=curl%2Flib%2Fsmtp.c) |
| twin | [0 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=twin_v0.8.1_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=twin_v0.8.1_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=New) | [4 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=twin_v0.8.1_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=twin_v0.8.1_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved) | all ArrayBoundV2: one [confusing mess](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=twin_v0.8.1_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=twin_v0.8.1_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&report-id=6808281&report-hash=1542d877b70c5b48920affa437d5858b&report-filepath=twin%2Fclients%2Fterm.c) and 3 FPs like [this](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=twin_v0.8.1_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=twin_v0.8.1_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&report-id=6808523&report-hash=fcd2a55365ae55ea8a17441ba691416d&report-filepath=twin%2Fserver%2Ftty.c) |
| vim | [0 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=vim_v8.2.1920_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=vim_v8.2.1920_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=New) | [1 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=vim_v8.2.1920_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=vim_v8.2.1920_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved) | [clear ArrayBoundV2 FP](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=vim_v8.2.1920_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=vim_v8.2.1920_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&report-id=6812790&report-hash=bd3099e40489c823b3d9a9018cf76e9c&report-filepath=vim%2Fsrc%2Fvim9compile.c) |
| openssl | [0 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=openssl_openssl-3.0.0-alpha7_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=openssl_openssl-3.0.0-alpha7_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=New) | [2 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=openssl_openssl-3.0.0-alpha7_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=openssl_openssl-3.0.0-alpha7_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved) | both clear ArrayBoundV2 FPs |
| sqlite | [0 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=sqlite_version-3.33.0_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=sqlite_version-3.33.0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=New) | [1 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=sqlite_version-3.33.0_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=sqlite_version-3.33.0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved) | ArrayBoundV2, [probably FP](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=sqlite_version-3.33.0_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=sqlite_version-3.33.0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&report-id=6826935&report-hash=58a93e6b2dc084f6c0d3c8188920dc5a&report-filepath=sqlite%2Fsqlite3.c) |
| ffmpeg | [0 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=New) | [131 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved) | 128 ArrayBoundV2 reports -- I looked at 10 randomly picked ones ([1](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&report-id=6837328&report-hash=434d44c83dc07af2c1bd1f85865ee943&report-filepath=ffmpeg%2Flibavformat%2Frl2.c), [2](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&page=3&report-id=6839631&report-hash=0fccb34b631f4336237cbe3894b9fcd9&report-filepath=ffmpeg%2Flibavcodec%2Fmpegaudiodec_template.c), [3](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&page=3&report-id=6839026&report-hash=db56d1022e3e28a02be5fead6bb8a722&report-filepath=ffmpeg%2Flibavcodec%2Fh264_refs.c), [4](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&page=3&report-id=6839759&report-hash=03e2a88d3eedfd4f6cd58f6c3fc31fe9&report-filepath=ffmpeg%2Flibavcodec%2Fon2avc.c), [5](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&page=3&report-id=6839562&report-hash=e55ff8c6b0ebe2ee7f5baa104fef38fb&report-filepath=ffmpeg%2Flibavcodec%2Fmpegaudioenc_template.c), [6](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&page=4&report-id=6839873&report-hash=8aabce391d2d49dc9300ebc940e4191f&report-filepath=ffmpeg%2Flibavcodec%2Fopusdec.c), [7](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&page=4&report-id=6840093&report-hash=6a18751ce41d4328a6a9ea9049425662&report-filepath=ffmpeg%2Flibavcodec%2Fqdmc.c), [8](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&page=4&report-id=6839877&report-hash=7ef35db8dbd61ce37af3ccf1a968537a&report-filepath=ffmpeg%2Flibavcodec%2Fopusdec.c), [9](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&page=4&report-id=6840213&report-hash=551902ed44db11243ca758fb205e3a0a&report-filepath=ffmpeg%2Flibavcodec%2Fsbcenc.c), [10](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&page=6&report-id=6841676&report-hash=f723e9a199ed145c4351593408a61157&report-filepath=ffmpeg%2Ffftools%2Fffmpeg_opt.c) ) and those are all FPs, two UndefinedBinaryOperatorResult false positives [[1](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&page=4&report-id=6839900&report-hash=c8ca84aaf994178f26da2efc9ab6b03c&report-filepath=ffmpeg%2Flibavcodec%2Fopus_silk.c), [2](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&page=4&report-id=6839901&report-hash=219306cb7ad2d22e2570a55f9b3c9d8b&report-filepath=ffmpeg%2Flibavcodec%2Fopus_silk.c)], and a complex [NullDereference false positive](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&report-id=6836414&report-hash=e2f4049c7a98e03996a8e626f492b8cc&report-filepath=ffmpeg%2Flibavfilter%2Fvf_fps.c) |
| postgres | [0 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=postgres_REL_13_0_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=postgres_REL_13_0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=New) | [5 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=postgres_REL_13_0_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=postgres_REL_13_0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved) | three ArrayBoundV2 FPs (two simple [1](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=postgres_REL_13_0_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=postgres_REL_13_0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&report-id=6857081&report-hash=2806dbce8b1438e81f12599f32c1aae7&report-filepath=postgres%2Fsrc%2Fbackend%2Faccess%2Fcommon%2Freloptions.c) [2](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=postgres_REL_13_0_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=postgres_REL_13_0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&report-id=6857083&report-hash=2806dbce8b1438e81f12599f32c1aae7&report-filepath=postgres%2Fsrc%2Fbackend%2Faccess%2Fcommon%2Freloptions.c) and one [very creative](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=postgres_REL_13_0_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=postgres_REL_13_0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&report-id=6857933&report-hash=65d64088b0963a771337624620139fd6&report-filepath=postgres%2Fsrc%2Fbackend%2Foptimizer%2Futil%2Fappendinfo.c)), a [confusing and probably FP NullDereference report](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=postgres_REL_13_0_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=postgres_REL_13_0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&report-id=6859162&report-hash=707cfd55634fe644c644974a7becf667&report-filepath=postgres%2Fsrc%2Fbackend%2Futils%2Fmisc%2Fguc.c) and an [uninitialized.ArraySubscript](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=postgres_REL_13_0_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=postgres_REL_13_0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&report-id=6859166&report-hash=9cc1fd752255b9141bb1c5bf421706db&report-filepath=postgres%2Fsrc%2Fbackend%2Futils%2Fmisc%2Fguc-file.c) in code generated by yacc
| tinyxml2 | [0 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=tinyxml2_8.0.0_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=tinyxml2_8.0.0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=New) | [0 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=tinyxml2_8.0.0_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=tinyxml2_8.0.0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved) | |
| libwebm | [0 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=libwebm_libwebm-1.0.0.27_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=libwebm_libwebm-1.0.0.27_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=New) | [2 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=libwebm_libwebm-1.0.0.27_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=libwebm_libwebm-1.0.0.27_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved) | both straightforward ArrayBoundV2 FPs like [this one](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=libwebm_libwebm-1.0.0.27_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=libwebm_libwebm-1.0.0.27_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&report-id=6866714&report-hash=20f2e348894cf3bbfd80437f9aa20d57&report-filepath=libwebm%2Fmkvmuxer.cpp) |
| xerces | [0 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=xerces_v3.2.3_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=xerces_v3.2.3_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=New) | [1 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=xerces_v3.2.3_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=xerces_v3.2.3_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved) | [ArrayBoundV2 FP](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=xerces_v3.2.3_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=xerces_v3.2.3_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&report-id=6867038&report-hash=f3b75ee0b824e4265171be6c058c0711&report-filepath=xerces%2Ftests%2Fsrc%2FDOM%2FDOMTest%2FDTest.cpp) in a test file |
| bitcoin | [0 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=bitcoin_v0.20.1_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=bitcoin_v0.20.1_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=New) | [0 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=bitcoin_v0.20.1_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=bitcoin_v0.20.1_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved) | |
| protobuf | [0 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=protobuf_v3.13.0_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=protobuf_v3.13.0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=New) | [0 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=protobuf_v3.13.0_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=protobuf_v3.13.0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved) | |
| qtbase | [0 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=qtbase_v6.2.0_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=qtbase_v6.2.0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=New) | [6 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=qtbase_v6.2.0_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=qtbase_v6.2.0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved) | 5 ArrayBoundV2 results (three FPs [1](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=qtbase_v6.2.0_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=qtbase_v6.2.0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&report-id=6869649&report-hash=ea2123fcaefa8ef361b00aca40fe9f7b&report-filepath=qtbase%2Fsrc%2Fcorelib%2Ftext%2Fqstringconverter.cpp) [2](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=qtbase_v6.2.0_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=qtbase_v6.2.0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&report-id=6869935&report-hash=a6ba2a52fc01e82cfa28f713f3cfd36d&report-filepath=qtbase%2Fsrc%2Fgui%2Fvulkan%2Fqvulkanwindow.cpp) [3](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=qtbase_v6.2.0_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=qtbase_v6.2.0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&report-id=6870593&report-hash=58a93e6b2dc084f6c0d3c8188920dc5a&report-filepath=qtbase%2Fsrc%2F3rdparty%2Fsqlite%2Fsqlite3.c) and two results [1](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=qtbase_v6.2.0_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=qtbase_v6.2.0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&report-id=6870084&report-hash=939d8ec857fdfee4528a40d25d921cc1&report-filepath=qtbase%2Fsrc%2F3rdparty%2Flibjpeg%2Fsrc%2Fjccoefct.c) [2](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=qtbase_v6.2.0_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=qtbase_v6.2.0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&report-id=6870085&report-hash=db4852f3f470386292db2956fc900ef4&report-filepath=qtbase%2Fsrc%2F3rdparty%2Flibjpeg%2Fsrc%2Fjccoefct.c) that I don't understand); one [confusing NonNullParamChecker result](https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=qtbase_v6.2.0_edonnag_Ericsson-just-suppress-third-iteration_012d8f7&newcheck=qtbase_v6.2.0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&diff-type=Resolved&report-id=6870748&report-hash=0f41a5ea7ccb02d5a3e37d8ba34baf3e&report-filepath=qtbase%2Fsrc%2F3rdparty%2Fsqlite%2Fsqlite3.c) |
_Note that it's not surprising that most of the eliminated false positives are coming from the alpha checker `alpha.security.ArrayBoundV2`: as the analyzer engine was always making these unjustified assumptions, any checker that was affected by this was stuck in alpha -- and only checkers that are less dependent on values manipulated in loops were brought out of the alpha state._
**Comparison between intermediate and this commit:**
| Project | Only in this commit | Only in intermediate |
|---------|-------------|------------------|
| memcached | [0 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=memcached_1.6.8_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=memcached_1.6.8_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=New) | [0 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=memcached_1.6.8_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=memcached_1.6.8_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=Resolved)
| tmux | [3 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=tmux_2.6_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=tmux_2.6_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=New) | [0 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=tmux_2.6_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=tmux_2.6_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=Resolved)
| curl | [3 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=curl_curl-7_66_0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=curl_curl-7_66_0_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=New) | [1 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=curl_curl-7_66_0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=curl_curl-7_66_0_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=Resolved)
| twin | [1 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=twin_v0.8.1_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=twin_v0.8.1_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=New) | [1 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=twin_v0.8.1_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=twin_v0.8.1_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=Resolved)
| vim | [17 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=vim_v8.2.1920_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=vim_v8.2.1920_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=New) | [3 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=vim_v8.2.1920_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=vim_v8.2.1920_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=Resolved)
| openssl | [1 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=openssl_openssl-3.0.0-alpha7_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=openssl_openssl-3.0.0-alpha7_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=New) | [0 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=openssl_openssl-3.0.0-alpha7_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=openssl_openssl-3.0.0-alpha7_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=Resolved)
| sqlite | [6 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=sqlite_version-3.33.0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=sqlite_version-3.33.0_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=New) | [7 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=sqlite_version-3.33.0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=sqlite_version-3.33.0_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=Resolved)
| ffmpeg | [11 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=ffmpeg_n4.3.1_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=New) | [18 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=ffmpeg_n4.3.1_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=ffmpeg_n4.3.1_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=Resolved)
| postgres | [15 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=postgres_REL_13_0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=postgres_REL_13_0_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=New) | [5 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=postgres_REL_13_0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=postgres_REL_13_0_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=Resolved)
| tinyxml2 | [0 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=tinyxml2_8.0.0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=tinyxml2_8.0.0_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=New) | [0 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=tinyxml2_8.0.0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=tinyxml2_8.0.0_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=Resolved)
| libwebm | [3 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=libwebm_libwebm-1.0.0.27_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=libwebm_libwebm-1.0.0.27_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=New) | [2 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=libwebm_libwebm-1.0.0.27_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=libwebm_libwebm-1.0.0.27_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=Resolved)
| xerces | [6 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=xerces_v3.2.3_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=xerces_v3.2.3_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=New) | [6 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=xerces_v3.2.3_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=xerces_v3.2.3_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=Resolved)
| bitcoin | [1 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=bitcoin_v0.20.1_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=bitcoin_v0.20.1_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=New) | [1 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=bitcoin_v0.20.1_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=bitcoin_v0.20.1_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=Resolved)
| protobuf | [2 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=protobuf_v3.13.0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=protobuf_v3.13.0_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=New) | [0 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=protobuf_v3.13.0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=protobuf_v3.13.0_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=Resolved)
| qtbase | [21 new reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=qtbase_v6.2.0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=qtbase_v6.2.0_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=New) | [26 resolved reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=qtbase_v6.2.0_edonnag_Ericsson-just-suppress-third-iteration_a4c7718&newcheck=qtbase_v6.2.0_edonnag_Ericsson-dont-assume-third-iteration_a4c7718&diff-type=Resolved)
These differences are just random perturbations: as we don't waste time on analyzing the discarded execution paths, the "smart" graph traversal algorithms pick different branches and finds different results. As this perturbation is negligible compared to the total number of reports (among the largest projects: 1.8% on qtbase, 0.4% on postgres, 0.3% on ffmpeg), I think it's completely fine if this number of random results is replaced by the approximately same amount of other random results. (This perturbation would happen only once, when the user upgrades to a clang version that includes this commit.)
---
Full diff: https://github.com/llvm/llvm-project/pull/119388.diff
6 Files Affected:
- (modified) clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h (+8)
- (modified) clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h (+10-8)
- (modified) clang/lib/StaticAnalyzer/Core/CoreEngine.cpp (+26-1)
- (modified) clang/lib/StaticAnalyzer/Core/ExprEngine.cpp (+47-8)
- (modified) clang/test/Analysis/loop-unrolling.cpp (+20-15)
- (modified) clang/test/Analysis/misc-ps-region-store.m (+22-9)
``````````diff
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
index a6d05a3ac67b4e..80b79fd4e928f8 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
@@ -126,6 +126,14 @@ class CoreEngine {
ExplodedNode *generateCallExitBeginNode(ExplodedNode *N,
const ReturnStmt *RS);
+ /// Helper function called by `HandleBranch()`. If the currently handled
+ /// branch corresponds to a loop, this returns the number of already
+ /// completed iterations in that loop, otherwise the return value is
+ /// `std::nullopt`. Note that this counts _all_ earlier iterations, including
+ /// ones that were performed within an earlier iteration of an outer loop.
+ std::optional<unsigned> getCompletedIterationCount(const CFGBlock *B,
+ ExplodedNode *Pred) const;
+
public:
/// Construct a CoreEngine object to analyze the provided CFG.
CoreEngine(ExprEngine &exprengine,
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index 8c7493e27fcaa6..20c446e33ef9a5 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -321,14 +321,14 @@ class ExprEngine {
NodeBuilderWithSinks &nodeBuilder,
ExplodedNode *Pred);
- /// ProcessBranch - Called by CoreEngine. Used to generate successor
- /// nodes by processing the 'effects' of a branch condition.
- void processBranch(const Stmt *Condition,
- NodeBuilderContext& BuilderCtx,
- ExplodedNode *Pred,
- ExplodedNodeSet &Dst,
- const CFGBlock *DstT,
- const CFGBlock *DstF);
+ /// ProcessBranch - Called by CoreEngine. Used to generate successor nodes by
+ /// processing the 'effects' of a branch condition. If the branch condition
+ /// is a loop condition, IterationsCompletedInLoop is the number of completed
+ /// iterations (otherwise it's std::nullopt).
+ void processBranch(const Stmt *Condition, NodeBuilderContext &BuilderCtx,
+ ExplodedNode *Pred, ExplodedNodeSet &Dst,
+ const CFGBlock *DstT, const CFGBlock *DstF,
+ std::optional<unsigned> IterationsCompletedInLoop);
/// Called by CoreEngine.
/// Used to generate successor nodes for temporary destructors depending
@@ -588,6 +588,8 @@ class ExprEngine {
void evalEagerlyAssumeBifurcation(ExplodedNodeSet &Dst, ExplodedNodeSet &Src,
const Expr *Ex);
+ bool didEagerlyAssumeBifurcateAt(ProgramStateRef State, const Expr *Ex) const;
+
static std::pair<const ProgramPointTag *, const ProgramPointTag *>
getEagerlyAssumeBifurcationTags();
diff --git a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
index 67b7d30853d9de..775a22e18c6199 100644
--- a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
@@ -444,7 +444,8 @@ void CoreEngine::HandleBranch(const Stmt *Cond, const Stmt *Term,
NodeBuilderContext Ctx(*this, B, Pred);
ExplodedNodeSet Dst;
ExprEng.processBranch(Cond, Ctx, Pred, Dst, *(B->succ_begin()),
- *(B->succ_begin() + 1));
+ *(B->succ_begin() + 1),
+ getCompletedIterationCount(B, Pred));
// Enqueue the new frontier onto the worklist.
enqueue(Dst);
}
@@ -591,6 +592,30 @@ ExplodedNode *CoreEngine::generateCallExitBeginNode(ExplodedNode *N,
return isNew ? Node : nullptr;
}
+std::optional<unsigned>
+CoreEngine::getCompletedIterationCount(const CFGBlock *B,
+ ExplodedNode *Pred) const {
+ const LocationContext *LC = Pred->getLocationContext();
+ BlockCounter Counter = WList->getBlockCounter();
+ unsigned BlockCount =
+ Counter.getNumVisited(LC->getStackFrame(), B->getBlockID());
+
+ const Stmt *Term = B->getTerminatorStmt();
+ if (isa<ForStmt, WhileStmt, CXXForRangeStmt>(Term)) {
+ assert(BlockCount >= 1 &&
+ "Block count of currently analyzed block must be >= 1");
+ return BlockCount - 1;
+ }
+ if (isa<DoStmt>(Term)) {
+ // In a do-while loop one iteration happens before the first evaluation of
+ // the loop condition, so we don't subtract one.
+ return BlockCount;
+ }
+ // ObjCForCollectionStmt is skipped intentionally because the current
+ // application of the iteration counts is not relevant for it.
+ return std::nullopt;
+}
+
void CoreEngine::enqueue(ExplodedNodeSet &Set) {
for (const auto I : Set)
WList->enqueue(I);
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index b46cd9fe86fc11..db31206a13c36b 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2753,12 +2753,10 @@ assumeCondition(const Stmt *Condition, ExplodedNode *N) {
return State->assume(V);
}
-void ExprEngine::processBranch(const Stmt *Condition,
- NodeBuilderContext& BldCtx,
- ExplodedNode *Pred,
- ExplodedNodeSet &Dst,
- const CFGBlock *DstT,
- const CFGBlock *DstF) {
+void ExprEngine::processBranch(
+ const Stmt *Condition, NodeBuilderContext &BldCtx, ExplodedNode *Pred,
+ ExplodedNodeSet &Dst, const CFGBlock *DstT, const CFGBlock *DstF,
+ std::optional<unsigned> IterationsCompletedInLoop) {
assert((!Condition || !isa<CXXBindTemporaryExpr>(Condition)) &&
"CXXBindTemporaryExprs are handled by processBindTemporary.");
const LocationContext *LCtx = Pred->getLocationContext();
@@ -2801,8 +2799,35 @@ void ExprEngine::processBranch(const Stmt *Condition,
if (StTrue && StFalse)
assert(!isa<ObjCForCollectionStmt>(Condition));
- if (StTrue)
- Builder.generateNode(StTrue, true, PredN);
+ if (StTrue) {
+ // If we are processing a loop condition where two iterations have
+ // already been completed and the the false branch is also feasible, then
+ // don't assume a third iteration, because it is a redundant execution
+ // path (unlikely to be different from earlier loop exits) and can cause
+ // false positives if e.g. the loop iterates over a two-element structure
+ // with an opaque condition.
+ //
+ // The iteration count "2" is hardcoded because it's the natural limit:
+ // * the fact that the programmer wrote a loop (and not just an `if`)
+ // implies that they thought that the loop body may be executed twice;
+ // * however, there are situations where the programmer knows that there
+ // are at most two iterations, but writes a loop that appears to be
+ // generic, because there is no special syntax for "loop with at most
+ // two iterations". (This pattern is common in FFMPEG and appears in
+ // many other projects as well.)
+ bool CompletedTwoIterations = IterationsCompletedInLoop.value_or(0) >= 2;
+ bool FalseAlsoFeasible =
+ StFalse ||
+ didEagerlyAssumeBifurcateAt(PrevState, dyn_cast<Expr>(Condition));
+ bool SkipTrueBranch = CompletedTwoIterations && FalseAlsoFeasible;
+
+ // FIXME: This "don't assume third iteration" heuristic partially
+ // conflicts with the widen-loop analysis option (which is off by
+ // default). If we intend to support and stabilize the loop widening,
+ // we'll need to ensure that it 'plays nicely' with this logic.
+ if (!SkipTrueBranch || AMgr.options.ShouldWidenLoops)
+ Builder.generateNode(StTrue, true, PredN);
+ }
if (StFalse)
Builder.generateNode(StFalse, false, PredN);
@@ -3724,6 +3749,10 @@ ExprEngine::getEagerlyAssumeBifurcationTags() {
return std::make_pair(&TrueTag, &FalseTag);
}
+/// The last expression where EagerlyAssume produced two transitions (i.e. it
+/// activated and the true and false case were both feasible).
+REGISTER_TRAIT_WITH_PROGRAMSTATE(LastEagerlyAssumeBifurcationAt, const Expr *)
+
void ExprEngine::evalEagerlyAssumeBifurcation(ExplodedNodeSet &Dst,
ExplodedNodeSet &Src,
const Expr *Ex) {
@@ -3746,6 +3775,11 @@ void ExprEngine::evalEagerlyAssumeBifurcation(ExplodedNodeSet &Dst,
auto [StateTrue, StateFalse] = State->assume(*SEV);
+ if (StateTrue && StateFalse) {
+ StateTrue = StateTrue->set<LastEagerlyAssumeBifurcationAt>(Ex);
+ StateFalse = StateFalse->set<LastEagerlyAssumeBifurcationAt>(Ex);
+ }
+
// First assume that the condition is true.
if (StateTrue) {
SVal Val = svalBuilder.makeIntVal(1U, Ex->getType());
@@ -3763,6 +3797,11 @@ void ExprEngine::evalEagerlyAssumeBifurcation(ExplodedNodeSet &Dst,
}
}
+bool ExprEngine::didEagerlyAssumeBifurcateAt(ProgramStateRef State,
+ const Expr *Ex) const {
+ return Ex && State->get<LastEagerlyAssumeBifurcationAt>() == Ex;
+}
+
void ExprEngine::VisitGCCAsmStmt(const GCCAsmStmt *A, ExplodedNode *Pred,
ExplodedNodeSet &Dst) {
StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);
diff --git a/clang/test/Analysis/loop-unrolling.cpp b/clang/test/Analysis/loop-unrolling.cpp
index 66a828abfb5133..bf05a7739ce48c 100644
--- a/clang/test/Analysis/loop-unrolling.cpp
+++ b/clang/test/Analysis/loop-unrolling.cpp
@@ -63,7 +63,7 @@ int simple_no_unroll1() {
int a[9];
int k = 42;
for (int i = 0; i < 9; i++) {
- clang_analyzer_numTimesReached(); // expected-warning {{4}}
+ clang_analyzer_numTimesReached(); // expected-warning {{2}}
a[i] = 42;
foo(i);
}
@@ -76,7 +76,7 @@ int simple_no_unroll2() {
int k = 42;
int i;
for (i = 0; i < 9; i++) {
- clang_analyzer_numTimesReached(); // expected-warning {{4}}
+ clang_analyzer_numTimesReached(); // expected-warning {{2}}
a[i] = 42;
i += getNum();
}
@@ -309,9 +309,9 @@ int nested_inner_unrolled() {
int k = 42;
int j = 0;
for (int i = 0; i < getNum(); i++) {
- clang_analyzer_numTimesReached(); // expected-warning {{4}}
+ clang_analyzer_numTimesReached(); // expected-warning {{2}}
for (j = 0; j < 8; ++j) {
- clang_analyzer_numTimesReached(); // expected-warning {{32}}
+ clang_analyzer_numTimesReached(); // expected-warning {{16}}
a[j] = 22;
}
a[i] = 42;
@@ -346,11 +346,7 @@ int simple_known_bound_loop() {
int simple_unknown_bound_loop() {
for (int i = 2; i < getNum(); i++) {
-#ifdef DFS
- clang_analyzer_numTimesReached(); // expected-warning {{16}}
-#else
clang_analyzer_numTimesReached(); // expected-warning {{8}}
-#endif
}
return 0;
}
@@ -368,11 +364,7 @@ int nested_inlined_unroll1() {
int nested_inlined_no_unroll1() {
int k;
for (int i = 0; i < 9; i++) {
-#ifdef DFS
- clang_analyzer_numTimesReached(); // expected-warning {{18}}
-#else
- clang_analyzer_numTimesReached(); // expected-warning {{14}}
-#endif
+ clang_analyzer_numTimesReached(); // expected-warning {{10}}
k = simple_unknown_bound_loop(); // reevaluation without inlining, splits the state as well
}
int a = 22 / k; // no-warning
@@ -475,9 +467,13 @@ int num_steps_over_limit2() {
int num_steps_on_limit3() {
for (int i = 0; i < getNum(); i++) {
- clang_analyzer_numTimesReached(); // expected-warning {{4}}
+ clang_analyzer_numTimesReached(); // expected-warning {{2}}
for (int j = 0; j < 32; j++) {
- clang_analyzer_numTimesReached(); // expected-warning {{128}}
+ // Here the loop unrollig logic calculates with four potential iterations
+ // in the outer loop where it cannot determine the iteration count in
+ // advance; but after two loops the analyzer conservatively assumes that
+ // the (still opaque) loop condition is false.
+ clang_analyzer_numTimesReached(); // expected-warning {{64}}
}
}
return 0;
@@ -493,6 +489,15 @@ int num_steps_over_limit3() {
return 0;
}
+int num_steps_on_limit4() {
+ for (int i = 0; i < 4; i++) {
+ clang_analyzer_numTimesReached(); // expected-warning {{4}}
+ for (int j = 0; j < 32; j++) {
+ clang_analyzer_numTimesReached(); // expected-warning {{128}}
+ }
+ }
+ return 0;
+}
void pr34943() {
for (int i = 0; i < 6L; ++i) {
diff --git a/clang/test/Analysis/misc-ps-region-store.m b/clang/test/Analysis/misc-ps-region-store.m
index 668b5ffd7001a6..b0a04b836efb99 100644
--- a/clang/test/Analysis/misc-ps-region-store.m
+++ b/clang/test/Analysis/misc-ps-region-store.m
@@ -910,13 +910,13 @@ void pr6302(id x, Class y) {
//===----------------------------------------------------------------------===//
// Specially handle global variables that are declared constant. In the
-// example below, this forces the loop to take exactly 2 iterations.
+// example below, this forces the loop to take exactly 1 iterations.
//===----------------------------------------------------------------------===//
-const int pr6288_L_N = 2;
+const int pr6288_L_N = 1;
void pr6288_(void) {
- int x[2];
- int *px[2];
+ int x[1];
+ int *px[1];
int i;
for (i = 0; i < pr6288_L_N; i++)
px[i] = &x[i];
@@ -924,8 +924,8 @@ void pr6288_(void) {
}
void pr6288_pos(int z) {
- int x[2];
- int *px[2];
+ int x[1];
+ int *px[1];
int i;
for (i = 0; i < z; i++)
px[i] = &x[i]; // expected-warning{{Access out-of-bound array element (buffer overflow)}}
@@ -933,15 +933,28 @@ void pr6288_pos(int z) {
}
void pr6288_b(void) {
- const int L_N = 2;
- int x[2];
- int *px[2];
+ const int L_N = 1;
+ int x[1];
+ int *px[1];
int i;
for (i = 0; i < L_N; i++)
px[i] = &x[i];
*(px[0]) = 0; // no-warning
}
+void pr6288_no_third_iter(int z) {
+ int x[2];
+ int *px[2];
+ int i;
+ // If the loop condition is opaque, we assume that there may be two
+ // iterations (becasuse otherwise the loop could be replaced by an if); but
+ // we do not assume that there may be a third iteration. Therefore,
+ // unlike 'pr6288_pos', this testcase does not produce an out-of-bounds error.
+ for (i = 0; i < z; i++)
+ px[i] = &x[i];
+ *(px[0]) = 0; // expected-warning{{Dereference of undefined pointer value}}
+}
+
// A bug in RemoveDeadBindings was causing instance variable bindings to get
// prematurely pruned from the state.
@interface Rdar7817800 {
``````````
</details>
https://github.com/llvm/llvm-project/pull/119388
More information about the cfe-commits
mailing list