Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 49 additions & 2 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,53 @@ jobs:
if: steps.detect-simd.outputs.opt_level != ''
run: ctest --test-dir build --output-on-failure

# nsparse deserializes binary index files that may be truncated or corrupt, and
# the ~120 negative-path assertions only prove an exception was thrown -- not
# that no out-of-bounds read, overflow or misaligned access happened first. The
# jobs above build Release with no sanitizers, so nothing checks that today.
#
# UBSan carries as much weight as ASan here: overflow and misalignment are what
# binary parsing produces. RelWithDebInfo, because -O0 changes which code the
# optimizer would have folded away and the reports need line numbers.
#
# generic, and only generic: tests/CMakeLists.txt registers the per-ISA
# distance_kernel_equivalence_test binaries (and builds nsparse_avx2 /
# nsparse_avx512 to link them) at every NSPARSE_OPT_LEVEL, so this one job
# already runs the vectorized kernels -- where tail-element OOB would live --
# under ASan. A second SIMD-configured job would only re-cover them.
#
# Not in the container the other Linux jobs use: this needs the libasan/libubsan
# runtimes, which ubuntu-latest ships with gcc and the CI image does not
# promise.
Sanitize-nsparse-Linux:
name: ASan and UBSan nsparse on Linux
needs: check-files
if: needs.check-files.outputs.RUN_BUILD_AND_TEST == 'true'
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4

- name: Configure
run: |
cmake -B build \
-DNSPARSE_ENABLE_TESTS=ON \
-DNSPARSE_OPT_LEVEL=generic \
-DNSPARSE_ENABLE_SANITIZERS=ON \
-DCMAKE_BUILD_TYPE=RelWithDebInfo

- name: Build
run: cmake --build build -j$(nproc)

# print_stacktrace turns a UBSan one-liner into something you can act on;
# without it the report names the source line but not the caller that
# reached it.
- name: Test
env:
ASAN_OPTIONS: detect_leaks=1:detect_stack_use_after_return=1:strict_string_checks=1:check_initialization_order=1:strict_init_order=1
UBSAN_OPTIONS: print_stacktrace=1
run: ctest --test-dir build --output-on-failure

Build-nsparse-Python-Linux:
name: Build and Test nsparse Python bindings on Linux
needs: check-files
Expand Down Expand Up @@ -260,13 +307,13 @@ jobs:
run: ctest --test-dir build --build-config Release --output-on-failure

check-results:
needs: [check-files, Build-nsparse-Linux, Build-nsparse-Python-Linux, Build-nsparse-MacOS, Build-nsparse-Windows]
needs: [check-files, Build-nsparse-Linux, Sanitize-nsparse-Linux, Build-nsparse-Python-Linux, Build-nsparse-MacOS, Build-nsparse-Windows]
if: always()
name: Check results
runs-on: ubuntu-latest
steps:
- name: Fail if build or test failed
if: |
needs.check-files.outputs.RUN_BUILD_AND_TEST == 'true' &&
(needs.Build-nsparse-Linux.result == 'failure' || needs.Build-nsparse-Python-Linux.result == 'failure' || needs.Build-nsparse-MacOS.result == 'failure' || needs.Build-nsparse-Windows.result == 'failure')
(needs.Build-nsparse-Linux.result == 'failure' || needs.Sanitize-nsparse-Linux.result == 'failure' || needs.Build-nsparse-Python-Linux.result == 'failure' || needs.Build-nsparse-MacOS.result == 'failure' || needs.Build-nsparse-Windows.result == 'failure')
run: exit 1
4 changes: 4 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ if(WIN32)
add_compile_definitions(NOMINMAX WIN32_LEAN_AND_MEAN)
endif()

# Sanitizers, if requested. Included before any target exists so the flags reach
# every one of them, third-party dependencies included.
include(cmake/sanitizers.cmake)

# Third party dependencies
include(cmake/third_party.cmake)

Expand Down
34 changes: 34 additions & 0 deletions DEVELOPER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
- [SIMD Optimization Levels](#simd-optimization-levels)
- [GPU Acceleration](#gpu-acceleration)
- [Run Tests](#run-tests)
- [Python integration tests](#python-integration-tests)
- [Sanitizers](#sanitizers)
- [Run Benchmarks](#run-benchmarks)
- [Python Bindings](#python-bindings)
- [Build Python Bindings](#build-python-bindings)
Expand Down Expand Up @@ -98,6 +100,7 @@ cmake --build build -j
| `NSPARSE_ENABLE_TESTS` | `OFF` | Build unit tests |
| `NSPARSE_ENABLE_BENCHMARKS` | `OFF` | Build benchmarks |
| `NSPARSE_ENABLE_GPU` | `OFF` | GPU-accelerate index building via cuSPARSE (see [GPU Acceleration](#gpu-acceleration)) |
| `NSPARSE_ENABLE_SANITIZERS` | `OFF` | Build with ASan and UBSan (see [Sanitizers](#sanitizers)) |

Example with multiple options:
```bash
Expand Down Expand Up @@ -212,6 +215,37 @@ schedule the lists in.
subprocesses, which is required because the OpenMP runtime reads that variable
when it initialises.

### Sanitizers

`NSPARSE_ENABLE_SANITIZERS=ON` instruments the whole build with AddressSanitizer
and UndefinedBehaviorSanitizer. This matters most for the index-file readers: the
negative-path tests assert that a truncated or corrupt file raises an exception,
but only a sanitizer can tell you no out-of-bounds read, overflow or misaligned
access happened on the way to that exception.

```bash
cmake -S . -B build -DNSPARSE_ENABLE_TESTS=ON -DNSPARSE_ENABLE_SANITIZERS=ON \
-DCMAKE_BUILD_TYPE=RelWithDebInfo
cmake --build build -j
UBSAN_OPTIONS=print_stacktrace=1 ctest --test-dir build --output-on-failure
```

GCC and Clang only; you need the matching runtimes (`libasan`/`libubsan`, part of
`gcc` on Debian/Ubuntu, separate packages on Amazon Linux and RHEL). Findings
abort the process rather than print and continue, so any one of them fails the
test that hit it. `-DNSPARSE_SANITIZERS=<list>` overrides the default
`address,undefined` for a different `-fsanitize` set, e.g. `thread`.

Under GCC, the `null` check and the two nonnull-attribute checks are dropped:
they make the address of a function template instantiation non-constant, which
breaks abseil's `constexpr` hash-function dispatch in every translation unit that
includes `flat_hash_map`. A null dereference still shows up, as an ASan SEGV
report.

This runs per-PR in CI at the `generic` optimization level. The per-ISA
`distance_kernel_equivalence_test` binaries are built and run at every
optimization level, so that one configuration also covers the SIMD kernels.

## Run Benchmarks

Build with benchmarks enabled and run:
Expand Down
56 changes: 56 additions & 0 deletions cmake/sanitizers.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# Copyright OpenSearch Contributors
# SPDX-License-Identifier: Apache-2.0
#
# The OpenSearch Contributors require contributions made to
# this file be licensed under the Apache-2.0 license or a
# compatible open source license.

# Sanitizer instrumentation, off by default.
#
# nsparse parses binary index files that may be truncated or corrupt. The
# negative-path tests assert that an exception is thrown, but not that no
# out-of-bounds read, overflow or misaligned access happened on the way there --
# in an in-process native library those are far worse than an exception. ASan and
# UBSan are what turn those assertions into memory-safety assertions.
option(NSPARSE_ENABLE_SANITIZERS "Instrument the build with sanitizers" OFF)
set(NSPARSE_SANITIZERS "address,undefined" CACHE STRING
"Comma-separated -fsanitize list used when NSPARSE_ENABLE_SANITIZERS=ON")

if(NOT NSPARSE_ENABLE_SANITIZERS)
return()
endif()

if(MSVC)
# MSVC only ships /fsanitize=address, and spells it differently; nothing
# here would apply.
message(FATAL_ERROR "NSPARSE_ENABLE_SANITIZERS is only supported with GCC and Clang")
endif()

# Applied globally, before any target (including the FetchContent'ed abseil and
# GoogleTest) is defined: an ASan-instrumented translation unit and an
# uninstrumented one disagree about libstdc++ container annotations, which shows
# up as container-overflow false positives rather than a link error.
#
# -fno-sanitize-recover=all: by default UBSan prints and continues, so a
# findings-only run still exits 0 and ctest reports a pass. Aborting is what
# makes a UBSan finding fail CI.
set(NSPARSE_SANITIZER_FLAGS
-fsanitize=${NSPARSE_SANITIZERS}
-fno-sanitize-recover=all
-fno-omit-frame-pointer)

# GCC's null-pointer checks make the address of a function template
# instantiation non-constant, which breaks abseil's constexpr
# `get_hash_slot_fn() == nullptr` dispatch (hash_policy_traits.h) -- every
# translation unit that includes flat_hash_map fails to compile. Clang is
# unaffected. Dropping the three checks costs little here: a null dereference
# still surfaces, as an ASan SEGV report.
if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND NSPARSE_SANITIZERS MATCHES "undefined")
list(APPEND NSPARSE_SANITIZER_FLAGS
-fno-sanitize=null,nonnull-attribute,returns-nonnull-attribute)
endif()

add_compile_options(${NSPARSE_SANITIZER_FLAGS})
add_link_options(${NSPARSE_SANITIZER_FLAGS})

message(STATUS "Sanitizers enabled: ${NSPARSE_SANITIZERS}")
Loading