diff --git a/.github/workflows/unit-test-python.yml b/.github/workflows/unit-test-python.yml index 22832b608..348da318b 100644 --- a/.github/workflows/unit-test-python.yml +++ b/.github/workflows/unit-test-python.yml @@ -128,3 +128,84 @@ jobs: name: tsfile-${{ runner.os }}-py${{ matrix.python-version }}-whl path: python/dist/tsfile-*.whl retention-days: 1 + + # AddressSanitizer/LeakSanitizer coverage for the complete Python -> Cython + # -> C wrapper -> C++ chain. The build instruments libtsfile and every + # extension with the same compiler and check set; the scenarios run in fresh + # interpreters with leak detection on. See python/SANITIZER.md. + python-sanitizer: + runs-on: ubuntu-24.04 + timeout-minutes: 60 + steps: + - name: Checkout repository + uses: actions/checkout@v7 + + - name: Set up Python + uses: actions/setup-python@v7 + with: + python-version: "3.11" + + - name: Set up JDK 17 + uses: actions/setup-java@v6.0.1 + with: + distribution: corretto + java-version: 17 + + - name: Cache Maven packages + uses: actions/cache@v6 + with: + path: ~/.m2 + key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }} + restore-keys: ${{ runner.os }}-m2- + + - name: Install dependencies + shell: bash + run: | + sudo apt-get update + sudo apt-get install -y uuid-dev binutils + + - name: Build with ASan/LSan + shell: bash + run: | + # One compiler (and therefore one sanitizer runtime) for CMake, + # distutils and the preload below. + export CC=gcc CXX=g++ + ./mvnw -P with-python \ + -Dtsfile.sanitizer=address \ + -Dbuild.type=RelWithDebInfo \ + -Dbuild.test=OFF \ + -Dspotless.skip=true \ + clean test-compile + + - name: Verify sanitizer instrumentation + shell: bash + run: | + python/venv/bin/python python/check_sanitizer_build.py python/tsfile + + - name: Run ownership scenarios under ASan/LSan + shell: bash + run: | + set -o pipefail + # Suppressions must stay scoped to third-party/interpreter frames. + if grep -v '^#' python/tests/sanitizer/lsan_suppressions.txt \ + | grep -Eqi 'tsfile|dataset/_merge|dataset/_index'; then + echo "::error::suppression file must not match TsFile modules" + exit 1 + fi + ASAN_LIB="$(gcc -print-file-name=libasan.so)" + LIBSTDCXX="$(gcc -print-file-name=libstdc++.so.6)" + if [ ! -f "$ASAN_LIB" ] || [ ! -f "$LIBSTDCXX" ]; then + echo "::error::AddressSanitizer or libstdc++ runtime not found for gcc" + exit 1 + fi + # The pytest runner itself does not load tsfile; each scenario child + # re-enables leak detection (see test_sanitizer_ownership.py). + # + # libasan must stay first (runtime-first check); libstdc++ follows so + # ASan's __cxa_throw interceptor can resolve the real symbol. Without + # it, any C++ exception aborts with "CHECK failed ... __cxa_throw". + export TSFILE_SANITIZER=address + export ASAN_OPTIONS="detect_leaks=0:abort_on_error=1:halt_on_error=1" + export LSAN_OPTIONS="suppressions=$PWD/python/tests/sanitizer/lsan_suppressions.txt" + LD_PRELOAD="$ASAN_LIB:$LIBSTDCXX" python/venv/bin/python -m pytest \ + python/tests/test_sanitizer_ownership.py -v diff --git a/cpp/CLAUDE.md b/cpp/CLAUDE.md index 02dc4186e..a33e6d9f7 100644 --- a/cpp/CLAUDE.md +++ b/cpp/CLAUDE.md @@ -33,6 +33,8 @@ 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 -c=ON # Enable code coverage # Disable optional compression libraries @@ -69,6 +71,8 @@ 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) | | `ENABLE_SIMD` | SIMD acceleration | ## Source Structure diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index f9cb5741b..fff34cee6 100755 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -155,6 +155,20 @@ if (ENABLE_MEM_STAT) endif () +# Sanitizer configuration. Declared before the build-type block because the +# Release flags below must know whether LTO may be used. +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) endif () @@ -212,7 +226,14 @@ 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 (MINGW OR WIN32) + if (ENABLE_ASAN AND TSFILE_SANITIZER_SHARED_RUNTIME) + # 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") elseif (TSFILE_ENABLE_NATIVE_ARCH) set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -O3 -march=native -flto") @@ -229,11 +250,9 @@ else () endif () message("CMAKE DEBUG: CMAKE_CXX_FLAGS=${CMAKE_CXX_FLAGS}") -# disable asan by default. -option(ENABLE_ASAN "Enable Address Sanitizer" OFF) - if (ENABLE_ASAN) - message("Address Sanitizer is enabled.") + message("Address Sanitizer is enabled: checks=${TSFILE_SANITIZER_CHECKS}, " + "shared_runtime=${TSFILE_SANITIZER_SHARED_RUNTIME}") if (MSVC) # MSVC ships AddressSanitizer; it requires Visual Studio 2019 16.9 or # newer (MSVC_VERSION >= 1928). Only the address sanitizer is available @@ -259,13 +278,32 @@ if (ENABLE_ASAN) endforeach () add_link_options(/INCREMENTAL:NO /DEBUG) elseif (NOT WIN32) - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address,undefined -fno-omit-frame-pointer") - - if (NOT APPLE) + # 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=address,undefined -static-libasan -static-libubsan") + 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=address,undefined") + set(CMAKE_EXE_LINKER_FLAGS + "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=${TSFILE_SANITIZER_CHECKS}") endif () else () message(WARNING @@ -397,6 +435,16 @@ else () 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") +endif () + # CMake 3.11 cannot call target_link_libraries() on object-library targets, but # directory-level link_libraries() initializes both their usage requirements # and the final library's link dependencies. Apply every enabled project diff --git a/cpp/build.sh b/cpp/build.sh index 0ca86c4f2..47179ea13 100644 --- a/cpp/build.sh +++ b/cpp/build.sh @@ -28,6 +28,9 @@ enable_cov=0 debug_se=0 run_cov_only=0 enable_antlr4=ON +enable_asan=OFF +sanitizer_checks=address,undefined +sanitizer_shared_runtime=OFF enable_snappy=ON enable_lz4=ON @@ -56,6 +59,13 @@ Commands: Options: -t=, -t Build type: Debug, Release, RelWithDebInfo, MinSizeRel. -a= Enable or disable AddressSanitizer. + --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). @@ -89,6 +99,8 @@ function print_config() echo "use_cpp11=$use_cpp11" 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" @@ -125,6 +137,10 @@ parse_options() -a) shift 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) @@ -223,6 +239,9 @@ cmake ../../ \ -DCMAKE_BUILD_TYPE=$build_type \ -DUSE_CPP11=$use_cpp11 \ -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 7a4d960e3..14666e341 100644 --- a/cpp/pom.xml +++ b/cpp/pom.xml @@ -34,6 +34,14 @@ OFF Release OFF + + address,undefined + OFF ON ON BUNDLED @@ -110,6 +118,8 @@ + + @@ -291,6 +301,25 @@ + + with-sanitizer + + + + tsfile.sanitizer + + + + ON + ${tsfile.sanitizer} + ON + + with-code-coverage diff --git a/cpp/src/cwrapper/tsfile_cwrapper.cc b/cpp/src/cwrapper/tsfile_cwrapper.cc index c0fd7f842..be97ef484 100644 --- a/cpp/src/cwrapper/tsfile_cwrapper.cc +++ b/cpp/src/cwrapper/tsfile_cwrapper.cc @@ -1173,6 +1173,14 @@ static void free_device_schema_array(DeviceSchema* schemas, size_t size) { free(schemas); } +void tsfile_free_table_schema_array(TableSchema* schemas, uint32_t length) { + free_table_schema_array(schemas, length); +} + +void tsfile_free_device_schema_array(DeviceSchema* schemas, uint32_t length) { + free_device_schema_array(schemas, length); +} + ERRNO tsfile_reader_get_table_schema_checked(TsFileReader reader, const char* table_name, TableSchema* out_schema) { @@ -2170,6 +2178,8 @@ ERRNO tsfile_reader_get_tsfile_properties(TsFileReader reader, return common::E_INVALID_ARG; } + TsFileProperty* properties = nullptr; + uint32_t property_index = 0; try { auto* r = static_cast(reader); storage::TsFileProperties cpp_properties = r->get_tsfile_properties(); @@ -2183,14 +2193,13 @@ ERRNO tsfile_reader_get_tsfile_properties(TsFileReader reader, return common::E_OK; } - auto* properties = static_cast( + properties = static_cast( malloc(sizeof(TsFileProperty) * cpp_properties.size())); if (properties == nullptr) { return common::E_OOM; } memset(properties, 0, sizeof(TsFileProperty) * cpp_properties.size()); - uint32_t property_index = 0; for (const auto& cpp_property : cpp_properties) { TsFileProperty& property = properties[property_index]; if (cpp_property.first.size() > @@ -2241,6 +2250,8 @@ ERRNO tsfile_reader_get_tsfile_properties(TsFileReader reader, *out_length = static_cast(cpp_properties.size()); return common::E_OK; } catch (const std::bad_alloc&) { + // get_tsfile_properties() can throw after the array was allocated. + tsfile_free_tsfile_properties(properties, property_index + 1); return common::E_OOM; } } @@ -2296,6 +2307,8 @@ void free_table_schema(TableSchema schema) { } void free_column_schema(ColumnSchema schema) { free(schema.column_name); } +void tsfile_free_buffer(void* buffer) { free(buffer); } + void free_write_file(WriteFile* write_file) { auto f = static_cast(*write_file); delete f; diff --git a/cpp/src/cwrapper/tsfile_cwrapper.h b/cpp/src/cwrapper/tsfile_cwrapper.h index ee1158f61..6d9985c01 100644 --- a/cpp/src/cwrapper/tsfile_cwrapper.h +++ b/cpp/src/cwrapper/tsfile_cwrapper.h @@ -919,7 +919,8 @@ TSFILE_RESULT_SET_GET_VALUE_BY_NAME(double); /** * @brief Gets string value from current row by column name. * - * @return char* - String pointer. Caller must free this ptr after usage. + * @return char* - String pointer. The caller owns it and must release it with + * tsfile_free_buffer() so the deallocator matches the allocating module. */ char* tsfile_result_set_get_value_by_name_string(ResultSet result_set, const char* column_name); @@ -946,7 +947,8 @@ TSFILE_RESULT_SET_GET_VALUE_BY_INDEX(bool); /** * @brief Gets string value from current row by column index. * - * @return char* - String pointer. Caller must free this ptr after usage. + * @return char* - String pointer. The caller owns it and must release it with + * tsfile_free_buffer() so the deallocator matches the allocating module. */ char* tsfile_result_set_get_value_by_index_string(ResultSet result_set, uint32_t column_index); @@ -954,8 +956,10 @@ char* tsfile_result_set_get_value_by_index_string(ResultSet result_set, /** * @brief Copies a binary value from the current row by 1-based column index. * - * The caller owns the returned buffer and must release it with free(). An - * empty non-null value returns E_OK with a NULL buffer and length zero. + * The caller owns the returned buffer and must release it with + * tsfile_free_buffer(). An empty non-null value returns E_OK with a NULL + * buffer and length zero. A NULL cell returns E_INVALID_ARG; check + * tsfile_result_set_is_null_by_index() first. */ ERRNO tsfile_result_set_get_value_by_index_binary(ResultSet result_set, uint32_t column_index, @@ -1049,8 +1053,8 @@ ERRNO tsfile_reader_get_table_schema_checked(TsFileReader reader, * @return Schema array, or NULL when there are no tables or an error occurs. * Use tsfile_reader_get_all_table_schemas_checked() when the error code is * required. - * @note size is zero on failure. Release each schema with free_table_schema(), - * then release the array with free(). + * @note size is zero on failure. Release the result with + * tsfile_free_table_schema_array(). */ TableSchema* tsfile_reader_get_all_table_schemas(TsFileReader reader, uint32_t* size); @@ -1060,8 +1064,8 @@ TableSchema* tsfile_reader_get_all_table_schemas(TsFileReader reader, * Both output parameters are required. Non-null outputs are set to NULL/0 * before work begins and remain empty on failure. An empty file result is * successful with RET_OK and NULL/0 outputs. - * @note On success, release each schema with free_table_schema(), then free() - * the array. Partial results are released internally on failure. + * @note On success, release the result with tsfile_free_table_schema_array(). + * Partial results are released internally on failure. */ ERRNO tsfile_reader_get_all_table_schemas_checked(TsFileReader reader, TableSchema** out_schemas, @@ -1081,8 +1085,8 @@ TableSchema* tsfile_reader_get_all_table_schemas_with_error(TsFileReader reader, * Use tsfile_reader_get_all_timeseries_schemas_checked() when the error code * is required. * @return Array on success, or NULL for an empty result or an error. - * @note size is zero on failure. Release each schema with free_device_schema(), - * then release the array with free(). + * @note size is zero on failure. Release the result with + * tsfile_free_device_schema_array(). */ DeviceSchema* tsfile_reader_get_all_timeseries_schemas(TsFileReader reader, uint32_t* size); @@ -1092,8 +1096,9 @@ DeviceSchema* tsfile_reader_get_all_timeseries_schemas(TsFileReader reader, * Both output parameters are required. Non-null outputs are set to NULL/0 * before work begins and remain empty on failure. No devices is a successful * result with RET_OK and NULL/0 outputs. - * @note On success, release each schema with free_device_schema(), then free() - * the array. Partial results are released internally on failure. + * @note On success, release the result with + * tsfile_free_device_schema_array(). Partial results are released internally + * on failure. */ ERRNO tsfile_reader_get_all_timeseries_schemas_checked( TsFileReader reader, DeviceSchema** out_schemas, uint32_t* out_size); @@ -1251,6 +1256,29 @@ void free_table_schema(TableSchema schema); void free_column_schema(ColumnSchema schema); void free_write_file(WriteFile* write_file); +/** + * @brief Releases a schema array returned by the reader APIs. + * + * Frees the contents of every schema, then the array itself, with the same + * allocator that produced them. Use this for + * tsfile_reader_get_all_table_schemas*() and + * tsfile_reader_get_all_timeseries_schemas*() results instead of freeing + * elements and the array individually from another module. + */ +void tsfile_free_table_schema_array(TableSchema* schemas, uint32_t length); +void tsfile_free_device_schema_array(DeviceSchema* schemas, uint32_t length); + +/** + * @brief Releases a buffer allocated by this C wrapper. + * + * Result-value getters such as tsfile_result_set_get_value_by_index_string() + * and tsfile_result_set_get_value_by_index_binary() return caller-owned + * buffers. Release them here instead of calling free() in a binding: the + * deallocator then lives in the same module (and CRT) as the allocator, which + * matters for static-runtime builds where each DLL owns a separate heap. + */ +void tsfile_free_buffer(void* buffer); + // ---------- Generic Writer API ---------- // // Safety rules: diff --git a/python/CLAUDE.md b/python/CLAUDE.md index 2fc0ca88e..60c609313 100644 --- a/python/CLAUDE.md +++ b/python/CLAUDE.md @@ -48,6 +48,18 @@ pytest tests/ # Run a single test pytest tests/test_write_and_read.py -k "test_name" +# ASan/LSan coverage of the bindings (Linux; run from the repo root; see +# SANITIZER.md). TSFILE_SANITIZER is required or test_sanitizer_ownership.py +# skips every case, and the runtime must be preloaded or the instrumented +# extensions abort on import. +./mvnw -P with-python -Dtsfile.sanitizer=address -Dbuild.type=RelWithDebInfo \ + -Dbuild.test=OFF clean test-compile +TSFILE_SANITIZER=address \ +ASAN_OPTIONS="detect_leaks=0:abort_on_error=1:halt_on_error=1" \ +LSAN_OPTIONS="suppressions=$PWD/python/tests/sanitizer/lsan_suppressions.txt" \ +LD_PRELOAD="$(gcc -print-file-name=libasan.so):$(gcc -print-file-name=libstdc++.so.6)" \ + python/venv/bin/python -m pytest python/tests/test_sanitizer_ownership.py -v + # Format code cd ../ && ./mvnw spotless:apply # Uses Black 26.3.1 ``` diff --git a/python/README.md b/python/README.md index 60607fe7d..c99d11f9d 100644 --- a/python/README.md +++ b/python/README.md @@ -61,6 +61,17 @@ Build by python command: python setup.py build_ext --inplace ``` +## Sanitizer builds + +To build the C++ library and every Python native extension with +AddressSanitizer/LeakSanitizer and run the ownership scenarios in isolated +subprocesses, see [SANITIZER.md](SANITIZER.md): + +```sh +./mvnw -P with-python -Dtsfile.sanitizer=address -Dbuild.type=RelWithDebInfo \ + -Dbuild.test=OFF clean test-compile +``` + ## Dataset read resource limits `TsFileDataFrame` accepts keyword-only resource options when `use_index=True`: diff --git a/python/SANITIZER.md b/python/SANITIZER.md new file mode 100644 index 000000000..77670c5f6 --- /dev/null +++ b/python/SANITIZER.md @@ -0,0 +1,243 @@ + + +# Sanitizer builds for the Python bindings + +The Python package crosses four layers: + +``` +Python tests -> Cython extensions (.so/.dylib) -> C wrapper -> libtsfile (C++) +``` + +The ordinary C++ sanitizer jobs only cover the last layer. This mode builds +**all four** with AddressSanitizer, keeps LeakSanitizer enabled, and runs +ownership/lifetime scenarios in fresh interpreters so allocated buffers, +invalid frees, use-after-free and result-buffer lifetime errors are visible +across the language boundary. + +## Supported platforms + +| Platform | ASan | LSan | Status | +|----------|------|------|--------| +| Linux x86_64/aarch64 (GCC or Clang) | yes | yes | Supported; used by CI | +| macOS (Apple Clang) | yes | no | Build/run manually with `detect_leaks=0` | +| Windows (MSVC/MinGW) | — | — | Not supported for the bindings | + +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`. + +## Build + +### One command (C++ library + Cython extensions) + +```bash +./mvnw -P with-python \ + -Dtsfile.sanitizer=address \ + -Dbuild.type=RelWithDebInfo \ + -Dbuild.test=OFF \ + -Dspotless.skip=true \ + clean test-compile +``` + +`-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`; +* exports `TSFILE_SANITIZER=` to `setup.py`, which instruments every + extension (including `python_random_access_read_file.cc`) and verifies the + result; +* disables wheel packaging and the ordinary pytest execution. Sanitizer + binaries are never release artifacts, and the pytest process would abort + while importing the instrumented extensions without the preloaded runtime. + Use `test-compile` and run the ownership suite by hand (see Run below); + `mvn test`/`mvn verify` are not supported in this mode. + +`RelWithDebInfo` is recommended: it provides `-O2 -g`, and the sanitizer mode +additionally adds `-g`. Release builds skip LTO when ASan is enabled because LTO +removes the frames reports need. `-Dbuild.test=OFF` only skips the C++ gtest +target; the Python module is still built (and the ownership suite is run +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 + +# 2. Point setup.py at that build and instrument the extensions +cd python +TSFILE_CPP_BUILD=../cpp/build/relwithdebinfo \ +TSFILE_SANITIZER=address \ +python setup.py build_ext --inplace +``` + +`TSFILE_SANITIZER` takes the `-fsanitize=` list (use `address` for the +supported ASan + LSan mode). Use the same list for the C++ build +(`--sanitizer-checks=...` / `-DTSFILE_SANITIZER_CHECKS=...`), otherwise the +instrumented modules disagree about the runtime they need. The instrumentation +verification is ASan-specific. + +`setup.py` inspects every built extension and the copied `libtsfile` with `nm` +and `readelf`/`otool` and fails if any of them lacks ASan references **or** a +link-time dependency on the sanitizer runtime. It also refuses `bdist_wheel` +and refuses to package any staged binary that carries `__asan_` symbols, so a +stale sanitizer object cannot slip into a release wheel. This makes `nm` +(`binutils` on Linux) a build-time requirement for every wheel build, not just +sanitizer builds. + +## Run + +A standard (non-instrumented) Python interpreter must load the sanitizer +runtime **before** any instrumented module; otherwise ASan aborts with +"ASan runtime does not come first in initial library list". Preload it: + +```bash +ASAN_LIB="$(gcc -print-file-name=libasan.so)" # GCC +LIBSTDCXX="$(gcc -print-file-name=libstdc++.so.6)" +# Or, with Clang: "$(clang -print-file-name=libclang_rt.asan-x86_64.so)" + +TSFILE_SANITIZER=address \ +LD_PRELOAD="$ASAN_LIB:$LIBSTDCXX" \ +ASAN_OPTIONS="detect_leaks=1:abort_on_error=1:halt_on_error=1" \ +LSAN_OPTIONS="suppressions=$PWD/python/tests/sanitizer/lsan_suppressions.txt" \ +python -m pytest python/tests/test_sanitizer_ownership.py -v +``` + +`libasan.so` must be first so it owns process startup. `libstdc++.so.6` +immediately follows: GCC's ASan resolves the real `__cxa_throw` when the +interceptor initializes, and if libstdc++ is not in the initial library list it +aborts with `AddressSanitizer: CHECK failed ... real___cxa_throw != 0` the first +time a C++ exception is thrown. Putting libstdc++ before libasan instead fails +ASan's runtime-first check. + +Run from the repository root (the suite sets the child working directory +itself) so `tests.sanitizer_scenarios` is importable in the children. +The pytest runner is started with `detect_leaks=0` in CI to avoid interpreter +noise; every scenario child forces `detect_leaks=1` back on, and the ownership +assertions only apply to those children. The import-only and runtime scenarios +show what the interpreter maps, and the positive controls prove the harness +fails on a real defect. + +On macOS (manual ASan runs only; there is no `LD_PRELOAD`) use +`DYLD_INSERT_LIBRARIES`: + +```bash +DYLD_INSERT_LIBRARIES="$(clang -print-file-name=libclang_rt.asan_osx_dynamic.dylib)" \ +ASAN_OPTIONS="detect_leaks=0:halt_on_error=1" ... +``` + +### Manual scenario runs + +```bash +cd python +ASAN_LIB="$(gcc -print-file-name=libasan.so)" +LIBSTDCXX="$(gcc -print-file-name=libstdc++.so.6)" +LD_PRELOAD="$ASAN_LIB:$LIBSTDCXX" TSFILE_SANITIZER=address \ + python -m tests.sanitizer_scenarios tree_full /tmp/tsfile-sanitizer +``` + +Use `build_fixtures` first; it writes `tree_all`, `table_all`, `tree_numeric` +and `table_numeric` files into the directory. + +## What is covered + +* Low-level tree and table reads, row and batch APIs, dataset reads. +* Numeric values plus STRING/TEXT/BLOB buffers, including NULLs and BLOBs with + embedded NUL bytes. +* Full consumption, early termination, exception cleanup, repeated open/close, + and reader/result close-order permutations. +* Arrow C data interface import and release. +* The file-object adapter (`python_random_access_read_file.cc`). + +## Symbolization + +Build with `RelWithDebInfo` (or `Debug`) and keep the binaries unstripped. +GCC's runtime uses `llvm-symbolizer` when available and falls back to +`addr2line` from binutils. To pin a symbolizer: + +```bash +export ASAN_SYMBOLIZER_PATH="$(command -v llvm-symbolizer)" +``` + +Cython stacks show the generated frame followed by the `.pyx` line; the TsFile +allocation site appears as `libtsfile.so!...` or the extension module name. + +## Suppression policy + +The file is `python/tests/sanitizer/lsan_suppressions.txt`. Rules: + +* Prefer no entries. The import-only baseline exists to identify noise. +* Scope every entry to a specific third-party module or allocator function. No + wildcard modules. +* Never suppress `tsfile/`, `libtsfile`, or the extension modules. A leak there + is the defect this job exists to find. +* Never set `detect_leaks=0` for scenario processes. +* Add a comment above each rule explaining the upstream allocation. + +A normal CPython process does not release every code object, interned string or +thread lock before the sanitizer's exit-time check, and binary wheels keep +process-lifetime caches. The file therefore suppresses specific CPython +allocator functions (`PyObject_Malloc`, `PyMem_Malloc`, ...) and a few +third-party C extension modules; it never contains a TsFile module. + +Reports that survive the suppressions are attributed by the harness +(`test_sanitizer_ownership.py`): a leak **fails the run only when its allocation +stack contains a `libtsfile` or extension frame**. Blocks that involve only +CPython or third-party code are counted and printed but do not fail the run. +This is attribution, not suppression: any leak that passes through the TsFile +modules fails, and the positive controls prove that a real defect is reported. + +## Positive controls + +`positive_control_leak` (ctypes `malloc` without free) and +`positive_control_uaf` (`malloc`/`free`/write-after-free) intentionally fail. +`test_sanitizer_ownership.py` asserts that they exit nonzero with a +`LeakSanitizer`/`AddressSanitizer` report. If a suppression file becomes too +broad, these controls fail first. + +To validate that a real binding regression is detected, temporarily remove a +buffer release (for example the `tsfile_free_buffer(string)` call in +`get_value_by_index`) and re-run the suite: `tree_full`/`table_full` must fail +with a leak whose allocation stack names +`tsfile_result_set_get_value_by_index_string`. + +## Troubleshooting + +* **"ASan runtime does not come first in initial library list"** — `LD_PRELOAD` + is missing or points at a different compiler's runtime. Use the runtime + printed by the compiler that built the library. +* **`CHECK failed: asan_interceptors.cpp ... real___cxa_throw != 0`** — a C++ + exception was thrown while libstdc++ was not in the initial library list. + Preload it immediately after libasan: + `LD_PRELOAD="$ASAN_LIB:$(gcc -print-file-name=libstdc++.so.6)"`. +* **undefined symbol `__ubsan_*`** — the C++ library and extensions were built + with different check lists. Rebuild both with the same + `-Dtsfile.sanitizer=...` / `TSFILE_SANITIZER=...` value. +* **Leak reports in `libpython`/`pyarrow`/`numpy` only** — add a narrowly + scoped suppression for that symbol; do not touch TsFile entries. +* **No leaks reported at all on macOS** — LeakSanitizer is not available on + Darwin; run with `detect_leaks=0` and rely on ASan's access checks. diff --git a/python/_build_support.py b/python/_build_support.py index b050c6378..e2a22395f 100644 --- a/python/_build_support.py +++ b/python/_build_support.py @@ -18,10 +18,12 @@ """Read native SDK build metadata and prepare runtime-only wheel files.""" +import importlib.machinery import json from pathlib import Path import shutil import subprocess +import sys MSVC_RUNTIME_FLAGS = { "MultiThreaded": "/MT", @@ -99,3 +101,113 @@ def strip_wheel_binaries(build_lib, platform): subprocess.run( ["codesign", "--force", "--sign", "-", str(path)], check=True ) + + +SANITIZER_SYMBOL_PREFIX = "__asan_" + + +def is_runtime_library(path): + return Path(path).name.startswith(("libtsfile", "tsfile.dll")) + + +def native_binaries(root): + """Return the binaries built for the running interpreter under *root*. + + Only the current platform/ABI extension suffix is matched, so stale + artifacts from another OS or Python version in a reused tree are ignored. + The copied TsFile runtime library is included explicitly. + """ + extension_suffix = importlib.machinery.EXTENSION_SUFFIXES[0] + root = Path(root) + return sorted( + path + for path in root.rglob("*") + if path.is_file() + and (path.name.endswith(extension_suffix) or is_runtime_library(path)) + ) + + +SANITIZER_RUNTIME_HINTS = ("libasan", "libclang_rt.asan") + + +def _run_tool(command, path): + try: + completed = subprocess.run(command, capture_output=True, text=True) + except FileNotFoundError as exc: + raise RuntimeError( + f"{command[0]} is required to verify sanitizer instrumentation" + ) from exc + if completed.returncode != 0: + raise RuntimeError( + f"failed to inspect {path} with {command[0]}: {completed.stderr.strip()}" + ) + return completed.stdout + + +def _sanitizer_symbols(path): + # Linux lists both defined and undefined dynamic symbols with -D; macOS + # -gU lists only undefined external symbols, which is where the ASan + # instrumentation references appear in a shared object. + if sys.platform.startswith("linux"): + command = ["nm", "-D", str(path)] + elif sys.platform == "darwin": + command = ["nm", "-gU", str(path)] + else: + raise RuntimeError( + f"Sanitizer instrumentation checks are not supported on {sys.platform}" + ) + return _run_tool(command, path) + + +def _linked_libraries(path): + if sys.platform.startswith("linux"): + command = ["readelf", "-d", str(path)] + elif sys.platform == "darwin": + command = ["otool", "-L", str(path)] + else: + raise RuntimeError(f"Sanitizer link checks are not supported on {sys.platform}") + return _run_tool(command, path) + + +def is_sanitizer_instrumented(path): + return SANITIZER_SYMBOL_PREFIX in _sanitizer_symbols(path) + + +def links_sanitizer_runtime(path): + """True when the binary records a dependency on the sanitizer runtime.""" + linked = _linked_libraries(path) + return any(hint in linked for hint in SANITIZER_RUNTIME_HINTS) + + +def verify_sanitizer_instrumentation(paths, expect_instrumented): + """Check each native binary and return the inspected (path, bool) pairs. + + Instrumented binaries must have both compile-time ASan references and a + link-time dependency on the sanitizer runtime. Raises RuntimeError with + every offending path when a binary does not match *expect_instrumented*. + Printing happens here so build logs show which artifacts were verified. + """ + inspected = [] + failures = [] + for path in paths: + path = Path(path) + if not path.is_file(): + raise RuntimeError(f"Missing native binary: {path}") + instrumented = is_sanitizer_instrumented(path) + if expect_instrumented: + instrumented = instrumented and links_sanitizer_runtime(path) + inspected.append((path, instrumented)) + if instrumented != expect_instrumented: + failures.append(path) + state = "instrumented" if instrumented else "not instrumented" + print(f"sanitizer check: {state}: {path}") + + if failures: + expected = "instrumented" if expect_instrumented else "uninstrumented" + details = "\n".join(f" {path}" for path in failures) + raise RuntimeError( + f"Expected every native binary to be {expected}:\n{details}\n" + "Use '-Dtsfile.sanitizer=' for a coordinated sanitizer " + "build, or run a clean normal build for release artifacts." + ) + return inspected diff --git a/python/check_sanitizer_build.py b/python/check_sanitizer_build.py new file mode 100644 index 000000000..d6785d649 --- /dev/null +++ b/python/check_sanitizer_build.py @@ -0,0 +1,75 @@ +#!/usr/bin/env python3 +# +# 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. +# + +"""Verify that Python native extensions and libtsfile are all instrumented. + +Used by the sanitizer CI job after the build so the log proves that ASan +covers the complete Python -> Cython -> C wrapper -> C++ library chain. The +same check runs inside setup.py; this entry point is for standalone runs. + +Usage: + check_sanitizer_build.py [--expect-clean] + + is the directory containing the built tsfile package, for +example ``python/tsfile``. With ``--expect-clean`` every binary must be free +of ASan references (useful to keep sanitizer artifacts out of release builds). +""" + +from __future__ import annotations + +from pathlib import Path +import sys + +from _build_support import native_binaries, verify_sanitizer_instrumentation + + +def main(argv: list[str]) -> int: + arguments = [item for item in argv[1:] if item != "--expect-clean"] + expect_clean = "--expect-clean" in argv[1:] + if len(arguments) != 1: + print( + f"usage: {Path(argv[0]).name} [--expect-clean]", + file=sys.stderr, + ) + return 2 + + package = Path(arguments[0]).expanduser().resolve() + if not package.is_dir(): + print(f"error: package directory not found: {package}", file=sys.stderr) + return 2 + + binaries = native_binaries(package) + if not binaries: + print(f"error: no native binaries found under {package}", file=sys.stderr) + return 1 + + try: + verify_sanitizer_instrumentation(binaries, expect_instrumented=not expect_clean) + except RuntimeError as exc: + print(f"error: {exc}", file=sys.stderr) + return 1 + + state = "clean" if expect_clean else "instrumented" + print(f"sanitizer check: {len(binaries)} native binaries are {state}") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main(sys.argv)) diff --git a/python/pom.xml b/python/pom.xml index f47c92ac2..33d3338cb 100644 --- a/python/pom.xml +++ b/python/pom.xml @@ -36,6 +36,14 @@ -Dtsfile.cpp.build=. --> ${project.basedir}/../cpp/target/build + + + false tsfile ${project.build.directory}/build-wrapper-output @@ -84,6 +92,7 @@ --> ${tsfile.cpp.build} + ${tsfile.python.sanitizer} @@ -209,6 +218,7 @@ exec + ${tsfile.python.wheel.skip} ${python.venv.bin}${python.exe.bin} setup.py @@ -275,4 +285,51 @@ + + + with-sanitizer + + + + tsfile.sanitizer + + + + ${tsfile.sanitizer} + true + + + + + org.codehaus.mojo + exec-maven-plugin + + + + run-python-tests + + true + + + + + + + + diff --git a/python/setup.py b/python/setup.py index 5b906324d..2672565bf 100644 --- a/python/setup.py +++ b/python/setup.py @@ -18,6 +18,7 @@ import os import platform +import re import shutil import sys @@ -37,12 +38,29 @@ from _build_support import ( copy_runtime_library, msvc_runtime_flag, + native_binaries, read_sdk_config, strip_wheel_binaries, + verify_sanitizer_instrumentation, ) version = "2.5.1.dev" +# Comma-separated GCC/Clang sanitizer checks (for example "address"). Empty +# means a normal build. Maven sets this from -Dtsfile.sanitizer=. +SANITIZER = os.environ.get("TSFILE_SANITIZER", "").strip() +SANITIZER_SUPPORTED = sys.platform.startswith("linux") or sys.platform == "darwin" +if SANITIZER and not re.fullmatch(r"[A-Za-z0-9,_+-]+", SANITIZER): + raise RuntimeError( + f"Invalid TSFILE_SANITIZER value: {SANITIZER!r}; expected a " + "comma-separated -fsanitize= list such as 'address'." + ) +if SANITIZER and not SANITIZER_SUPPORTED: + raise RuntimeError( + "Sanitizer builds of the Python bindings are supported on Linux and " + "macOS only." + ) + def _find_cpp_build(): """Locate the C++ build output directory (one containing include/). @@ -96,6 +114,9 @@ def _find_lib(root, patterns): # "mingw" links against a MinGW import library (libtsfile.dll.a). win_toolchain = None +# Destination of the copied C++ runtime library, used by the sanitizer check. +runtime_lib_dst = None + if sys.platform.startswith("linux"): candidates = sorted( CPP_LIB.rglob("libtsfile.so*"), key=lambda p: len(p.name), reverse=True @@ -104,7 +125,7 @@ def _find_lib(root, patterns): raise FileNotFoundError("missing libtsfile.so* in build output") src = candidates[0] sdk_config = read_sdk_config(CPP_OUT, src) - copy_runtime_library(PKG, src, sdk_config) + runtime_lib_dst = copy_runtime_library(PKG, src, sdk_config) native_link_dir = src.parent elif sys.platform == "darwin": @@ -115,7 +136,7 @@ def _find_lib(root, patterns): raise FileNotFoundError("missing libtsfile*.dylib in build output") src = candidates[0] sdk_config = read_sdk_config(CPP_OUT, src) - copy_runtime_library(PKG, src, sdk_config) + runtime_lib_dst = copy_runtime_library(PKG, src, sdk_config) native_link_dir = src.parent elif sys.platform == "win32": @@ -144,6 +165,7 @@ def _find_lib(root, patterns): # table and Windows must find it by that exact name at runtime. sdk_config = read_sdk_config(CPP_OUT, dll_src) dll_dst = copy_runtime_library(PKG, dll_src, sdk_config) + runtime_lib_dst = dll_dst native_link_dir = PKG # Copy import library with a name matching the DLL. @@ -224,9 +246,37 @@ def run(self): class BuildExt(build_ext): def run(self): + wheel_build = ( + "bdist_wheel" in self.distribution.script_args and not self.inplace + ) + if wheel_build and SANITIZER: + raise RuntimeError( + "Refusing to package a wheel from a sanitizer build. " + "Unset TSFILE_SANITIZER and run a clean build for release " + "artifacts; sanitizer binaries never ship." + ) + super().run() - if "bdist_wheel" in self.distribution.script_args and not self.inplace: + + if wheel_build: + # Distutils can reuse object files from a previous sanitizer build + # in the same directory; never let them into a wheel. + staged = Path(self.build_lib) / "tsfile" + if staged.is_dir() and SANITIZER_SUPPORTED: + verify_sanitizer_instrumentation( + native_binaries(staged), expect_instrumented=False + ) strip_wheel_binaries(self.build_lib, sys.platform) + return + + if not SANITIZER: + return + targets = [ + Path(self.get_ext_fullpath(extension.name)) for extension in self.extensions + ] + if runtime_lib_dst is not None: + targets.append(Path(runtime_lib_dst)) + verify_sanitizer_instrumentation(targets, expect_instrumented=True) def finalize_options(self): # MinGW must be requested explicitly; MSVC is the default Windows @@ -246,14 +296,28 @@ def finalize_options(self): if sdk_include.is_dir(): include_dirs.append(str(sdk_include)) +# Link arguments that carry the sanitizer runtime into every extension, +# including the ones that do not link libtsfile. +sanitizer_link_args = [] + if sys.platform.startswith("linux"): libraries = ["tsfile"] - extra_compile_args += ["-O3", "-std=c++11", "-fvisibility=hidden", "-fPIC"] + extra_compile_args += [ + "-O1" if SANITIZER else "-O3", + "-std=c++11", + "-fvisibility=hidden", + "-fPIC", + ] runtime_library_dirs = ["$ORIGIN"] extra_link_args += ["-Wl,-rpath,$ORIGIN"] elif sys.platform == "darwin": libraries = ["tsfile"] - extra_compile_args += ["-O3", "-std=c++11", "-fvisibility=hidden", "-fPIC"] + extra_compile_args += [ + "-O1" if SANITIZER else "-O3", + "-std=c++11", + "-fvisibility=hidden", + "-fPIC", + ] extra_link_args += ["-Wl,-rpath,@loader_path", "-stdlib=libc++"] elif sys.platform == "win32": if win_toolchain == "mingw": @@ -293,6 +357,23 @@ def finalize_options(self): else: raise RuntimeError(f"Unsupported platform: {sys.platform}") +if SANITIZER: + # -O1 keeps reports readable; frame pointers and debug info make the + # allocation/free stacks actionable. Every extension is instrumented and + # linked with the same check set as libtsfile. + extra_compile_args += [ + "-g", + "-fno-omit-frame-pointer", + "-fno-optimize-sibling-calls", + f"-fsanitize={SANITIZER}", + ] + sanitizer_link_args.append(f"-fsanitize={SANITIZER}") + extra_link_args += sanitizer_link_args + print( + "setup.py: sanitizer build enabled: " + f"checks={SANITIZER} (extensions use -O1, -g, -fno-omit-frame-pointer)" + ) + common = dict( language="c++", include_dirs=include_dirs, @@ -307,7 +388,9 @@ def finalize_options(self): merge_common.update( library_dirs=[], libraries=[], - extra_link_args=[], + # These extensions do not link libtsfile, but they still contain + # instrumented code and need the sanitizer runtime at link time. + extra_link_args=list(sanitizer_link_args), runtime_library_dirs=[], ) diff --git a/python/tests/sanitizer/lsan_suppressions.txt b/python/tests/sanitizer/lsan_suppressions.txt new file mode 100644 index 000000000..5c5030ec5 --- /dev/null +++ b/python/tests/sanitizer/lsan_suppressions.txt @@ -0,0 +1,49 @@ +# +# 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. +# +# LeakSanitizer suppressions for the Python binding sanitizer job. +# +# Policy: +# * Prefer no suppressions. The import-only baseline scenario exists to show +# whether interpreter or third-party noise exists at all. +# * Every entry must be narrowly scoped to a third-party library or the +# Python interpreter itself (a specific module and symbol, never a wildcard +# module). +# * Never suppress anything under tsfile/, libtsfile, or the Cython extension +# modules. A leak there is a defect this job exists to find. +# * Never disable leak detection globally; ASAN_OPTIONS/LSAN_OPTIONS keep +# detect_leaks=1 for every scenario process. +# +# Syntax (one rule per line, same as the compiler suppression format): +# leak:: +# Add rules below this line only with a comment explaining the upstream cause. + +# CPython does not release every import-time object (code objects, interned +# strings, thread locks) before the sanitizer performs its exit-time check. +# These rules match the CPython allocator functions, not any TsFile frame: +# a native buffer leaked by libtsfile is allocated with malloc() directly and +# still has a TsFile frame at the top of its stack. +leak:PyObject_Malloc +leak:PyMem_Malloc +leak:PyMem_RawMalloc +leak:PyThread_allocate_lock + +# Process-lifetime caches in third-party extension modules. +leak:_multiarray_umath +leak:libarrow +leak:lib.cpython diff --git a/python/tests/sanitizer_scenarios.py b/python/tests/sanitizer_scenarios.py new file mode 100644 index 000000000..b5fd3b352 --- /dev/null +++ b/python/tests/sanitizer_scenarios.py @@ -0,0 +1,645 @@ +# 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. +# +"""Ownership and lifetime scenarios for the sanitizer job. + +Each scenario is executed in a fresh interpreter by +``tests/test_sanitizer_ownership.py`` so that LeakSanitizer can run at normal +process exit. Run one manually with:: + + python -m tests.sanitizer_scenarios + +The module must run from the ``python/`` directory with an instrumented build +whose sanitizer runtime was preloaded (see ``python/SANITIZER.md``). +""" + +import ctypes +import gc +import io +import sys +import threading +from datetime import date +from pathlib import Path + +from tsfile import ( + ColumnCategory, + ColumnSchema, + Field, + RowRecord, + TableSchema, + Tablet, + TSDataType, + TimeseriesSchema, + TsFileDataFrame, + TsFileReader, + TsFileTableWriter, + TsFileWriter, +) + +ROWS = 1024 +BATCH_SIZE = 64 +RESOURCES = Path(__file__).resolve().parent / "resources" + +TREE_MEASUREMENTS = [ + ("i32", TSDataType.INT32), + ("i64", TSDataType.INT64), + ("f", TSDataType.FLOAT), + ("d", TSDataType.DOUBLE), + ("b", TSDataType.BOOLEAN), + ("s", TSDataType.STRING), + ("t", TSDataType.TEXT), + ("blob", TSDataType.BLOB), + ("dt", TSDataType.DATE), + ("ts", TSDataType.TIMESTAMP), +] +TREE_DEVICES = ["root.ds.d1", "root.ds.d2"] + +TABLE_COLUMNS = [ + ColumnSchema("device", TSDataType.STRING, ColumnCategory.TAG), + ColumnSchema("i32", TSDataType.INT32, ColumnCategory.FIELD), + ColumnSchema("i64", TSDataType.INT64, ColumnCategory.FIELD), + ColumnSchema("f", TSDataType.FLOAT, ColumnCategory.FIELD), + ColumnSchema("d", TSDataType.DOUBLE, ColumnCategory.FIELD), + ColumnSchema("b", TSDataType.BOOLEAN, ColumnCategory.FIELD), + ColumnSchema("s", TSDataType.STRING, ColumnCategory.FIELD), + ColumnSchema("t", TSDataType.TEXT, ColumnCategory.FIELD), + ColumnSchema("blob", TSDataType.BLOB, ColumnCategory.FIELD), + ColumnSchema("dt", TSDataType.DATE, ColumnCategory.FIELD), + ColumnSchema("ts", TSDataType.TIMESTAMP, ColumnCategory.FIELD), +] + + +def _tree_value(name, data_type, row): + if data_type == TSDataType.INT32: + return row * 2 + if data_type == TSDataType.INT64: + return row * 3 + if data_type == TSDataType.FLOAT: + return row * 1.5 + if data_type == TSDataType.DOUBLE: + return row * 2.5 + if data_type == TSDataType.BOOLEAN: + return row % 2 == 0 + if data_type == TSDataType.STRING: + return f"str_{row}" + if data_type == TSDataType.TEXT: + return f"text_{row}" + if data_type == TSDataType.BLOB: + return b"blob\x00" + str(row).encode("ascii") + if data_type == TSDataType.DATE: + return date(2025, 1, 1 + row % 28) + if data_type == TSDataType.TIMESTAMP: + return row * 1000 + raise AssertionError(f"unexpected type {data_type}") + + +def _skip_tree_value(name, row): + if name == "s": + return row % 7 == 3 + if name == "blob": + return row % 5 == 2 + return False + + +def _build_tree(base): + path = base / "tree_all.tsfile" + writer = TsFileWriter(str(path)) + for device in TREE_DEVICES: + for name, data_type in TREE_MEASUREMENTS: + writer.register_timeseries(device, TimeseriesSchema(name, data_type)) + writer.add_tsfile_property("owner", b"apache\x00tsfile") + for row in range(ROWS): + for device in TREE_DEVICES: + fields = [ + Field(name, _tree_value(name, data_type, row), data_type) + for name, data_type in TREE_MEASUREMENTS + if not _skip_tree_value(name, row) + ] + writer.write_row_record(RowRecord(device, row, fields)) + writer.close() + return path + + +def _build_table(base): + path = base / "table_all.tsfile" + schema = TableSchema("measures", TABLE_COLUMNS) + with TsFileTableWriter(str(path), schema) as writer: + tablet = Tablet( + [column.column_name for column in TABLE_COLUMNS], + [column.data_type for column in TABLE_COLUMNS], + ROWS, + ) + for row in range(ROWS): + tablet.add_timestamp(row, row) + tablet.add_value_by_name("device", row, "d1") + tablet.add_value_by_name("i32", row, row * 2) + tablet.add_value_by_name("i64", row, row * 3) + tablet.add_value_by_name("f", row, row * 1.5) + tablet.add_value_by_name("d", row, row * 2.5) + if row % 3: + tablet.add_value_by_name("b", row, row % 2 == 0) + if row % 4 != 1: + tablet.add_value_by_name("s", row, f"str_{row}") + tablet.add_value_by_name("t", row, f"text_{row}") + if row % 5 != 2: + tablet.add_value_by_name( + "blob", row, b"blob\x00" + str(row).encode("ascii") + ) + tablet.add_value_by_name("dt", row, date(2025, 1, 1 + row % 28)) + tablet.add_value_by_name("ts", row, row * 1000) + writer.write_table(tablet) + return path + + +def _build_tree_numeric(base): + path = base / "tree_numeric.tsfile" + writer = TsFileWriter(str(path)) + writer.register_timeseries("root.ds.d1", TimeseriesSchema("s1", TSDataType.INT64)) + writer.register_timeseries("root.ds.d1", TimeseriesSchema("s2", TSDataType.DOUBLE)) + for row in range(ROWS): + writer.write_row_record( + RowRecord( + "root.ds.d1", + row, + [ + Field("s1", row * 2, TSDataType.INT64), + Field("s2", row * 0.5, TSDataType.DOUBLE), + ], + ) + ) + writer.close() + return path + + +def _build_table_numeric(base): + path = base / "table_numeric.tsfile" + schema = TableSchema( + "metrics", + [ + ColumnSchema("device", TSDataType.STRING, ColumnCategory.TAG), + ColumnSchema("value1", TSDataType.INT64, ColumnCategory.FIELD), + ColumnSchema("value2", TSDataType.DOUBLE, ColumnCategory.FIELD), + ], + ) + with TsFileTableWriter(str(path), schema) as writer: + tablet = Tablet( + ["device", "value1", "value2"], + [TSDataType.STRING, TSDataType.INT64, TSDataType.DOUBLE], + ROWS, + ) + for row in range(ROWS): + tablet.add_timestamp(row, row) + tablet.add_value_by_name("device", row, "d1") + tablet.add_value_by_name("value1", row, row * 2) + tablet.add_value_by_name("value2", row, row * 0.5) + writer.write_table(tablet) + return path + + +def build_fixtures(base): + base.mkdir(parents=True, exist_ok=True) + for name in ( + "tree_all.tsfile", + "table_all.tsfile", + "tree_numeric.tsfile", + "table_numeric.tsfile", + ): + (base / name).unlink(missing_ok=True) + paths = [ + _build_tree(base), + _build_table(base), + _build_tree_numeric(base), + _build_table_numeric(base), + ] + for path in paths: + assert path.is_file() and path.stat().st_size > 0, path + + +def _fixture(base, name): + path = base / name + assert path.is_file(), f"missing fixture {path}; run build_fixtures first" + return str(path) + + +def tree_full(base): + path = _fixture(base, "tree_all.tsfile") + names = [name for name, _ in TREE_MEASUREMENTS] + with TsFileReader(path) as reader: + result = reader.query_timeseries(TREE_DEVICES[0], names, 0, ROWS + 1) + try: + row = 0 + while result.next(): + assert result.get_value_by_index(1) == row + assert result.get_value_by_index(2) == row * 2 + assert result.get_value_by_index(3) == row * 3 + assert result.get_value_by_index(5) == row * 2.5 + assert result.get_value_by_index(6) == (row % 2 == 0) + if _skip_tree_value("s", row): + assert result.is_null_by_index(7) + else: + assert result.get_value_by_index(7) == f"str_{row}" + assert result.get_value_by_index(8) == f"text_{row}" + if _skip_tree_value("blob", row): + assert result.is_null_by_index(9) + else: + assert result.get_value_by_index(9) == ( + b"blob\x00" + str(row).encode("ascii") + ) + row += 1 + assert row == ROWS + finally: + result.close() + + by_row = reader.query_tree_by_row([TREE_DEVICES[0]], names[:3], 8, 16) + try: + row = 8 + while by_row.next(): + assert by_row.get_value_by_index(1) == row + assert by_row.get_value_by_index(2) == row * 2 + row += 1 + assert row == 24 + finally: + by_row.close() + + +def table_full(base): + path = _fixture(base, "table_all.tsfile") + columns = ["device", "i32", "i64", "d", "b", "s", "blob"] + with TsFileReader(path) as reader: + result = reader.query_table("measures", columns, 0, ROWS + 1) + try: + row = 0 + while result.next(): + assert result.get_value_by_index(1) == row + assert result.get_value_by_index(2) == "d1" + assert result.get_value_by_index(3) == row * 2 + assert result.get_value_by_index(4) == row * 3 + assert result.get_value_by_index(5) == row * 2.5 + if row % 3: + assert result.get_value_by_index(6) == (row % 2 == 0) + else: + assert result.is_null_by_index(6) + if row % 4 != 1: + assert result.get_value_by_index(7) == f"str_{row}" + else: + assert result.is_null_by_index(7) + if row % 5 != 2: + assert result.get_value_by_index(8) == ( + b"blob\x00" + str(row).encode("ascii") + ) + else: + assert result.is_null_by_index(8) + row += 1 + assert row == ROWS + finally: + result.close() + + by_row = reader.query_table_by_row("measures", columns, 4, 8) + try: + row = 4 + while by_row.next(): + assert by_row.get_value_by_index(1) == row + row += 1 + assert row == 12 + finally: + by_row.close() + + frame = reader.query_table("measures", columns, 0, ROWS + 1) + try: + assert frame.next() + dataframe = frame.read_data_frame(max_row_num=ROWS) + finally: + frame.close() + assert len(dataframe) == ROWS + assert dataframe["i64"].iloc[-1] == (ROWS - 1) * 3 + + +def metadata_reads(base): + tree_path = _fixture(base, "tree_all.tsfile") + table_path = _fixture(base, "table_all.tsfile") + for _ in range(3): + with TsFileReader(table_path) as reader: + schema = reader.get_table_schema("measures") + assert schema.get_table_name() == "measures" + assert reader.get_all_table_schemas() + assert reader.get_all_devices() + properties = reader.get_tsfile_properties() + assert properties["encryptType"] == ( + b"org.apache.tsfile.encrypt.UNENCRYPTED" + ) + assert "owner" not in properties + with TsFileReader(tree_path) as reader: + schemas = reader.get_all_timeseries_schemas() + assert set(schemas) == set(TREE_DEVICES) + for device in TREE_DEVICES: + assert len(schemas[device].get_timeseries_list()) == len( + TREE_MEASUREMENTS + ) + assert len(reader.get_all_devices()) == len(TREE_DEVICES) + assert reader.get_timeseries_metadata() + properties = reader.get_tsfile_properties() + assert properties["owner"] == b"apache\x00tsfile" + + +def file_object_read(base): + source = io.BytesIO((RESOURCES / "simple_table_t1.tsfile").read_bytes()) + source.seek(11) + with TsFileReader(source) as reader: + result = reader.query_table("test", ["s0", "s1"]) + try: + assert result.next() + assert result.get_value_by_index(1) == 1760106020000 + assert result.get_value_by_index(2) == "a" + finally: + result.close() + assert source.tell() == 11 + + +def dataset_read(base): + tree_path = _fixture(base, "tree_numeric.tsfile") + table_path = _fixture(base, "table_numeric.tsfile") + for use_index in (False, True): + with TsFileDataFrame( + tree_path, show_progress=False, use_index=use_index + ) as frame: + names = frame.list_timeseries() + assert names + for name in names: + values = frame[name][:] + assert len(values) == ROWS + assert float(values[0]) == 0.0 + with TsFileDataFrame( + table_path, show_progress=False, use_index=use_index + ) as frame: + names = frame.list_timeseries() + assert names + for name in names: + values = frame[name][:] + assert len(values) == ROWS + assert float(values[0]) == 0.0 + + +def arrow_full(base): + path = _fixture(base, "table_all.tsfile") + columns = ["device", "i64", "d", "s"] + with TsFileReader(path) as reader: + result = reader.query_table( + "measures", columns, 0, ROWS + 1, batch_size=BATCH_SIZE + ) + batches = [] + total = 0 + try: + while True: + batch = result.read_arrow_record_batch() + if batch is None: + break + batches.append(batch) + total += batch.num_rows + assert total == ROWS + # Ownership transfers to pyarrow; dropping them must release the + # native Arrow buffers. + del batches + gc.collect() + finally: + result.close() + + table_result = reader.query_table( + "measures", columns, 0, ROWS + 1, batch_size=BATCH_SIZE + ) + try: + table = table_result.read_arrow_batch() + assert table is not None + assert table.num_rows > 0 + assert table.num_rows <= BATCH_SIZE + del table + gc.collect() + finally: + table_result.close() + + +def arrow_early_release(base): + path = _fixture(base, "table_all.tsfile") + with TsFileReader(path) as reader: + result = reader.query_table( + "measures", ["i64", "d"], 0, ROWS + 1, batch_size=BATCH_SIZE + ) + try: + batch = result.read_arrow_record_batch() + assert batch is not None + del batch + gc.collect() + finally: + result.close() + + +def early_termination(base): + tree_path = _fixture(base, "tree_all.tsfile") + table_path = _fixture(base, "table_all.tsfile") + with TsFileReader(tree_path) as reader: + with reader.query_timeseries( + TREE_DEVICES[1], ["s", "blob"], 0, ROWS + 1 + ) as result: + assert result.next() + assert result.next() + with TsFileReader(table_path) as reader: + with reader.query_table( + "measures", ["s", "d"], 0, ROWS + 1, batch_size=BATCH_SIZE + ) as result: + assert result.read_arrow_record_batch() is not None + + +def exception_cleanup(base): + path = _fixture(base, "table_all.tsfile") + for name in ("bad_tree.tsfile", "bad_table.tsfile"): + (base / name).unlink(missing_ok=True) + with TsFileReader(path) as reader: + try: + with reader.query_table("measures", ["i64", "s"]) as result: + assert result.next() + raise RuntimeError("intentional cleanup test") + except RuntimeError: + pass + + try: + result = reader.query_table("missing_table", ["nope"]) + try: + result.next() + finally: + result.close() + except Exception: + pass + + bad_tree_path = str(base / "bad_tree.tsfile") + writer = TsFileWriter(bad_tree_path) + try: + writer.register_timeseries("root.bad", TimeseriesSchema("bad", None)) + except ValueError: + pass + else: + raise AssertionError("invalid timeseries schema was accepted") + writer.close() + + bad_table_path = str(base / "bad_table.tsfile") + try: + TsFileTableWriter( + bad_table_path, + TableSchema("bad", [ColumnSchema("x", 123)]), + ) + except ValueError: + pass + else: + raise AssertionError("invalid table schema was accepted") + + +def repeated_open_close(base): + tree_path = _fixture(base, "tree_all.tsfile") + table_path = _fixture(base, "table_all.tsfile") + for _ in range(40): + with TsFileReader(tree_path) as reader: + with reader.query_timeseries( + TREE_DEVICES[0], ["i64", "s"], 0, ROWS + 1 + ) as result: + assert result.next() + with TsFileReader(table_path) as reader: + with reader.query_table("measures", ["i64"], 0, ROWS + 1) as result: + assert result.next() + + +def lifetime_interactions(base): + path = _fixture(base, "table_all.tsfile") + + reader = TsFileReader(path) + result = reader.query_table("measures", ["i64", "s"], 0, ROWS + 1) + assert result.next() + reader.close() + try: + result.next() + except Exception as exc: + assert "Invalid result set" in str(exc), exc + else: + raise AssertionError("result remained readable after the reader closed") + result.close() + result.close() + reader.close() + + reader = TsFileReader(path) + result = reader.query_table("measures", ["i64"], 0, ROWS + 1) + assert result.next() + result.close() + result.close() + reader.close() + + def scope_result_first(): + local_reader = TsFileReader(path) + local_result = local_reader.query_table("measures", ["i64"], 0, ROWS + 1) + assert local_result.next() + local_result.close() + return local_reader + + scope_result_first() + gc.collect() + + def scope_reader_first(): + local_reader = TsFileReader(path) + local_result = local_reader.query_table("measures", ["i64"], 0, ROWS + 1) + assert local_result.next() + return local_result + + scope_reader_first() + gc.collect() + + +def import_only(base): + assert sys.modules["tsfile"].__name__ == "tsfile" + + +def verify_runtime(base): + maps = Path("/proc/self/maps").read_text() + assert "libasan" in maps, "ASan runtime is not mapped into the process" + package = Path(sys.modules["tsfile"].__file__).resolve().parent + assert any( + "libtsfile" in line and str(package) in line for line in maps.splitlines() + ), "libtsfile from the instrumented build is not mapped" + + +def _leak_beyond_recognition(): + libc = ctypes.CDLL(None) + libc.malloc.restype = ctypes.c_void_p + + def worker(): + assert libc.malloc(4096), "malloc failed" + + # The pointer lives only on the worker's stack, which is gone after join(); + # no live stack or register can keep the block reachable for LeakSanitizer. + thread = threading.Thread(target=worker) + thread.start() + thread.join() + + +def positive_control_leak(base): + _leak_beyond_recognition() + gc.collect() + + +def positive_control_uaf(base): + libc = ctypes.CDLL(None) + libc.malloc.restype = ctypes.c_void_p + pointer = libc.malloc(64) + assert pointer, "malloc failed" + libc.free(ctypes.c_void_p(pointer)) + # ASan intercepts both calls and reports the write after free. + libc.memset(ctypes.c_void_p(pointer), 0x41, 8) + + +SCENARIOS = { + "build_fixtures": build_fixtures, + "tree_full": tree_full, + "table_full": table_full, + "metadata_reads": metadata_reads, + "file_object_read": file_object_read, + "dataset_read": dataset_read, + "arrow_full": arrow_full, + "arrow_early_release": arrow_early_release, + "early_termination": early_termination, + "exception_cleanup": exception_cleanup, + "repeated_open_close": repeated_open_close, + "lifetime_interactions": lifetime_interactions, + "import_only": import_only, + "verify_runtime": verify_runtime, + "positive_control_leak": positive_control_leak, + "positive_control_uaf": positive_control_uaf, +} + + +def main(argv): + if len(argv) != 3: + print( + f"usage: {Path(argv[0]).name} ", + file=sys.stderr, + ) + return 2 + scenario = SCENARIOS.get(argv[1]) + if scenario is None: + print(f"unknown scenario: {argv[1]}", file=sys.stderr) + return 2 + scenario(Path(argv[2]).resolve()) + print(f"scenario {argv[1]}: ok") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main(sys.argv)) diff --git a/python/tests/test_sanitizer_ownership.py b/python/tests/test_sanitizer_ownership.py new file mode 100644 index 000000000..ac1810a3d --- /dev/null +++ b/python/tests/test_sanitizer_ownership.py @@ -0,0 +1,210 @@ +# 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. +# +"""Run native ownership scenarios under AddressSanitizer/LeakSanitizer. + +Enabled only for a Linux sanitizer build (``TSFILE_SANITIZER`` is set). Every +scenario runs in a fresh interpreter that exits normally, which is required for +LeakSanitizer to report at exit. The pytest runner itself disables leak +checking to avoid interpreter noise; each scenario process forces it back on. +""" + +import os +from pathlib import Path +import subprocess +import sys + +import pytest + +SANITIZER = os.environ.get("TSFILE_SANITIZER", "").strip() +PYTHON_DIR = Path(__file__).resolve().parents[1] +SUPPRESSIONS = Path(__file__).resolve().parent / "sanitizer" / "lsan_suppressions.txt" + +pytestmark = pytest.mark.skipif( + not SANITIZER or sys.platform != "linux", + reason=( + "native sanitizer scenarios require a Linux build with " + "TSFILE_SANITIZER set (see python/SANITIZER.md)" + ), +) + +SCENARIOS = [ + "tree_full", + "table_full", + "metadata_reads", + "file_object_read", + "dataset_read", + "arrow_full", + "arrow_early_release", + "early_termination", + "exception_cleanup", + "repeated_open_close", + "lifetime_interactions", + "import_only", + "verify_runtime", +] + + +def _set_option(options, key, value): + items = [item for item in options.split(":") if item] + items = [item for item in items if not item.startswith(f"{key}=")] + items.append(f"{key}={value}") + return ":".join(items) + + +def _scenario_env(): + env = os.environ.copy() + asan = env.get("ASAN_OPTIONS", "") + asan = _set_option(asan, "detect_leaks", "1") + asan = _set_option(asan, "exitcode", "23") + asan = _set_option(asan, "abort_on_error", "1") + asan = _set_option(asan, "halt_on_error", "1") + env["ASAN_OPTIONS"] = asan + lsan = env.get("LSAN_OPTIONS", "") + lsan = _set_option(lsan, "suppressions", str(SUPPRESSIONS)) + lsan = _set_option(lsan, "print_suppressions", "1") + env["LSAN_OPTIONS"] = lsan + env.setdefault("UBSAN_OPTIONS", "halt_on_error=1:print_stacktrace=1") + return env + + +def _run_scenario(scenario, work_dir, timeout=300): + return subprocess.run( + [ + sys.executable, + "-X", + "faulthandler", + "-m", + "tests.sanitizer_scenarios", + scenario, + str(work_dir), + ], + cwd=PYTHON_DIR, + env=_scenario_env(), + capture_output=True, + text=True, + timeout=timeout, + ) + + +def _combined_output(completed): + return completed.stdout + completed.stderr + + +# A leak is attributable to this project when the first meaningful frame of +# its allocation stack (after allocator/runtime frames) is the native library +# or one of the Cython extensions. Interpreter and third-party frames found +# first mean the allocation belongs to them, even if a TsFile frame appears +# much further down (for example, our Cython module importing pandas). +TSFILE_FRAME_TOKENS = ( + "libtsfile", + "tsfile_py_cpp", + "tsfile_reader", + "tsfile_writer", + "dataset/_merge", + "dataset/_index", +) +INTERPRETER_FRAME_TOKENS = ("python3.", "libpython", "site-packages") + + +def _leak_blocks(output): + blocks = [] + current = [] + for line in output.splitlines(): + starts = line.startswith("Direct leak of") or line.startswith( + "Indirect leak of" + ) + if starts: + if current: + blocks.append(current) + current = [line] + elif current: + if line.strip(): + current.append(line) + else: + blocks.append(current) + current = [] + if current: + blocks.append(current) + return blocks + + +def _is_tsfile_leak(block): + for line in block: + if not line.lstrip().startswith("#"): + continue + if any(token in line for token in TSFILE_FRAME_TOKENS): + return True + if any(token in line for token in INTERPRETER_FRAME_TOKENS): + return False + return False + + +def _tsfile_leak_blocks(output): + return [block for block in _leak_blocks(output) if _is_tsfile_leak(block)] + + +def _assert_no_sanitizer_report(completed): + output = _combined_output(completed) + assert ": ok" in completed.stdout, output + assert "ERROR: AddressSanitizer" not in output, output + + if "ERROR: LeakSanitizer" not in output: + assert completed.returncode == 0, output + return + + # CPython does not release every import-time object before the sanitizer's + # exit check, and third-party wheels keep process-lifetime caches. Those + # blocks carry no TsFile frame; a leak from libtsfile or an extension does. + tsfile_blocks = _tsfile_leak_blocks(output) + assert not tsfile_blocks, "TsFile-attributable leaks:\n" + "\n".join( + "\n".join(block) for block in tsfile_blocks + ) + ignored = len(_leak_blocks(output)) - len(tsfile_blocks) + print(f"ignored {ignored} interpreter/third-party leak block(s)") + + +@pytest.fixture(scope="module") +def fixture_dir(tmp_path_factory): + work_dir = tmp_path_factory.mktemp("sanitizer_fixtures") + completed = _run_scenario("build_fixtures", work_dir) + _assert_no_sanitizer_report(completed) + return work_dir + + +@pytest.mark.parametrize("scenario", SCENARIOS) +def test_ownership_scenario_reports_no_defects(scenario, fixture_dir): + _assert_no_sanitizer_report(_run_scenario(scenario, fixture_dir)) + + +@pytest.mark.parametrize( + "scenario, marker", + [ + ("positive_control_leak", "LeakSanitizer"), + ("positive_control_uaf", "AddressSanitizer"), + ], +) +def test_positive_control_fails_the_run(scenario, marker, tmp_path): + """The harness must fail when a real defect exists. + + These controls use ctypes allocations, not TsFile code, so they also prove + that the suppression file does not silently hide leaks. + """ + completed = _run_scenario(scenario, tmp_path) + output = _combined_output(completed) + assert completed.returncode != 0, output + assert marker in output, output diff --git a/python/tests/test_value_buffers.py b/python/tests/test_value_buffers.py new file mode 100644 index 000000000..d749cbc02 --- /dev/null +++ b/python/tests/test_value_buffers.py @@ -0,0 +1,105 @@ +# 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. +# +"""Regression tests for caller-owned native value buffers. + +The sanitizer job covers the same paths with LeakSanitizer; these assertions +keep the functional behavior (BLOB round trips and repeated schema +registration) correct in ordinary builds. +""" + +from tsfile import ( + ColumnCategory, + ColumnSchema, + Field, + RowRecord, + TableSchema, + TSDataType, + Tablet, + TimeseriesSchema, + TsFileReader, + TsFileTableWriter, + TsFileWriter, +) + + +def test_blob_preserves_embedded_nul_bytes(tmp_path): + path = str(tmp_path / "blob_nul.tsfile") + schema = TableSchema( + "t", + [ + ColumnSchema("device", TSDataType.STRING, ColumnCategory.TAG), + ColumnSchema("payload", TSDataType.BLOB, ColumnCategory.FIELD), + ], + ) + payloads = [b"a\x00b", b"\x00\x00", b"", b"tail\x00"] + + with TsFileTableWriter(path, schema) as writer: + tablet = Tablet( + ["device", "payload"], + [TSDataType.STRING, TSDataType.BLOB], + len(payloads), + ) + for row, payload in enumerate(payloads): + tablet.add_timestamp(row, row) + tablet.add_value_by_name("device", row, "d1") + tablet.add_value_by_name("payload", row, payload) + writer.write_table(tablet) + + with TsFileReader(path) as reader: + result = reader.query_table("t", ["device", "payload"]) + try: + values = [] + while result.next(): + values.append(result.get_value_by_index(3)) + finally: + result.close() + + assert values == payloads + + +def test_repeated_schema_registration_round_trip(tmp_path): + path = str(tmp_path / "many_series.tsfile") + series_count = 64 + writer = TsFileWriter(path) + for index in range(series_count): + writer.register_timeseries( + "root.device1", TimeseriesSchema(f"s{index}", TSDataType.INT64) + ) + for timestamp in range(4): + fields = [ + Field(f"s{index}", timestamp * 1000 + index, TSDataType.INT64) + for index in range(series_count) + ] + writer.write_row_record(RowRecord("root.device1", timestamp, fields)) + writer.close() + + with TsFileReader(path) as reader: + schemas = reader.get_all_timeseries_schemas() + assert set(schemas) == {"root.device1"} + assert len(schemas["root.device1"].get_timeseries_list()) == series_count + result = reader.query_timeseries( + "root.device1", [f"s{index}" for index in range(series_count)], 0, 10 + ) + try: + rows = 0 + while result.next(): + assert result.get_value_by_index(2) == rows * 1000 + rows += 1 + assert rows == 4 + finally: + result.close() diff --git a/python/tsfile/tsfile_cpp.pxd b/python/tsfile/tsfile_cpp.pxd index 2015359ee..d5f2539cf 100644 --- a/python/tsfile/tsfile_cpp.pxd +++ b/python/tsfile/tsfile_cpp.pxd @@ -379,6 +379,13 @@ cdef extern from "cwrapper/tsfile_cwrapper.h": void tsfile_free_device_id_array(DeviceID * devices, uint32_t length); + void free_table_schema(TableSchema schema); + void free_device_schema(DeviceSchema schema); + void tsfile_free_table_schema_array(TableSchema * schemas, + uint32_t length); + void tsfile_free_device_schema_array(DeviceSchema * schemas, + uint32_t length); + ErrorCode tsfile_reader_get_timeseries_metadata_all( TsFileReader reader, DeviceTimeseriesMetadataMap * out_map); ErrorCode tsfile_reader_get_timeseries_metadata_for_devices( @@ -454,9 +461,11 @@ cdef extern from "cwrapper/tsfile_cwrapper.h": float tsfile_result_set_get_value_by_index_float(ResultSet result_set, uint32_t column_index); double tsfile_result_set_get_value_by_index_double(ResultSet result_set, uint32_t column_index); char * tsfile_result_set_get_value_by_index_string(ResultSet result_set, uint32_t column_index); + ErrorCode tsfile_result_set_get_value_by_index_binary(ResultSet result_set, uint32_t column_index, uint8_t ** out_value, uint32_t * out_length); ResultSetMetaData tsfile_result_set_get_metadata(ResultSet result_set); void free_result_set_meta_data(ResultSetMetaData result_set_meta_data); + void tsfile_free_buffer(void * buffer); # Arrow structures ctypedef struct ArrowSchema: diff --git a/python/tsfile/tsfile_py_cpp.pxd b/python/tsfile/tsfile_py_cpp.pxd index d20a66c63..b7106a03e 100644 --- a/python/tsfile/tsfile_py_cpp.pxd +++ b/python/tsfile/tsfile_py_cpp.pxd @@ -39,6 +39,9 @@ cdef public api void free_c_table_schema(TableSchema* c_schema) cdef public api void free_c_column_schema(ColumnSchema* c_schema) cdef public api void free_c_timeseries_schema(TimeseriesSchema* c_schema) cdef public api void free_c_device_schema(DeviceSchema* c_schema) +cdef public api void free_c_timeseries_schema_allocated(TimeseriesSchema* c_schema) +cdef public api void free_c_device_schema_allocated(DeviceSchema* c_schema) +cdef public api void free_c_table_schema_allocated(TableSchema* c_schema) cdef public api void free_c_tablet(Tablet tablet) cdef public api void free_c_row_record(TsRecord record) cdef public api TsFileWriter tsfile_writer_new_c(object pathname, uint64_t memory_threshold) except NULL diff --git a/python/tsfile/tsfile_py_cpp.pyx b/python/tsfile/tsfile_py_cpp.pyx index 6fa557bf5..b6a7795b6 100644 --- a/python/tsfile/tsfile_py_cpp.pyx +++ b/python/tsfile/tsfile_py_cpp.pyx @@ -75,12 +75,14 @@ cdef object from_c_column_schema(ColumnSchema schema): return ColumnSchemaPy(column_name, data_type, category_type) cdef object from_c_table_schema(TableSchema schema): + """Convert a by-value C schema. The caller keeps ownership of the C data + and releases it after this returns (see get_table_schema and + tsfile_free_table_schema_array).""" cdef int i table_name = schema.table_name.decode('utf-8') columns = [] for i in range(schema.column_num): columns.append(from_c_column_schema(schema.column_schemas[i])) - free_c_table_schema(&schema) return TableSchemaPy(table_name, columns) cdef object from_c_timeseries_schema(TimeseriesSchema schema): @@ -91,12 +93,14 @@ cdef object from_c_timeseries_schema(TimeseriesSchema schema): return TimeseriesSchemaPy(timeseries_name, data_type, encoding, compression) cdef object from_c_device_schema(DeviceSchema schema): + """Convert a by-value C schema. The caller keeps ownership of the C data + and releases it after this returns (see + tsfile_free_device_schema_array).""" cdef int i device_name = schema.device_name.decode('utf-8') timeseries = [] for i in range(schema.timeseries_num): timeseries.append(from_c_timeseries_schema(schema.timeseries_schema[i])) - free_c_device_schema(&schema) return DeviceSchemaPy(device_name, timeseries) # Convert from python to c struct @@ -176,54 +180,75 @@ cdef CompressionType to_c_compression_type(object compression_type): cdef TimeseriesSchema * to_c_timeseries_schema(object py_schema): cdef TimeseriesSchema * c_schema - c_schema = malloc(sizeof(TimeseriesSchema)) - c_schema.timeseries_name = strdup(py_schema.timeseries_name.encode('utf-8')) - if py_schema.data_type is not None: - c_schema.data_type = to_c_data_type(py_schema.data_type) - else: + cdef TSDataType data_type + cdef TSEncoding encoding + cdef CompressionType compression + # Convert before allocating: a conversion error must not leak the schema. + if py_schema.data_type is None: raise ValueError("data_type cannot be None") - if py_schema.encoding_type is not None: - c_schema.encoding = to_c_encoding_type(py_schema.encoding_type) - else: + if py_schema.encoding_type is None: raise ValueError("encoding_type cannot be None") - if py_schema.compression_type is not None: - c_schema.compression = to_c_compression_type(py_schema.compression_type) - else: + if py_schema.compression_type is None: raise ValueError("compression_type cannot be None") + data_type = to_c_data_type(py_schema.data_type) + encoding = to_c_encoding_type(py_schema.encoding_type) + compression = to_c_compression_type(py_schema.compression_type) + c_schema = malloc(sizeof(TimeseriesSchema)) + c_schema.timeseries_name = strdup(py_schema.timeseries_name.encode('utf-8')) + c_schema.data_type = data_type + c_schema.encoding = encoding + c_schema.compression = compression return c_schema cdef DeviceSchema * to_c_device_schema(object py_schema): cdef DeviceSchema * c_schema + cdef Py_ssize_t i + cdef list names = [] + cdef list data_types = [] + cdef list encodings = [] + cdef list compressions = [] + # Convert before allocating so a bad timeseries entry cannot leak the + # partially built device schema. + for timeseries in py_schema.timeseries_list: + names.append(timeseries.timeseries_name.encode('utf-8')) + data_types.append(to_c_data_type(timeseries.data_type)) + encodings.append(to_c_encoding_type(timeseries.encoding_type)) + compressions.append(to_c_compression_type(timeseries.compression_type)) c_schema = malloc(sizeof(DeviceSchema)) c_schema.device_name = strdup(py_schema.device_name.encode('utf-8')) - c_schema.timeseries_num = len(py_schema.timeseries_list) + c_schema.timeseries_num = len(names) c_schema.timeseries_schema = malloc(c_schema.timeseries_num * sizeof(TimeseriesSchema)) for i in range(c_schema.timeseries_num): - c_schema.timeseries_schema[i].timeseries_name = strdup( - py_schema.timeseries_list[i].timeseries_name.encode('utf-8')) - c_schema.timeseries_schema[i].data_type = to_c_data_type(py_schema.timeseries_list[i].data_type) - c_schema.timeseries_schema[i].encoding = to_c_encoding_type(py_schema.timeseries_list[i].encoding_type) - c_schema.timeseries_schema[i].compression = to_c_compression_type(py_schema.timeseries_list[i].compression_type) + c_schema.timeseries_schema[i].timeseries_name = strdup(names[i]) + c_schema.timeseries_schema[i].data_type = data_types[i] + c_schema.timeseries_schema[i].encoding = encodings[i] + c_schema.timeseries_schema[i].compression = compressions[i] return c_schema cdef ColumnSchema * to_c_column_schema(object py_schema): cdef ColumnSchema * c_schema + cdef TSDataType data_type = to_c_data_type(py_schema.data_type) c_schema = malloc(sizeof(ColumnSchema)) - c_schema.data_type = to_c_data_type(py_schema.data_type) + c_schema.data_type = data_type c_schema.column_category = py_schema.category c_schema.column_name = strdup(py_schema.column_name.encode('utf-8')) return c_schema cdef TableSchema * to_c_table_schema(object py_schema): cdef TableSchema * c_schema + cdef Py_ssize_t i + cdef list columns = py_schema.columns + cdef list names = [column.column_name.encode('utf-8') for column in columns] + cdef list categories = [to_c_category_type(column.category) for column in columns] + cdef list data_types = [to_c_data_type(column.data_type) for column in columns] c_schema = malloc(sizeof(TableSchema)) c_schema.table_name = strdup(py_schema.table_name.encode('utf-8')) - c_schema.column_num = len(py_schema.columns) + c_schema.column_num = len(columns) c_schema.column_schemas = malloc(c_schema.column_num * sizeof(ColumnSchema)) for i in range(c_schema.column_num): - c_schema.column_schemas[i].column_name = strdup(py_schema.columns[i].column_name.encode('utf-8')) - c_schema.column_schemas[i].column_category = to_c_category_type(py_schema.columns[i].category) - c_schema.column_schemas[i].data_type = to_c_data_type(py_schema.columns[i].data_type) + c_schema.column_schemas[i].column_name = strdup(names[i]) + c_schema.column_schemas[i].column_category = categories[i] + c_schema.column_schemas[i].data_type = data_types[i] return c_schema cdef Tablet to_c_tablet(object tablet): @@ -766,6 +791,24 @@ cdef void free_c_device_schema(DeviceSchema * c_schema): free_c_timeseries_schema(&(c_schema.timeseries_schema[i])) free(c_schema.timeseries_schema) +# The helpers above free only the contents. They are used for schemas built by +# to_c_*_schema, where this extension owns both the allocation and the +# deallocation. Reader-owned schemas are released through the C API instead +# (free_table_schema / tsfile_free_*_schema_array), so the allocator and the +# deallocator stay in the same module. These variants also release the struct +# itself and are used by writer callers. +cdef void free_c_timeseries_schema_allocated(TimeseriesSchema * c_schema): + free_c_timeseries_schema(c_schema) + free(c_schema) + +cdef void free_c_device_schema_allocated(DeviceSchema * c_schema): + free_c_device_schema(c_schema) + free(c_schema) + +cdef void free_c_table_schema_allocated(TableSchema * c_schema): + free_c_table_schema(c_schema) + free(c_schema) + cdef void free_c_tablet(Tablet tablet): free_tablet(&tablet) @@ -1243,7 +1286,12 @@ cdef object get_table_schema(TsFileReader reader, object table_name): code = tsfile_reader_get_table_schema_checked( reader, table_name_c, &schema) check_error(code) - return from_c_table_schema(schema) + try: + return from_c_table_schema(schema) + finally: + # Release through the C API so the allocator and the deallocator live + # in the same module (see tsfile_free_buffer). + free_table_schema(schema) cdef object get_all_table_schema(TsFileReader reader): cdef uint32_t table_num = 0 @@ -1253,11 +1301,16 @@ cdef object get_all_table_schema(TsFileReader reader): table_schemas = {} schemas = tsfile_reader_get_all_table_schemas_with_error(reader, &table_num, &error_code) - check_error(error_code) - for i in range(table_num): - schema_py = from_c_table_schema(schemas[i]) - table_schemas.update([(schema_py.get_table_name(), schema_py)]) - free(schemas) + try: + check_error(error_code) + for i in range(table_num): + schema_py = from_c_table_schema(schemas[i]) + table_schemas.update([(schema_py.get_table_name(), schema_py)]) + finally: + # Frees every element's contents and the array itself inside the C + # wrapper, including when conversion fails mid-loop. table_num is zero + # (and schemas NULL) when the C call reported an error. + tsfile_free_table_schema_array(schemas, table_num) return table_schemas cdef object get_all_timeseries_schema(TsFileReader reader): @@ -1269,11 +1322,13 @@ cdef object get_all_timeseries_schema(TsFileReader reader): device_schemas = {} error_code = tsfile_reader_get_all_timeseries_schemas_checked( reader, &schemas, &device_num) - check_error(error_code) - for i in range(device_num): - schema_py = from_c_device_schema(schemas[i]) - device_schemas.update([(schema_py.get_device_name(), schema_py)]) - free(schemas) + try: + check_error(error_code) + for i in range(device_num): + schema_py = from_c_device_schema(schemas[i]) + device_schemas.update([(schema_py.get_device_name(), schema_py)]) + finally: + tsfile_free_device_schema_array(schemas, device_num) return device_schemas cdef object _c_str_to_py_utf8_or_none(char* p): diff --git a/python/tsfile/tsfile_reader.pyx b/python/tsfile/tsfile_reader.pyx index a5d7446b5..c4fa4fd7a 100644 --- a/python/tsfile/tsfile_reader.pyx +++ b/python/tsfile/tsfile_reader.pyx @@ -23,7 +23,6 @@ import weakref from typing import List, Optional, Dict import pandas as pd -from libc.string cimport strlen from libc.stdlib cimport free, malloc from cpython.bytes cimport PyBytes_FromStringAndSize from cpython.ref cimport PyObject @@ -80,9 +79,11 @@ cdef class ResultSetPy: cdef ResultSetMetaData metadata_c self.result = result metadata_c = tsfile_result_set_get_metadata(self.result) - self.metadata = from_c_result_set_meta_data(metadata_c) - self.metadata.set_table_name(device_name) - free_result_set_meta_data(metadata_c) + try: + self.metadata = from_c_result_set_meta_data(metadata_c) + self.metadata.set_table_name(device_name) + finally: + free_result_set_meta_data(metadata_c) def next(self): """ @@ -210,6 +211,9 @@ cdef class ResultSetPy: NOTE: index start from 1. """ cdef char * string = NULL + cdef uint8_t * blob = NULL + cdef uint32_t blob_len = 0 + cdef ErrorCode binary_code = 0 self.check_result_set_invalid() # Well when we check is null, id from 0, so there index -1. if tsfile_result_set_is_null_by_index(self.result, index): @@ -230,21 +234,25 @@ cdef class ResultSetPy: elif data_type == TSDataTypePy.BOOLEAN: return tsfile_result_set_get_value_by_index_bool(self.result, index) elif data_type == TSDataTypePy.STRING or data_type == TSDataTypePy.TEXT: + string = tsfile_result_set_get_value_by_index_string(self.result, index) + if string == NULL: + return None try: - string = tsfile_result_set_get_value_by_index_string(self.result, index) - if string == NULL: - return None return string.decode('utf-8') finally: - pass + tsfile_free_buffer(string) elif data_type == TSDataTypePy.BLOB: + binary_code = tsfile_result_set_get_value_by_index_binary( + self.result, index, &blob, &blob_len) try: - string = tsfile_result_set_get_value_by_index_string(self.result, index) - if string == NULL: - return None - return PyBytes_FromStringAndSize(string, strlen(string)) + check_error(binary_code) + if blob == NULL: + return b"" + # Use the length-returning getter: strlen would truncate a + # BLOB at its first NUL byte. + return PyBytes_FromStringAndSize(blob, blob_len) finally: - pass + tsfile_free_buffer(blob) def get_value_by_name(self, column_name : str): """ diff --git a/python/tsfile/tsfile_writer.pyx b/python/tsfile/tsfile_writer.pyx index 4df03b449..7ae95c635 100644 --- a/python/tsfile/tsfile_writer.pyx +++ b/python/tsfile/tsfile_writer.pyx @@ -49,7 +49,7 @@ cdef class TsFileWriterPy: errno = tsfile_writer_register_timeseries_py_cpp(self.writer, device_name, c_schema) check_error(errno) finally: - free_c_timeseries_schema(c_schema) + free_c_timeseries_schema_allocated(c_schema) def register_device(self, device_schema : DeviceSchemaPy): """ @@ -62,7 +62,7 @@ cdef class TsFileWriterPy: errno = tsfile_writer_register_device_py_cpp(self.writer, device_schema_c) check_error(errno) finally: - free_c_device_schema(device_schema_c) + free_c_device_schema_allocated(device_schema_c) def register_table(self, table_schema : TableSchemaPy): """ @@ -75,7 +75,7 @@ cdef class TsFileWriterPy: errno = tsfile_writer_register_table_py_cpp(self.writer, c_schema) check_error(errno) finally: - free_c_table_schema(c_schema) + free_c_table_schema_allocated(c_schema) def write_tablet(self, tablet : TabletPy): """