Run static analysis on OCaml C stubs in the CI (#6338)

Similar to the static analyzer introduced in xha, but for OCaml C stubs.

This uses my https://ocaml.org/p/dune-compiledb/latest to product a
`compile_commands.json` for the C stubs out of the `dune` rules (which
can also be useful if you want to use `clangd` and get some editor
integration about compiler warnings). This requires installing enough of
the build dependencies to be able to run `dune` successfully (perhaps in
the future that restriction can be removed). Caching is used though, and
we only need to install the build deps when `dune` files change,
otherwise we can reuse a cached `compile_commands.json`.

Static analyzers, like CodeChecker support reading
`compile_commands.json` and invoking static analyzers with the
appropriate flags to preprocess, and analyze the C source code. We use
`clang`, `clang-tidy` and `cppcheck` as the default analyzers, although
more analyzers could be added in the future (CodeChecker supports
converting `gcc -fanalyzer` output for example. GCC also supports
emiting SARIF format directly, but github cannot parse it, because it
doesn't implement the full SARIF spec).

At the end we convert the results back to the standard SARIF format that
Github also supports for its code scanning results, which will make it
automatically add comments on PRs that introduce *new* bugs, without
necessarily gating on them.

I fixed some of the most obvious warnings, and suppressed some minor
ones that we cannot fix (where the warning is caused by a Xen or OCaml
header). More warnings can be skipped by adding to `.codechecker.json`
if needed.

So far it seems to have found a file descriptor leak in `unixpwd.c` on
an error path, but I haven't gone through all the reports in detail yet.

Author: edwintorok

.codechecker.json

@@ -0,0 +1,6 @@
+{
+  "analyze": [
+    "--disable=misc-header-include-cycle",
+    "--disable=clang-diagnostic-unused-parameter"
+  ]
+}

.github/workflows/codechecker.yml

@@ -0,0 +1,96 @@
+name: Run CodeChecker static analyzer on XAPI's C stubs
+permissions: {}
+
+on:
+  push:
+  pull_request:
+    branches:
+      - master
+      - 'feature/**'
+      - '*-lcm'
+
+concurrency: # On new push, cancel old workflows from the same PR, branch or tag:
+  group: ${{ github.workflow }}-${{github.event_name}}-${{ github.event.pull_request.number || github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  staticanalyzer:
+    name: Static analyzer for OCaml C stubs
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      security-events: write
+    env:
+      XAPI_VERSION: "v0.0.0-${{ github.sha }}"
+
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v4
+
+      - name: Restore cache for compile_commands.json
+        uses: actions/cache/restore@v4
+        id: cache-cmds
+        with:
+          path: compile_commands.json
+          key: compile_commands.json-v1-${{ hashFiles('**/dune') }}
+
+      - name: Setup XenAPI environment
+        if: steps.cache-cmds.outputs.cache-hit != 'true'
+        uses: ./.github/workflows/setup-xapi-environment
+        with:
+          xapi_version: ${{ env.XAPI_VERSION }}
+
+      - name: Install dune-compiledb to generate compile_commands.json
+        if: steps.cache-cmds.outputs.cache-hit != 'true'
+        run: |
+          opam pin add -y ezjsonm https://github.com/mirage/ezjsonm/releases/download/v1.3.0/ezjsonm-1.3.0.tbz
+          opam pin add -y dune-compiledb https://github.com/edwintorok/dune-compiledb/releases/download/0.6.0/dune-compiledb-0.6.0.tbz
+
+      - name: Trim dune cache
+        if: steps.cache-cmds.outputs.cache-hit != 'true'
+        run: opam exec -- dune cache trim --size=2GiB         
+
+      - name: Generate compile_commands.json
+        if: steps.cache-cmds.outputs.cache-hit != 'true'
+        run: opam exec -- make compile_commands.json
+
+      - name: Save cache for cmds.json
+        uses: actions/cache/save@v4
+        with:
+          path: compile_commands.json
+          key: ${{ steps.cache-cmds.outputs.cache-primary-key }}
+
+      - name: Upload compile commands json
+        uses: actions/upload-artifact@v4
+        with:
+          path: ${{ github.workspace }}/compile_commands.json
+
+      - uses: whisperity/codechecker-analysis-action@v1
+        id: codechecker
+        with:
+          ctu: true
+          logfile: ${{ github.workspace }}/compile_commands.json
+          analyze-output: "codechecker_results"
+
+      - name: Upload CodeChecker report
+        uses: actions/upload-artifact@v4
+        with:
+          name: codechecker_results
+          path: "${{ steps.codechecker.outputs.result-html-dir }}"
+
+      # cppcheck even for other analyzers apparently, this is
+      # codechecker's output
+      - name: convert to SARIF
+        shell: bash
+        run: report-converter "codechecker_results" --type cppcheck --output codechecker.sarif --export sarif
+
+      - name: Upload CodeChecker SARIF report
+        uses: actions/upload-artifact@v4
+        with:
+          name: codechecker_sarif
+          path: codechecker.sarif
+
+      - name: Upload SARIF report
+        uses: github/codeql-action/upload-sarif@v3
+        with:
+         sarif_file: codechecker.sarif

Makefile

@@ -6,7 +6,7 @@ JOBS = $(shell getconf _NPROCESSORS_ONLN)
 PROFILE=release
 OPTMANDIR ?= $(OPTDIR)/man/man1/
 
-.PHONY: build clean test doc python format install uninstall coverage
+.PHONY: build clean test doc python format install uninstall coverage analyze
 
 # if we have XAPI_VERSION set then set it in dune-project so we use that version number instead of the one obtained from git
 # this is typically used when we're not building from a git repo
@@ -196,6 +196,17 @@ uninstall:
 	dune uninstall $(DUNE_IU_PACKAGES3)
 	dune uninstall $(DUNE_IU_PACKAGES4)
 
+# An approximation, we actually depend on all dune files recursively
+# Also fixup the directory paths to remove _build
+# (we must refer to paths that exist in the repository for static analysis results)
+compile_commands.json: Makefile dune
+	mkdir -p _build/
+	dune rules | dune-compiledb -o _build/
+	sed -e 's/"directory".*/"directory": ".",/' <_build/$@ >$@
+
+analyze: compile_commands.json Makefile .codechecker.json
+	CodeChecker check --config .codechecker.json -l compile_commands.json
+
 compile_flags.txt: Makefile
 	(ocamlc -config-var ocamlc_cflags;\
 	ocamlc -config-var ocamlc_cppflags;\

ocaml/auth/xa_auth.h

@@ -11,8 +11,8 @@
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU Lesser General Public License for more details.
  */
-#ifndef _XA_AUTH_H_
-#define _XA_AUTH_H_
+#ifndef XA_AUTH_H_
+#define XA_AUTH_H_
 
 #define XA_SUCCESS 0
 #define XA_ERR_EXTERNAL 1

ocaml/libs/log/syslog_stubs.c

@@ -20,12 +20,12 @@
 #include <caml/custom.h>
 #include <caml/signals.h>
 
-static int __syslog_level_table[] = {
+static int syslog_level_table[] = {
 	LOG_EMERG, LOG_ALERT, LOG_CRIT, LOG_ERR, LOG_WARNING,
 	LOG_NOTICE, LOG_INFO, LOG_DEBUG
 };
 
-static int __syslog_facility_table[] = {
+static int syslog_facility_table[] = {
 	LOG_AUTH, LOG_AUTHPRIV, LOG_CRON, LOG_DAEMON, LOG_FTP, LOG_KERN,
 	LOG_LOCAL0, LOG_LOCAL1, LOG_LOCAL2, LOG_LOCAL3,
 	LOG_LOCAL4, LOG_LOCAL5, LOG_LOCAL6, LOG_LOCAL7,
@@ -54,8 +54,8 @@ value stub_syslog(value facility, value level, value msg)
 {
 	CAMLparam3(facility, level, msg);
 	char *c_msg = strdup(String_val(msg));
-	int c_facility = __syslog_facility_table[Int_val(facility)]
-	               | __syslog_level_table[Int_val(level)];
+	int c_facility = syslog_facility_table[Int_val(facility)]
+	               | syslog_level_table[Int_val(level)];
 
 	caml_enter_blocking_section();
 	syslog(c_facility, "%s", c_msg);

ocaml/libs/vhd/vhd_format_lwt/blkgetsize64_stubs.c

@@ -69,7 +69,8 @@ CAMLprim value stub_blkgetsize64(value filename){
     }
 #endif
     close(fd);
-  }
+  } else
+    size_in_bytes = -1;
   caml_leave_blocking_section();
   free((void*)filename_c);
 

unixpwd/c/unixpwd.c

@@ -144,7 +144,7 @@ unixpwd_setspw(const char *user, char *password)
                    *sp;
     char            buf[BUFLEN];
     int             tmp;
-    FILE           *tmp_file;
+    FILE           *tmp_file = NULL;
     char            tmp_name[PATH_MAX];
     struct stat     statbuf;
     int             rc;
@@ -164,6 +164,8 @@ unixpwd_setspw(const char *user, char *password)
         return rc;
     }
     if (lckpwdf() != 0) {
+        if (tmp_file)
+            fclose(tmp_file);
         close(tmp);
         unlink(tmp_name);
         return ENOLCK;