From 6042764e41f8eca26b0a022b00189cf8d540d426 Mon Sep 17 00:00:00 2001 From: ColinLee Date: Tue, 3 Nov 2026 12:30:32 +0800 Subject: [PATCH] [C++] Instrument TsFile sources in ENABLE_ASAN builds The ASan compile flags were appended before add_subdirectory(third_party), where CMAKE_CXX_FLAGS is reset, and were only re-applied for the Python binding build. C++-only ASan jobs therefore linked the sanitizer runtime without compiling any instrumentation: leaks were still visible through malloc/free interposition, but heap-buffer-overflow, use-after-free and all UBSan checks were inactive for TsFile code. - Keep the sanitizer compile flags in TSFILE_SANITIZER_COMPILE_FLAGS and re-apply them after third_party for every ENABLE_ASAN build; third-party code stays uninstrumented. - Link the sanitizer runtime dynamically into libtsfile, the executables and the tools. A macOS dylib cannot carry unresolved __asan_* symbols, and two private runtimes in one process are unsupported, so the static-runtime branch is removed along with TSFILE_SANITIZER_SHARED_RUNTIME. - Skip LTO in Release builds whenever ASan is enabled, matching the binding build. - Add an asan_negative_control CTest case that deliberately writes past a heap allocation. A CMake driver checks the captured output for the sanitizer report instead of relying on the exit status, because ASan aborts on some platforms and CTest turns a subprocess abort into a failure. A build that loses instrumentation fails the test. Fixes #1062. --- cpp/CLAUDE.md | 6 +-- cpp/CMakeLists.txt | 72 ++++++++++------------------ cpp/build.sh | 9 ---- cpp/pom.xml | 16 +++---- cpp/test/CMakeLists.txt | 14 ++++++ cpp/test/asan_negative_control.cc | 38 +++++++++++++++ cpp/test/asan_negative_control.cmake | 45 +++++++++++++++++ python/SANITIZER.md | 19 ++++---- 8 files changed, 141 insertions(+), 78 deletions(-) create mode 100644 cpp/test/asan_negative_control.cc create mode 100644 cpp/test/asan_negative_control.cmake diff --git a/cpp/CLAUDE.md b/cpp/CLAUDE.md index a33e6d9f7..32d045e22 100644 --- a/cpp/CLAUDE.md +++ b/cpp/CLAUDE.md @@ -33,8 +33,7 @@ bash build.sh bash build.sh -t=Debug bash build.sh -t=RelWithDebInfo bash build.sh -a=ON # Enable AddressSanitizer -bash build.sh -a=ON --sanitizer-shared-runtime=ON \ - --sanitizer-checks=address # For the Python bindings (see python/SANITIZER.md) +bash build.sh -a=ON --sanitizer-checks=address # Match the Python binding build (see python/SANITIZER.md) bash build.sh -c=ON # Enable code coverage # Disable optional compression libraries @@ -71,8 +70,7 @@ All `ON` by default unless noted: | `ENABLE_LZMA2` | LZMA2 compression (`OFF` by default; bundled XZ requires CMake 3.20+) | | `ENABLE_THREADS` | Multi-threaded read/write via pthreads | | `ENABLE_ASAN` | AddressSanitizer (`OFF` by default) | -| `TSFILE_SANITIZER_CHECKS` | `-fsanitize=` list used by `ENABLE_ASAN` (default `address,undefined`) | -| `TSFILE_SANITIZER_SHARED_RUNTIME` | Link the sanitizer runtime dynamically into shared libraries; required when Python bindings preload it (`OFF` by default) | +| `TSFILE_SANITIZER_CHECKS` | `-fsanitize=` list used by `ENABLE_ASAN` (default `address,undefined`); TsFile targets are compiled and linked dynamically against it | | `ENABLE_SIMD` | SIMD acceleration | ## Source Structure diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index fff34cee6..3be4a7fd6 100755 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -161,13 +161,6 @@ option(ENABLE_ASAN "Enable Address Sanitizer" OFF) set(TSFILE_SANITIZER_CHECKS "address,undefined" CACHE STRING "Sanitizer checks passed to -fsanitize= when ENABLE_ASAN is ON (for example 'address' or 'address,undefined')") -# Python bindings preload the sanitizer runtime, so instrumented shared -# libraries must not embed a private copy of it. Keep this OFF for the C++ test -# builds that use -static-libasan, and turn it ON for the coordinated -# C++/Python binding build. -option(TSFILE_SANITIZER_SHARED_RUNTIME - "Link the sanitizer runtime dynamically into shared libraries (for preloaded Python bindings)" - OFF) if (NOT CMAKE_BUILD_TYPE) set(CMAKE_BUILD_TYPE "Release" CACHE STRING "Choose the type of build." FORCE) @@ -226,12 +219,9 @@ else () # (see TSFILE_ENABLE_NATIVE_ARCH above): the default target for the # deployment platform is the portable baseline, and on macOS arm64 # it still includes +crc so snappy's fast path stays available. - if (ENABLE_ASAN AND TSFILE_SANITIZER_SHARED_RUNTIME) + if (ENABLE_ASAN) # LTO loses the stack frames sanitizer reports need, and its # additional optimization pipeline complicates symbolization. - # Only the binding mode changes optimization: the C++ test mode - # keeps its historical flags so its static runtime stays in - # sync with how libtsfile is linked. set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -O3") elseif (MINGW OR WIN32) set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -O3") @@ -251,8 +241,7 @@ endif () message("CMAKE DEBUG: CMAKE_CXX_FLAGS=${CMAKE_CXX_FLAGS}") if (ENABLE_ASAN) - message("Address Sanitizer is enabled: checks=${TSFILE_SANITIZER_CHECKS}, " - "shared_runtime=${TSFILE_SANITIZER_SHARED_RUNTIME}") + message("Address Sanitizer is enabled: checks=${TSFILE_SANITIZER_CHECKS}") if (MSVC) # MSVC ships AddressSanitizer; it requires Visual Studio 2019 16.9 or # newer (MSVC_VERSION >= 1928). Only the address sanitizer is available @@ -278,33 +267,24 @@ if (ENABLE_ASAN) endforeach () add_link_options(/INCREMENTAL:NO /DEBUG) elseif (NOT WIN32) - # Compile flags here only survive until add_subdirectory(third_party) - # resets CMAKE_CXX_FLAGS. The binding mode re-applies them after that - # call; the C++ test mode keeps its historical behavior. Link flags set - # here persist. - set(CMAKE_CXX_FLAGS - "${CMAKE_CXX_FLAGS} -fsanitize=${TSFILE_SANITIZER_CHECKS} -fno-omit-frame-pointer") - - if (TSFILE_SANITIZER_SHARED_RUNTIME) - # The runtime stays dynamic so one preloaded copy serves Python, the - # extensions and libtsfile. Debug info is required for useful - # reports in every build type. - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g") - set(CMAKE_SHARED_LINKER_FLAGS - "${CMAKE_SHARED_LINKER_FLAGS} -fsanitize=${TSFILE_SANITIZER_CHECKS}") - set(CMAKE_EXE_LINKER_FLAGS - "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=${TSFILE_SANITIZER_CHECKS}") - elseif (NOT APPLE) - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -static-libasan") - set(CMAKE_EXE_LINKER_FLAGS - "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=${TSFILE_SANITIZER_CHECKS} -static-libasan") - if (TSFILE_SANITIZER_CHECKS MATCHES "undefined") - set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -static-libubsan") - endif () - else () - set(CMAKE_EXE_LINKER_FLAGS - "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=${TSFILE_SANITIZER_CHECKS}") - endif () + # Every TsFile target (libtsfile, the tools, the test executable) is + # compiled with instrumentation, and every binary that contains + # instrumented code links the sanitizer runtime dynamically: a macOS + # dylib cannot carry unresolved __asan_* symbols, and embedding a + # private runtime in both libtsfile and the executable is unsupported. + # + # Compile flags set here do not survive the CMAKE_CXX_FLAGS reset before + # add_subdirectory(third_party); they are stored in + # TSFILE_SANITIZER_COMPILE_FLAGS and re-applied for the TsFile targets + # once the bundled third-party code is configured (so that code stays + # uninstrumented). Link flags set here persist. + set(TSFILE_SANITIZER_COMPILE_FLAGS + "-fsanitize=${TSFILE_SANITIZER_CHECKS} -fno-omit-frame-pointer -g") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${TSFILE_SANITIZER_COMPILE_FLAGS}") + set(CMAKE_SHARED_LINKER_FLAGS + "${CMAKE_SHARED_LINKER_FLAGS} -fsanitize=${TSFILE_SANITIZER_CHECKS}") + set(CMAKE_EXE_LINKER_FLAGS + "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=${TSFILE_SANITIZER_CHECKS}") else () message(WARNING "ENABLE_ASAN on Windows is only supported with the MSVC toolchain; " @@ -436,13 +416,11 @@ endif () add_subdirectory(third_party) # third_party intentionally rebuilds with a minimal CMAKE_CXX_FLAGS so bundled -# projects do not inherit TsFile's warning flags. The binding mode re-applies -# the sanitizer instrumentation for the TsFile targets that follow. The C++ -# test mode keeps its historical static-runtime setup, so re-applying here -# would mix a dynamic runtime in libtsfile with the executable's static one. -if (ENABLE_ASAN AND TSFILE_SANITIZER_SHARED_RUNTIME AND NOT MSVC AND NOT WIN32) - set(CMAKE_CXX_FLAGS - "${CMAKE_CXX_FLAGS} -fsanitize=${TSFILE_SANITIZER_CHECKS} -fno-omit-frame-pointer -g") +# projects do not inherit TsFile's warning flags. Re-apply the sanitizer +# instrumentation for the TsFile targets that follow; without it TsFile sources +# would link the sanitizer runtime but carry no compiler-inserted checks. +if (ENABLE_ASAN AND NOT MSVC AND NOT WIN32) + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${TSFILE_SANITIZER_COMPILE_FLAGS}") endif () # CMake 3.11 cannot call target_link_libraries() on object-library targets, but diff --git a/cpp/build.sh b/cpp/build.sh index 47179ea13..fb0ff1cbf 100644 --- a/cpp/build.sh +++ b/cpp/build.sh @@ -30,7 +30,6 @@ run_cov_only=0 enable_antlr4=ON enable_asan=OFF sanitizer_checks=address,undefined -sanitizer_shared_runtime=OFF enable_snappy=ON enable_lz4=ON @@ -62,10 +61,6 @@ Options: --sanitizer-checks= Checks passed to -fsanitize= when ASan is enabled (default: address,undefined). - --sanitizer-shared-runtime= - Link the sanitizer runtime dynamically into shared - libraries; required when building the Python bindings, - which preload libasan (default: OFF). -c= Enable or disable code coverage. --build-shared= Build libtsfile as a shared library (default: ON). @@ -100,7 +95,6 @@ function print_config() echo "enable_cov=$enable_cov" echo "enable_asan=$enable_asan" echo "sanitizer_checks=$sanitizer_checks" - echo "sanitizer_shared_runtime=$sanitizer_shared_runtime" echo "enable_antlr4=$enable_antlr4" echo "enable_snappy=$enable_snappy" echo "enable_lz4=$enable_lz4" @@ -139,8 +133,6 @@ parse_options() enable_asan=$(get_key_value "$1");; --sanitizer-checks=*) sanitizer_checks=$(get_key_value "$1");; - --sanitizer-shared-runtime=*) - sanitizer_shared_runtime=$(get_key_value "$1");; -c=*) enable_cov=$(get_key_value "$1");; -c) @@ -241,7 +233,6 @@ cmake ../../ \ -DENABLE_COV=$enable_cov \ -DENABLE_ASAN=$enable_asan \ -DTSFILE_SANITIZER_CHECKS=$sanitizer_checks \ - -DTSFILE_SANITIZER_SHARED_RUNTIME=$sanitizer_shared_runtime \ -DDEBUG_SE=$debug_se \ -DENABLE_ANTLR4=$enable_antlr4 \ -DBUILD_TSFILE_ONLY=$build_tsfile_only \ diff --git a/cpp/pom.xml b/cpp/pom.xml index 14666e341..97428f194 100644 --- a/cpp/pom.xml +++ b/cpp/pom.xml @@ -35,13 +35,12 @@ Release OFF address,undefined - OFF ON ON BUNDLED @@ -119,7 +118,6 @@ - @@ -305,9 +303,10 @@ with-sanitizer @@ -317,7 +316,6 @@ ON ${tsfile.sanitizer} - ON diff --git a/cpp/test/CMakeLists.txt b/cpp/test/CMakeLists.txt index a9e1fa439..e640a0880 100644 --- a/cpp/test/CMakeLists.txt +++ b/cpp/test/CMakeLists.txt @@ -227,6 +227,20 @@ add_test( -DTEST_ANTLR4_ARCHIVE=${TSFILE_ANTLR4_ARCHIVE_RESOLVED} -P ${CMAKE_SOURCE_DIR}/cmake/tests/ANTLR4DependencyTest.cmake) +if (ENABLE_ASAN) + # Negative control for the sanitizer build: the program performs a + # deliberate out-of-bounds heap write. The driver script decides the + # outcome from the sanitizer report, because CTest treats the abort ASan + # uses on some platforms as a failure regardless of the exit status that + # PASS_REGULAR_EXPRESSION would otherwise absorb. + add_executable(asan_negative_control asan_negative_control.cc) + add_test( + NAME asan_negative_control + COMMAND ${CMAKE_COMMAND} + "-DASAN_NEGATIVE_CONTROL=$" + -P ${CMAKE_CURRENT_SOURCE_DIR}/asan_negative_control.cmake) +endif () + file(GLOB_RECURSE TEST_SRCS "common/*_test.cc" "encoding/*_test.cc" diff --git a/cpp/test/asan_negative_control.cc b/cpp/test/asan_negative_control.cc new file mode 100644 index 000000000..c4c193b9f --- /dev/null +++ b/cpp/test/asan_negative_control.cc @@ -0,0 +1,38 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// Built and registered as a CTest case only when ENABLE_ASAN is ON. +// +// The program deliberately writes past the end of a heap allocation. The +// accompanying CTest entry passes only when the sanitizer reports the access, +// so a build that links the runtime without instrumenting TsFile code fails +// this test instead of silently losing its memory-safety checks. + +#include + +int main() { + char* buffer = static_cast(std::malloc(4)); + if (buffer == nullptr) { + return 0; + } + volatile char* access = buffer; + access[8] = 1; + std::free(buffer); + return 0; +} diff --git a/cpp/test/asan_negative_control.cmake b/cpp/test/asan_negative_control.cmake new file mode 100644 index 000000000..f0c1967b5 --- /dev/null +++ b/cpp/test/asan_negative_control.cmake @@ -0,0 +1,45 @@ +#[[ +Licensed to the Apache Software Foundation (ASF) under one +or more contributor license agreements. See the NOTICE file +distributed with this work for additional information +regarding copyright ownership. The ASF licenses this file +to you under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance +with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, +software distributed under the License is distributed on an +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +KIND, either express or implied. See the License for the +specific language governing permissions and limitations +under the License. +]] + +# CTest driver for asan_negative_control. The program performs a deliberate +# out-of-bounds heap write, so the test passes only when the sanitizer reports +# it. The exit status is ignored on purpose: ASan aborts on some platforms and +# exits with a code on others, and CTest marks a subprocess abort as a failure +# regardless of PASS_REGULAR_EXPRESSION. + +if (NOT DEFINED ASAN_NEGATIVE_CONTROL OR NOT EXISTS "${ASAN_NEGATIVE_CONTROL}") + message(FATAL_ERROR + "ASAN_NEGATIVE_CONTROL does not name an existing executable: " + "${ASAN_NEGATIVE_CONTROL}") +endif () + +execute_process( + COMMAND "${ASAN_NEGATIVE_CONTROL}" + RESULT_VARIABLE _result + OUTPUT_VARIABLE _stdout + ERROR_VARIABLE _stderr) + +set(_combined "${_stdout}\n${_stderr}") +string(FIND "${_combined}" "AddressSanitizer" _report_position) +if (_report_position EQUAL -1) + message(FATAL_ERROR + "The sanitizer did not report the deliberate out-of-bounds write " + "(exit status: ${_result}). The build is probably not compiled " + "with instrumentation.\nOutput:\n${_combined}") +endif () diff --git a/python/SANITIZER.md b/python/SANITIZER.md index 77670c5f6..988ab9a80 100644 --- a/python/SANITIZER.md +++ b/python/SANITIZER.md @@ -43,10 +43,11 @@ across the language boundary. The CI job runs on `ubuntu-24.04` with the distribution GCC and Python 3.11. -The C++-only `ENABLE_ASAN` jobs keep their historical static-runtime -configuration; this mode is the coordinated C++ + bindings build, where the -sanitizer compile flags are also re-applied after the bundled third-party -projects reset `CMAKE_CXX_FLAGS`. +Every `ENABLE_ASAN` build (C++ only or with bindings) now compiles the TsFile +targets with `-fsanitize` and links the sanitizer runtime dynamically; +`third_party` code stays uninstrumented. The bindings build additionally +instruments the Cython extensions and preloads the single shared runtime into +the interpreter. ## Build @@ -64,9 +65,9 @@ projects reset `CMAKE_CXX_FLAGS`. `-Dtsfile.sanitizer=` is the single switch. It: * enables `ENABLE_ASAN` and passes `` to `-fsanitize=` for the C++ - library, adding the same flags to the shared-library link step; -* keeps the sanitizer runtime dynamic (`TSFILE_SANITIZER_SHARED_RUNTIME=ON`) so - one preloaded runtime can serve Python, the extensions and `libtsfile`; + library, adding the same flags to the shared-library and executable links; +* keeps the sanitizer runtime dynamically linked so one preloaded runtime can + serve Python, the extensions and `libtsfile`; * exports `TSFILE_SANITIZER=` to `setup.py`, which instruments every extension (including `python_random_access_read_file.cc`) and verifies the result; @@ -85,8 +86,8 @@ separately, as described below). ### Direct CMake + setup.py ```bash -# 1. C++ library, dynamic sanitizer runtime -bash cpp/build.sh -t=RelWithDebInfo -a=ON --sanitizer-shared-runtime=ON +# 1. C++ library with dynamic sanitizer runtime +bash cpp/build.sh -t=RelWithDebInfo -a=ON # 2. Point setup.py at that build and instrument the extensions cd python