Merge pull request #2 from Ximaz/v1.2.0

V1.2.0

Dockerfile

@@ -1,11 +1,11 @@
 FROM debian:stable
 
-RUN apt update -y          \
-    && apt upgrade -y      \
+RUN apt update -y                              \
+    && apt upgrade -y                          \
     && apt install -y build-essential valgrind
 
 WORKDIR /root/
 
-COPY valgrind.sh /root/valgrind.sh
+COPY valgrind.bash /root/valgrind.bash
 
-ENTRYPOINT [ "/root/valgrind.sh" ]
+ENTRYPOINT [ "/root/valgrind.bash" ]

README.md

@@ -1,44 +1,95 @@
 # Valgrind Checker Action
 
-A GitHub action allowing you to check for memory leaks on your binaries, libraries and unit tests.
+A GitHub Action for checking your memory management using [Valgrind](https://valgrind.org).
 
-# Usage
+This Action will check for :
+- un`free`'d memory,
+- invalid `read` or `write` operations,
+- un`close`'d file descriptors (files and sockets),
+- invalid usage of `free`, `delete`, `delete []` or `realloc`,
+- uninitialised `syscall` params,
+- overlaps between sources and destinations for `memcpy`, `memmove`, etc...,
+- `fishy` arguments (possibly negative values) for `unsigned` expected,
+- memory allocation with a size of `0`,
+- invalid alignment values.
+
+If you want to have more details about the errors this Action supports, you can
+check the [`4.2. Explanation of error messages from Memcheck`](https://valgrind.org/docs/manual/mc-manual.html) section.
 
-Here is how you may use the action :
+# Usage
 
 ```yml
-name: Valgrind Tester
-
-on:
-    - push
-
-jobs:
-    check-memory-leaks:
-        runs-on: ubuntu-latest
-        steps:
-            -   name: "Checkout"
-                uses: actions/checkout@v4.1.4
-
-            -   name: "Compile program"
-                run: gcc -g src/*.c -o program
-
-            -   name: "Valgrind Checks"
-                uses: Ximaz/valgrind-action@v1.1.1
-                with:
-                    binary_path: "./program"
-                    binary_args: "--arg1 value1 --arg2 value2"
-```
+-   uses: Ximaz/valgrind-action@v1.2.0
+    with:
+        # Either absolute or reative path to the binary you want to check the
+        # memory on.
+        binary_path: ""
 
-# Customization
+        # A string containing all the arguments to pass the the binary when it
+        # gets checked by Valgrind.
+        #
+        # Default: ""
+        binary_args: ""
 
-`binary_args`: The args to pass to the binary when checked (string format)
+        # The value of the `LD_LIBRARY_PATH` environment variable so that the
+        # binary can be executed with your custom libraries, if any.
+        #
+        # Default: ""
+        ld_library_path: ""
 
-`ld_library_path`: Custom library path (for dymanic locally-compiled libraries)
+        # Redzone size used by Valgrind. It represents the blocks that Valgrind
+        # will check before, and after any allocated pointer so that it will
+        # look if you're trying to operate on those bytes, which you are not
+        # supposed to.
+        #
+        # Default: 16
+        redzone_size: 16
 
-`track_file_descriptors`: Whether or not to track file descriptors (default: `true`)
+        # Whether or not to track unclosed file descriptors.
+        # Default: true
+        track_file_descriptors: true
 
-`treat_error_as_warning`: The workflow won't exit as error, and error annotations will be replaced by warnings (default: `false`)
+        # Whether or not to treat error as warning. This implies that, if set
+        # to true, then the action will not exit with an error status, even if
+        # Valgrind found issues during the binary execution. Also, this implies
+        # that in your workflow summary, you will see warning annotations
+        # instead of error ones.
+        #
+        # Default: false
+        treat_error_as_warning: false
 
-`valgrind_suppressions`: String containing Valgrind suppressions (will be put inside a suppressions file)
+        # Valgrind suppressions. If specified, the content will be written into
+        # a local file which will be passed to Valgrind. It represents a set of
+        # specific checks to avoid. For instance, you can avoid checks for a
+        # specific function, inside a specific program or library, etc...
+        # Foe more detail, you can check the `2.5. Suppressing errors` section
+        # on the Valgrind's documentation :
+        # https://valgrind.org/docs/manual/manual-core.html
+        #
+        # For each found error by Valgrind, a generated suppression will be
+        # logged inside your workflow's logs. That way, you will be able to see
+        # if any dependency, unrelated to your implementation, is faulty for
+        # not free'ing it's own memory or file descriptors.
+        #
+        # Generally, you want to avoid using this because you may tend to use
+        # it badly in order to suppress errors about your own implemnetations,
+        # so be careful.
+        #
+        # Default: ""
+        #
+        # The example below showcases how to avoid checking for the `free`
+        # function, inside the `main` function.
+        valgrind_suppressions: |
+            {
+                DontCheckFreeInMain
+                Memcheck:Free
+                fun:free
+                fun:main
+            }
 
-`verbose`: Asking Valgrind to be verbose (default: `false`)
+        # Whether or not Valgrind should be verbose. It may be useful in case
+        # you want to debug things.
+        #
+        # Default: false
+        verbose: false
+```

action.yml

@@ -13,6 +13,10 @@ inputs:
   ld_library_path:
     description: "Custom library path (for dymanic locally-compiled libraries)"
     required: false
+  redzone_size:
+    description: "Valgrind's padding blocks protection (redzone) in bytes"
+    required: false
+    default: 16
   track_file_descriptors:
     description: "Whether or not to track file descriptors (true or false)"
     required: false

docker-compose.yml

@@ -1,5 +1,3 @@
-version: "3.9"
-
 services:
   valgrind-action:
     build:

valgrind.sh → valgrind.bash

@@ -1,5 +1,5 @@
 #!/usr/bin/env bash
-# set -xeu
+set -xeu
 
 prepare_valgrind_flags() {
     local SUPPRESSIONS_FILE="valgrind.supp"
@@ -13,6 +13,7 @@ prepare_valgrind_flags() {
     echo "--errors-for-leak-kinds=all"
     echo "--expensive-definedness-checks=yes"
     echo "--gen-suppressions=all"
+    echo "--redzone-size=${INPUT_REDZONE_SIZE}"
     [[ "${INPUT_TRACK_FILE_DESCRIPTORS}" == "true" ]] && echo "--track-fds=yes"
     [[ "${INPUT_VERBOSE}" == "true" ]] && echo "--verbose"
     if [[ "${INPUT_VALGRIND_SUPPRESSIONS}" != "" ]]; then
@@ -21,18 +22,10 @@ prepare_valgrind_flags() {
     fi
 }
 
-match_pattern() {
-    if [[ $(echo "${1}" | grep "${2}") != "" ]]; then
-        echo "1"
-    else
-        echo "0"
-    fi
-}
-
 skip_criterion_pipe_leaks() {
     # Reason of this skip: https://github.com/Snaipe/Criterion/issues/533
-    if [[ $(echo "${1}" | grep '^==.*==    by 0x.*: stdpipe_options (in /usr/local/lib/libcriterion.so.3.2.0)') == "" &&
-          $(echo "${1}" | grep '^==.*== Open file descriptor .*: /dev/shm/bxf_arena_.* (deleted)') == "" ]]; then
+    if [[ $(echo "${1}" | grep "^==.*==    by 0x.*: stdpipe_options (in /usr/local/lib/libcriterion.so.3.2.0)") == "" &&
+          $(echo "${1}" | grep "^==.*== Open file descriptor .*: /dev/shm/bxf_arena_.* (deleted)") == "" ]]; then
         echo "1"
     else
         echo "0"
@@ -41,6 +34,19 @@ skip_criterion_pipe_leaks() {
 
 parse_valgrind_reports() {
     local VALGRIND_REPORTS="${1}"
+    declare -a VALGRIND_RULES=(
+        "^==.*== .* bytes in .* blocks are definitely lost in loss record .* of .*$"
+        "^==.*== .* bytes in .* blocks are still reachable in loss record .* of .*$"
+        "^==.*== Invalid .* of size .*$"
+        "^==.*== Open file descriptor .*: .*$"
+        "^==.*== Invalid free() / delete / delete\[\] / realloc()$"
+        "^==.*== Mismatched free() / delete / delete \[\].*$"
+        "^==.*== Syscall param .* points to uninitialised byte(s).*$"
+        "^==.*== Source and destination overlap in .*$"
+        "^==.*== Argument .* of function .* has a fishy (possibly negative) value: .*$"
+        "^==.*== .*alloc() with size 0$"
+        "^==.*== Invalid alignment value: .* (should be power of 2)$"
+    )
     local report_id=1
     local status=0
     local error=""
@@ -60,35 +66,15 @@ parse_valgrind_reports() {
                 error="${error}%0A${line}"
             fi
         fi
-        if [[ $(echo "${line}" | grep '^==.*== .* bytes in .* blocks are definitely lost in loss record .* of .*$') ]]; then
-            error="${line}"
-        elif [[ $(echo "${line}" | grep '^==.*== .* bytes in .* blocks are still reachable in loss record .* of .*$') ]]; then
-            error="${line}"
-        elif [[ $(echo "${line}" | grep '^==.*== Invalid .* of size .*$') ]]; then
-            error="${line}"
-        elif [[ $(echo "${line}" | grep '^==.*== Open file descriptor .*: .*$') ]]; then
-            error="${line}"
-        elif [[ $(echo "${line}" | grep '^==.*== Invalid free().*$') ]]; then
-            error="${line}"
-        elif [[ $(echo "${line}" | grep '^==.*== Mismatched free() / delete / delete \[\].*$') ]]; then
-            error="${line}"
-        elif [[ $(echo "${line}" | grep '^==.*== Syscall param .* points to uninitialised byte(s).*$') ]]; then
-            error="${line}"
-        elif [[ $(echo "${line}" | grep '^==.*== Source and destination overlap in .*$') ]]; then
-            error="${line}"
-        elif [[ $(echo "${line}" | grep '^==.*== Argument .* of function .* has a fishy (possibly negative) value: .*$') ]]; then
-            error="${line}"
-        elif [[ $(echo "${line}" | grep '^==.*== .*alloc() with size 0$') ]]; then
-            error="${line}"
-        elif [[ $(echo "${line}" | grep '^==.*== Invalid alignment value: .* (should be power of 2)$') ]]; then
-            error="${line}"
-        fi
+        for rule in "${VALGRIND_RULES[@]}"; do
+            if [[ $(echo "${line}" | grep "${rule}") ]]; then
+                error="${line}"
+                break
+            fi
+        done
     done < "${VALGRIND_REPORTS}"
     rm -f "${VALGRIND_REPORTS}"
-    if [[ "${kind}" == "warning" ]]; then
-        exit 0
-    fi
-    exit "${status}"
+    [[ "${kind}" == "warning" ]] && exit 0 || exit "${status}"
 }
 
 main() {