[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker
Donát Nagy via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 26 05:05:34 PDT 2023
donat.nagy added a comment.
I tested this commit on several open source projects (in the default mode Pedantic=false), the following table summarizes the results:
| vim_v8.2.1920_baseline_vs_vim_v8.2.1920_with_bitwise_shift | New reports <https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=vim_v8.2.1920_baseline&newcheck=vim_v8.2.1920_with_bitwise_shift&is-unique=on&diff-mode=New> | Lost reports <https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=vim_v8.2.1920_baseline&newcheck=vim_v8.2.1920_with_bitwise_shift&is-unique=on&diff-mode=Resolved> | no differences |
| openssl_openssl-3.0.0-alpha7_baseline_vs_openssl_openssl-3.0.0-alpha7_with_bitwise_shift | New reports <https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=openssl_openssl-3.0.0-alpha7_baseline&newcheck=openssl_openssl-3.0.0-alpha7_with_bitwise_shift&is-unique=on&diff-mode=New> | Lost reports <https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=openssl_openssl-3.0.0-alpha7_baseline&newcheck=openssl_openssl-3.0.0-alpha7_with_bitwise_shift&is-unique=on&diff-mode=Resolved> | +2 FPs from same invalid loop assumption [1] |
| sqlite_version-3.33.0_baseline_vs_sqlite_version-3.33.0_with_bitwise_shift | New reports <https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=sqlite_version-3.33.0_baseline&newcheck=sqlite_version-3.33.0_with_bitwise_shift&is-unique=on&diff-mode=New> | Lost reports <https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=sqlite_version-3.33.0_baseline&newcheck=sqlite_version-3.33.0_with_bitwise_shift&is-unique=on&diff-mode=Resolved> | one FP lost for unclear reasons |
| ffmpeg_n4.3.1_baseline_vs_ffmpeg_n4.3.1_with_bitwise_shift | New reports <https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=ffmpeg_n4.3.1_baseline&newcheck=ffmpeg_n4.3.1_with_bitwise_shift&is-unique=on&diff-mode=New> | Lost reports <https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=ffmpeg_n4.3.1_baseline&newcheck=ffmpeg_n4.3.1_with_bitwise_shift&is-unique=on&diff-mode=Resolved> | 21 new reports, 14 lost reports, mostly FPs [2] |
| postgres_REL_13_0_baseline_vs_postgres_REL_13_0_with_bitwise_shift | New reports <https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=postgres_REL_13_0_baseline&newcheck=postgres_REL_13_0_with_bitwise_shift&is-unique=on&diff-mode=New> | Lost reports <https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=postgres_REL_13_0_baseline&newcheck=postgres_REL_13_0_with_bitwise_shift&is-unique=on&diff-mode=Resolved> | two FPs are "converted" to new checker, one unreadable report lost |
| libwebm_libwebm-1.0.0.27_baseline_vs_libwebm_libwebm-1.0.0.27_with_bitwise_shift | New reports <https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=libwebm_libwebm-1.0.0.27_baseline&newcheck=libwebm_libwebm-1.0.0.27_with_bitwise_shift&is-unique=on&diff-mode=New> | Lost reports <https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=libwebm_libwebm-1.0.0.27_baseline&newcheck=libwebm_libwebm-1.0.0.27_with_bitwise_shift&is-unique=on&diff-mode=Resolved> | no differences |
| xerces_v3.2.3_baseline_vs_xerces_v3.2.3_with_bitwise_shift | New reports <https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=xerces_v3.2.3_baseline&newcheck=xerces_v3.2.3_with_bitwise_shift&is-unique=on&diff-mode=New> | Lost reports <https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=xerces_v3.2.3_baseline&newcheck=xerces_v3.2.3_with_bitwise_shift&is-unique=on&diff-mode=Resolved> | no differences |
| bitcoin_v0.20.1_baseline_vs_bitcoin_v0.20.1_with_bitwise_shift | New reports <https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=bitcoin_v0.20.1_baseline&newcheck=bitcoin_v0.20.1_with_bitwise_shift&is-unique=on&diff-mode=New> | Lost reports <https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=bitcoin_v0.20.1_baseline&newcheck=bitcoin_v0.20.1_with_bitwise_shift&is-unique=on&diff-mode=Resolved> | 6 new FPs caused by incorrect config of our CI [3], one FP lost |
| protobuf_v3.13.0_baseline_vs_protobuf_v3.13.0_with_bitwise_shift | New reports <https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=protobuf_v3.13.0_baseline&newcheck=protobuf_v3.13.0_with_bitwise_shift&is-unique=on&diff-mode=New> | Lost reports <https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=protobuf_v3.13.0_baseline&newcheck=protobuf_v3.13.0_with_bitwise_shift&is-unique=on&diff-mode=Resolved> | three reports "converted" to new checker |
| qtbase_v6.2.0_baseline_vs_qtbase_v6.2.0_with_bitwise_shift | New reports <https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=qtbase_v6.2.0_baseline&newcheck=qtbase_v6.2.0_with_bitwise_shift&is-unique=on&diff-mode=New> | Lost reports <https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=qtbase_v6.2.0_baseline&newcheck=qtbase_v6.2.0_with_bitwise_shift&is-unique=on&diff-mode=Resolved> | 3 new reports, 3 lost reports [4] |
| contour_v0.2.0.173_baseline_vs_contour_v0.2.0.173_with_bitwise_shift | New reports <https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=contour_v0.2.0.173_baseline&newcheck=contour_v0.2.0.173_with_bitwise_shift&is-unique=on&diff-mode=New> | Lost reports <https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=contour_v0.2.0.173_baseline&newcheck=contour_v0.2.0.173_with_bitwise_shift&is-unique=on&diff-mode=Resolved> | no differences |
| acid_2022-08-02-codechecker-test_baseline_vs_acid_2022-08-02-codechecker-test_with_bitwise_shift | New reports <https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=acid_2022-08-02-codechecker-test_baseline&newcheck=acid_2022-08-02-codechecker-test_with_bitwise_shift&is-unique=on&diff-mode=New> | Lost reports <https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=acid_2022-08-02-codechecker-test_baseline&newcheck=acid_2022-08-02-codechecker-test_with_bitwise_shift&is-unique=on&diff-mode=Resolved> | no differences |
|
More detailed summary:
[1] When the engine encounters a loop, it arbitrarily assumes that (there is a valid path where) the loop body will be executed 0 times. These unfounded assumptions produce false positives in many stable checkers.
[2] It seems that most of these warnings report the same underlying issues in different ways, but I didn't do a through analysis yet.
[3] The build/analysis procedure of bitcoin was misconfigured in our CI and the VERIFY_CHECK() assertions were preprocessed incorrectly.
[4] Logical relationships between these reports:
- two new false positives (1 <https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=qtbase_v6.2.0_baseline&newcheck=qtbase_v6.2.0_with_bitwise_shift&is-unique=on&diff-type=New&report-id=1973364&report-hash=2cb403dafe0fd0af00f0d50f90a37137&report-filepath=%2ajdhuff.c> and 2 <https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=qtbase_v6.2.0_baseline&newcheck=qtbase_v6.2.0_with_bitwise_shift&is-unique=on&diff-type=New&report-id=1973365&report-hash=3fce48fa039d1d6f797008e0e4337b50&report-filepath=%2ajdhuff.c>) are caused by the same underlying engine misunderstanding which had caused a different false positve <https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=qtbase_v6.2.0_baseline&newcheck=qtbase_v6.2.0_with_bitwise_shift&is-unique=on&diff-type=Resolved&report-id=1971465&report-hash=6940a82030821b9638a3aab239a3e2c0&report-filepath=%2ajdhuff.c> in the baseline;
- one old report <https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=qtbase_v6.2.0_baseline&newcheck=qtbase_v6.2.0_with_bitwise_shift&is-unique=on&diff-type=Resolved&report-id=1971732&report-hash=7b5ed42f3de9290c469b6bf1176b0159&report-filepath=%2aqdistancefield.cpp> is eliminated because core.BitwiseShift is non-pedantic by default;
- one old false positive <https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=qtbase_v6.2.0_baseline&newcheck=qtbase_v6.2.0_with_bitwise_shift&is-unique=on&diff-type=Resolved&report-id=1972205&report-hash=fc05d62894e821baeeb4a9d1e8c203e9&report-filepath=%2asqlite3.c> is lost for unclear reasons;
- one new false positive <https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=qtbase_v6.2.0_baseline&newcheck=qtbase_v6.2.0_with_bitwise_shift&is-unique=on&diff-type=New&report-id=1974427&report-hash=3a1b00bbed9399a099e0dd39fcf9d905&report-filepath=%2aqgifhandler.cpp> appeared, it's caused by an incorrect assumption that an 'if' and a 'while' are logically independent (in fact, the loop body will never run in cases where the 'if' condition is false).
================
Comment at: clang/test/Analysis/symbol-simplification-nonloc-loc.cpp:46-47
// Bitwise operators:
- // expected-warning at +2 {{The result of the left shift is undefined due to shifting by '1028', which is greater or equal to the width of type 'int' [core.UndefinedBinaryOperatorResult]}}
- // expected-warning at +2 {{The result of the right shift is undefined due to shifting by '1028', which is greater or equal to the width of type 'int' [core.UndefinedBinaryOperatorResult]}}
nonloc_OP_loc(p, BINOP(<<)); // no-crash
----------------
In this test file these warnings were irrelevant, so I picked a different arbitrary constant to eliminate them.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D156312/new/
https://reviews.llvm.org/D156312
More information about the cfe-commits
mailing list