From 65df21f9e99abf1853b589f7f7366b39b84a0aba Mon Sep 17 00:00:00 2001 From: "F.D.Castel" Date: Fri, 10 Apr 2026 20:27:32 -0300 Subject: [PATCH 01/18] Add ASAN and Valgrind integration for CI test suite Add CMake options ENABLE_ASAN and ENABLE_VALGRIND (Linux/GCC/Clang only, mutually exclusive) with corresponding compiler/linker flags and CTest memcheck configuration. CMake changes: - ENABLE_ASAN appends -fsanitize=address -fno-omit-frame-pointer to compile and link flags; suppresses -DLOGGING in Debug builds for cleaner sanitizer output - ENABLE_VALGRIND finds the valgrind binary and configures MEMORYCHECK_COMMAND for ctest -T memcheck - Mutual exclusion enforced via FATAL_ERROR CMake presets: - Add 'asan' and 'valgrind' configure/build/test presets inheriting from 'debug', with ASAN_OPTIONS env and Valgrind timeout multiplier Build script (firebird-odbc-driver.build.ps1): - Add -Sanitizer parameter (None, Asan, Valgrind) that passes the corresponding CMake option; sets ASAN_OPTIONS at runtime; skips sanitizers on Windows with a warning CI (build-and-test.yml): - Add Linux x64 ASAN matrix entry (ubuntu-22.04, Debug) - Add Linux x64 Valgrind matrix entry (ubuntu-22.04, Debug) with valgrind package installation - Sanitizer jobs do not upload release artifacts Also adds valgrind.supp suppressions file (seeded with fbclient rule). Closes #288 --- .github/workflows/build-and-test.yml | 27 +++++++++++---- CMakeLists.txt | 49 ++++++++++++++++++++++++++-- CMakePresets.json | 46 ++++++++++++++++++++++++++ firebird-odbc-driver.build.ps1 | 22 ++++++++++++- valgrind.supp | 18 ++++++++++ 5 files changed, 153 insertions(+), 9 deletions(-) create mode 100644 valgrind.supp diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 142d3adc..afc59899 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -19,14 +19,14 @@ jobs: fail-fast: false matrix: include: - # ── Windows x64 native ───────────────────────────────────────────── + # ── Windows x64 native ────────────────────────────────────────────── - os: windows-2022 artifact-name: windows-x64-binaries firebird-version: '5.0.3' - os: windows-2022 firebird-branch: master - # ── Windows x86 native (WoW64) ───────────────────────────────────── + # ── Windows x86 native (WoW64) ────────────────────────────────────── - os: windows-2022 artifact-name: windows-x86-binaries arch: Win32 @@ -35,26 +35,34 @@ jobs: arch: Win32 firebird-branch: master - # ── Windows ARM64 native ─────────────────────────────────────────── + # ── Windows ARM64 native ───────────────────────────────────────────── # Official Firebird releases have no win-arm64 binaries; snapshots do. - os: windows-11-arm artifact-name: windows-arm64-binaries firebird-branch: master - # ── Linux x64 native ─────────────────────────────────────────────── + # ── Linux x64 native ───────────────────────────────────────────────── - os: ubuntu-22.04 artifact-name: linux-x64-binaries firebird-version: '5.0.3' - os: ubuntu-22.04 firebird-branch: master - # ── Linux ARM64 native ───────────────────────────────────────────── + # ── Linux ARM64 native ────────────────────────────────────────────── - os: ubuntu-22.04-arm artifact-name: linux-arm64-binaries firebird-version: '5.0.3' - os: ubuntu-22.04-arm firebird-branch: master + # ── Linux x64 sanitizers (Debug, Firebird 5.0.3) ───────────────────── + - os: ubuntu-22.04 + sanitizer: Asan + firebird-version: '5.0.3' + - os: ubuntu-22.04 + sanitizer: Valgrind + firebird-version: '5.0.3' + runs-on: ${{ matrix.os }} steps: @@ -70,6 +78,10 @@ jobs: if: runner.os == 'Linux' run: sudo apt-get update && sudo apt-get install -y unixodbc unixodbc-dev + - name: Install Valgrind + if: matrix.sanitizer == 'Valgrind' + run: sudo apt-get install -y valgrind + - name: Build, install and test shell: pwsh env: @@ -79,7 +91,10 @@ jobs: run: | $archArgs = @{} if ('${{ matrix.arch }}') { $archArgs['Architecture'] = '${{ matrix.arch }}' } - Invoke-Build test -Configuration Release @archArgs -File ./firebird-odbc-driver.build.ps1 + $sanitizerArgs = @{} + if ('${{ matrix.sanitizer }}') { $sanitizerArgs['Sanitizer'] = '${{ matrix.sanitizer }}' } + $config = if ('${{ matrix.sanitizer }}') { 'Debug' } else { 'Release' } + Invoke-Build test -Configuration $config @archArgs @sanitizerArgs -File ./firebird-odbc-driver.build.ps1 - name: Upload artifacts (Windows) if: runner.os == 'Windows' && matrix.artifact-name diff --git a/CMakeLists.txt b/CMakeLists.txt index 577e318d..2e9bc8d1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -41,6 +41,38 @@ set(CMAKE_C_STANDARD_REQUIRED ON) option(BUILD_SHARED_LIBS "Build shared libraries" ON) option(BUILD_TESTING "Build tests" ON) +# --------------------------------------------------------------------------- +# Sanitizer options (Linux / GCC / Clang only) +# --------------------------------------------------------------------------- +if(NOT MSVC) + option(ENABLE_ASAN "Enable AddressSanitizer (-fsanitize=address)" OFF) + option(ENABLE_VALGRIND "Enable Valgrind memcheck via CTest" OFF) + + if(ENABLE_ASAN AND ENABLE_VALGRIND) + message(FATAL_ERROR "ENABLE_ASAN and ENABLE_VALGRIND are mutually exclusive. " + "ASAN instruments the binary at compile time; Valgrind instruments at runtime. " + "They must not be combined.") + endif() + + if(ENABLE_ASAN) + message(STATUS "AddressSanitizer: ENABLED") + add_compile_options(-fsanitize=address -fno-omit-frame-pointer) + add_link_options(-fsanitize=address) + endif() + + if(ENABLE_VALGRIND) + find_program(VALGRIND_COMMAND valgrind) + if(NOT VALGRIND_COMMAND) + message(FATAL_ERROR "Valgrind not found but ENABLE_VALGRIND is ON. " + "Install it with: sudo apt-get install valgrind") + endif() + message(STATUS "Valgrind memcheck: ENABLED (${VALGRIND_COMMAND})") + set(MEMORYCHECK_COMMAND ${VALGRIND_COMMAND}) + set(MEMORYCHECK_COMMAND_OPTIONS + "--leak-check=full --error-exitcode=1 --suppressions=${CMAKE_SOURCE_DIR}/valgrind.supp") + endif() +endif() + # --------------------------------------------------------------------------- # CPU architecture detection (from PR #248) # --------------------------------------------------------------------------- @@ -94,8 +126,17 @@ else() set(CMAKE_POSITION_INDEPENDENT_CODE ON) # Compiler optimization flags (from PR #248 / old makefile.linux) + if(ENABLE_ASAN) + # ASAN builds: debug flags without extra logging noise + add_compile_options( + "$<$:-O0;-g3;-D_DEBUG;-DDEBUG;-fexceptions>" + ) + else() + add_compile_options( + "$<$:-O0;-g3;-D_DEBUG;-DDEBUG;-DLOGGING;-fexceptions>" + ) + endif() add_compile_options( - "$<$:-O0;-g3;-D_DEBUG;-DDEBUG;-DLOGGING;-fexceptions>" "$<$:-O3;-DNDEBUG;-ftree-loop-vectorize>" "$<$:-O2;-g;-DNDEBUG>" "$<$:-Os;-DNDEBUG>" @@ -249,8 +290,12 @@ endif() # Debug-specific definitions (matching .vcxproj: DEBUG;LOGGING for Debug configs) target_compile_definitions(OdbcFb PRIVATE $<$:DEBUG> - $<$:LOGGING> ) +if(NOT ENABLE_ASAN) + target_compile_definitions(OdbcFb PRIVATE + $<$:LOGGING> + ) +endif() # --------------------------------------------------------------------------- # Testing diff --git a/CMakePresets.json b/CMakePresets.json index fa3fc630..828a0579 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -30,6 +30,22 @@ "cacheVariables": { "CMAKE_BUILD_TYPE": "Debug" } + }, + { + "name": "asan", + "displayName": "Debug + AddressSanitizer", + "inherits": "debug", + "cacheVariables": { + "ENABLE_ASAN": "ON" + } + }, + { + "name": "valgrind", + "displayName": "Debug + Valgrind", + "inherits": "debug", + "cacheVariables": { + "ENABLE_VALGRIND": "ON" + } } ], "buildPresets": [ @@ -42,6 +58,16 @@ "name": "debug", "configurePreset": "default", "configuration": "Debug" + }, + { + "name": "asan", + "configurePreset": "asan", + "configuration": "Debug" + }, + { + "name": "valgrind", + "configurePreset": "valgrind", + "configuration": "Debug" } ], "testPresets": [ @@ -51,6 +77,26 @@ "output": { "outputOnFailure": true } + }, + { + "name": "asan", + "configurePreset": "asan", + "output": { + "outputOnFailure": true + }, + "environment": { + "ASAN_OPTIONS": "detect_leaks=1:halt_on_error=1:print_stats=1" + } + }, + { + "name": "valgrind", + "configurePreset": "valgrind", + "output": { + "outputOnFailure": true + }, + "execution": { + "timeout": 600 + } } ] } diff --git a/firebird-odbc-driver.build.ps1 b/firebird-odbc-driver.build.ps1 index f205c6c0..f8f49b31 100644 --- a/firebird-odbc-driver.build.ps1 +++ b/firebird-odbc-driver.build.ps1 @@ -19,7 +19,10 @@ param( [string]$Configuration = 'Debug', [ValidateSet('', 'Win32')] - [string]$Architecture = '' + [string]$Architecture = '', + + [ValidateSet('None', 'Asan', 'Valgrind')] + [string]$Sanitizer = 'None' ) # Detect OS @@ -66,6 +69,16 @@ task build { if ($IsWindowsOS -and $Architecture) { $cmakeArgs += @('-A', $Architecture) } + if ($Sanitizer -ne 'None') { + if ($IsWindowsOS) { + print Yellow "WARNING: Sanitizer=$Sanitizer is not supported on Windows. Building without sanitizer." + } else { + switch ($Sanitizer) { + 'Asan' { $cmakeArgs += '-DENABLE_ASAN=ON' } + 'Valgrind' { $cmakeArgs += '-DENABLE_VALGRIND=ON' } + } + } + } exec { cmake @cmakeArgs } if ($IsWindowsOS) { @@ -160,6 +173,13 @@ task test build, build-test-databases, install, { print Yellow 'WARNING: FIREBIRD_ODBC_CONNECTION environment variable is not set. Using built-in connection strings.' } + # Set sanitizer runtime options + if ($Sanitizer -eq 'Asan' -and -not $IsWindowsOS) { + $env:ASAN_OPTIONS = 'detect_leaks=1:halt_on_error=1:print_stats=1' + $env:LSAN_OPTIONS = "suppressions=$(Join-Path $BuildRoot 'lsan.supp')" + print Cyan "ASAN_OPTIONS=$env:ASAN_OPTIONS" + } + # Test suites that exercise charset/encoding-sensitive code paths. $charsetSensitiveSuites = @( 'WCharTest' diff --git a/valgrind.supp b/valgrind.supp new file mode 100644 index 00000000..5b6252fd --- /dev/null +++ b/valgrind.supp @@ -0,0 +1,18 @@ +# Valgrind suppressions for Firebird ODBC Driver test suite +# +# This file suppresses known false positives from third-party libraries +# (Firebird client, unixODBC, system allocators, etc.). +# Populate as false positives are discovered during Valgrind runs. +# +# Usage: +# valgrind --leak-check=full --suppressions=valgrind.supp ./firebird_odbc_tests +# +# Or via CTest (when ENABLE_VALGRIND=ON): +# ctest -T memcheck + +{ + fbclient_global_init + Memcheck:Leak + ... + obj:*/libfbclient.so* +} From 1b21cf212c5c4f525518ca58b33768f54af38f57 Mon Sep 17 00:00:00 2001 From: "F.D.Castel" Date: Fri, 10 Apr 2026 20:30:27 -0300 Subject: [PATCH 02/18] Add lsan.supp and fix suppressions file path LSAN_OPTIONS referenced a relative 'lsan.supp' path but CTest runs from the build directory. Use an absolute path derived from the source/workspace root so the file is always found. Also adds the lsan.supp suppressions file with initial rules for libfbclient, libodbc, and libodbcinst. --- lsan.supp | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 lsan.supp diff --git a/lsan.supp b/lsan.supp new file mode 100644 index 00000000..7355db56 --- /dev/null +++ b/lsan.supp @@ -0,0 +1,12 @@ +# LeakSanitizer suppressions for Firebird ODBC Driver test suite +# +# This file suppresses known leak reports from third-party libraries +# (Firebird client, unixODBC, system allocators, etc.). +# Populate as false positives are discovered during ASAN/LSAN runs. +# +# Usage: +# export LSAN_OPTIONS="suppressions=lsan.supp" + +leak:libfbclient +leak:libodbc +leak:libodbcinst From 2e5af485d3cda7b8991f1ecb40ae2ed0a9826c87 Mon Sep 17 00:00:00 2001 From: "F.D.Castel" Date: Fri, 10 Apr 2026 20:33:42 -0300 Subject: [PATCH 03/18] Mark ASAN CI job as continue-on-error ASAN correctly detected a pre-existing heap-buffer-overflow in OdbcError::sqlGetDiagRec (strcpy into an undersized ConvertingString buffer). This proves the sanitizer integration works, but the existing bug should not block PRs. Mark the ASAN matrix entry continue-on-error: true until the underlying memory bugs are fixed in a separate PR. --- .github/workflows/build-and-test.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index afc59899..2258dfbb 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -59,11 +59,15 @@ jobs: - os: ubuntu-22.04 sanitizer: Asan firebird-version: '5.0.3' + # ASAN found pre-existing memory bugs (e.g. heap-buffer-overflow in + # OdbcError::sqlGetDiagRec). Allow failure until those are fixed. + continue-on-error: true - os: ubuntu-22.04 sanitizer: Valgrind firebird-version: '5.0.3' runs-on: ${{ matrix.os }} + continue-on-error: ${{ matrix.continue-on-error || false }} steps: - uses: actions/checkout@v6 From 832d8e7a1592edc0d8665228e08cfcc34cc8edc3 Mon Sep 17 00:00:00 2001 From: "F.D.Castel" Date: Fri, 10 Apr 2026 20:56:31 -0300 Subject: [PATCH 04/18] Fix heap-buffer-overflow in ConvertingString<> state buffer allocation On Linux, sizeof(wchar_t) = 4 bytes but sizeof(SQLWCHAR) = 2 bytes (unixODBC defines SQLWCHAR as unsigned short). The ConvertingString constructor used sizeof(wchar_t) to convert a byte-count argument into the number of narrow characters needed: lengthString = length / sizeof(wchar_t); // = 12/4 = 3 on Linux For SQLGetDiagRecW the state buffer is declared as State(12, sqlState), giving lengthString=3 and Alloc() allocating 3+2=5 bytes. strcpy() then writes the 5-character SQL state plus its NUL terminator (6 bytes) into that 5-byte buffer, producing a 1-byte heap-buffer-overflow caught by AddressSanitizer. The same latent bug exists in SQLErrorW (same State(12, sqlState) pattern). Fix: divide by sizeof(SQLWCHAR) instead of sizeof(wchar_t). sizeof(SQLWCHAR) == 2 on all platforms (Windows: SQLWCHAR=wchar_t=2; Linux/unixODBC: SQLWCHAR=unsigned short=2), so the formula now yields: lengthString = 12 / sizeof(SQLWCHAR) = 6 and Alloc() allocates 6+2=8 bytes, comfortably holding the SQL state. On Windows sizeof(wchar_t)==sizeof(SQLWCHAR)==2, so this change is a no-op there. Found by: AddressSanitizer (introduced in CI via PR #288/#289) Test: DataTypeTest.SmallintRoundTrip -> ExecIgnoreError -> SQLExecDirect -> unixODBC dispatcher -> SQLGetDiagRecW -> sqlGetDiagRec(strcpy) --- MainUnicode.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MainUnicode.cpp b/MainUnicode.cpp index 1d3c8486..7c5b674e 100644 --- a/MainUnicode.cpp +++ b/MainUnicode.cpp @@ -85,7 +85,7 @@ class ConvertingString if ( length == SQL_NTS ) lengthString = 0; else if ( retCountOfBytes ) - lengthString = length / sizeof(wchar_t); + lengthString = length / sizeof(SQLWCHAR); else lengthString = length; } From 8ea653f6c3118261746241f2e445cca567b7b5d7 Mon Sep 17 00:00:00 2001 From: "F.D.Castel" Date: Fri, 10 Apr 2026 20:59:08 -0300 Subject: [PATCH 05/18] Remove continue-on-error from ASAN CI job The heap-buffer-overflow in ConvertingString (sizeof(wchar_t) vs sizeof(SQLWCHAR)) has been fixed. ASAN now passes all 375 tests cleanly on ubuntu-22.04/GCC, so the soft-fail guard is no longer needed. --- .github/workflows/build-and-test.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 2258dfbb..afc59899 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -59,15 +59,11 @@ jobs: - os: ubuntu-22.04 sanitizer: Asan firebird-version: '5.0.3' - # ASAN found pre-existing memory bugs (e.g. heap-buffer-overflow in - # OdbcError::sqlGetDiagRec). Allow failure until those are fixed. - continue-on-error: true - os: ubuntu-22.04 sanitizer: Valgrind firebird-version: '5.0.3' runs-on: ${{ matrix.os }} - continue-on-error: ${{ matrix.continue-on-error || false }} steps: - uses: actions/checkout@v6 From 84b9dec5d24e260ccdd07e2fb5c0af59a5f8340c Mon Sep 17 00:00:00 2001 From: "F.D.Castel" Date: Wed, 15 Apr 2026 16:22:20 -0300 Subject: [PATCH 06/18] Address PR review: rename to BUILD_WITH_*, restructure sanitizer options - Move option() declarations outside if(NOT MSVC) guard; emit warning on MSVC instead of hiding the options entirely - Rename ENABLE_ASAN -> BUILD_WITH_ASAN and ENABLE_VALGRIND -> BUILD_WITH_VALGRIND to follow existing BUILD_* naming convention - Remove duplicate target_compile_definitions for DEBUG/LOGGING (already defined via add_compile_options in Linux section) - Separate LOGGING into add_definitions() and conditionally skip it for both ASAN and Valgrind builds (not just ASAN) --- CMakeLists.txt | 52 ++++++++++++++-------------------- CMakePresets.json | 4 +-- firebird-odbc-driver.build.ps1 | 4 +-- 3 files changed, 25 insertions(+), 35 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 2e9bc8d1..4dc26985 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -42,28 +42,31 @@ option(BUILD_SHARED_LIBS "Build shared libraries" ON) option(BUILD_TESTING "Build tests" ON) # --------------------------------------------------------------------------- -# Sanitizer options (Linux / GCC / Clang only) +# Sanitizer options # --------------------------------------------------------------------------- -if(NOT MSVC) - option(ENABLE_ASAN "Enable AddressSanitizer (-fsanitize=address)" OFF) - option(ENABLE_VALGRIND "Enable Valgrind memcheck via CTest" OFF) +option(BUILD_WITH_ASAN "Enable AddressSanitizer (-fsanitize=address)" OFF) +option(BUILD_WITH_VALGRIND "Enable Valgrind memcheck via CTest" OFF) - if(ENABLE_ASAN AND ENABLE_VALGRIND) - message(FATAL_ERROR "ENABLE_ASAN and ENABLE_VALGRIND are mutually exclusive. " - "ASAN instruments the binary at compile time; Valgrind instruments at runtime. " - "They must not be combined.") - endif() +if(BUILD_WITH_ASAN AND BUILD_WITH_VALGRIND) + message(FATAL_ERROR "BUILD_WITH_ASAN and BUILD_WITH_VALGRIND are mutually exclusive. " + "ASAN instruments the binary at compile time; Valgrind instruments at runtime. " + "They must not be combined.") +endif() - if(ENABLE_ASAN) +if(MSVC AND (BUILD_WITH_ASAN OR BUILD_WITH_VALGRIND)) + message(WARNING "Sanitizers are not yet supported on MSVC. " + "BUILD_WITH_ASAN / BUILD_WITH_VALGRIND will be ignored.") +elseif(NOT MSVC) + if(BUILD_WITH_ASAN) message(STATUS "AddressSanitizer: ENABLED") add_compile_options(-fsanitize=address -fno-omit-frame-pointer) add_link_options(-fsanitize=address) endif() - if(ENABLE_VALGRIND) + if(BUILD_WITH_VALGRIND) find_program(VALGRIND_COMMAND valgrind) if(NOT VALGRIND_COMMAND) - message(FATAL_ERROR "Valgrind not found but ENABLE_VALGRIND is ON. " + message(FATAL_ERROR "Valgrind not found but BUILD_WITH_VALGRIND is ON. " "Install it with: sudo apt-get install valgrind") endif() message(STATUS "Valgrind memcheck: ENABLED (${VALGRIND_COMMAND})") @@ -126,15 +129,12 @@ else() set(CMAKE_POSITION_INDEPENDENT_CODE ON) # Compiler optimization flags (from PR #248 / old makefile.linux) - if(ENABLE_ASAN) - # ASAN builds: debug flags without extra logging noise - add_compile_options( - "$<$:-O0;-g3;-D_DEBUG;-DDEBUG;-fexceptions>" - ) - else() - add_compile_options( - "$<$:-O0;-g3;-D_DEBUG;-DDEBUG;-DLOGGING;-fexceptions>" - ) + add_compile_options( + "$<$:-O0;-g3;-D_DEBUG;-DDEBUG;-fexceptions>" + ) + # LOGGING definition: enable for Debug builds, suppress for sanitizer builds + if(NOT (BUILD_WITH_ASAN OR BUILD_WITH_VALGRIND)) + add_definitions(-DLOGGING) endif() add_compile_options( "$<$:-O3;-DNDEBUG;-ftree-loop-vectorize>" @@ -287,16 +287,6 @@ else() ) endif() -# Debug-specific definitions (matching .vcxproj: DEBUG;LOGGING for Debug configs) -target_compile_definitions(OdbcFb PRIVATE - $<$:DEBUG> -) -if(NOT ENABLE_ASAN) - target_compile_definitions(OdbcFb PRIVATE - $<$:LOGGING> - ) -endif() - # --------------------------------------------------------------------------- # Testing # --------------------------------------------------------------------------- diff --git a/CMakePresets.json b/CMakePresets.json index 828a0579..9ef7c3e1 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -36,7 +36,7 @@ "displayName": "Debug + AddressSanitizer", "inherits": "debug", "cacheVariables": { - "ENABLE_ASAN": "ON" + "BUILD_WITH_ASAN": "ON" } }, { @@ -44,7 +44,7 @@ "displayName": "Debug + Valgrind", "inherits": "debug", "cacheVariables": { - "ENABLE_VALGRIND": "ON" + "BUILD_WITH_VALGRIND": "ON" } } ], diff --git a/firebird-odbc-driver.build.ps1 b/firebird-odbc-driver.build.ps1 index f8f49b31..2a43704a 100644 --- a/firebird-odbc-driver.build.ps1 +++ b/firebird-odbc-driver.build.ps1 @@ -74,8 +74,8 @@ task build { print Yellow "WARNING: Sanitizer=$Sanitizer is not supported on Windows. Building without sanitizer." } else { switch ($Sanitizer) { - 'Asan' { $cmakeArgs += '-DENABLE_ASAN=ON' } - 'Valgrind' { $cmakeArgs += '-DENABLE_VALGRIND=ON' } + 'Asan' { $cmakeArgs += '-DBUILD_WITH_ASAN=ON' } + 'Valgrind' { $cmakeArgs += '-DBUILD_WITH_VALGRIND=ON' } } } } From e28f2556c6631add91908e872e51381b1e2df6b8 Mon Sep 17 00:00:00 2001 From: "F.D.Castel" Date: Wed, 15 Apr 2026 16:47:31 -0300 Subject: [PATCH 07/18] Restructure LOGGING handling to use remove_definitions pattern MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per PR review feedback: consolidate the build-type options matrix and express the sanitizer carve-out via add_definitions/remove_definitions instead of an inverted if-NOT guard. Behavior is unchanged — LOGGING is defined for Debug builds and stripped for ASAN/Valgrind builds. --- CMakeLists.txt | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 4dc26985..8941ac96 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -131,17 +131,19 @@ else() # Compiler optimization flags (from PR #248 / old makefile.linux) add_compile_options( "$<$:-O0;-g3;-D_DEBUG;-DDEBUG;-fexceptions>" - ) - # LOGGING definition: enable for Debug builds, suppress for sanitizer builds - if(NOT (BUILD_WITH_ASAN OR BUILD_WITH_VALGRIND)) - add_definitions(-DLOGGING) - endif() - add_compile_options( "$<$:-O3;-DNDEBUG;-ftree-loop-vectorize>" "$<$:-O2;-g;-DNDEBUG>" "$<$:-Os;-DNDEBUG>" ) + # LOGGING: Debug builds only; sanitizer builds strip it for cleaner output. + if(CMAKE_BUILD_TYPE STREQUAL "Debug") + add_definitions(-DLOGGING) + endif() + if(BUILD_WITH_ASAN OR BUILD_WITH_VALGRIND) + remove_definitions(-DLOGGING) + endif() + # SSE4.1 for x86/x86_64 (from PR #248) if(FBODBC_ARCH STREQUAL "x86" OR FBODBC_ARCH STREQUAL "i686" OR FBODBC_ARCH STREQUAL "x86_64" OR FBODBC_ARCH STREQUAL "AMD64") From 52f049562ff87fb2afa75d61dc3767e4ac5ee1e5 Mon Sep 17 00:00:00 2001 From: "F.D.Castel" Date: Thu, 16 Apr 2026 14:49:59 -0300 Subject: [PATCH 08/18] Simplify LOGGING guard to a single combined condition Per review feedback: remove_definitions() cannot undo definitions added via add_compile_options(), so the add-then-remove pattern was misleading. Collapse to a single if() with the combined condition instead. --- CMakeLists.txt | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8941ac96..748b82b9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -137,12 +137,9 @@ else() ) # LOGGING: Debug builds only; sanitizer builds strip it for cleaner output. - if(CMAKE_BUILD_TYPE STREQUAL "Debug") + if(CMAKE_BUILD_TYPE STREQUAL "Debug" AND NOT BUILD_WITH_ASAN AND NOT BUILD_WITH_VALGRIND) add_definitions(-DLOGGING) endif() - if(BUILD_WITH_ASAN OR BUILD_WITH_VALGRIND) - remove_definitions(-DLOGGING) - endif() # SSE4.1 for x86/x86_64 (from PR #248) if(FBODBC_ARCH STREQUAL "x86" OR FBODBC_ARCH STREQUAL "i686" From b7aa955f102a03f5b72e7ab45467156fa3c4d70a Mon Sep 17 00:00:00 2001 From: "F.D.Castel" Date: Thu, 16 Apr 2026 15:44:59 -0300 Subject: [PATCH 09/18] Fix version detection when no stable release tag exists The first git describe call used --always, which made it return the short commit hash instead of failing when --exclude "*-*" ruled out every matching tag (as is the case today: only prerelease tags like v3.5.0-rc5 exist). The non-zero exit that was supposed to trigger the fallback without --exclude never fired, so parsing dropped to 0.0.0.0. Drop --always from both calls: if the stable-only call finds nothing it exits non-zero and the fallback runs; if the fallback also finds nothing the existing warning path reports it. --- cmake/GetVersionFromGit.cmake | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/cmake/GetVersionFromGit.cmake b/cmake/GetVersionFromGit.cmake index b2adaa26..9c8e3421 100644 --- a/cmake/GetVersionFromGit.cmake +++ b/cmake/GetVersionFromGit.cmake @@ -22,11 +22,15 @@ if(NOT GIT_FOUND) return() endif() -# Try to find the latest semver tag matching v.. -# We use --match to only consider tags in vX.Y.Z format (not rc, temp, etc.) +# Find the latest semver tag matching v... +# Prefer a stable vX.Y.Z tag (--exclude "*-*" filters out prereleases like +# v3.5.0-rc1). If no stable tag exists yet, fall back to any matching tag; +# the parser regex below handles the optional - suffix. +# NOTE: --always is intentionally NOT used on the first call so that a +# "no stable tag" case fails with non-zero exit and triggers the fallback. execute_process( COMMAND ${GIT_EXECUTABLE} describe --tags --match "v[0-9]*.[0-9]*.[0-9]*" - --exclude "*-*" --long --always + --exclude "*-*" --long WORKING_DIRECTORY ${CMAKE_SOURCE_DIR} OUTPUT_VARIABLE GIT_DESCRIBE_OUTPUT OUTPUT_STRIP_TRAILING_WHITESPACE @@ -35,10 +39,9 @@ execute_process( ) if(NOT GIT_DESCRIBE_RESULT EQUAL 0) - # No matching tag found at all, try without --exclude execute_process( COMMAND ${GIT_EXECUTABLE} describe --tags --match "v[0-9]*.[0-9]*.[0-9]*" - --long --always + --long WORKING_DIRECTORY ${CMAKE_SOURCE_DIR} OUTPUT_VARIABLE GIT_DESCRIBE_OUTPUT OUTPUT_STRIP_TRAILING_WHITESPACE From 532321435a3ed4a3e34f2a0963a947e60084925d Mon Sep 17 00:00:00 2001 From: "F.D.Castel" Date: Thu, 16 Apr 2026 17:46:00 -0300 Subject: [PATCH 10/18] Exclude DEBUG macro from sanitizer builds DEBUG adds extra logging; exclude it alongside LOGGING for cleaner output. --- CMakeLists.txt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 748b82b9..8798d9e5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -130,14 +130,16 @@ else() # Compiler optimization flags (from PR #248 / old makefile.linux) add_compile_options( - "$<$:-O0;-g3;-D_DEBUG;-DDEBUG;-fexceptions>" + "$<$:-O0;-g3;-D_DEBUG;-fexceptions>" "$<$:-O3;-DNDEBUG;-ftree-loop-vectorize>" "$<$:-O2;-g;-DNDEBUG>" "$<$:-Os;-DNDEBUG>" ) - # LOGGING: Debug builds only; sanitizer builds strip it for cleaner output. + # Debug macros: DEBUG and LOGGING are for Debug builds only; + # sanitizer builds strip them for cleaner output (less noise in reports). if(CMAKE_BUILD_TYPE STREQUAL "Debug" AND NOT BUILD_WITH_ASAN AND NOT BUILD_WITH_VALGRIND) + add_definitions(-DDEBUG) add_definitions(-DLOGGING) endif() From 836e6d48d58b9d757e0dd8058e79b854ec9b4365 Mon Sep 17 00:00:00 2001 From: "F.D.Castel" Date: Fri, 17 Apr 2026 17:37:14 -0300 Subject: [PATCH 11/18] Fix SQLWCHAR conversion path in MainUnicode --- MainUnicode.cpp | 66 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 54 insertions(+), 12 deletions(-) diff --git a/MainUnicode.cpp b/MainUnicode.cpp index 7c5b674e..df79d7a5 100644 --- a/MainUnicode.cpp +++ b/MainUnicode.cpp @@ -22,6 +22,7 @@ #include #else #include +#include #endif #include #include "OdbcJdbc.h" @@ -38,6 +39,18 @@ extern FILE *logFile; using namespace OdbcJdbcLibrary; +static int SqlWcharLength(const SQLWCHAR* text) +{ + int len = 0; + if ( !text ) + return 0; + + while ( text[len] != (SQLWCHAR)0 ) + ++len; + + return len; +} + #ifdef _WINDOWS extern UINT codePage; // from Main.cpp #endif @@ -135,13 +148,42 @@ class ConvertingString if ( len > 0 ) len--; #else - len = mbstowcs( (wchar_t*)unicodeString, (const char*)byteString, lengthString ); + // SQLWCHAR is UTF-16 on Linux; convert multibyte input directly into SQLWCHAR units. + mbstate_t state = mbstate_t(); + const char* p = (const char*)byteString; + size_t inRemaining = (size_t)lengthString; + char16_t* out = (char16_t*)unicodeString; + size_t outCount = 0; + const size_t outCapacity = (lengthString > 0) ? (size_t)lengthString - 1 : 0; + + while ( inRemaining > 0 && outCount < outCapacity ) + { + size_t rc = std::mbrtoc16( out + outCount, p, inRemaining, &state ); + + if ( rc == (size_t)-1 || rc == (size_t)-2 ) + break; + + if ( rc == (size_t)-3 ) + { + ++outCount; + continue; + } + + if ( rc == 0 ) + break; + + p += rc; + inRemaining -= rc; + ++outCount; + } + + len = outCount; #endif } - if ( len > 0 ) + if ( lengthString > 0 ) { - *(LPWSTR)(unicodeString + len) = L'\0'; + unicodeString[len] = (SQLWCHAR)0; if ( realLength ) { @@ -166,16 +208,16 @@ class ConvertingString SQLCHAR * convUnicodeToString( SQLWCHAR *wcString, int length ) { size_t bytesNeeded; - wchar_t *ptEndWC = NULL; - wchar_t saveWC; + SQLWCHAR *ptEndWC = NULL; + SQLWCHAR saveWC; if ( length == SQL_NTS ) - length = (int)wcslen( (const wchar_t*)wcString ); - else if ( wcString[length] != L'\0' ) + length = SqlWcharLength( wcString ); + else if ( wcString[length] != (SQLWCHAR)0 ) { - ptEndWC = (wchar_t*)&wcString[length]; + ptEndWC = &wcString[length]; saveWC = *ptEndWC; - *ptEndWC = L'\0'; + *ptEndWC = (SQLWCHAR)0; } if ( connection ) @@ -749,7 +791,7 @@ SQLRETURN SQL_API SQLNativeSqlW( SQLHDBC hDbc, GUARD_HDBC( hDbc ); if ( cbSqlStrIn == SQL_NTS ) - cbSqlStrIn = (SQLINTEGER)wcslen( (const wchar_t*)szSqlStrIn ); + cbSqlStrIn = (SQLINTEGER)SqlWcharLength( szSqlStrIn ); bool isByte = !( cbSqlStrIn % 2 ); @@ -1161,9 +1203,9 @@ SQLRETURN SQL_API SQLSetDescFieldW( SQLHDESC hDesc, int len; if ( bufferLength == SQL_NTS ) - len = (int)wcslen( (const wchar_t*)value ); + len = SqlWcharLength( (const SQLWCHAR*)value ); else - len = bufferLength / sizeof(wchar_t); + len = bufferLength / sizeof(SQLWCHAR); ConvertingString<> Value( GETCONNECT_DESC( hDesc ), (SQLWCHAR *)value, len ); From f20e6fc36f3dcdca3b5dc93ec600dc6446160c18 Mon Sep 17 00:00:00 2001 From: "F.D.Castel" Date: Sat, 18 Apr 2026 17:37:51 -0300 Subject: [PATCH 12/18] Revert "Fix SQLWCHAR conversion path in MainUnicode" This reverts commit 836e6d48d58b9d757e0dd8058e79b854ec9b4365. --- MainUnicode.cpp | 66 +++++++++---------------------------------------- 1 file changed, 12 insertions(+), 54 deletions(-) diff --git a/MainUnicode.cpp b/MainUnicode.cpp index df79d7a5..7c5b674e 100644 --- a/MainUnicode.cpp +++ b/MainUnicode.cpp @@ -22,7 +22,6 @@ #include #else #include -#include #endif #include #include "OdbcJdbc.h" @@ -39,18 +38,6 @@ extern FILE *logFile; using namespace OdbcJdbcLibrary; -static int SqlWcharLength(const SQLWCHAR* text) -{ - int len = 0; - if ( !text ) - return 0; - - while ( text[len] != (SQLWCHAR)0 ) - ++len; - - return len; -} - #ifdef _WINDOWS extern UINT codePage; // from Main.cpp #endif @@ -148,42 +135,13 @@ class ConvertingString if ( len > 0 ) len--; #else - // SQLWCHAR is UTF-16 on Linux; convert multibyte input directly into SQLWCHAR units. - mbstate_t state = mbstate_t(); - const char* p = (const char*)byteString; - size_t inRemaining = (size_t)lengthString; - char16_t* out = (char16_t*)unicodeString; - size_t outCount = 0; - const size_t outCapacity = (lengthString > 0) ? (size_t)lengthString - 1 : 0; - - while ( inRemaining > 0 && outCount < outCapacity ) - { - size_t rc = std::mbrtoc16( out + outCount, p, inRemaining, &state ); - - if ( rc == (size_t)-1 || rc == (size_t)-2 ) - break; - - if ( rc == (size_t)-3 ) - { - ++outCount; - continue; - } - - if ( rc == 0 ) - break; - - p += rc; - inRemaining -= rc; - ++outCount; - } - - len = outCount; + len = mbstowcs( (wchar_t*)unicodeString, (const char*)byteString, lengthString ); #endif } - if ( lengthString > 0 ) + if ( len > 0 ) { - unicodeString[len] = (SQLWCHAR)0; + *(LPWSTR)(unicodeString + len) = L'\0'; if ( realLength ) { @@ -208,16 +166,16 @@ class ConvertingString SQLCHAR * convUnicodeToString( SQLWCHAR *wcString, int length ) { size_t bytesNeeded; - SQLWCHAR *ptEndWC = NULL; - SQLWCHAR saveWC; + wchar_t *ptEndWC = NULL; + wchar_t saveWC; if ( length == SQL_NTS ) - length = SqlWcharLength( wcString ); - else if ( wcString[length] != (SQLWCHAR)0 ) + length = (int)wcslen( (const wchar_t*)wcString ); + else if ( wcString[length] != L'\0' ) { - ptEndWC = &wcString[length]; + ptEndWC = (wchar_t*)&wcString[length]; saveWC = *ptEndWC; - *ptEndWC = (SQLWCHAR)0; + *ptEndWC = L'\0'; } if ( connection ) @@ -791,7 +749,7 @@ SQLRETURN SQL_API SQLNativeSqlW( SQLHDBC hDbc, GUARD_HDBC( hDbc ); if ( cbSqlStrIn == SQL_NTS ) - cbSqlStrIn = (SQLINTEGER)SqlWcharLength( szSqlStrIn ); + cbSqlStrIn = (SQLINTEGER)wcslen( (const wchar_t*)szSqlStrIn ); bool isByte = !( cbSqlStrIn % 2 ); @@ -1203,9 +1161,9 @@ SQLRETURN SQL_API SQLSetDescFieldW( SQLHDESC hDesc, int len; if ( bufferLength == SQL_NTS ) - len = SqlWcharLength( (const SQLWCHAR*)value ); + len = (int)wcslen( (const wchar_t*)value ); else - len = bufferLength / sizeof(SQLWCHAR); + len = bufferLength / sizeof(wchar_t); ConvertingString<> Value( GETCONNECT_DESC( hDesc ), (SQLWCHAR *)value, len ); From 9daab276f24d13e6a062936c2ee203486596947b Mon Sep 17 00:00:00 2001 From: "F.D.Castel" Date: Sun, 19 Apr 2026 14:40:34 -0300 Subject: [PATCH 13/18] Make sanitizer presets explicitly mutually exclusive --- CMakePresets.json | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/CMakePresets.json b/CMakePresets.json index 9ef7c3e1..443bc701 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -36,7 +36,8 @@ "displayName": "Debug + AddressSanitizer", "inherits": "debug", "cacheVariables": { - "BUILD_WITH_ASAN": "ON" + "BUILD_WITH_ASAN": "ON", + "BUILD_WITH_VALGRIND": "OFF" } }, { @@ -44,7 +45,8 @@ "displayName": "Debug + Valgrind", "inherits": "debug", "cacheVariables": { - "BUILD_WITH_VALGRIND": "ON" + "BUILD_WITH_VALGRIND": "ON", + "BUILD_WITH_ASAN": "OFF" } } ], From 8258a27d827e6f3627f7dadfc8419bd179d04a12 Mon Sep 17 00:00:00 2001 From: "F.D.Castel" Date: Sun, 19 Apr 2026 14:56:39 -0300 Subject: [PATCH 14/18] Pin sanitizer flags OFF in standard presets Prevents stale BUILD_WITH_ASAN/BUILD_WITH_VALGRIND values in the CMake cache from leaking into default/release/debug configure runs after a prior sanitizer preset. --- CMakePresets.json | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/CMakePresets.json b/CMakePresets.json index 443bc701..ebd4bdab 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -12,7 +12,9 @@ "description": "Default preset — uses 'build/' as the binary directory so that all tools (command-line, Visual Studio, CLion, VS Code) share the same layout.", "binaryDir": "${sourceDir}/build", "cacheVariables": { - "CMAKE_INSTALL_PREFIX": "${sourceDir}/install" + "CMAKE_INSTALL_PREFIX": "${sourceDir}/install", + "BUILD_WITH_ASAN": "OFF", + "BUILD_WITH_VALGRIND": "OFF" } }, { @@ -20,7 +22,9 @@ "displayName": "Release", "inherits": "default", "cacheVariables": { - "CMAKE_BUILD_TYPE": "Release" + "CMAKE_BUILD_TYPE": "Release", + "BUILD_WITH_ASAN": "OFF", + "BUILD_WITH_VALGRIND": "OFF" } }, { @@ -28,7 +32,9 @@ "displayName": "Debug", "inherits": "default", "cacheVariables": { - "CMAKE_BUILD_TYPE": "Debug" + "CMAKE_BUILD_TYPE": "Debug", + "BUILD_WITH_ASAN": "OFF", + "BUILD_WITH_VALGRIND": "OFF" } }, { From 0f8f90fced65b633f9c26cee4abe6123a11e6136 Mon Sep 17 00:00:00 2001 From: "F.D.Castel" Date: Sun, 19 Apr 2026 15:32:50 -0300 Subject: [PATCH 15/18] Fix ConvertingString heap overflow without causing stack smash Commit 832d8e7 changed lengthString = length/sizeof(wchar_t) to length/sizeof(SQLWCHAR) to stop a 1-byte heap overflow in the internal byteString when OdbcError::sqlGetDiagRec strcpy's a 6-byte SQL state into the 5-byte buffer allocated on Linux (wchar_t=4). That fix is wrong in the opposite direction: lengthString is also the count passed to mbstowcs((wchar_t*)unicodeString, byteString, lengthString) in the destructor, which writes lengthString * sizeof(wchar_t) bytes into the caller's SQLWCHAR buffer. With lengthString=6 and a 12-byte caller buffer, mbstowcs overruns by 12 bytes and smashes the caller's stack (reproducible without ASAN). Revert line 88 to sizeof(wchar_t) to restore the safe mbstowcs bound, and instead floor the internal byteString allocation at 8 bytes so the strcpy no longer overflows. This is a targeted fix that keeps both the ASAN job and normal runs green until the ConvertingString / mbstowcs rewrite lands (issue #287 Tier 9.1). --- MainUnicode.cpp | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/MainUnicode.cpp b/MainUnicode.cpp index 7c5b674e..0b78688e 100644 --- a/MainUnicode.cpp +++ b/MainUnicode.cpp @@ -85,7 +85,7 @@ class ConvertingString if ( length == SQL_NTS ) lengthString = 0; else if ( retCountOfBytes ) - lengthString = length / sizeof(SQLWCHAR); + lengthString = length / sizeof(wchar_t); else lengthString = length; } @@ -219,8 +219,16 @@ class ConvertingString case BYTESCHARS: if ( lengthString ) { - byteString = new SQLCHAR[ lengthString + 2 ]; - memset(byteString, 0, lengthString + 2); + // Floor the internal buffer at 8 bytes so that callers which pass a + // small SQLWCHAR output buffer (e.g. SQLGetDiagRecW with a 12-byte + // SQL state, yielding lengthString=3 on Linux where sizeof(wchar_t)=4) + // still have room for the 6-byte SQL state ("HY000\0") that + // OdbcError::sqlGetDiagRec strcpy's into this buffer. Keeping + // lengthString itself unchanged preserves the mbstowcs writeback + // bound and avoids smashing the caller's stack buffer. + const size_t bufSize = (lengthString + 2 < 8) ? 8 : (size_t)lengthString + 2; + byteString = new SQLCHAR[ bufSize ]; + memset(byteString, 0, bufSize); } else byteString = NULL; From bea6403a65226969cdfa9a321e84cecf76d9b7f2 Mon Sep 17 00:00:00 2001 From: "F.D.Castel" Date: Sun, 19 Apr 2026 16:10:57 -0300 Subject: [PATCH 16/18] Use byte-level widening/narrowing on Linux SQLWCHAR paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the ConvertingString discussion in #289. The 8-byte floor in Alloc() introduced by the previous commit was a crutch: it avoided the strcpy heap overflow for the SQL-state case but left the destructor's mbstowcs((wchar_t*)unicodeString, ..., lengthString) writing 4-byte wchar_t units into a caller buffer that is SQLWCHAR-sized (2 bytes). Even when that did not overflow, the data was wrong — 'HY000' became 'H' after the client reinterpreted the bytes as UCS-2. Replace the Linux non-connection paths in both directions with the same byte-widening / byte-narrowing loop that unixODBC itself uses internally (ansi_to_unicode_copy / unicode_to_ansi_copy). This is correct for the ASCII-only error/state strings that reach this code; non-ASCII handling remains the subject of the broader rewrite tracked as Tier 9.1 in #287. Changes in MainUnicode.cpp: - lengthString now uses sizeof(SQLWCHAR) again (correct SQLWCHAR count). - Destructor Linux branch widens bytes into SQLWCHAR units. - convUnicodeToString Linux branch narrows SQLWCHAR to low bytes. - Temporary NUL is written as a SQLWCHAR-sized zero (not wchar_t). - Remove the Alloc() floor — no longer needed. - Add sqlwcharLen() helper; wcslen() on SQLWCHAR data is unsafe on Linux. --- MainUnicode.cpp | 78 ++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 61 insertions(+), 17 deletions(-) diff --git a/MainUnicode.cpp b/MainUnicode.cpp index 0b78688e..5222a7c2 100644 --- a/MainUnicode.cpp +++ b/MainUnicode.cpp @@ -38,6 +38,21 @@ extern FILE *logFile; using namespace OdbcJdbcLibrary; +#ifndef _WINDOWS +// SQLWCHAR-aware length (in SQLWCHAR units), safe on Linux where +// sizeof(wchar_t) != sizeof(SQLWCHAR). Do NOT use wcslen() on SQLWCHAR +// data on Linux — it reads two SQLWCHARs per wchar_t and runs off the end. +static size_t sqlwcharLen( const SQLWCHAR *s ) +{ + size_t n = 0; + if ( !s ) + return 0; + while ( s[n] ) + ++n; + return n; +} +#endif + #ifdef _WINDOWS extern UINT codePage; // from Main.cpp #endif @@ -85,7 +100,7 @@ class ConvertingString if ( length == SQL_NTS ) lengthString = 0; else if ( retCountOfBytes ) - lengthString = length / sizeof(wchar_t); + lengthString = length / sizeof(SQLWCHAR); else lengthString = length; } @@ -135,13 +150,33 @@ class ConvertingString if ( len > 0 ) len--; #else - len = mbstowcs( (wchar_t*)unicodeString, (const char*)byteString, lengthString ); + // SQLWCHAR is 2 bytes on Linux (unixODBC defines it as unsigned short), + // but wchar_t is 4 bytes, so mbstowcs((wchar_t*)unicodeString, ...) + // both corrupts the output and risks overflowing the caller's buffer. + // Widen byte-by-byte into SQLWCHAR units, matching what unixODBC's + // ansi_to_unicode_copy() does internally. This is correct for the + // ASCII-only error/state strings that reach this code path; non-ASCII + // input will be handled by the broader ConvertingString rewrite tracked + // in issue #287 (Tier 9.1). + { + const SQLCHAR *src = byteString; + size_t i = 0; + while ( i < (size_t)lengthString && src[i] != 0 ) + { + unicodeString[i] = (SQLWCHAR)( src[i] & 0xFF ); + ++i; + } + len = i; + } #endif } if ( len > 0 ) { - *(LPWSTR)(unicodeString + len) = L'\0'; + // NUL-terminate in SQLWCHAR units. LPWSTR assignment of L'\0' writes + // sizeof(wchar_t) bytes, which overruns the output buffer by 2 bytes + // on Linux. + unicodeString[len] = 0; if ( realLength ) { @@ -170,12 +205,18 @@ class ConvertingString wchar_t saveWC; if ( length == SQL_NTS ) +#ifdef _WINDOWS length = (int)wcslen( (const wchar_t*)wcString ); - else if ( wcString[length] != L'\0' ) +#else + length = (int)sqlwcharLen( wcString ); +#endif + else if ( wcString[length] != 0 ) { ptEndWC = (wchar_t*)&wcString[length]; saveWC = *ptEndWC; - *ptEndWC = L'\0'; + // Write a SQLWCHAR-sized NUL so we don't overrun the input by 2 bytes + // on Linux (wchar_t is 4 bytes there). + wcString[length] = 0; } if ( connection ) @@ -185,7 +226,10 @@ class ConvertingString #ifdef _WINDOWS bytesNeeded = WideCharToMultiByte( codePage, (DWORD)0, wcString, length, NULL, (int)0, NULL, NULL ); #else - bytesNeeded = wcstombs( NULL, (const wchar_t*)wcString, length ); + // See the symmetric comment in the destructor above: wcstombs assumes + // wchar_t-sized input, which corrupts SQLWCHAR data on Linux. The + // byte-narrowing loop below produces exactly `length` output bytes. + bytesNeeded = (size_t)length; #endif } @@ -198,7 +242,15 @@ class ConvertingString #ifdef _WINDOWS bytesNeeded = WideCharToMultiByte( codePage, 0, wcString, length, (LPSTR)byteString, (int)bytesNeeded, NULL, NULL ); #else - bytesNeeded = wcstombs( (char *)byteString, (const wchar_t*)wcString, bytesNeeded ); + { + size_t i = 0; + while ( i < (size_t)length && wcString[i] != 0 ) + { + byteString[i] = (SQLCHAR)( wcString[i] & 0xFF ); + ++i; + } + bytesNeeded = i; + } #endif } @@ -219,16 +271,8 @@ class ConvertingString case BYTESCHARS: if ( lengthString ) { - // Floor the internal buffer at 8 bytes so that callers which pass a - // small SQLWCHAR output buffer (e.g. SQLGetDiagRecW with a 12-byte - // SQL state, yielding lengthString=3 on Linux where sizeof(wchar_t)=4) - // still have room for the 6-byte SQL state ("HY000\0") that - // OdbcError::sqlGetDiagRec strcpy's into this buffer. Keeping - // lengthString itself unchanged preserves the mbstowcs writeback - // bound and avoids smashing the caller's stack buffer. - const size_t bufSize = (lengthString + 2 < 8) ? 8 : (size_t)lengthString + 2; - byteString = new SQLCHAR[ bufSize ]; - memset(byteString, 0, bufSize); + byteString = new SQLCHAR[ lengthString + 2 ]; + memset( byteString, 0, lengthString + 2 ); } else byteString = NULL; From 09b934a49eac7e98b70bb6094eb4b1835ecec5fd Mon Sep 17 00:00:00 2001 From: "F.D.Castel" Date: Mon, 20 Apr 2026 12:50:55 -0300 Subject: [PATCH 17/18] Trim PR #289 to ASAN/Valgrind CI scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Revert the MainUnicode.cpp changes (832d8e7, 0f8f90f, bea6403) per irodushka's plan in #289 comment 4279111183 — the Unicode rewrite is moving to its own PR, preceded by a widechar-tests PR. Restore continue-on-error on the ASAN matrix job temporarily; Valgrind stays strict. ASAN will flag the heap-buffer-overflow in SQLGetDiagRecW again; this is the known issue the follow-up PRs will fix and is tracked as issue #287 Tier 1b. --- .github/workflows/build-and-test.yml | 6 +++ MainUnicode.cpp | 68 ++++------------------------ 2 files changed, 14 insertions(+), 60 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index afc59899..44db8034 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -65,6 +65,12 @@ jobs: runs-on: ${{ matrix.os }} + # Temporary: the ASAN job is expected to fail on the heap-buffer-overflow + # inside SQLGetDiagRecW until the Linux widechar conversion paths in + # MainUnicode.cpp are rewritten. Tracked in issue #287 Tier 1b. Revert + # this back to strict once the Unicode fix PR lands. + continue-on-error: ${{ matrix.sanitizer == 'Asan' }} + steps: - uses: actions/checkout@v6 with: diff --git a/MainUnicode.cpp b/MainUnicode.cpp index 5222a7c2..1d3c8486 100644 --- a/MainUnicode.cpp +++ b/MainUnicode.cpp @@ -38,21 +38,6 @@ extern FILE *logFile; using namespace OdbcJdbcLibrary; -#ifndef _WINDOWS -// SQLWCHAR-aware length (in SQLWCHAR units), safe on Linux where -// sizeof(wchar_t) != sizeof(SQLWCHAR). Do NOT use wcslen() on SQLWCHAR -// data on Linux — it reads two SQLWCHARs per wchar_t and runs off the end. -static size_t sqlwcharLen( const SQLWCHAR *s ) -{ - size_t n = 0; - if ( !s ) - return 0; - while ( s[n] ) - ++n; - return n; -} -#endif - #ifdef _WINDOWS extern UINT codePage; // from Main.cpp #endif @@ -100,7 +85,7 @@ class ConvertingString if ( length == SQL_NTS ) lengthString = 0; else if ( retCountOfBytes ) - lengthString = length / sizeof(SQLWCHAR); + lengthString = length / sizeof(wchar_t); else lengthString = length; } @@ -150,33 +135,13 @@ class ConvertingString if ( len > 0 ) len--; #else - // SQLWCHAR is 2 bytes on Linux (unixODBC defines it as unsigned short), - // but wchar_t is 4 bytes, so mbstowcs((wchar_t*)unicodeString, ...) - // both corrupts the output and risks overflowing the caller's buffer. - // Widen byte-by-byte into SQLWCHAR units, matching what unixODBC's - // ansi_to_unicode_copy() does internally. This is correct for the - // ASCII-only error/state strings that reach this code path; non-ASCII - // input will be handled by the broader ConvertingString rewrite tracked - // in issue #287 (Tier 9.1). - { - const SQLCHAR *src = byteString; - size_t i = 0; - while ( i < (size_t)lengthString && src[i] != 0 ) - { - unicodeString[i] = (SQLWCHAR)( src[i] & 0xFF ); - ++i; - } - len = i; - } + len = mbstowcs( (wchar_t*)unicodeString, (const char*)byteString, lengthString ); #endif } if ( len > 0 ) { - // NUL-terminate in SQLWCHAR units. LPWSTR assignment of L'\0' writes - // sizeof(wchar_t) bytes, which overruns the output buffer by 2 bytes - // on Linux. - unicodeString[len] = 0; + *(LPWSTR)(unicodeString + len) = L'\0'; if ( realLength ) { @@ -205,18 +170,12 @@ class ConvertingString wchar_t saveWC; if ( length == SQL_NTS ) -#ifdef _WINDOWS length = (int)wcslen( (const wchar_t*)wcString ); -#else - length = (int)sqlwcharLen( wcString ); -#endif - else if ( wcString[length] != 0 ) + else if ( wcString[length] != L'\0' ) { ptEndWC = (wchar_t*)&wcString[length]; saveWC = *ptEndWC; - // Write a SQLWCHAR-sized NUL so we don't overrun the input by 2 bytes - // on Linux (wchar_t is 4 bytes there). - wcString[length] = 0; + *ptEndWC = L'\0'; } if ( connection ) @@ -226,10 +185,7 @@ class ConvertingString #ifdef _WINDOWS bytesNeeded = WideCharToMultiByte( codePage, (DWORD)0, wcString, length, NULL, (int)0, NULL, NULL ); #else - // See the symmetric comment in the destructor above: wcstombs assumes - // wchar_t-sized input, which corrupts SQLWCHAR data on Linux. The - // byte-narrowing loop below produces exactly `length` output bytes. - bytesNeeded = (size_t)length; + bytesNeeded = wcstombs( NULL, (const wchar_t*)wcString, length ); #endif } @@ -242,15 +198,7 @@ class ConvertingString #ifdef _WINDOWS bytesNeeded = WideCharToMultiByte( codePage, 0, wcString, length, (LPSTR)byteString, (int)bytesNeeded, NULL, NULL ); #else - { - size_t i = 0; - while ( i < (size_t)length && wcString[i] != 0 ) - { - byteString[i] = (SQLCHAR)( wcString[i] & 0xFF ); - ++i; - } - bytesNeeded = i; - } + bytesNeeded = wcstombs( (char *)byteString, (const wchar_t*)wcString, bytesNeeded ); #endif } @@ -272,7 +220,7 @@ class ConvertingString if ( lengthString ) { byteString = new SQLCHAR[ lengthString + 2 ]; - memset( byteString, 0, lengthString + 2 ); + memset(byteString, 0, lengthString + 2); } else byteString = NULL; From 98115a17884972d2ac7232644bb0fe5e29c958fe Mon Sep 17 00:00:00 2001 From: "F.D.Castel" Date: Mon, 20 Apr 2026 14:58:57 -0300 Subject: [PATCH 18/18] Disable ASAN CI job until Linux widechar paths are fixed Per #289 discussion with @irodushka: rather than keep ASAN running under continue-on-error, disable it outright until the Unicode rewrite lands (draft PR #291, tracked as #287 Tier 1b). Re-enable the matrix entry once #291 merges. Valgrind remains strict. --- .github/workflows/build-and-test.yml | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 44db8034..38a41edd 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -56,21 +56,19 @@ jobs: firebird-branch: master # ── Linux x64 sanitizers (Debug, Firebird 5.0.3) ───────────────────── - - os: ubuntu-22.04 - sanitizer: Asan - firebird-version: '5.0.3' + # ASAN is temporarily disabled — re-enable once the Linux widechar + # conversion paths in MainUnicode.cpp are rewritten (#287 Tier 1b, + # draft PR #291). Until then it fails on a heap-buffer-overflow in + # SQLGetDiagRecW. + # - os: ubuntu-22.04 + # sanitizer: Asan + # firebird-version: '5.0.3' - os: ubuntu-22.04 sanitizer: Valgrind firebird-version: '5.0.3' runs-on: ${{ matrix.os }} - # Temporary: the ASAN job is expected to fail on the heap-buffer-overflow - # inside SQLGetDiagRecW until the Linux widechar conversion paths in - # MainUnicode.cpp are rewritten. Tracked in issue #287 Tier 1b. Revert - # this back to strict once the Unicode fix PR lands. - continue-on-error: ${{ matrix.sanitizer == 'Asan' }} - steps: - uses: actions/checkout@v6 with: