Reenable and fix tests

- Add ASAN workflows - Remove flag messages from configure; these are misleading as they exclude `COMPILE_OPTIONS` - Fix non-working tests - Fix WavPack file loading on Windows - Fix ASAN traps in LoFi code due to a SIMD `loadu` call in HIIR resamplers - Add tests for st_audiofile -
atsushieno · Aug 7, 2023 · 1e771a4 · 1e771a4
1 parent 507c61d
commit 1e771a4
Show file tree

Hide file tree

Showing 14 changed files with 228 additions and 105 deletions.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -20,13 +20,8 @@ env:
 # shell: pwsh on windows, bash all the rest
 # working-directory: ${{ github.workspace }}
 
-# FIXME:
-# Tests disabled (again); they were working one day and broken the day after
-# without changes: ./build/tests/sfizz_tests: No such file or directory
-# It didn't work either by specifying the full path (that was OK in previous builds)
 jobs:
   clang_tidy:
-  # if: ${{ false }}
     name: Clang Tidy
     runs-on: ubuntu-20.04
     steps:
@@ -39,8 +34,57 @@ jobs:
     - name: Run
       run: ./scripts/run_clang_tidy.sh
 
+  test_with_asan:
+    name: ASAN
+    runs-on: ubuntu-22.04 # abseil (libabsl) is not available in 20.04 repository
+    env:
+      install_name: sfizz-${{ github.ref_name }}-linux
+    strategy:
+      matrix:
+        build_type: [Debug, RelWithDebInfo]
+    steps:
+    - name: Checkout
+      uses: actions/checkout@v3
+      with:
+        submodules: recursive
+    - name: Install dependencies
+      run: |
+        sudo apt-get update && sudo apt-get install \
+          ninja-build \
+          libjack-jackd2-dev \
+          libabsl-dev \
+          libabsl20210324
+    - name: Configure CMake
+      run: |
+        options=(
+          -G Ninja
+          -B build
+          -S .
+          -D CMAKE_BUILD_TYPE=${{ matrix.build_type }}
+          -D SFIZZ_TESTS=ON
+          -D SFIZZ_USE_SYSTEM_ABSEIL=ON
+          -D SFIZZ_ASAN=ON
+        )
+        cmake "${options[@]}"
+    - name: Build tests
+      run: |
+        options=(
+          --build build
+          --config ${{ matrix.build_type }}
+          --parallel 2
+          --target sfizz_tests
+        )
+        cmake "${options[@]}"
+    - name: Run tests
+      run: |
+        options=(
+          --build-config ${{ matrix.build_type }}
+          --output-on-failure
+          --test-dir build
+        )
+        ctest "${options[@]}"
+
   build_for_linux:
-  # if: ${{ false }}
     name: Linux Ubuntu 22.04
     runs-on: ubuntu-22.04 # abseil (libabsl) is not available in 20.04 repository
     env:
@@ -83,7 +127,6 @@ jobs:
         echo "-- github.workspace: ${{ github.workspace }}"
         echo "-- runner.workspace: ${{ runner.workspace }}"
     - name: Build tests
-      if: ${{ false }}
       run: |
         options=(
           --build build
@@ -94,15 +137,10 @@ jobs:
         )
         cmake "${options[@]}"
     - name: Run tests
-      if: ${{ false }}
       run: |
-        ./build/tests/sfizz_tests
         options=(
           --build-config ${{ env.build_type }}
-          --debug
-          --extra-verbose
           --output-on-failure
-          --parallel 2
           --test-dir build
         )
         ctest "${options[@]}"
@@ -129,7 +167,6 @@ jobs:
         path: "${{ github.workspace }}/${{ env.install_name }}.tar.gz"
 
   build_for_macos:
-  # if: ${{ false }}
     name: macOS 11
     runs-on: macos-11
     env:
@@ -161,7 +198,6 @@ jobs:
         echo "-- runner.workspace: ${{ runner.workspace }}"
         echo "-- github.workspace: ${{ github.workspace }}"
     - name: Build tests
-      if: ${{ false }}
       run: |
         options=(
           --build build
@@ -172,15 +208,10 @@ jobs:
         )
         cmake "${options[@]}"
     - name: Run tests
-      if: ${{ false }}
       run: |
-        ./build/tests/sfizz_tests
         options=(
           --build-config ${{ env.build_type }}
-          --debug
-          --extra-verbose
           --output-on-failure
-          --parallel 2
           --test-dir build
         )
         ctest "${options[@]}"
@@ -207,7 +238,6 @@ jobs:
         path: "${{ github.workspace }}/${{ env.install_name }}.tar.gz"
 
   build_for_windows:
-  # if: ${{ false }}
     name: Windows 2019
     runs-on: windows-2019
     strategy:
@@ -235,12 +265,10 @@ jobs:
           -A "${{ matrix.release_arch }}" `
           -B build `
           -S . `
-          -D CMAKE_BUILD_TYPE=${{ env.build_type }} `
           -D SFIZZ_TESTS=ON
         echo "-- runner.workspace: ${{ runner.workspace }}"
         echo "-- github.workspace: ${{ github.workspace }}"
     - name: Build tests
-      if: ${{ false }}
       run: |
         cmake `
           --build build `
@@ -249,15 +277,10 @@ jobs:
           --target sfizz_tests `
           --verbose `
     - name: Run tests
-      if: ${{ false }}
       run: |
-        .\build\tests\${{ env.build_type }}\sfizz_tests.exe
         ctest `
           --build-config ${{ env.build_type }} `
-          --debug `
-          --extra-verbose `
           --output-on-failure `
-          --parallel 2 `
           --test-dir build `
     - name: Build all
       run: |
@@ -277,7 +300,6 @@ jobs:
         path: "${{ github.workspace }}/${{ env.install_name }}.zip"
 
   archive_source_code:
-    # if: startsWith(github.ref, 'refs/tags/')
     if: ${{ github.ref_type == 'tag' }}
     name: Source code archive
     runs-on: ubuntu-20.04
@@ -304,7 +326,6 @@ jobs:
         path: "${{ github.workspace }}/${{ env.install_name }}.tar.gz"
 
   build_with_libsndfile:
-  # if: ${{ false }}
     name: Linux libsndfile
     runs-on: ubuntu-20.04
     steps:
@@ -332,7 +353,6 @@ jobs:
         )
         cmake "${options[@]}"
     - name: Build tests
-      if: ${{ false }}
       run: |
         options=(
           --build build
@@ -343,11 +363,15 @@ jobs:
         )
         cmake "${options[@]}"
     - name: Run tests
-      if: ${{ false }}
-      run: ./build/tests/sfizz_tests
+      run: |
+        options=(
+          --build-config ${{ env.build_type }}
+          --output-on-failure
+          --test-dir build
+        )
+        ctest "${options[@]}"
 
   build_with_makefile:
-  # if: ${{ false }}
     name: Linux makefile
     runs-on: ubuntu-20.04
     steps:

diff --git a/.gitignore b/.gitignore
@@ -26,3 +26,4 @@ clients/sfzprint
 *.code-*
 .kak.tags.namecache
 .clangd
+venv
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -41,6 +41,7 @@ option_ex(SFIZZ_USE_SYSTEM_CATCH    "Use Catch libraries preinstalled on system"
 option_ex(SFIZZ_RELEASE_ASSERTS     "Forced assertions in release builds" OFF)
 option_ex(SFIZZ_PROFILE_BUILD       "Profile the build time" OFF)
 option_ex(SFIZZ_SNDFILE_STATIC      "Link the sndfile library statically" OFF)
+option_ex(SFIZZ_ASAN                "Use address sanitizer on all sfizz targets" OFF)
 
 # Continuous Controller count (0 to 511)
 set(MIDI_CC_COUNT 512 CACHE STRING "Maximum number of managed Control Change messages")

diff --git a/cmake/SfizzConfig.cmake b/cmake/SfizzConfig.cmake
@@ -116,6 +116,11 @@ function(sfizz_enable_fast_math NAME)
     endif()
 endfunction()
 
+function(sfizz_enable_release_asserts NAME)
+    target_compile_definitions("${NAME}" PUBLIC "SFIZZ_ENABLE_RELEASE_ASSERT=1")
+    target_compile_definitions("${NAME}" PUBLIC "SFIZZ_ENABLE_RELEASE_DBG=1")
+endfunction()
+
 # If we build with Clang, optionally use libc++. Enabled by default on Apple OS.
 cmake_dependent_option(USE_LIBCPP "Use libc++ with clang" "${APPLE}"
     "CMAKE_CXX_COMPILER_ID MATCHES Clang" OFF)
@@ -133,6 +138,13 @@ else()
     set(PROJECT_IS_MAIN FALSE)
 endif()
 
+if(SFIZZ_ASAN)
+    add_compile_options($<$<COMPILE_LANGUAGE:C,CXX>:-fsanitize=address>)
+    add_link_options($<$<COMPILE_LANGUAGE:C,CXX>:-fsanitize=address>)
+    add_compile_options($<$<COMPILE_LANGUAGE:C,CXX>:-fno-omit-frame-pointer>)
+    add_link_options($<$<COMPILE_LANGUAGE:C,CXX>:-fno-omit-frame-pointer>)
+endif()
+
 # Don't show build information when building a different project
 function(show_build_info_if_needed)
     if(PROJECT_IS_MAIN)
@@ -155,6 +167,7 @@ Statically link sndfile:       ${SFIZZ_SNDFILE_STATIC}
 
 Use clang libc++:              ${USE_LIBCPP}
 Release asserts:               ${SFIZZ_RELEASE_ASSERTS}
+Use ASAN:                      ${SFIZZ_ASAN}
 
 Use system abseil-cpp:         ${SFIZZ_USE_SYSTEM_ABSEIL}
 Use system catch:              ${SFIZZ_USE_SYSTEM_CATCH}
@@ -182,11 +195,6 @@ VST3 destination directory:    ${VST3_PLUGIN_INSTALL_DIR}")
         endif()
         message(STATUS "
 Install prefix:                ${CMAKE_INSTALL_PREFIX}
-
-CXX Debug flags:               ${CMAKE_CXX_FLAGS_DEBUG}
-CXX Release flags:             ${CMAKE_CXX_FLAGS_RELEASE}
-CXX MinSize flags:             ${CMAKE_CXX_FLAGS_MINSIZEREL}
-CXX RelWithDebInfo flags:      ${CMAKE_CXX_FLAGS_RELWITHDEBINFO}
 ")
     endif()
 endfunction()
diff --git a/external/st_audiofile/src/st_audiofile.c b/external/st_audiofile/src/st_audiofile.c
@@ -9,6 +9,9 @@
 #include "st_audiofile_libs.h"
 #include <stdlib.h>
 
+#ifdef _WIN32
+#include <windows.h>
+#endif
 #define WAVPACK_MEMORY_ASSUMED_VERSION 5
 
 struct st_audio_file {
@@ -179,10 +182,18 @@ static st_audio_file* st_generic_open_file(const void* filename, int widepath)
 
     // Try WV
     {
-        af->wv =
 #if defined(_WIN32)
-            WavpackOpenFileInput((const char*)filename, NULL, OPEN_FILE_UTF8, 0);
+        // WavPack expects an UTF8 input and has no widechar api, so we convert the filename back...
+        unsigned wsize = wcslen(filename);
+        unsigned size = WideCharToMultiByte(CP_UTF8, 0, filename, wsize, NULL, 0, NULL, NULL);
+        char *buffer = (char*)malloc((size+1) * sizeof(char));
+        WideCharToMultiByte(CP_UTF8, 0, filename, wsize, buffer, size, NULL, NULL);
+        buffer[size] = '\0';
+        af->wv =
+            WavpackOpenFileInput(buffer, NULL, OPEN_FILE_UTF8, 0);
+        free(buffer);
 #else
+        af->wv =
             WavpackOpenFileInput((const char*)filename, NULL, 0, 0);
 #endif
         if (af->wv) {

diff --git a/external/st_audiofile/src/st_audiofile.hpp b/external/st_audiofile/src/st_audiofile.hpp
@@ -60,13 +60,13 @@ inline ST_AudioFile::~ST_AudioFile() noexcept
     reset();
 }
 
-ST_AudioFile::ST_AudioFile(ST_AudioFile&& other) noexcept
+inline ST_AudioFile::ST_AudioFile(ST_AudioFile&& other) noexcept
     : af_(other.af_)
 {
     other.af_ = nullptr;
 }
 
-ST_AudioFile& ST_AudioFile::operator=(ST_AudioFile&& other) noexcept
+inline ST_AudioFile& ST_AudioFile::operator=(ST_AudioFile&& other) noexcept
 {
     if (this != &other) {
         if (af_)
@@ -91,14 +91,14 @@ inline void ST_AudioFile::reset(st_audio_file* new_af) noexcept
     }
 }
 
-bool ST_AudioFile::open_file(const char* filename)
+inline bool ST_AudioFile::open_file(const char* filename)
 {
     st_audio_file* new_af = st_open_file(filename);
     reset(new_af);
     return new_af != nullptr;
 }
 
-bool ST_AudioFile::open_memory(const void* memory, size_t length)
+inline bool ST_AudioFile::open_memory(const void* memory, size_t length)
 {
     st_audio_file* new_af = st_open_memory(memory, length);
     reset(new_af);

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
@@ -231,9 +231,7 @@ target_include_directories(sfizz_parser PUBLIC sfizz)
 target_link_libraries(sfizz_parser
     PUBLIC sfizz::filesystem sfizz::simde absl::strings
     PRIVATE absl::flat_hash_map)
-if(SFIZZ_RELEASE_ASSERTS)
-    target_compile_definitions(sfizz_parser PUBLIC "SFIZZ_ENABLE_RELEASE_ASSERT=1")
-endif()
+sfizz_enable_release_asserts(sfizz_parser)
 
 # OSC messaging library
 set(SFIZZ_MESSAGING_HEADERS
@@ -250,9 +248,8 @@ target_sources(sfizz_messaging PRIVATE
     ${SFIZZ_MESSAGING_HEADERS} ${SFIZZ_MESSAGING_SOURCES})
 target_include_directories(sfizz_messaging PUBLIC ".")
 target_link_libraries(sfizz_messaging PUBLIC absl::strings)
-if(SFIZZ_RELEASE_ASSERTS)
-    target_compile_definitions(sfizz_messaging PUBLIC "SFIZZ_ENABLE_RELEASE_ASSERT=1")
-endif()
+sfizz_enable_release_asserts(sfizz_messaging)
+
 
 # Import library
 set(SFIZZ_IMPORT_HEADERS
@@ -303,9 +300,8 @@ if(SFIZZ_USE_SNDFILE)
     target_compile_definitions(sfizz_internal PUBLIC "SFIZZ_USE_SNDFILE=1")
     target_link_libraries(sfizz_internal PUBLIC st_audiofile)
 endif()
-if(SFIZZ_RELEASE_ASSERTS)
-    target_compile_definitions(sfizz_internal PUBLIC "SFIZZ_ENABLE_RELEASE_ASSERT=1")
-endif()
+sfizz_enable_release_asserts(sfizz_internal)
+
 if(SFIZZ_IMPLEMENT_CXX17_ALIGNED_NEW_SUPPORT)
     target_compile_definitions(sfizz_internal PRIVATE "SFIZZ_IMPLEMENT_CXX17_ALIGNED_NEW_SUPPORT=1")
 endif()

diff --git a/src/sfizz/FilePool.cpp b/src/sfizz/FilePool.cpp
@@ -477,7 +477,7 @@ void sfz::FilePool::loadingJob(const QueuedFileData& data) noexcept
     AudioReaderPtr reader = createAudioReader(file, id->isReverse(), &readError);
 
     if (readError) {
-        DBG("[sfizz] libsndfile errored for " << *id << " with message " << readError.message());
+        DBG("[sfizz] reading the file errored for " << *id << " with code " << readError << ": " << readError.message());
         return;
     }
 

diff --git a/src/sfizz/Voice.cpp b/src/sfizz/Voice.cpp
@@ -1069,6 +1069,10 @@ void Voice::Impl::fillWithData(AudioSpan<float> buffer) noexcept
     }
 
     auto source = currentPromise_->getData();
+    if (source.getNumFrames() == 0) {
+        DBG("[Voice] Empty source in promise");
+        return;
+    }
 
     BufferPool& bufferPool = resources_.getBufferPool();
     const CurveSet& curves = resources_.getCurves();
-Original file line number
+Diff line change
@@ Expand Up / @@ -26,3 +26,4 @@ clients/sfzprint @@
     *.code-*
     .kak.tags.namecache
     .clangd
+    venv