Add a Valgrind build on debug Python 3.9 (#2746)

* Adding a valgrind build on debug Python 3.9

Co-authored-by: Boris Staletic <boris.staletic@gmail.com>

* Add Valgrind suppression files

- Introduce suppression file, populate it with a first suppression taken from CPython, and fix one leak in the tests
- Suppress leak in NumPy
- More clean tests!
- Tests with names a-e passing (except for test_buffer)
- Suppress multiprocessing errors
- Merge multiprocessing suppressions into other suppression files
- Numpy seems to be spelled with a big P
- Append single entry from valgrind-misc.supp to valgrind-python.supp, and make clear valgrind-python.supp is only CPython

Co-authored-by: Boris Staletic <boris.staletic@gmail.com>

* Enable test_virtual_functions with a workaround

* Add a memcheck cmake target

- Add a memcheck cmake target
- Reformat cmake
- Appease the formatting overlords - they are angry
- Format CMake valgrind target decently

* Update CI config to new action versions

* fix: separate memcheck from pytest

* ci: cleanup

* Merge Valgrind and other deadsnakes builds

Co-authored-by: Boris Staletic <boris.staletic@gmail.com>
Co-authored-by: Henry Schreiner <henryschreineriii@gmail.com>

.github/workflows/ci.yml

@@ -169,30 +169,61 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        python:
-        - version: 3.9
-          debug: true
-        - version: 3.10-dev
-          debug: false
+        include:
+        - python-version: 3.9
+          python-debug: true
+          valgrind: true
+        - python-version: 3.10-dev
+          python-debug: false
 
-    name: "🐍 ${{ matrix.python.version }}${{ matrix.python.debug && ' (debug)' || '' }} • deadsnakes • x64"
+    name: "🐍 ${{ matrix.python-version }}${{ matrix.python-debug && '-dbg' || '' }} (deadsnakes)${{ matrix.valgrind && ' • Valgrind' || '' }} • x64"
     runs-on: ubuntu-latest
 
     steps:
     - uses: actions/checkout@v2
 
-    - name: Setup Python ${{ matrix.python.version }} (deadsnakes)
+    - name: Setup Python ${{ matrix.python-version }} (deadsnakes)
       uses: deadsnakes/action@v2.1.1
       with:
-        python-version: ${{ matrix.python.version }}
-        debug: ${{ matrix.python.debug }}
+        python-version: ${{ matrix.python-version }}
+        debug: ${{ matrix.python-debug }}
+
+    - name: Update CMake
+      uses: jwlawson/actions-setup-cmake@v1.6
+
+    - name: Valgrind cache
+      if: matrix.valgrind
+      uses: actions/cache@v2
+      id: cache-valgrind
+      with:
+        path: valgrind
+        key: 3.16.1 # Valgrind version
+
+    - name: Compile Valgrind
+      if: matrix.valgrind && steps.cache-valgrind.outputs.cache-hit != 'true'
+      run: |
+        VALGRIND_VERSION=3.16.1
+        curl https://sourceware.org/pub/valgrind/valgrind-$VALGRIND_VERSION.tar.bz2 -o - | tar xj
+        mv valgrind-$VALGRIND_VERSION valgrind
+        cd valgrind
+        ./configure
+        make -j 2 > /dev/null
+
+    - name: Install Valgrind
+      if: matrix.valgrind
+      working-directory: valgrind
+      run: |
+        sudo make install
+        sudo apt-get update
+        sudo apt-get install libc6-dbg  # Needed by Valgrind
 
     - name: Prepare env
       run: python -m pip install -r tests/requirements.txt --prefer-binary
 
     - name: Configure
       run: >
         cmake -S . -B build
+        -DCMAKE_BUILD_TYPE=Debug
         -DPYBIND11_WERROR=ON
         -DDOWNLOAD_CATCH=ON
         -DDOWNLOAD_EIGEN=ON
@@ -207,6 +238,10 @@ jobs:
     - name: C++ tests
       run: cmake --build build --target cpptest
 
+    - name: Run Valgrind on Python tests
+      if: matrix.valgrind
+      run: cmake --build build --target memcheck
+
 
   # Testing on clang using the excellent silkeh clang docker images
   clang:

tests/CMakeLists.txt

@@ -370,12 +370,17 @@ endif()
 string(REPLACE "test_" "${CMAKE_CURRENT_SOURCE_DIR}/test_" PYBIND11_ABS_PYTEST_FILES
                "${PYBIND11_PYTEST_FILES}")
 
+set(PYBIND11_TEST_PREFIX_COMMAND
+    ""
+    CACHE STRING "Put this before pytest, use for checkers and such")
+
 # A single command to compile and run the tests
 add_custom_target(
   pytest
-  COMMAND ${PYTHON_EXECUTABLE} -m pytest ${PYBIND11_ABS_PYTEST_FILES}
+  COMMAND ${PYBIND11_TEST_PREFIX_COMMAND} ${PYTHON_EXECUTABLE} -m pytest
+          ${PYBIND11_ABS_PYTEST_FILES}
   DEPENDS ${test_targets}
-  WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
+  WORKING_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}"
   USES_TERMINAL)
 
 if(PYBIND11_TEST_OVERRIDE)
@@ -386,6 +391,27 @@ if(PYBIND11_TEST_OVERRIDE)
             "Note: not all tests run: -DPYBIND11_TEST_OVERRIDE is in effect")
 endif()
 
+# cmake-format: off
+add_custom_target(
+  memcheck
+  COMMAND
+    PYTHONMALLOC=malloc
+    valgrind
+    --leak-check=full
+    --show-leak-kinds=definite,indirect
+    --errors-for-leak-kinds=definite,indirect
+    --error-exitcode=1
+    --read-var-info=yes
+    --track-origins=yes
+    --suppressions="${CMAKE_CURRENT_SOURCE_DIR}/valgrind-python.supp"
+    --suppressions="${CMAKE_CURRENT_SOURCE_DIR}/valgrind-numpy-scipy.supp"
+    --gen-suppressions=all
+    ${PYTHON_EXECUTABLE} -m pytest ${PYBIND11_ABS_PYTEST_FILES}
+  DEPENDS ${test_targets}
+  WORKING_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}"
+  USES_TERMINAL)
+# cmake-format: on
+
 # Add a check target to run all the tests, starting with pytest (we add dependencies to this below)
 add_custom_target(check DEPENDS pytest)
 

tests/requirements.txt

@@ -6,4 +6,4 @@ pytest==4.6.9; python_version<"3.5"
 pytest==6.1.2; python_version=="3.5"
 pytest==6.2.1; python_version>="3.6"
 scipy==1.2.3; (platform_python_implementation!="PyPy" or sys_platform=="linux") and python_version<"3.6"
-scipy==1.5.2; (platform_python_implementation!="PyPy" or sys_platform=="linux") and python_version>="3.6" and python_version<"3.9"
+scipy==1.5.4; (platform_python_implementation!="PyPy" or sys_platform=="linux") and python_version>="3.6" and python_version<"3.10"

tests/test_virtual_functions.py

@@ -251,8 +251,7 @@ def dispatch(self):
                 == 'Tried to call pure virtual function "Base::dispatch"'
             )
 
-            p = PyClass1()
-            return m.dispatch_issue_go(p)
+            return m.dispatch_issue_go(PyClass1())
 
     b = PyClass2()
     assert m.dispatch_issue_go(b) == "Yay.."

tests/valgrind-numpy-scipy.supp

@@ -0,0 +1,118 @@
+# Valgrind suppression file for NumPy & SciPy errors and leaks in pybind11 tests
+
+{
+   Leaks when importing NumPy
+   Memcheck:Leak
+   fun:malloc
+   fun:_PyMem_RawMalloc
+   fun:PyObject_Malloc
+   fun:_PyObject_GC_Alloc
+   fun:_PyObject_GC_Malloc
+   fun:_PyObject_GC_NewVar
+   fun:tuple_alloc
+   fun:PyTuple_Pack
+   ...
+   fun:__pyx_pymod_exec_*
+}
+
+{
+   Leaks when importing NumPy (bis)
+   Memcheck:Leak
+   fun:malloc
+   fun:_PyMem_RawMalloc
+   fun:PyObject_Malloc
+   fun:_PyObject_New
+   fun:PyCode_NewWithPosOnlyArgs
+   fun:PyCode_New
+   ...
+   fun:__pyx_pymod_exec_*
+}
+
+{
+   Leaks when importing NumPy (tris)
+   Memcheck:Leak
+   fun:malloc
+   fun:_PyMem_RawMalloc
+   fun:PyObject_Malloc
+   fun:_PyObject_GC_Alloc
+   fun:_PyObject_GC_Malloc
+   fun:_PyObject_GC_NewVar
+   fun:tuple_alloc
+   fun:_PyTuple_FromArray
+   fun:_PyObject_MakeTpCall
+   fun:_PyObject_VectorcallTstate
+   fun:PyObject_Vectorcall
+   fun:call_function
+   fun:_PyEval_EvalFrameDefault
+   fun:_PyEval_EvalFrame
+   fun:function_code_fastcall
+   fun:_PyFunction_Vectorcall
+}
+
+{
+   Leaks when importing NumPy (quater)
+   Memcheck:Leak
+   fun:malloc
+   fun:_PyMem_RawMalloc
+   fun:PyObject_Malloc
+   fun:_PyObject_GC_Alloc
+   fun:_PyObject_GC_Malloc
+   fun:_PyObject_GC_NewVar
+   fun:tuple_alloc
+   fun:PyTuple_New
+   fun:r_object
+   fun:r_object
+   fun:r_object
+   fun:r_object
+}
+
+{
+   Leaks when importing NumPy (quinquies)
+   Memcheck:Leak
+   fun:malloc
+   fun:_PyMem_RawMalloc
+   fun:PyObject_Malloc
+   fun:_PyObject_GC_Alloc
+   fun:_PyObject_GC_Malloc
+   fun:_PyObject_GC_NewVar
+   fun:tuple_alloc
+   fun:PyTuple_New
+   fun:dictiter_iternextitem
+   fun:list_extend
+   fun:_PyList_Extend
+   fun:PySequence_List
+}
+
+{
+   Leak when importing scipy.fft
+   Memcheck:Leak
+   fun:_Znwm
+   fun:PyInit_pypocketfft
+   fun:_PyImport_LoadDynamicModuleWithSpec
+   fun:_imp_create_dynamic_impl.constprop.3
+   fun:_imp_create_dynamic
+   fun:cfunction_vectorcall_FASTCALL
+   fun:PyVectorcall_Call
+   fun:_PyObject_Call
+   fun:PyObject_Call
+   fun:do_call_core
+   fun:_PyEval_EvalFrameDefault
+   fun:_PyEval_EvalFrame
+   fun:_PyEval_EvalCode
+}
+
+{
+   NumPy leaks when spawning a subprocess
+   Memcheck:Leak
+   fun:malloc
+   ...
+   fun:_buffer_get_info
+   fun:array_getbuffer
+   fun:PyObject_GetBuffer
+   fun:__Pyx__GetBufferAndValidate*
+   fun:__pyx_f_5numpy_6random_13bit_generator_12SeedSequence_mix_entropy
+   fun:__pyx_pw_5numpy_6random_13bit_generator_12SeedSequence_1__init__
+   fun:type_call
+   fun:__Pyx__PyObject_CallOneArg
+   fun:__pyx_pw_5numpy_6random_13bit_generator_12BitGenerator_1__init__
+}