AztecProtocol
Incorporate static analysis tools on barretenberg
See what is reasonable to run, investigate false positive rates
- https://cppcheck.sourceforge.io/
- https://clang.llvm.org/docs/ClangStaticAnalyzer.html
- https://pvs-studio.com/en/pvs-studio/
- try to run them
- share report with team (me is fine) see where to go from there
I ran the ClangStaticAnalyzer but it only finds bugs in this mdb.c which I think is just some dependency. Is it because most of our compilation is cached and I should disable caching?
@ludamad Do you know?
I didn't attach the report because it's only mdb.c stuff.
% scan-build ninja ~/aztec-packages/barretenberg/cpp/build-debug-no-avm pass-verifier-com...+ jonathan-box
scan-build: Using '/usr/lib/llvm-18/bin/clang' for static analysis
[31/647] Performing download step (git clone) for 'lmdb_repo'
Cloning into 'lmdb_repo'...
HEAD is now at ddd0a77 LMDB: tweak mdb_load.1 manpage
[36/647] Performing download step (git clone) for 'msgpack-c'
Cloning into 'msgpack-c'...
HEAD is now at 5ee9a1c8 fix: don't shim try/catch/throw from inside msgpack
[58/647] Performing build step for 'lmdb_repo'
make: Entering directory '/mnt/user-data/jonathan/aztec-packages/barretenberg/cpp/build-debug-no-avm/_deps/lmdb/src/lmdb_repo/libraries/liblmdb'
/usr/share/clang/scan-build-18/bin/../libexec/ccc-analyzer -pthread -O2 -g -W -Wall -Wno-unused-parameter -Wbad-function-cast -Wuninitialized -fPIC -c mdb.c
/usr/share/clang/scan-build-18/bin/../libexec/ccc-analyzer -pthread -O2 -g -W -Wall -Wno-unused-parameter -Wbad-function-cast -Wuninitialized -fPIC -c midl.c
mdb.c:2750:14: warning: Array access (via field 'mt_spill_pgs') results in a null pointer dereference [core.NullDereference]
2750 | if (x == txn->mt_spill_pgs[0])
| ^~~~~~~~~~~~~~~~~~~~
mdb.c:3147:31: warning: Use of memory after it is freed [unix.Malloc]
3147 | txn->mt_u.dirty_list[0].mid = 0;
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
mdb.c:3537:19: warning: Use of memory after it is freed [unix.Malloc]
3537 | for (; mp; mp = NEXT_LOOSE_PAGE(mp)) {
| ^~~~~~~~~~~~~~~~~~~
mdb.c:1085:29: note: expanded from macro 'NEXT_LOOSE_PAGE'
1085 | #define NEXT_LOOSE_PAGE(p) (*(MDB_page **)((p) + 2))
| ^~~~~~~~~~~~~~~~~~~~~~~~~
mdb.c:5016:17: warning: Assigned value is garbage or undefined [core.uninitialized.Assign]
5016 | env->me_psize = meta.mm_psize;
| ^ ~~~~~~~~~~~~~
mdb.c:7011:16: warning: Access to field 'mv_size' results in a dereference of a null pointer (loaded from variable 'key') [core.NullDereference]
7011 | key->mv_size = mc->mc_db->md_pad;
| ~~~ ^
mdb.c:7097:16: warning: Access to field 'mv_size' results in a dereference of a null pointer (loaded from variable 'key') [core.NullDereference]
7097 | key->mv_size = mc->mc_db->md_pad;
| ~~~ ^
mdb.c:7267:6: warning: Access to field 'mn_flags' results in a dereference of a null pointer (loaded from variable 'leaf') [core.NullDereference]
7267 | if (F_ISSET(leaf->mn_flags, F_DUPDATA)) {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
mdb.c:723:26: note: expanded from macro 'F_ISSET'
723 | #define F_ISSET(w, f) (((w) & (f)) == (f))
| ^~~
mdb.c:8624:27: warning: Dereference of null pointer [core.NullDereference]
8624 | mx->mx_cursor.mc_xcursor = NULL;
| ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
mdb.c:9209:2: warning: Value stored to 'pdst' is never read [deadcode.DeadStores]
9209 | pdst = cdst->mc_pg[cdst->mc_top];
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~
mdb.c:10012:38: warning: Array access (via field 'mp_ptrs') results in a null pointer dereference [core.NullDereference]
10012 | node = (MDB_node *)((char *)mp + copy->mp_ptrs[i] + PAGEBASE);
| ^~~~~~~~~~~~~~~~
mdb.c:10042:11: warning: Access to field 'mp2_lower' results in a dereference of a null pointer (loaded from variable 'copy') [core.NullDereference]
10042 | nkeys = NUMKEYS(copy);
| ^~~~~~~~~~~~~
mdb.c:1055:23: note: expanded from macro 'NUMKEYS'
1055 | #define NUMKEYS(p) ((MP_LOWER(p) - (PAGEHDRSZ-PAGEBASE)) >> 1)
| ^~~~~~~~~~~
mdb.c:1041:21: note: expanded from macro 'MP_LOWER'
1041 | #define MP_LOWER(p) (((MDB_page2 *)(void *)(p))->mp2_lower)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
mdb.c:11130:19: warning: Assigned value is garbage or undefined [core.uninitialized.Assign]
11130 | mc->mc_pg[i] = mx.mc_pg[i];
| ^ ~~~~~~~~~~~
12 warnings generated.
ar rs liblmdb.a mdb.o midl.o
ar: creating liblmdb.a
make: Leaving directory '/mnt/user-data/jonathan/aztec-packages/barretenberg/cpp/build-debug-no-avm/_deps/lmdb/src/lmdb_repo/libraries/liblmdb'
[647/647] Linking CXX executable bin/sumcheck_tests
scan-build: Analysis run complete.
scan-build: 12 bugs found.
scan-build: Run 'scan-view /tmp/scan-build-2025-06-18-134040-1190543-1' to examine bug reports.
I also tried scan-build ninja clean beforehand to make sure it's a clean build before scan-build ninja.
I have got some partial cppchecker results as well in https://docs.google.com/document/d/1Qs6ZPYuySVcdu-x0TPI1AF4HeiSlOlVSGi3KqtuuJw0/edit?usp=sharing
I was able to run ClangStaticAnalyzer with CodeChecker instead. Here are the reports https://drive.google.com/drive/folders/1Xpr52o6yC7CvEqDu5HB0tBPG4VGf1XJ_?usp=sharing.
I'll go through them and get a feeling of the false-positive rate.
The impression I got overall is that pvs-studio et al could add value to our dev cycle, but mostly it flagged code smells that, if they amounted to a bug, we'd surely know. Low priority to integrate, nice to have