bisect: handle crashes as "bad" / added hot-patch for 1.88/1.89 compilation failures / added expected output as optional parameter (#4804)

Author: firewave

tools/bisect/README.md

@@ -1,41 +1,39 @@
 # Bisecting
 
-NOTE: THIS IS WORK IN PROGRESS
+`bisect.sh` is a script to bisect regressions in Cppcheck utilizing `git-bisect`.
 
-`bisect.sh` is a script to bisect issues.
+To learn more about bisecting please refer to https://git-scm.com/docs/git-bisect. 
 
 ## Command
 
 ```
-./bisect.sh <hash-good> <hash-bad> "<cppcheck-options>"
+./bisect.sh <hash-good> <hash-bad> "<cppcheck-options>" "[expected]"
 ```
 
-`hash-good` - the last known good commit hash - in case of daca it is the last tagged minor release (not patch release - i.e. 2.x)
-`hash-bad` - the known bad commit hash - in case of daca the one from the `head-info:` line
-`cppcheck-options` - the options for the Cppcheck invokation - in case of daca the ones from the `cppcheck-options:` line and the path to the folder/file to scan
+`hash-good` the latest known good commit hash or tag<br/>
+`hash-bad` the earliest known bad commit hash or tag<br/>
+`cppcheck-options` the options for the Cppcheck invokatio<br/>
+`expected` (optional) a string that is expected in the output. Will be used instead of the exitcode
 
 If possible use `main` as the function to test stuff with since it won't emit an `unusedFunction` warning.
 
-## Bisecting scan time regressions
-
-We use daca to track differences in scan time. An overview of regressions in scan time can be found at http://cppcheck1.osuosl.org:8000/time_gt.html.
-
-You need to download the archive as specified by the second line in the output and extract it.
-
-If the overall scan time regressed you need to specify the whole folder.
+## Bisecting result regressions
 
-If a timeout (potential hang) was introduced you can simply specify the file from `error: Internal error: Child process crashed with signal 15 [cppcheckError]`.
+Results regressions are being bisected based on the `--error-exitcode=` result.
 
+If nothing is found the result will be `0` and it is treated as a _good_ commit.<br/>
+If a finding occurs the result will be `1` which is treated as a _bad_ commit.<br/>
+If a crash occurs it is treated as a _bad_ commit.
 
-## Bisecting result regressions
+You can also bisect based on expected output via the `expected` parameter.
 
-Results regressions are being bisected based on the `--error-exitcode=` result.
-If nothing is found the result will be `0` and it is treated as a _good_ commit.
-If a finding occurs the result will be `1` which is treated as a _bad_ commit.
+If the given string is found in the output it is treated as a _good_ commit.<br/>
+If the given string is _not_ found in the output it is treated as a _bad_ commit.<br/>
+If a crash occurs it is treated as a _bad_ commit.
 
 ### False positive
 
-Provide a code sample which will trigger the false postive.
+Provide a code sample which will trigger a single(!) false postive only. Trying to bisect multiple issues at the same time will most likely result in an incorrect result (see below).
 
 ```cpp
 // cppcheck-suppress unusedFunction
@@ -45,8 +43,16 @@ static void f()
 }
 ```
 
+```
+./bisect.sh <hash-good> <hash-bad> "<cppcheck-options>"
+```
+
+After the bisecting check the output to make sure that only expected false positive and no additional finding was reported for the _bad_ commits. Any other finding will also cause the commit to be marked as _bad_ leading to an incorrect result.  
+
 ### False negative
 
+#### Via suppression
+
 Provide a code sample which will trigger a `unmatchedSuppression`.
 
 ```cpp
@@ -58,15 +64,53 @@ static void f()
 }
 ```
 
+```
+./bisect.sh <hash-good> <hash-bad> "<cppcheck-options>"
+```
+
+#### Via output
+
+```cpp
+static void f()
+{
+    int i;
+}
+```
+
+Provide the expected error ID (`unreadVariable`) as the `expected` parameter.
+
+```
+./bisect.sh <hash-good> <hash-bad> "<cppcheck-options>" "unreadVariable"
+```
+
+## Bisecting scan time regressions
+
+We use daca@home to track differences in scan time. An overview of regressions in scan time can be found at http://cppcheck1.osuosl.org:8000/time_gt.html.
+
+You need to download the archive as specified by the second line in the output and extract it.
+
+If the overall scan time regressed you need to specify the whole folder.
+
+If a timeout (potential hang) was introduced you can simply specify the file from `error: Internal error: Child process crashed with signal 15 [cppcheckError]`.
+
 ## Notes
 
-### Compilation issues:
+### Bisecting daca@home issues
+
+Use the following data as respective parameters:
+
+`hash-good` the latest tagged release - the second value from the `cppcheck:` line<br/>
+`hash-bad` the commit hash from the `head-info:` line<br/>
+`cppcheck-options` the `cppcheck-options:` line and the path to the folder/file to scan<br/>
+
+### Known compilation issues:
 
 - 2.5 and before can only be built with GCC<=10 because of missing includes caused by cleanups within the standard headers. You need to specify `CXX=g++-10`.
 - 1.88 and 1.89 cannot be compiled:
 ```
 make: python: No such file or directory
 ```
+RESOLVED: a hot-patch is applied before compilation.
 - 1.39 to 1.49 (possibly more versions - 1.54 and up work) cannot be compiled:
 ```
 lib/mathlib.cpp:70:42: error: invalid conversion from ‘char’ to ‘char**’ [-fpermissive]

tools/bisect/bisect.sh

@@ -7,11 +7,11 @@ set -x
 hash_good=$1
 hash_bad=$2
 options=$3
+expected=$4
 
 # TODO: verify "good" commit happened before "bad" commit
 
 hang=0
-expected=""
 
 script_dir="$(dirname "$(realpath "$0")")"
 

tools/bisect/bisect_common.py

@@ -20,6 +20,11 @@ def build_cppcheck(bisect_path):
     if os.path.exists(os.path.join(bisect_repo_dir, 'cppcheck')):
         os.remove(os.path.join(bisect_repo_dir, 'cppcheck'))
 
+    # for versions 1.88 and 1.89
+    print('patching Makefile')
+    subprocess.check_call(['sed', '-i', 's/shell python /shell python3 /g', os.path.join(bisect_repo_dir, 'Makefile')])
+
+    # for versions between 2.0 and 2.2
     print('patching cli/cppcheckexecutor.cpp')
     subprocess.check_call(['sed', '-i', 's/SIGSTKSZ/32768/g', os.path.join(bisect_repo_dir, 'cli', 'cppcheckexecutor.cpp')])
 

tools/bisect/bisect_res.py

@@ -17,7 +17,7 @@ def run(cppcheck_path, options):
     # signals are report as negative exitcode (e.g. SIGSEGV -> -11)
     if p.returncode < 0:
         print('crash')
-        return None, None, None
+        return p.returncode, stderr, stdout
     print('done')
     return p.returncode, stderr, stdout
 
@@ -54,6 +54,8 @@ def run(cppcheck_path, options):
 # if no ec is set we encountered an unexpected error
 if run_ec is None:
     sys.exit(EC_SKIP)  # error occured
+elif run_ec < 0:
+    sys.exit(EC_BAD) # crash occured
 
 # check output for expected string
 if expected is not None: