Add ASAN and Valgrind integration for CI test suite by fdcastel · Pull Request #289 · FirebirdSQL/firebird-odbc-driver

fdcastel · 2026-04-10T23:27:58Z

Summary

Integrate AddressSanitizer (ASAN) and Valgrind into the CI test pipeline so that memory bugs (buffer overflows, use-after-free, leaks, etc.) are caught automatically on every PR.

Closes #288

Changes

CMake (`CMakeLists.txt`)

ENABLE_ASAN option (default OFF, Linux/GCC/Clang only): appends -fsanitize=address -fno-omit-frame-pointer to compile and link flags. Suppresses -DLOGGING in Debug builds for cleaner sanitizer output.
ENABLE_VALGRIND option (default OFF): finds the valgrind binary and configures MEMORYCHECK_COMMAND / MEMORYCHECK_COMMAND_OPTIONS for ctest -T memcheck.
Mutual exclusion enforced: enabling both emits a FATAL_ERROR.
Both options are guarded behind if(NOT MSVC) since MSVC ASAN has different semantics (future work).

CMake Presets (`CMakePresets.json`)

asan configure/build/test presets inheriting from debug, with ASAN_OPTIONS environment variable.
valgrind configure/build/test presets inheriting from debug, with a 600s test timeout (Valgrind is ~10-20x slower).

Build script (`firebird-odbc-driver.build.ps1`)

New -Sanitizer parameter accepting None, Asan, Valgrind.
Passes the corresponding -DENABLE_ASAN=ON or -DENABLE_VALGRIND=ON to CMake.
Sets ASAN_OPTIONS / LSAN_OPTIONS environment variables at runtime for ASAN builds.
Emits a warning and falls back to normal build on Windows.

CI (`build-and-test.yml`)

New Linux x64 ASAN matrix entry (ubuntu-22.04, Debug config).
New Linux x64 Valgrind matrix entry (ubuntu-22.04, Debug config) with valgrind package installation.
Sanitizer jobs set ASAN_OPTIONS and LSAN_OPTIONS environment variables.
Sanitizer jobs do not upload release artifacts.

Valgrind suppressions (`valgrind.supp`)

Initial suppressions file seeded with a rule for libfbclient.so global initialization leaks.
To be populated as false positives are discovered.

Local usage

# ASAN build
cmake --preset asan
cmake --build --preset asan
ctest --preset asan

# Valgrind build
cmake --preset valgrind
cmake --build --preset valgrind
ctest --preset valgrind

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 FirebirdSQL#288/FirebirdSQL#289) Test: DataTypeTest.SmallintRoundTrip -> ExecIgnoreError -> SQLExecDirect -> unixODBC dispatcher -> SQLGetDiagRecW -> sqlGetDiagRec(strcpy)

fdcastel · 2026-04-10T23:58:21Z

Yay! They are working!!!

fdcastel · 2026-04-11T00:05:44Z

Bug found and fixed by ASAN

The ASAN CI job introduced by this PR immediately caught a real memory bug in the driver.

What ASAN reported

The first ASAN run failed on DataTypeTest.SmallintRoundTrip with:

AddressSanitizer: heap-buffer-overflow
WRITE of size 6 at ... (0 bytes past end of 5-byte region)
  #0 __interceptor_strcpy
  #1 OdbcError::sqlGetDiagRec
  #2 SQLGetDiagRecW

The full output is in the CI job log: GitHub Actions -> Build and Test -> job ubuntu-22.04 / Debug / Asan -> step Run build script.

Root cause

SQLGetDiagRecW allocates a narrow-char work buffer for the SQL state:

ConvertingString<> State( 12, sqlState );

In ConvertingString's constructor, the byte-count to char-count conversion was:

lengthString = length / sizeof(wchar_t);   // BUG

On Windows sizeof(wchar_t) == sizeof(SQLWCHAR) == 2, so 12/2 = 6 -> 8-byte buffer, no overflow.
On Linux sizeof(wchar_t) == 4 but sizeof(SQLWCHAR) == 2 (unixODBC defines SQLWCHAR as unsigned short), so 12/4 = 3 -> 5-byte buffer.

OdbcError::sqlGetDiagRec then copies the SQL state ("HY000\0", 6 bytes) into that 5-byte buffer via strcpy -> 1-byte heap overflow. The same latent bug exists in SQLErrorW (same State(12, sqlState) pattern).

The bug was silently harmless on Windows and went undetected for years.

Fix

One-line change in MainUnicode.cpp (commit 636972b):

// Before
lengthString = length / sizeof(wchar_t);
// After
lengthString = length / sizeof(SQLWCHAR);

sizeof(SQLWCHAR) == 2 on all platforms, giving a correct 8-byte buffer on Linux.

CI after fix

All 4 matrix jobs now pass with no continue-on-error guard:

Job	Result
ubuntu-22.04 / Release	pass
ubuntu-22.04 / Debug / ASAN	pass (375/375 tests)
ubuntu-22.04 / Debug / Valgrind	pass
windows-latest / Release	pass

This is a concrete demonstration that ASAN integration pays off immediately.

fdcastel · 2026-04-11T00:10:09Z

@irodushka Enjoy! 😉

irodushka · 2026-04-11T18:57:48Z

Hi @fdcastel

Can you please resolve the conflicts after merging PR#286
I'll look at this PR after Orthodox Easter)

Cheers!

fdcastel · 2026-04-11T20:05:37Z

Can you please resolve the conflicts after merging PR#286 I'll look at this PR after Orthodox Easter)

Sure! I’ll look into this in a few hours.

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 FirebirdSQL#288

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.

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.

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 FirebirdSQL#288/FirebirdSQL#289) Test: DataTypeTest.SmallintRoundTrip -> ExecIgnoreError -> SQLExecDirect -> unixODBC dispatcher -> SQLGetDiagRecW -> sqlGetDiagRec(strcpy)

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.

fdcastel · 2026-04-11T20:31:41Z

@irodushka Rebased onto current master.

Conflicts were in build-and-test.yml and firebird-odbc-driver.build.ps1, caused by the expanded matrix (Windows ARM64, Windows x86/WoW64, Linux ARM64, multi-Firebird-version runs) introduced by master.

Resolution: kept all new upstream matrix entries and added the sanitizer entries on top; merged the Architecture and Sanitizer params in the build script so both features coexist.

fdcastel marked this pull request as draft April 10, 2026 23:45

fdcastel marked this pull request as ready for review April 11, 2026 00:09

fdcastel mentioned this pull request Apr 11, 2026

[EPIC] vNext Migration Plan #287

Open

fdcastel added 5 commits April 11, 2026 17:24

Remove continue-on-error from ASAN CI job

8ea653f

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.

fdcastel force-pushed the fix-issue-288 branch from d1a651d to 8ea653f Compare April 11, 2026 20:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ASAN and Valgrind integration for CI test suite#289

Add ASAN and Valgrind integration for CI test suite#289
fdcastel wants to merge 5 commits intoFirebirdSQL:masterfrom
fdcastel:fix-issue-288

fdcastel commented Apr 10, 2026 •

edited

Loading

Uh oh!

fdcastel commented Apr 10, 2026

Uh oh!

fdcastel commented Apr 11, 2026

Uh oh!

fdcastel commented Apr 11, 2026

Uh oh!

irodushka commented Apr 11, 2026

Uh oh!

fdcastel commented Apr 11, 2026

Uh oh!

fdcastel commented Apr 11, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

fdcastel commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

CMake (CMakeLists.txt)

CMake Presets (CMakePresets.json)

Build script (firebird-odbc-driver.build.ps1)

CI (build-and-test.yml)

Valgrind suppressions (valgrind.supp)

Local usage

Uh oh!

fdcastel commented Apr 10, 2026

Uh oh!

fdcastel commented Apr 11, 2026

Bug found and fixed by ASAN

What ASAN reported

Root cause

Fix

CI after fix

Uh oh!

fdcastel commented Apr 11, 2026

Uh oh!

irodushka commented Apr 11, 2026

Uh oh!

fdcastel commented Apr 11, 2026

Uh oh!

fdcastel commented Apr 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fdcastel commented Apr 10, 2026 •

edited

Loading

CMake (`CMakeLists.txt`)

CMake Presets (`CMakePresets.json`)

Build script (`firebird-odbc-driver.build.ps1`)

CI (`build-and-test.yml`)

Valgrind suppressions (`valgrind.supp`)

fdcastel commented Apr 11, 2026 •

edited

Loading