Conversation
slabko
force-pushed
the
use-system-stdlib
branch
from
September 29, 2026 17:36
9d09dc1 to
0cfe85d
Compare
slabko
force-pushed
the
use-system-stdlib
branch
from
September 29, 2026 17:54
0cfe85d to
e96d1ff
Compare
The driver is loaded into arbitrary host processes, so it must not depend on the host's C++ runtime version. Link libstdc++/libgcc statically on Linux/FreeBSD and use the static CRT (/MT) on MSVC unconditionally, and remove the CH_ODBC_RUNTIME_LINK_STATIC option along with the runtime_link CI matrix axis. Third-party libraries keep their existing static/bundled switches for packagers. With a static CRT there is nothing to redistribute on Windows, so drop InstallRequiredSystemLibraries and the RuntimeLibraries CPack component. Add CI checks that the built driver has no shared C++ runtime dependency (readelf on Linux, dumpbin on Windows).
Third-party libraries have been bundled and linked statically for a
long time, yet a set of CMake options still suggested otherwise:
- CH_ODBC_PREFER_BUNDLED_{POCO,ICU} were never referenced.
- CH_ODBC_PREFER_BUNDLED_SSL called find_package(OpenSSL) after the
bundled OpenSSL::SSL/Crypto targets already existed, so it was a no-op.
- CH_ODBC_PREFER_BUNDLED_NANODBC only printed an "unsupported" warning.
- CH_ODBC_THIRD_PARTY_LINK_STATIC only fed the dead system-SSL path.
- CH_ODBC_PREFER_BUNDLED_GOOGLETEST worked, but the vendored copy is the
only one that is tested; always use it.
- CH_ODBC_ENABLE_SSL=OFF compiled out the HTTPS code but still linked
OpenSSL and NetSSL. SSL is now always enabled.
Remove all of them together with the umbrella
CH_ODBC_PREFER_BUNDLED_THIRD_PARTIES, and force BUILD_SHARED_LIBS off
since bundled OpenSSL and Poco read it.
Drop the corresponding CI matrix axis and the now-unneeded system
poco/openssl/icu packages from the workflows and the README.
The project sets CMP0091 to NEW, under which CMake no longer places /MD in CMAKE_<LANG>_FLAGS_<CONFIG>; the runtime is chosen through CMAKE_MSVC_RUNTIME_LIBRARY instead. The /MD -> /MT string replacement therefore never matched anything, and the driver DLL was still linked against the shared CRT, as the new CI check showed. Set CMAKE_MSVC_RUNTIME_LIBRARY to the static runtime before project(), so every target (including bundled third parties) picks it up, and keep the flag replacement only as a fallback for CMake versions without CMP0091.
None of these platforms are supported or tested. Drop their branches from cmake/os.cmake (they now hit the generic 'not supported' error), the FreeBSD-only compile flags, and the FreeBSD case of the static C++ runtime block, whose comment now also points at where the version script actually lives.
…link option The configure step now requires a static libstdc++ on Linux, so list the libstdc++-static package among the Red Hat/CentOS build-time dependencies. Also remove the commented-out CH_ODBC_RUNTIME_LINK_STATIC reference from the test Dockerfile, as the option no longer exists.
With BUILD_TESTING=ON, the test CMake file linked ch_contrib::nanodbc
and ch_contrib::unixodbc PUBLIC into ${libname}-impl, the object library
the driver itself is built from. nanodbc references the Driver Manager's
SQL* entry points, so the whole Driver Manager was pulled into the
driver .so, where its SQLConnect, SQLAllocHandle etc. shadowed the
driver's own ones (same names, matched by the version script's SQL*
glob). Depending on what the linker happened to pull in, this produced
a driver whose SQLAllocHandle was demoted to LOCAL, which unixODBC then
reported as "Driver's SQLAllocHandle on SQL_HANDLE_HENV failed".
- Link nanodbc and the Driver Manager into the test executables only.
- Set the hidden visibility presets before adding contrib/, so that the
bundled third parties (unixODBC, Poco, OpenSSL, ICU...) are compiled
with hidden visibility too instead of relying solely on the version
script at link time.
- Apply the version script in sanitizer builds as well; there was no
reason for the exemption, and it hid this problem.
The driver library now exports exactly the ODBC API (62 symbols) in
every configuration.
Clang links the sanitizer runtimes statically into executables and exports their symbols (--export-dynamic). With the C++ runtime also linked statically into both the test executable and the driver .so, the process ended up with two copies of libstdc++/libgcc_eh, and exceptions thrown inside the driver were unwound with a mix of the two: __cxa_throw resolved into the executable, the personality routine into the driver, and _Unwind_SetGR aborted. In CI this showed up as every ASan test aborting on the first thrown SqlException and every UBSan test failing in SetUp() with an empty exception message. Sanitizer builds are never distributed, so simply use the shared system runtime there. Also document why CMAKE_ENABLE_EXPORTS is needed for UBSan: the driver's __ubsan_handle_* references are resolved against the executable.
The GCC code path in the workflow (install step, CC/CXX mapping, compiler-suffixed artifact name) already existed but was not enabled. Use GCC 12 explicitly, since Ubuntu 22.04 defaults to GCC 11, whose C++23 support is insufficient.
Google Test had no printer for DateTimeParams and fell back to dumping the object's raw bytes, including its uninitialized padding, which was the only thing Valgrind's Memcheck reported across the whole integration suite.
MSan requires every library in the process, including the C++ standard library, to be MSan-instrumented. With the vendored libc++ gone and the driver using the system libstdc++, that is no longer available, and keeping it would mean building and caching an instrumented libc++ in CI, tied to the exact Clang version, plus a no-asm OpenSSL - a lot of machinery for a sanitizer that has found nothing in this code base so far. Valgrind's Memcheck covers the same class of bugs (reads of uninitialized memory, including uninitialized bytes passed to system calls such as send()) without instrumentation, on the regular build, and with no toolchain coupling. The full integration suite runs clean under it for both drivers and with compression on, with no suppressions, in about 2.5 minutes per DSN. The Sanitizer workflow matrix becomes checker = [address, undefined, valgrind]; only the configure flag and the test step differ between the sanitizer and the Valgrind legs. SANITIZE=memory is rejected by CMake with a pointer to Valgrind.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.