ci: Run analysis using CodeChecker

.codechecker-ignore

@@ -0,0 +1,4 @@
+-*/external/*
+-*/vendor/*
+-/usr/*
+-*/atomic_base.h

.gitignore

@@ -15,5 +15,8 @@ leak-build
 .venv
 __pycache__
 
+# Devtools
+CodeChecker
+
 # Coverage
 coverage

CONTRIBUTING.md

@@ -84,6 +84,8 @@ Some of its behavior is controlled by env-variables:
     tool for builds.
   - `kcov`: Uses [`kcov`](https://github.com/SimonKagstrom/kcov) to collect
     code-coverage statistics.
+  - `valgrind`: Uses [`valgrind`](https://valgrind.org/) to check for memory
+    issues such as leaks.
   - `gcc`: Use the `-fanalyzer` flag of `gcc > 10`.
     This is currently not stable enough to use, as it leads to false positives
     and internal compiler errors.
@@ -93,6 +95,15 @@ Some of its behavior is controlled by env-variables:
   `ARCH`. The test runner assumes an already running simulator matching the
   `ARCH`, and will run the tests on that.
 
+**Analyzer Requirements**:
+
+Some tools, such as `kcov` and `valgrind` have their own distribution packages.
+Clang-based tools may require an up-to-date clang, and a separate `clang-tools`
+packages.
+`CodeChecker` has its own
+[install instructions](https://github.com/Ericsson/codechecker#install-guide)
+with a list of needed dependencies.
+
 **Running examples manually**:
 
     $ cmake -B build -D CMAKE_RUNTIME_OUTPUT_DIRECTORY=$(pwd)/build

azure-pipelines.yml

@@ -46,9 +46,9 @@ stages:
               CXX: clang++-10
               ERROR_ON_WARNINGS: 1
               RUN_ANALYZER: kcov
-            Linux (scan-build):
+            Linux (code-checker):
               vmImage: "ubuntu-20.04"
-              RUN_ANALYZER: scan-build
+              RUN_ANALYZER: code-checker
             macOS (xcode llvm):
               vmImage: "macOs-latest"
               ERROR_ON_WARNINGS: 1
@@ -89,6 +89,14 @@ stages:
           - script: sudo apt update && sudo apt install libcurl4 libcurl4-openssl-dev kcov g++-10 clang-10 clang-tools llvm
             condition: and(eq(variables['Agent.OS'], 'Linux'), not(variables['TEST_X86']))
             displayName: Installing Linux Dependencies
+          - script: |
+              sudo apt install clang-tidy clang-tools curl doxygen gcc-multilib python3-dev python3-virtualenv
+              git clone https://github.com/Ericsson/CodeChecker.git --depth 1
+              cd CodeChecker
+              BUILD_LOGGER_64_BIT_ONLY=YES make standalone_package
+              echo "##vso[task.prependpath]$PWD/build/CodeChecker/bin"
+            condition: contains(variables['RUN_ANALYZER'], 'code-checker')
+            displayName: Installing CodeChecker
           - script: sudo apt update && sudo apt install gcc-multilib g++-multilib
             condition: and(eq(variables['Agent.OS'], 'Linux'), variables['TEST_X86'])
             displayName: Installing Linux 32-bit Dependencies

examples/example.c

@@ -38,9 +38,7 @@ static void *invalid_mem = (void *)1;
 static void
 trigger_crash()
 {
-    // Triggers a segfault by writing to `NULL`. We actually do a `1 - 1` to
-    // defeat static analyzers which would warn for the trivial case.
-    memset((char *)invalid_mem - 1, 1, 100);
+    memset((char *)invalid_mem, 1, 100);
 }
 
 int

src/modulefinder/sentry_modulefinder_linux.c

@@ -427,6 +427,7 @@ load_modules(sentry_value_t modules)
             break;
         }
         if (sentry__stringbuilder_append_buf(&sb, buf, n)) {
+            sentry__stringbuilder_cleanup(&sb);
             close(fd);
             return;
         }

src/path/sentry_path_unix.c

@@ -156,23 +156,19 @@ sentry_path_t *
 sentry__path_from_str(const char *s)
 {
     char *path = sentry__string_clone(s);
-    sentry_path_t *rv = NULL;
     if (!path) {
         return NULL;
     }
-    rv = sentry__path_from_str_owned(path);
-    if (rv) {
-        return rv;
-    }
-    sentry_free(path);
-    return NULL;
+    // NOTE: function will free `path` on error
+    return sentry__path_from_str_owned(path);
 }
 
 sentry_path_t *
 sentry__path_from_str_owned(char *s)
 {
     sentry_path_t *rv = SENTRY_MAKE(sentry_path_t);
     if (!rv) {
+        sentry_free(s);
         return NULL;
     }
     rv->path = s;

src/sentry_core.c

@@ -67,6 +67,12 @@ sentry_init(sentry_options_t *options)
     }
     sentry__logger_set_global(logger);
 
+    // we need to ensure the dir exists, otherwise `path_absolute` will fail.
+    if (sentry__path_create_dir_all(options->database_path)) {
+        sentry_options_free(options);
+        return 1;
+    }
+
     sentry_path_t *database_path = options->database_path;
     options->database_path = sentry__path_absolute(database_path);
     if (options->database_path) {
@@ -78,11 +84,6 @@ sentry_init(sentry_options_t *options)
     SENTRY_DEBUGF("using database path \"%" SENTRY_PATH_PRI "\"",
         options->database_path->path);
 
-    if (sentry__path_create_dir_all(options->database_path)) {
-        sentry_options_free(options);
-        return 1;
-    }
-
     // try to create and lock our run folder as early as possibly, since it is
     // fallible. since it does locking, it will not interfere with run folder
     // enumeration.

src/sentry_envelope.c

@@ -86,8 +86,11 @@ static sentry_envelope_item_t *
 envelope_add_from_owned_buffer(
     sentry_envelope_t *envelope, char *buf, size_t buf_len, const char *type)
 {
+    if (!buf) {
+        return NULL;
+    }
     sentry_envelope_item_t *item = envelope_add_item(envelope);
-    if (!item || !buf) {
+    if (!item) {
         sentry_free(buf);
         return NULL;
     }
@@ -157,6 +160,9 @@ sentry__envelope_from_path(const sentry_path_t *path)
     size_t buf_len;
     char *buf = sentry__path_read_to_buffer(path, &buf_len);
     if (!buf) {
+        SENTRY_WARNF("failed to read raw envelope from \"%" SENTRY_PATH_PRI
+                     "\"",
+            path->path);
         return NULL;
     }
 
@@ -230,12 +236,10 @@ sentry__envelope_add_session(
         return NULL;
     }
     sentry__session_to_json(session, jw);
-    size_t payload_len;
+    size_t payload_len = 0;
     char *payload = sentry__jsonwriter_into_string(jw, &payload_len);
-    if (!payload) {
-        return NULL;
-    }
 
+    // NOTE: function will check for `payload` internally and free it on error
     return envelope_add_from_owned_buffer(
         envelope, payload, payload_len, "session");
 }
@@ -244,6 +248,8 @@ sentry_envelope_item_t *
 sentry__envelope_add_from_buffer(sentry_envelope_t *envelope, const char *buf,
     size_t buf_len, const char *type)
 {
+    // NOTE: function will check for the clone of `buf` internally and free it
+    // on error
     return envelope_add_from_owned_buffer(
         envelope, sentry__string_clonen(buf, buf_len), buf_len, type);
 }
@@ -255,15 +261,13 @@ sentry__envelope_add_from_path(
     size_t buf_len;
     char *buf = sentry__path_read_to_buffer(path, &buf_len);
     if (!buf) {
+        SENTRY_WARNF("failed to read envelope item from \"%" SENTRY_PATH_PRI
+                     "\"",
+            path->path);
         return NULL;
     }
-    sentry_envelope_item_t *rv
-        = envelope_add_from_owned_buffer(envelope, buf, buf_len, type);
-    if (!rv) {
-        sentry_free(buf);
-        return NULL;
-    }
-    return rv;
+    // NOTE: function will free `buf` on error
+    return envelope_add_from_owned_buffer(envelope, buf, buf_len, type);
 }
 
 static void

src/sentry_session.c

@@ -58,6 +58,7 @@ sentry__session_new(void)
 
     sentry_session_t *rv = SENTRY_MAKE(sentry_session_t);
     if (!rv) {
+        sentry_free(release);
         return NULL;
     }
 
@@ -159,6 +160,7 @@ sentry__session_from_json(const char *buf, size_t buflen)
 
     sentry_session_t *rv = SENTRY_MAKE(sentry_session_t);
     if (!rv) {
+        sentry_free(release);
         return NULL;
     }
     rv->session_id

src/sentry_string.c

@@ -17,7 +17,7 @@ static int
 append(sentry_stringbuilder_t *sb, const char *s, size_t len)
 {
     size_t needed = sb->len + len + 1;
-    if (needed > sb->allocated) {
+    if (!sb->buf || needed > sb->allocated) {
         size_t new_alloc_size = sb->allocated;
         if (new_alloc_size == 0) {
             new_alloc_size = INITIAL_BUFFER_SIZE;
@@ -41,6 +41,7 @@ append(sentry_stringbuilder_t *sb, const char *s, size_t len)
 
     // make sure we're always zero terminated
     sb->buf[sb->len] = '\0';
+
     return 0;
 }
 

src/sentry_utils.c

@@ -52,7 +52,7 @@ sentry__url_parse(sentry_url_t *url_out, const char *url)
     }
     url_out->scheme = sentry__string_clonen(ptr, tmp - ptr);
 
-    if (!is_scheme_valid(url_out->scheme)) {
+    if (!url_out->scheme || !is_scheme_valid(url_out->scheme)) {
         goto error;
     }
     sentry__string_ascii_lower(url_out->scheme);

src/sentry_value.c

@@ -308,7 +308,11 @@ sentry_value_new_list(void)
     list_t *l = SENTRY_MAKE(list_t);
     if (l) {
         memset(l, 0, sizeof(list_t));
-        return new_thing_value(l, THING_TYPE_LIST);
+        sentry_value_t rv = new_thing_value(l, THING_TYPE_LIST);
+        if (sentry_value_is_null(rv)) {
+            sentry_free(l);
+        }
+        return rv;
     } else {
         return sentry_value_new_null();
     }
@@ -328,7 +332,12 @@ sentry__value_new_list_with_size(size_t size)
                 return sentry_value_new_null();
             }
         }
-        return new_thing_value(l, THING_TYPE_LIST);
+        sentry_value_t rv = new_thing_value(l, THING_TYPE_LIST);
+        if (sentry_value_is_null(rv)) {
+            sentry_free(l->items);
+            sentry_free(l);
+        }
+        return rv;
     } else {
         return sentry_value_new_null();
     }
@@ -340,7 +349,11 @@ sentry_value_new_object(void)
     obj_t *o = SENTRY_MAKE(obj_t);
     if (o) {
         memset(o, 0, sizeof(obj_t));
-        return new_thing_value(o, THING_TYPE_OBJECT);
+        sentry_value_t rv = new_thing_value(o, THING_TYPE_OBJECT);
+        if (sentry_value_is_null(rv)) {
+            sentry_free(o);
+        }
+        return rv;
     } else {
         return sentry_value_new_null();
     }
@@ -349,18 +362,23 @@ sentry_value_new_object(void)
 sentry_value_t
 sentry__value_new_object_with_size(size_t size)
 {
-    obj_t *l = SENTRY_MAKE(obj_t);
-    if (l) {
-        memset(l, 0, sizeof(obj_t));
-        l->allocated = size;
+    obj_t *o = SENTRY_MAKE(obj_t);
+    if (o) {
+        memset(o, 0, sizeof(obj_t));
+        o->allocated = size;
         if (size) {
-            l->pairs = sentry_malloc(sizeof(obj_pair_t) * size);
-            if (!l->pairs) {
-                sentry_free(l);
+            o->pairs = sentry_malloc(sizeof(obj_pair_t) * size);
+            if (!o->pairs) {
+                sentry_free(o);
                 return sentry_value_new_null();
             }
         }
-        return new_thing_value(l, THING_TYPE_OBJECT);
+        sentry_value_t rv = new_thing_value(o, THING_TYPE_OBJECT);
+        if (sentry_value_is_null(rv)) {
+            sentry_free(o->pairs);
+            sentry_free(o);
+        }
+        return rv;
     } else {
         return sentry_value_new_null();
     }
@@ -404,7 +422,7 @@ sentry_value_set_by_key(sentry_value_t value, const char *k, sentry_value_t v)
 {
     thing_t *thing = value_as_unfrozen_thing(value);
     if (!thing || thing_get_type(thing) != THING_TYPE_OBJECT) {
-        return 1;
+        goto fail;
     }
     obj_t *o = thing->payload;
     for (size_t i = 0; i < o->len; i++) {
@@ -418,17 +436,21 @@ sentry_value_set_by_key(sentry_value_t value, const char *k, sentry_value_t v)
 
     if (!reserve((void **)&o->pairs, sizeof(o->pairs[0]), &o->allocated,
             o->len + 1)) {
-        return 1;
+        goto fail;
     }
 
     obj_pair_t pair;
     pair.k = sentry__string_clone(k);
     if (!pair.k) {
-        return 1;
+        goto fail;
     }
     pair.v = v;
     o->pairs[o->len++] = pair;
     return 0;
+
+fail:
+    sentry_value_decref(v);
+    return 1;
 }
 
 int
@@ -525,12 +547,8 @@ sentry__value_clone(sentry_value_t value)
         const obj_t *obj = thing->payload;
         sentry_value_t rv = sentry__value_new_object_with_size(obj->len);
         for (size_t i = 0; i < obj->len; i++) {
-            char *key = sentry__string_clone(obj->pairs[i].k);
-            if (!key) {
-                continue;
-            }
             sentry_value_incref(obj->pairs[i].v);
-            sentry_value_set_by_key(rv, key, obj->pairs[i].v);
+            sentry_value_set_by_key(rv, obj->pairs[i].k, obj->pairs[i].v);
         }
         return rv;
     }
@@ -846,7 +864,12 @@ sentry_value_to_msgpack(sentry_value_t value, size_t *size_out)
 sentry_value_t
 sentry__value_new_string_owned(char *s)
 {
-    return new_thing_value(s, THING_TYPE_STRING | THING_TYPE_FROZEN);
+    sentry_value_t rv
+        = new_thing_value(s, THING_TYPE_STRING | THING_TYPE_FROZEN);
+    if (sentry_value_is_null(rv)) {
+        sentry_free(s);
+    }
+    return rv;
 }
 
 #ifdef SENTRY_PLATFORM_WINDOWS

tests/__init__.py

@@ -76,7 +76,14 @@ def run(cwd, exe, args, env=dict(os.environ), **kwargs):
             coverage_dir,
             *cmd,
         ]
-    return subprocess.run([*cmd, *args], cwd=cwd, env=env, **kwargs)
+    try:
+        return subprocess.run([*cmd, *args], cwd=cwd, env=env, **kwargs)
+    except subprocess.CalledProcessError:
+        raise pytest.fail.Exception(
+            "running command failed: {cmd} {args}".format(
+                cmd=" ".join(cmd), args=" ".join(args)
+            )
+        ) from None
 
 
 def check_output(*args, **kwargs):