diff --git a/.clang-tidy b/.clang-tidy index 48ce291..fe018a3 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -16,7 +16,6 @@ Checks: > readability-duplicate-include, readability-else-after-return, readability-implicit-bool-conversion, - readability-inconsistent-declaration-parameter-name, readability-misleading-indentation, readability-misplaced-array-index, readability-non-const-parameter, @@ -44,6 +43,9 @@ Checks: > -modernize-use-trailing-return-type, -readability-function-cognitive-complexity, -readability-magic-numbers, + # low signal for a C library: header and implementation legitimately use + # different, more descriptive parameter names + -readability-inconsistent-declaration-parameter-name, -bugprone-reserved-identifier, -bugprone-branch-clone, -bugprone-macro-parentheses, diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c0b4e90..209bdba 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -239,6 +239,86 @@ jobs: - name: Build kernel modules run: ./scripts/container.sh ./scripts/ci-cd/step_build_kernel_module.sh + # ============================================ + # Static analysis (clang-tidy + cppcheck, semantic subset) + # Single-TU semantic lint. Cheap gate for confidently-wrong local + # mistakes; the cross-TU / runtime bugs are caught by the sanitizer legs. + # ============================================ + static-analysis: + runs-on: ubuntu-latest + needs: [changes, checks] + if: | + !failure() && !cancelled() && + (github.event_name == 'push' || + needs.changes.outputs.code == 'true' || + needs.changes.outputs.tests == 'true' || + needs.changes.outputs.ci == 'true') + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Load CI-Container + uses: ./.github/actions/load-container + with: + token: ${{ secrets.GITHUB_TOKEN }} + + # clang-tidy runs diff-only (changed lines) so the decoder's pre-existing + # backlog does not block new work; cppcheck runs on the full tree. + - name: Determine diff base + id: diffbase + run: | + if [ "${{ github.event_name }}" = "pull_request" ]; then + echo "base=${{ github.event.pull_request.base.sha }}" >> $GITHUB_OUTPUT + else + echo "base=${{ github.sha }}^" >> $GITHUB_OUTPUT + fi + + - name: Static analysis (clang-tidy diff + cppcheck) + run: ./scripts/container.sh ./scripts/ci-cd/step_static_analysis.sh --all --werror --diff "${{ steps.diffbase.outputs.base }}" + + # ============================================ + # Sanitizers - runtime verification of the semantics static lint can't see + # (byte packing, endianness, 48-bit offsets, shared-memory sync). + # asan = AddressSanitizer + UBSan tsan = ThreadSanitizer + # ============================================ + sanitizers: + runs-on: ubuntu-latest + needs: [changes, checks] + if: | + !failure() && !cancelled() && + (github.event_name == 'push' || + needs.changes.outputs.code == 'true' || + needs.changes.outputs.tests == 'true' || + needs.changes.outputs.ci == 'true') + strategy: + fail-fast: false + matrix: + kind: [asan, tsan] + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Setup ccache + run: mkdir -p ~/.ccache + + - name: Restore ccache + uses: actions/cache@v4 + with: + path: ~/.ccache + key: ccache-${{ runner.os }}-san-${{ matrix.kind }}-${{ hashFiles('**/CMakeLists.txt', '**/*.cmake') }}-${{ github.sha }} + restore-keys: | + ccache-${{ runner.os }}-san-${{ matrix.kind }}- + + - name: Load CI-Container + uses: ./.github/actions/load-container + with: + token: ${{ secrets.GITHUB_TOKEN }} + + - name: Run ${{ matrix.kind }} + run: ./scripts/container.sh ./scripts/ci-cd/step_sanitizers.sh ${{ matrix.kind }} + # ============================================ # Packaging (only for code/ci changes, or push to main) # ============================================ diff --git a/CMakeLists.txt b/CMakeLists.txt index b95c0c0..7a687a8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -11,7 +11,7 @@ include(cmake/ToolchainVersionCheck.cmake) include(cmake/CreateVersion.cmake) include(cmake/CheckPICEnabled.cmake) include(cmake/CodeCoverage.cmake) -include(cmake/ASAN.cmake) +include(cmake/Sanitizers.cmake) include(cmake/StaticAnalysis.cmake) include(cmake/CompileWarnings.cmake) diff --git a/CMakePresets.json b/CMakePresets.json index 4889edd..3a978bf 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -35,6 +35,23 @@ "CMAKE_INTERPROCEDURAL_OPTIMIZATION": "OFF" } }, + { + "name": "unittests-asan", + "inherits": "unittests-nolto", + "description": "ASan + UBSan, no LTO", + "cacheVariables": { + "ENABLE_ASAN": "ON", + "ENABLE_UBSAN": "ON" + } + }, + { + "name": "unittests-tsan", + "inherits": "unittests-nolto", + "description": "ThreadSanitizer, no LTO", + "cacheVariables": { + "ENABLE_TSAN": "ON" + } + }, { "name": "rpm", "inherits": "default", @@ -80,6 +97,14 @@ "all" ] }, + { + "name": "unittests-asan", + "configurePreset": "unittests-asan" + }, + { + "name": "unittests-tsan", + "configurePreset": "unittests-tsan" + }, { "name": "rpm", "configurePreset": "rpm", @@ -109,6 +134,26 @@ "environment": { "LLVM_PROFILE_FILE": "%p.profraw" } + }, + { + "name": "unittests-asan", + "configurePreset": "unittests-asan", + "output": { + "outputOnFailure": true + }, + "execution": { + "noTestsAction": "error" + } + }, + { + "name": "unittests-tsan", + "configurePreset": "unittests-tsan", + "output": { + "outputOnFailure": true + }, + "execution": { + "noTestsAction": "error" + } } ], "packagePresets": [ @@ -229,4 +274,4 @@ ] } ] -} \ No newline at end of file +} diff --git a/VERSION.md b/VERSION.md index 3285f83..15ee7b0 100644 --- a/VERSION.md +++ b/VERSION.md @@ -1,6 +1,14 @@ -1.7.0 +1.7.1 # Change log +## 1.7.1 +- fix: unaligned access on the byte-packed trace format in the writer and both decoders + (memcpy instead of typed-pointer loads/stores) - undefined behavior that faults on + strict-alignment targets such as s390x; no format change. +- fix: the unique-stack lookup index is freed on close (was leaked via the raw API). +- fix: the per-call-site offset cache is accessed with relaxed atomics (data race on + concurrent first use). Behavior preserved throughout; patch-level. + ## 1.7.0 - perf: registration lookups use a persisted open-addressing index instead of scanning the stack linearly. The index lives in memory per tracebuffer and is periodically persisted diff --git a/cmake/ASAN.cmake b/cmake/ASAN.cmake deleted file mode 100644 index e74938b..0000000 --- a/cmake/ASAN.cmake +++ /dev/null @@ -1,11 +0,0 @@ -option(ENABLE_ASAN "Enable AddressSanitizer" OFF) - -if(ENABLE_ASAN) - message(STATUS "AddressSanitizer enabled") -endif() - -# Add this after add_executable or add_library -if(ENABLE_ASAN) - add_compile_options(-fsanitize=address -fno-omit-frame-pointer) - add_link_options(-fsanitize=address) -endif() diff --git a/cmake/Sanitizers.cmake b/cmake/Sanitizers.cmake new file mode 100644 index 0000000..4d56683 --- /dev/null +++ b/cmake/Sanitizers.cmake @@ -0,0 +1,44 @@ +# Copyright (c) 2024, International Business Machines +# SPDX-License-Identifier: BSD-2-Clause-Patent +# +# Runtime sanitizer support. These matter more than static lint for this +# library: it packs bytes, swaps endianness, uses 48-bit bitfield offsets, +# and coordinates through shared-memory mutexes and atomics - exactly the +# semantics static analysis of a single translation unit cannot see. +# +# ENABLE_ASAN AddressSanitizer use-after-free, heap/stack overflow, leaks +# ENABLE_UBSAN UndefinedBehavior signed overflow, bad shifts, misalignment +# ENABLE_TSAN ThreadSanitizer data races +# +# ASan and TSan cannot be combined (both hook memory); UBSan composes with +# either. Sanitizers disable LTO here - instrumentation wants real frames. + +option(ENABLE_ASAN "Enable AddressSanitizer" OFF) +option(ENABLE_UBSAN "Enable UndefinedBehaviorSanitizer" OFF) +option(ENABLE_TSAN "Enable ThreadSanitizer" OFF) + +if(ENABLE_ASAN AND ENABLE_TSAN) + message(FATAL_ERROR "ENABLE_ASAN and ENABLE_TSAN are mutually exclusive") +endif() + +set(_clltk_sanitizers "") +if(ENABLE_ASAN) + list(APPEND _clltk_sanitizers address) +endif() +if(ENABLE_UBSAN) + list(APPEND _clltk_sanitizers undefined) +endif() +if(ENABLE_TSAN) + list(APPEND _clltk_sanitizers thread) +endif() + +if(_clltk_sanitizers) + string(REPLACE ";" "," _clltk_sanitizer_flag "${_clltk_sanitizers}") + message(STATUS "Sanitizers enabled: ${_clltk_sanitizer_flag}") + add_compile_options(-fsanitize=${_clltk_sanitizer_flag} -fno-omit-frame-pointer -g) + add_link_options(-fsanitize=${_clltk_sanitizer_flag}) + # UBSan defaults to warn-and-continue; make findings fatal so CI can gate. + if(ENABLE_UBSAN) + add_compile_options(-fno-sanitize-recover=undefined) + endif() +endif() diff --git a/command_line_tool/commands/decode/meta.cpp b/command_line_tool/commands/decode/meta.cpp index a5079ee..57bb772 100644 --- a/command_line_tool/commands/decode/meta.cpp +++ b/command_line_tool/commands/decode/meta.cpp @@ -65,7 +65,8 @@ static void print_entry_row(Output &out, const MetaEntryInfo &entry, bool full) } else { std::string format_display = entry.format; if (format_display.size() > 50) { - format_display = format_display.substr(0, 47) + "..."; + format_display.resize(47); + format_display += "..."; } std::string file_display = entry.file; diff --git a/decoder_tool/cpp/CMakeLists.txt b/decoder_tool/cpp/CMakeLists.txt index ac22857..5f71480 100644 --- a/decoder_tool/cpp/CMakeLists.txt +++ b/decoder_tool/cpp/CMakeLists.txt @@ -8,6 +8,16 @@ endif() find_package(Boost REQUIRED CONFIG) +# Treat Boost headers as system headers so static analysis (clang-tidy / +# clang-analyzer) does not report findings inside them. +if(TARGET Boost::boost) + get_target_property(_clltk_boost_inc Boost::boost INTERFACE_INCLUDE_DIRECTORIES) + if(_clltk_boost_inc) + set_target_properties(Boost::boost PROPERTIES + INTERFACE_SYSTEM_INCLUDE_DIRECTORIES "${_clltk_boost_inc}") + endif() +endif() + set(TARGET_SOURCES source/low_level/file.cpp source/low_level/ringbuffer.cpp diff --git a/decoder_tool/cpp/include/CommonLowLevelTracingKit/decoder/Inline.hpp b/decoder_tool/cpp/include/CommonLowLevelTracingKit/decoder/Inline.hpp index 3821fd4..29fe8ad 100644 --- a/decoder_tool/cpp/include/CommonLowLevelTracingKit/decoder/Inline.hpp +++ b/decoder_tool/cpp/include/CommonLowLevelTracingKit/decoder/Inline.hpp @@ -58,6 +58,8 @@ template CONST_INLINE constexpr To safe_cast(From v } else { // both signed or both unsigned using Common = std::common_type_t; + // NOLINTNEXTLINE(bugprone-signed-char-misuse,cert-str34-c): a signed source + // value must keep its sign here - this is a saturating numeric cast const Common v = static_cast(value); const Common lo = static_cast(std::numeric_limits::min()); const Common hi = static_cast(std::numeric_limits::max()); diff --git a/decoder_tool/cpp/include/CommonLowLevelTracingKit/decoder/Tracebuffer.hpp b/decoder_tool/cpp/include/CommonLowLevelTracingKit/decoder/Tracebuffer.hpp index 97a4b50..47e1901 100644 --- a/decoder_tool/cpp/include/CommonLowLevelTracingKit/decoder/Tracebuffer.hpp +++ b/decoder_tool/cpp/include/CommonLowLevelTracingKit/decoder/Tracebuffer.hpp @@ -164,7 +164,7 @@ namespace CommonLowLevelTracingKit::decoder { uint64_t pending = 0; // current entries in buffer (entries - dropped) uint64_t dropped = 0; uint64_t wrapped = 0; - std::filesystem::file_time_type modified{}; + std::filesystem::file_time_type modified; std::optional error; bool valid() const noexcept { return !error.has_value(); } diff --git a/decoder_tool/cpp/source/Meta.cpp b/decoder_tool/cpp/source/Meta.cpp index f7766eb..dba5038 100644 --- a/decoder_tool/cpp/source/Meta.cpp +++ b/decoder_tool/cpp/source/Meta.cpp @@ -49,7 +49,7 @@ namespace { } MetaSourceInfo readTraceBufferMeta(const fs::path &path) { - MetaSourceInfo info; + MetaSourceInfo info{}; info.path = path; info.source_type = MetaSourceType::Tracebuffer; @@ -70,7 +70,7 @@ namespace { } MetaSourceInfo readRawMetaMeta(const fs::path &path) { - MetaSourceInfo info; + MetaSourceInfo info{}; info.path = path; info.source_type = MetaSourceType::RawBlob; info.name = path.stem().string(); diff --git a/decoder_tool/cpp/source/Tracepoint.cpp b/decoder_tool/cpp/source/Tracepoint.cpp index 5f0e721..db9c174 100644 --- a/decoder_tool/cpp/source/Tracepoint.cpp +++ b/decoder_tool/cpp/source/Tracepoint.cpp @@ -12,6 +12,7 @@ #include "file.hpp" #include "formatter.hpp" #include "ringbuffer.hpp" +#include using namespace CommonLowLevelTracingKit::decoder; using ToString = CommonLowLevelTracingKit::decoder::source::low_level::ToString; @@ -78,7 +79,7 @@ TracepointDynamic::TracepointDynamic(std::string tb_name, source::Ringbuffer::En return; } const char *const line_start = current; - m_line = *std::bit_cast(line_start); + std::memcpy(&m_line, line_start, sizeof(m_line)); if (e->foreignEndian()) { m_line = source::internal::byteswapValue(m_line); } current += line_len; remaining -= line_len; diff --git a/decoder_tool/cpp/source/TracepointInternal.hpp b/decoder_tool/cpp/source/TracepointInternal.hpp index 7f18126..6dfe6ac 100644 --- a/decoder_tool/cpp/source/TracepointInternal.hpp +++ b/decoder_tool/cpp/source/TracepointInternal.hpp @@ -5,6 +5,7 @@ #include "CommonLowLevelTracingKit/decoder/Tracepoint.hpp" #include "file.hpp" #include "ringbuffer.hpp" +#include #include #include #include @@ -16,8 +17,10 @@ template INLINE static constexpr T get(R r, size_t offset = 0, bool foreign_endian = false) { const uintptr_t base = std::bit_cast(r.data()); const uintptr_t position = base + offset; - const T *const ptr = reinterpret_cast(position); - T value = *ptr; + // trace data is byte-packed, so the source may be unaligned for T; + // copy the bytes out instead of a misaligned load + T value; + std::memcpy(&value, reinterpret_cast(position), sizeof(T)); if constexpr (CommonLowLevelTracingKit::decoder::source::internal::ByteSwappable) { if (foreign_endian) { value = CommonLowLevelTracingKit::decoder::source::internal::byteswapValue(value); diff --git a/decoder_tool/cpp/source/low_level/file.hpp b/decoder_tool/cpp/source/low_level/file.hpp index 5c95f4e..a6694da 100644 --- a/decoder_tool/cpp/source/low_level/file.hpp +++ b/decoder_tool/cpp/source/low_level/file.hpp @@ -60,7 +60,11 @@ namespace CommonLowLevelTracingKit::decoder::source { return value; } template INLINE T get(size_t offset = 0) const { - T value = *(const T *)getPtr(offset, sizeof(T)); + // trace data is byte-packed, so the source may be unaligned for T; + // copy the bytes out instead of a misaligned load + T value; + std::memcpy(&value, reinterpret_cast(getPtr(offset, sizeof(T))), + sizeof(T)); if constexpr (internal::ByteSwappable) { if (m_foreign_endian) { value = internal::byteswapValue(value); } } diff --git a/decoder_tool/cpp/source/low_level/formatter.cpp b/decoder_tool/cpp/source/low_level/formatter.cpp index 79f3dac..7f42bfb 100644 --- a/decoder_tool/cpp/source/low_level/formatter.cpp +++ b/decoder_tool/cpp/source/low_level/formatter.cpp @@ -454,7 +454,8 @@ std::string formatter::dump(const std::string_view format, const std::span(args_raw.data())); + uint32_t dump_size; + std::memcpy(&dump_size, args_raw.data(), sizeof(dump_size)); if (foreign_endian) { dump_size = CommonLowLevelTracingKit::decoder::source::internal::byteswapValue(dump_size); } diff --git a/examples/complex_c/complex_c.c b/examples/complex_c/complex_c.c index 5692644..5aefa84 100644 --- a/examples/complex_c/complex_c.c +++ b/examples/complex_c/complex_c.c @@ -23,7 +23,7 @@ int main(int argc, char *argv[]) LOOPS = atoi(a); } - printf("LOOPS %lu\n", (size_t)LOOPS); + printf("LOOPS %zu\n", (size_t)LOOPS); CLLTK_TRACEPOINT(COMPLEX_C, "LOOPS %lu", (size_t)LOOPS); for (size_t i = 0; i < (size_t)LOOPS; i++) { diff --git a/examples/complex_cpp/complex_cpp.cpp b/examples/complex_cpp/complex_cpp.cpp index 1f4dd62..6e2ba7c 100644 --- a/examples/complex_cpp/complex_cpp.cpp +++ b/examples/complex_cpp/complex_cpp.cpp @@ -34,7 +34,7 @@ int main(int argc, char *argv[]) LOOPS = std::stoi(a); } - printf("LOOPS %lu\n", (size_t)LOOPS); + printf("LOOPS %zu\n", (size_t)LOOPS); CLLTK_TRACEPOINT(COMPLEX_CPP, "LOOPS %lu", (size_t)LOOPS); for (size_t i = 0; i < (size_t)LOOPS; i++) { diff --git a/examples/process_threads/process_threads.cpp b/examples/process_threads/process_threads.cpp index 4c9d2b8..3dc93ed 100644 --- a/examples/process_threads/process_threads.cpp +++ b/examples/process_threads/process_threads.cpp @@ -39,7 +39,7 @@ static int func(char const *msg) } constexpr size_t t_n = 10; -void run_threads(std::string str) +void run_threads(const std::string &str) { std::array threads; std::array strings; diff --git a/scripts/ci-cd/step_sanitizers.sh b/scripts/ci-cd/step_sanitizers.sh new file mode 100755 index 0000000..7cc94c9 --- /dev/null +++ b/scripts/ci-cd/step_sanitizers.sh @@ -0,0 +1,51 @@ +#!/usr/bin/env bash +# Copyright (c) 2024, International Business Machines +# SPDX-License-Identifier: BSD-2-Clause-Patent + +# CI Step: build and run the test suite under a sanitizer. +# Usage: ./scripts/ci-cd/step_sanitizers.sh +# Exit: 0 = clean, 1 = sanitizer findings or build failure + +set -euo pipefail + +ROOT_PATH=$(git rev-parse --show-toplevel) +cd "${ROOT_PATH}" + +KIND="${1:-asan}" +case "$KIND" in + asan) PRESET="unittests-asan" ;; + tsan) PRESET="unittests-tsan" ;; + *) echo "usage: $0 "; exit 1 ;; +esac + +echo "========================================" +echo "CI Step: Sanitizer ($KIND)" +echo "========================================" + +# halt_on_error=1 makes the first finding fail the run instead of scrolling past. +# Do NOT set abort_on_error for ASan: the death tests expect ASan's default +# exit(1) on a caught error (EXPECT_EXIT(ExitedWithCode(1))); abort() would +# raise SIGABRT and fail those tests. +export ASAN_OPTIONS="detect_leaks=1:halt_on_error=1:${ASAN_OPTIONS:-}" +export UBSAN_OPTIONS="halt_on_error=1:print_stacktrace=1:${UBSAN_OPTIONS:-}" +# TSan understands pthreads but not our shared-memory futex mutexes; a +# suppression file (if present) keeps known-safe patterns from failing CI. +export TSAN_OPTIONS="halt_on_error=1:${TSAN_OPTIONS:-}" +if [[ "$KIND" == "tsan" && -f "$ROOT_PATH/scripts/ci-cd/tsan.suppressions" ]]; then + export TSAN_OPTIONS="suppressions=$ROOT_PATH/scripts/ci-cd/tsan.suppressions:$TSAN_OPTIONS" +fi + +echo "Configuring ($PRESET)..." +cmake --preset "$PRESET" --fresh + +echo "Building..." +cmake --build --preset "$PRESET" -- -j"$(nproc)" + +echo "Testing under $KIND..." +if ! ctest --preset "$PRESET" --output-on-failure; then + echo "FAILED: $KIND sanitizer found issues" + exit 1 +fi + +echo "PASSED: $KIND sanitizer clean" +exit 0 diff --git a/scripts/ci-cd/step_static_analysis.sh b/scripts/ci-cd/step_static_analysis.sh index 84c35a0..d56744e 100755 --- a/scripts/ci-cd/step_static_analysis.sh +++ b/scripts/ci-cd/step_static_analysis.sh @@ -53,12 +53,19 @@ usage() { exit 0 } +# When set, clang-tidy runs only on lines changed since this git ref +# (clang-tidy-diff). The decoder carries a large pre-existing clang-tidy +# backlog; diff mode gates new code without requiring the whole backlog to be +# cleared first. cppcheck still runs on the full tree. +DIFF_BASE="" + while [[ $# -gt 0 ]]; do case $1 in --clang-tidy) RUN_CLANG_TIDY=true; shift ;; --cppcheck) RUN_CPPCHECK=true; shift ;; --iwyu) RUN_IWYU=true; shift ;; --all) RUN_CLANG_TIDY=true; RUN_CPPCHECK=true; shift ;; + --diff) DIFF_BASE="$2"; shift 2 ;; --fix) FIX=true; shift ;; --werror) WERROR=true; shift ;; --filter) FILTER="$2"; shift 2 ;; @@ -140,7 +147,59 @@ ERRORS_FOUND=0 # ============================================================================= # clang-tidy # ============================================================================= +find_clang_tidy_diff() { + for p in /usr/share/clang/clang-tidy-diff.py \ + /usr/lib64/llvm*/share/clang/clang-tidy-diff.py \ + "$(command -v clang-tidy-diff.py 2>/dev/null)"; do + [[ -f "$p" ]] && { echo "$p"; return 0; } + done + return 1 +} + +# Diff mode: run clang-tidy only on lines changed since DIFF_BASE. +run_clang_tidy_diff() { + echo "----------------------------------------" + echo "Running clang-tidy on changes since ${DIFF_BASE}..." + echo "----------------------------------------" + + local diff_tool + if ! diff_tool=$(find_clang_tidy_diff); then + echo "clang-tidy-diff.py not found; cannot run diff mode" + ERRORS_FOUND=1 + return + fi + + # container repo is checked out by the host user but the container runs as + # root; tell git the mount is trusted so `git diff` works + git config --global --add safe.directory "$ROOT_PATH" 2>/dev/null || true + + # Only source files: headers are not translation units (not in + # compile_commands.json), and some intentionally #error when compiled + # standalone. Header changes are still covered by the full builds and by + # clang-tidy of the sources that include them. + # + # -U0: no context lines, so only changed lines are analyzed. -p1 strips the + # a/ b/ diff prefix. clang-tidy-diff exits non-zero on findings, so guard the + # assignment (set -e) and decide pass/fail from the output itself. + local out + out=$(git diff -U0 "${DIFF_BASE}" -- '*.c' '*.cpp' \ + | python3 "$diff_tool" -p1 -path "$BUILD_DIR" -clang-tidy-binary clang-tidy \ + -extra-arg=-Wno-unknown-warning-option 2>&1) || true + echo "$out" + + if echo "$out" | grep -qE ': (warning|error):'; then + echo "clang-tidy: FAILED (findings on changed lines)" + ERRORS_FOUND=1 + else + echo "clang-tidy: PASSED (no findings on changed lines)" + fi +} + run_clang_tidy() { + if [[ -n "$DIFF_BASE" ]]; then + run_clang_tidy_diff + return + fi echo "----------------------------------------" echo "Running clang-tidy..." echo "----------------------------------------" @@ -152,8 +211,12 @@ run_clang_tidy() { echo "Analyzing $file_count files..." - local tidy_args=("-p=$BUILD_DIR") - + # compile_commands.json comes from the gcc build, so it carries GCC-only + # warning flags (-Wlogical-op, -Wduplicated-cond, ...) that clang-tidy's + # clang front-end does not know. Silence those meta-diagnostics; they are + # not findings about the code. + local tidy_args=("-p=$BUILD_DIR" "--extra-arg=-Wno-unknown-warning-option") + if $FIX; then tidy_args+=("--fix" "--fix-errors") fi @@ -212,6 +275,31 @@ run_cppcheck() { cppcheck_args+=("--suppress=*:*boost*") cppcheck_args+=("--suppress=*:/usr/include/*") cppcheck_args+=("--suppress=*:/usr/local/include/*") + # Known false positives, scoped to the file: + # - stack_alloc() uses alloca() by design for the hot path; cppcheck cannot + # see that it initializes the buffer, hence allocaCalled + legacyUninitvar. + cppcheck_args+=("--suppress=allocaCalled:*/tracepoint.c") + cppcheck_args+=("--suppress=legacyUninitvar:*/tracepoint.c") + # - test code may use alloca directly (it is testing the allocator). + cppcheck_args+=("--suppress=allocaCalled:*/tests/*") + # - the discovery-section bounds are distinct linker symbols; comparing them + # is the intended way to iterate the section. + cppcheck_args+=("--suppress=comparePointers:*/_user_tracing.h") + # - cppcheck's C++ parser chokes on some test constructs (not a code defect). + cppcheck_args+=("--suppress=syntaxError:*/tests/*") + # - test code intentionally uses terse casts and skips malloc-failure checks. + cppcheck_args+=("--suppress=dangerousTypeCast:*/tests/*") + cppcheck_args+=("--suppress=nullPointerOutOfResources:*/tests/*") + # - each *.tests directory is a separate executable, so identically named + # test fixtures in different suites never share a translation unit; the + # whole-program ODR check is a false positive across separate binaries. + cppcheck_args+=("--suppress=ctuOneDefinitionRuleViolation:*/tests/*") + # - COMMAND_INIT registration runs at startup and is intentionally + # noexcept: an exception there should fail-fast (terminate), not unwind. + cppcheck_args+=("--suppress=throwInNoexceptFunction:*/command_line_tool/*") + # - false positive: FilePart grow() mutates m_file->size() between the outer + # and inner checks, so the inner condition is not always true. + cppcheck_args+=("--suppress=identicalInnerCondition:*/file.cpp") if ! cppcheck "${cppcheck_args[@]}" 2>&1; then echo "cppcheck: FAILED" diff --git a/scripts/ci-cd/tsan.suppressions b/scripts/ci-cd/tsan.suppressions new file mode 100644 index 0000000..28341b2 --- /dev/null +++ b/scripts/ci-cd/tsan.suppressions @@ -0,0 +1,12 @@ +# ThreadSanitizer suppressions - each entry is a proven-benign or +# intentional race, not a blanket mute. Real races must still fail CI. + +# parallelism.tests.cpp deliberately races a non-atomic increment against an +# atomic one to demonstrate why the atomic is needed (see the "will create +# race condition" comment). The race is the point of the test. +race:multi_thread_atomic_increment + +# sync.tests.cpp deliberately calls the mutex API from a signal handler to +# verify behavior; pthread_mutex_clocklock is not async-signal-safe, which +# is exactly what the test probes. +signal:signal_in_same_process__signal_handler diff --git a/scripts/container.Dockerfile b/scripts/container.Dockerfile index 3de451c..b49cee7 100644 --- a/scripts/container.Dockerfile +++ b/scripts/container.Dockerfile @@ -24,6 +24,9 @@ RUN dnf -y install \ lcov \ gcovr \ libasan \ + libubsan \ + libtsan \ + clang \ rpmdevtools \ cppcheck \ jq \ diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 2624722..51815aa 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -16,6 +16,9 @@ target_link_libraries(clltk_test_flags INTERFACE clltk_warnings_extra) if(ENABLE_ASAN) target_compile_definitions(clltk_test_flags INTERFACE CLLTK_ASAN_ENABLED) endif() +if(ENABLE_TSAN) + target_compile_definitions(clltk_test_flags INTERFACE CLLTK_TSAN_ENABLED) +endif() if(CMAKE_C_COMPILER_ID MATCHES "Clang") target_compile_options(clltk_test_flags INTERFACE -Wno-unneeded-internal-declaration diff --git a/tests/decoder.tests/Meta.tests.cpp b/tests/decoder.tests/Meta.tests.cpp index 4c75dcd..57cb5e1 100644 --- a/tests/decoder.tests/Meta.tests.cpp +++ b/tests/decoder.tests/Meta.tests.cpp @@ -258,7 +258,7 @@ TEST_F(MetaEntryInfoTest, argCharToTypeName) TEST_F(MetaEntryInfoTest, argumentTypeNames) { - MetaEntryInfo info; + MetaEntryInfo info{}; info.arg_types = "isL"; auto names = info.argumentTypeNames(); @@ -271,7 +271,7 @@ TEST_F(MetaEntryInfoTest, argumentTypeNames) TEST_F(MetaEntryInfoTest, argumentTypeNames_empty) { - MetaEntryInfo info; + MetaEntryInfo info{}; info.arg_types = ""; auto names = info.argumentTypeNames(); @@ -375,7 +375,7 @@ TEST_F(MetaIntegrationTest, getMetaInfo_filter_excludes) TEST_F(MetaIntegrationTest, MetaSourceInfo_valid) { - MetaSourceInfo info; + MetaSourceInfo info{}; EXPECT_TRUE(info.valid()); info.error = "some error"; diff --git a/tests/decoder.tests/file.tests.cpp b/tests/decoder.tests/file.tests.cpp index ad83295..a9d1d32 100644 --- a/tests/decoder.tests/file.tests.cpp +++ b/tests/decoder.tests/file.tests.cpp @@ -2,6 +2,7 @@ // SPDX-License-Identifier: BSD-2-Clause-Patent #include +#include #include #include #include @@ -15,6 +16,16 @@ #include "file.hpp" #include "gtest/gtest.h" +namespace +{ +template T read_unaligned(const void *p) +{ + T v; + std::memcpy(&v, p, sizeof(T)); + return v; +} +} // namespace + using namespace CommonLowLevelTracingKit; using namespace CommonLowLevelTracingKit::decoder; using namespace CommonLowLevelTracingKit::decoder::source; @@ -63,9 +74,9 @@ TEST_F(decoder_file_part, get) outFile.close(); const auto file = FilePart(m_file_name); ASSERT_EQ(file.get(), *reinterpret_cast(m_data.data())); - ASSERT_EQ(file.get(), *reinterpret_cast(m_data.data())); - ASSERT_EQ(file.get(), *reinterpret_cast(m_data.data())); - ASSERT_EQ(file.get(), *reinterpret_cast(m_data.data())); + ASSERT_EQ(file.get(), read_unaligned(m_data.data())); + ASSERT_EQ(file.get(), read_unaligned(m_data.data())); + ASSERT_EQ(file.get(), read_unaligned(m_data.data())); EXPECT_ANY_THROW(file.get(257)); } @@ -77,9 +88,9 @@ TEST_F(decoder_file_part, getRef) outFile.close(); const auto file = FilePart(m_file_name); ASSERT_EQ(file.getReference(), *reinterpret_cast(m_data.data())); - ASSERT_EQ(file.getReference(), *reinterpret_cast(m_data.data())); - ASSERT_EQ(file.getReference(), *reinterpret_cast(m_data.data())); - ASSERT_EQ(file.getReference(), *reinterpret_cast(m_data.data())); + ASSERT_EQ(file.getReference(), read_unaligned(m_data.data())); + ASSERT_EQ(file.getReference(), read_unaligned(m_data.data())); + ASSERT_EQ(file.getReference(), read_unaligned(m_data.data())); EXPECT_ANY_THROW(file.get(257)); } @@ -91,10 +102,18 @@ TEST_F(decoder_file_part, getFilePart) outFile.close(); const auto file = FilePart(m_file_name); const auto subFile = file.get(1); - ASSERT_EQ(subFile.getReference(), *reinterpret_cast(m_data.data() + 1)); - ASSERT_EQ(subFile.getReference(), *reinterpret_cast(m_data.data() + 1)); - ASSERT_EQ(subFile.getReference(), *reinterpret_cast(m_data.data() + 1)); - ASSERT_EQ(subFile.getReference(), *reinterpret_cast(m_data.data() + 1)); + // getReference is zero-copy (byte access only); multi-byte values live at + // unaligned offsets here, so read them with the copying get() accessor + // and compare against unaligned-safe reads of the source bytes + const auto raw = [this](size_t off, auto sample) { + decltype(sample) v; + std::memcpy(&v, m_data.data() + off, sizeof(v)); + return v; + }; + ASSERT_EQ(subFile.getReference(), raw(1, uint8_t{})); + ASSERT_EQ(subFile.get(), raw(1, uint16_t{})); + ASSERT_EQ(subFile.get(), raw(1, uint32_t{})); + ASSERT_EQ(subFile.get(), raw(1, uint64_t{})); ASSERT_EQ(&file.getReference(10), &subFile.getReference(10 - 1)); } diff --git a/tests/decoder.tests/ringbuffer.tests.cpp b/tests/decoder.tests/ringbuffer.tests.cpp index bbecca7..9f67f26 100644 --- a/tests/decoder.tests/ringbuffer.tests.cpp +++ b/tests/decoder.tests/ringbuffer.tests.cpp @@ -313,6 +313,17 @@ TEST_F(decoder_ringbuffer, read_parallel_overwhelmed) }; } std::for_each(threads.begin(), threads.end(), [](std::thread &t) { t.join(); }); + // The reader loop above stops after 100ms of no new entries. Under heavy + // load (e.g. ThreadSanitizer's slowdown) a writer can pause longer than + // that while still having produced entries, so drain whatever is left now + // that all writers have finished, making the "no pending" check deterministic. + for (;;) { + auto rc = rb.getNextEntry(); + if (rc.index() != 0) break; + auto tp = std::move(get<0>(rc)); + if (tp == nullptr) break; + nrs_vector.push_back(tp->nr); + } EXPECT_EQ(rb.pendingBytes(), 0) << "there should be no new tracepoints "; std::map frequency; diff --git a/tests/tracing.tests/abstraction.tests/file.tests.cpp b/tests/tracing.tests/abstraction.tests/file.tests.cpp index 3918bd3..4035a95 100644 --- a/tests/tracing.tests/abstraction.tests/file.tests.cpp +++ b/tests/tracing.tests/abstraction.tests/file.tests.cpp @@ -171,10 +171,10 @@ TEST_F(file, mmap) const char name[] = "temp_mmap_file_test"; auto temp_file = ::file_create_temp(name, memory_get_page_size()); EXPECT_TRUE(temp_file); + // ASAN's shadow memory layout may make the computed address writable, and + // TSAN does not fault on out-of-bounds access, so skip the death test there +#if !defined(CLLTK_ASAN_ENABLED) && !defined(CLLTK_TSAN_ENABLED) char *const base = std::bit_cast(file_mmap_ptr(temp_file)); - // ASAN's shadow memory layout may make the computed address writable, - // so skip the out-of-bounds death test under ASAN -#ifndef CLLTK_ASAN_ENABLED char *const invalid = base + file_mmap_size(temp_file) + 2 * memory_get_page_size(); EXPECT_EXIT({ *invalid = 'A'; }, KilledBySignal(SIGSEGV), ".*"); #endif diff --git a/tests/tracing.tests/abstraction.tests/memory.tests.cpp b/tests/tracing.tests/abstraction.tests/memory.tests.cpp index b3a244c..f7e4e7b 100644 --- a/tests/tracing.tests/abstraction.tests/memory.tests.cpp +++ b/tests/tracing.tests/abstraction.tests/memory.tests.cpp @@ -23,6 +23,10 @@ TEST(memory, heap_allocation) #ifdef CLLTK_ASAN_ENABLED EXPECT_EXIT({ *ptr = 'A'; }, ::testing::ExitedWithCode(1), ".*"); EXPECT_EXIT({ [[maybe_unused]] char c = *ptr; }, ::testing::ExitedWithCode(1), ".*"); +#elif defined(CLLTK_TSAN_ENABLED) + // ThreadSanitizer does not fault on use-after-free, so the death checks + // below would not trigger; skip them under TSan + (void)ptr; #else EXPECT_EXIT({ *ptr = 'A'; }, ::testing::KilledBySignal(SIGSEGV), ".*"); EXPECT_EXIT({ [[maybe_unused]] char c = *ptr; }, ::testing::KilledBySignal(SIGSEGV), ".*"); @@ -50,8 +54,9 @@ TEST(memory, equal_relocate) char *ptrA = std::bit_cast(memory_heap_allocation(first_size)); memcpy_and_flush(ptrA, data.data(), data.size() + 1); char *ptrB = std::bit_cast(memory_heap_realloc(ptrA, first_size)); - // ASAN's realloc always returns a new pointer to detect use-after-realloc bugs -#ifndef CLLTK_ASAN_ENABLED + // ASAN's (and TSAN's) realloc always returns a new pointer to detect + // use-after-realloc bugs, so in-place realloc cannot be assumed there +#if !defined(CLLTK_ASAN_ENABLED) && !defined(CLLTK_TSAN_ENABLED) ASSERT_EQ(ptrA, ptrB); #endif EXPECT_EQ(ptrB, data); diff --git a/tests/tracing.tests/arguments.tests/get_arguments.tests.cpp b/tests/tracing.tests/arguments.tests/get_arguments.tests.cpp index e5ecfef..60d531b 100644 --- a/tests/tracing.tests/arguments.tests/get_arguments.tests.cpp +++ b/tests/tracing.tests/arguments.tests/get_arguments.tests.cpp @@ -5,10 +5,21 @@ #include "arguments.h" #include "gtest/gtest.h" #include +#include #include #include #include +namespace +{ +template T read_unaligned(const void *p) +{ + T v; + std::memcpy(&v, p, sizeof(T)); + return v; +} +} // namespace + void _helper(const char *const format, void *const buffer, _clltk_argument_types_t *types, ...) { va_list args; @@ -47,19 +58,19 @@ TEST(get_argument, types_str_str_str) constexpr size_t argN_size = 5; // arg0 - EXPECT_EQ(*(uint32_t *)ptr, argN_size); + EXPECT_EQ(read_unaligned(ptr), argN_size); ptr += sizeof(uint32_t); EXPECT_TRUE(memcmp(ptr, "arg0", argN_size) == 0); ptr += argN_size; // arg1 - EXPECT_EQ(*(uint32_t *)ptr, argN_size); + EXPECT_EQ(read_unaligned(ptr), argN_size); ptr += sizeof(uint32_t); EXPECT_TRUE(memcmp(ptr, "arg1", argN_size) == 0); ptr += argN_size; // arg2 - EXPECT_EQ(*(uint32_t *)ptr, argN_size); + EXPECT_EQ(read_unaligned(ptr), argN_size); ptr += sizeof(uint32_t); EXPECT_TRUE(memcmp(ptr, "arg2", argN_size) == 0); ptr += argN_size; @@ -81,19 +92,19 @@ TEST(get_argument, types_str_int64_str) // arg0 const char *const arg0 = "some arg"; const size_t arg0_n = strlen(arg0) + 1; - EXPECT_EQ(*(uint32_t *)ptr, arg0_n); + EXPECT_EQ(read_unaligned(ptr), arg0_n); ptr += sizeof(uint32_t); EXPECT_TRUE(memcmp(ptr, arg0, arg0_n) == 0); ptr += arg0_n; // arg1 - EXPECT_EQ(0x4d61696e6c6f6f70, *(int64_t *)ptr); + EXPECT_EQ(0x4d61696e6c6f6f70, read_unaligned(ptr)); ptr += sizeof(int64_t); // arg2 const char *const arg2 = "work/folder/source-file.cpp line:62"; const size_t arg2_n = strlen(arg2) + 1; - EXPECT_EQ(*(uint32_t *)ptr, arg2_n); + EXPECT_EQ(read_unaligned(ptr), arg2_n); ptr += sizeof(uint32_t); EXPECT_TRUE(memcmp(ptr, arg2, arg2_n) == 0); ptr += arg2_n; @@ -113,7 +124,7 @@ TEST(get_argument, types_str) // arg0 const char *str0 = "Mainloop"; const size_t str0_n = strlen(str0) + 1; - EXPECT_EQ(*(uint32_t *)ptr, str0_n); + EXPECT_EQ(read_unaligned(ptr), str0_n); ptr += sizeof(uint32_t); EXPECT_TRUE(memcmp(ptr, str0, str0_n) == 0); ptr += str0_n; diff --git a/tests/tracing.tests/definition.tests/definition.test.cpp b/tests/tracing.tests/definition.tests/definition.test.cpp index c1ad687..ff0ec1a 100644 --- a/tests/tracing.tests/definition.tests/definition.test.cpp +++ b/tests/tracing.tests/definition.tests/definition.test.cpp @@ -397,8 +397,9 @@ TEST_F(DefinitionTest, binary_layout_verification) // Verify exact binary layout uint8_t *ptr = buffer.data(); - // [0-7]: body_size (uint64_t) - uint64_t body_size = *reinterpret_cast(ptr); + // [0-7]: body_size (uint64_t) - buffer is byte-aligned, read unaligned-safe + uint64_t body_size; + std::memcpy(&body_size, ptr, sizeof(body_size)); EXPECT_EQ(body_size, name_len + 1 + sizeof(definition_extended_t)); ptr += 8; diff --git a/tests/tracing.tests/meta.tests/meta_macro.tests.cpp b/tests/tracing.tests/meta.tests/meta_macro.tests.cpp index 5519a5c..2de2896 100644 --- a/tests/tracing.tests/meta.tests/meta_macro.tests.cpp +++ b/tests/tracing.tests/meta.tests/meta_macro.tests.cpp @@ -3,10 +3,21 @@ #include "CommonLowLevelTracingKit/tracing/tracing.h" #include "gtest/gtest.h" +#include #include #include #include +namespace +{ +template T read_unaligned(const void *p) +{ + T v; + std::memcpy(&v, p, sizeof(T)); + return v; +} +} // namespace + CLLTK_TRACEBUFFER(META_MACRO_00, 1024) TEST(meta_macro, str) { @@ -16,14 +27,15 @@ TEST(meta_macro, str) // the meta section holds {meta, offset-cache} pointer pairs per tracepoint const char *const meta = ((const char *const *)_clltk_META_MACRO_00.meta.start)[0]; const char magic = *reinterpret_cast(&meta[0]); - const uint32_t size = *reinterpret_cast(&meta[1]); - const _clltk_meta_enty_type type = *reinterpret_cast(&meta[5]); - const uint32_t line = *reinterpret_cast(&meta[6]); + const uint32_t size = read_unaligned(&meta[1]); + const _clltk_meta_enty_type type = read_unaligned<_clltk_meta_enty_type>(&meta[5]); + const uint32_t line = read_unaligned(&meta[6]); const uint8_t arg_count = *reinterpret_cast(&meta[10]); const char *const arg_types = reinterpret_cast(&meta[11]); - const char *const meta_file_name = reinterpret_cast(&meta[10 + arg_count + 2]); - const char *const meta_format = - reinterpret_cast(&meta[10 + arg_count + 2 + strlen(__FILE__) + 1]); + const char *const meta_file_name = + reinterpret_cast(&meta[10 + static_cast(arg_count) + 2]); + const char *const meta_format = reinterpret_cast( + &meta[10 + static_cast(arg_count) + 2 + strlen(__FILE__) + 1]); EXPECT_EQ(magic, '{'); EXPECT_EQ(size, 24 + strlen(__FILE__)); @@ -46,14 +58,15 @@ TEST(meta_macro, str_str) // the meta section holds {meta, offset-cache} pointer pairs per tracepoint const char *const meta = ((const char *const *)_clltk_META_MACRO_01.meta.start)[0]; const char magic = *reinterpret_cast(&meta[0]); - const uint32_t size = *reinterpret_cast(&meta[1]); - const _clltk_meta_enty_type type = *reinterpret_cast(&meta[5]); - const uint32_t line = *reinterpret_cast(&meta[6]); + const uint32_t size = read_unaligned(&meta[1]); + const _clltk_meta_enty_type type = read_unaligned<_clltk_meta_enty_type>(&meta[5]); + const uint32_t line = read_unaligned(&meta[6]); const uint8_t arg_count = *reinterpret_cast(&meta[10]); const char *const arg_types = reinterpret_cast(&meta[11]); - const char *const meta_file_name = reinterpret_cast(&meta[10 + arg_count + 2]); - const char *const meta_format = - reinterpret_cast(&meta[10 + arg_count + 2 + strlen(__FILE__) + 1]); + const char *const meta_file_name = + reinterpret_cast(&meta[10 + static_cast(arg_count) + 2]); + const char *const meta_format = reinterpret_cast( + &meta[10 + static_cast(arg_count) + 2 + strlen(__FILE__) + 1]); EXPECT_EQ(magic, '{'); EXPECT_EQ(size, 35 + strlen(__FILE__)); @@ -76,14 +89,15 @@ TEST(meta_macro, int64) // the meta section holds {meta, offset-cache} pointer pairs per tracepoint const char *const meta = ((const char *const *)_clltk_META_MACRO_02.meta.start)[0]; const char magic = *reinterpret_cast(&meta[0]); - const uint32_t size = *reinterpret_cast(&meta[1]); - const _clltk_meta_enty_type type = *reinterpret_cast(&meta[5]); - const uint32_t line = *reinterpret_cast(&meta[6]); + const uint32_t size = read_unaligned(&meta[1]); + const _clltk_meta_enty_type type = read_unaligned<_clltk_meta_enty_type>(&meta[5]); + const uint32_t line = read_unaligned(&meta[6]); const uint8_t arg_count = *reinterpret_cast(&meta[10]); const char *const arg_types = reinterpret_cast(&meta[11]); - const char *const meta_file_name = reinterpret_cast(&meta[10 + arg_count + 2]); - const char *const meta_format = - reinterpret_cast(&meta[10 + arg_count + 2 + strlen(__FILE__) + 1]); + const char *const meta_file_name = + reinterpret_cast(&meta[10 + static_cast(arg_count) + 2]); + const char *const meta_format = reinterpret_cast( + &meta[10 + static_cast(arg_count) + 2 + strlen(__FILE__) + 1]); EXPECT_EQ(magic, '{'); EXPECT_EQ(size, 25 + strlen(__FILE__)); @@ -109,16 +123,15 @@ TEST(meta_macro, two_tracepoints) CLLTK_TRACEPOINT(META_MACRO_03, "arg0 = %ld", arg0); const static uint32_t ref_line = __LINE__; const char magic = *reinterpret_cast(&meta[0]); - const uint32_t size = *reinterpret_cast(&meta[1]); - const _clltk_meta_enty_type type = - *reinterpret_cast(&meta[5]); - const uint32_t line = *reinterpret_cast(&meta[6]); + const uint32_t size = read_unaligned(&meta[1]); + const _clltk_meta_enty_type type = read_unaligned<_clltk_meta_enty_type>(&meta[5]); + const uint32_t line = read_unaligned(&meta[6]); const uint8_t arg_count = *reinterpret_cast(&meta[10]); const char *const arg_types = reinterpret_cast(&meta[11]); const char *const meta_file_name = - reinterpret_cast(&meta[10 + arg_count + 2]); - const char *const meta_format = - reinterpret_cast(&meta[10 + arg_count + 2 + strlen(__FILE__) + 1]); + reinterpret_cast(&meta[10 + static_cast(arg_count) + 2]); + const char *const meta_format = reinterpret_cast( + &meta[10 + static_cast(arg_count) + 2 + strlen(__FILE__) + 1]); EXPECT_EQ(magic, '{'); EXPECT_EQ(size, 25 + strlen(__FILE__)); @@ -136,16 +149,15 @@ TEST(meta_macro, two_tracepoints) CLLTK_TRACEPOINT(META_MACRO_03, "arg0 = %ld", arg0); const static uint32_t ref_line = __LINE__; const char magic = *reinterpret_cast(&meta[0]); - const uint32_t size = *reinterpret_cast(&meta[1]); - const _clltk_meta_enty_type type = - *reinterpret_cast(&meta[5]); - const uint32_t line = *reinterpret_cast(&meta[6]); + const uint32_t size = read_unaligned(&meta[1]); + const _clltk_meta_enty_type type = read_unaligned<_clltk_meta_enty_type>(&meta[5]); + const uint32_t line = read_unaligned(&meta[6]); const uint8_t arg_count = *reinterpret_cast(&meta[10]); const char *const arg_types = reinterpret_cast(&meta[11]); const char *const meta_file_name = - reinterpret_cast(&meta[10 + arg_count + 2]); - const char *const meta_format = - reinterpret_cast(&meta[10 + arg_count + 2 + strlen(__FILE__) + 1]); + reinterpret_cast(&meta[10 + static_cast(arg_count) + 2]); + const char *const meta_format = reinterpret_cast( + &meta[10 + static_cast(arg_count) + 2 + strlen(__FILE__) + 1]); EXPECT_EQ(magic, '{'); EXPECT_EQ(size, 25 + strlen(__FILE__)); @@ -176,12 +188,11 @@ TEST(meta_macro, span_meta_layout) { // first call site: span begin const char *const meta = meta_ptrs[0]; - const _clltk_meta_enty_type type = - *reinterpret_cast(&meta[5]); + const _clltk_meta_enty_type type = read_unaligned<_clltk_meta_enty_type>(&meta[5]); const uint8_t arg_count = *reinterpret_cast(&meta[10]); const char *const arg_types = reinterpret_cast(&meta[11]); - const char *const name = - reinterpret_cast(&meta[10 + arg_count + 2 + strlen(__FILE__) + 1]); + const char *const name = reinterpret_cast( + &meta[10 + static_cast(arg_count) + 2 + strlen(__FILE__) + 1]); EXPECT_EQ(type, _clltk_meta_enty_type_span_begin); EXPECT_EQ(arg_count, 2); @@ -191,8 +202,7 @@ TEST(meta_macro, span_meta_layout) } { // second call site: span end const char *const meta = meta_ptrs[2]; - const _clltk_meta_enty_type type = - *reinterpret_cast(&meta[5]); + const _clltk_meta_enty_type type = read_unaligned<_clltk_meta_enty_type>(&meta[5]); const uint8_t arg_count = *reinterpret_cast(&meta[10]); const char *const arg_types = reinterpret_cast(&meta[11]); @@ -214,7 +224,7 @@ TEST(meta_macro, three_tracepoints) int64_t arg0 = -1; CLLTK_TRACEPOINT(META_MACRO_04, "arg0 = %ld", arg0); const char magic = *reinterpret_cast(&meta[0]); - const uint32_t size = *reinterpret_cast(&meta[1]); + const uint32_t size = read_unaligned(&meta[1]); EXPECT_EQ(magic, '{'); EXPECT_EQ(size, 25 + strlen(__FILE__)); @@ -224,7 +234,7 @@ TEST(meta_macro, three_tracepoints) volatile char arg0[] = "Hello World!\n"; CLLTK_TRACEPOINT(META_MACRO_04, "arg0 = %s", arg0); const char magic = *reinterpret_cast(&meta[0]); - const uint32_t size = *reinterpret_cast(&meta[1]); + const uint32_t size = read_unaligned(&meta[1]); EXPECT_EQ(magic, '{'); EXPECT_GT(size, 0u); @@ -234,7 +244,7 @@ TEST(meta_macro, three_tracepoints) volatile double arg0 = 3e-23; CLLTK_TRACEPOINT(META_MACRO_04, "arg0 = %f", arg0); const char magic = *reinterpret_cast(&meta[0]); - const uint32_t size = *reinterpret_cast(&meta[1]); + const uint32_t size = read_unaligned(&meta[1]); EXPECT_EQ(magic, '{'); EXPECT_GT(size, 0u); diff --git a/tests/tracing.tests/snapshot.tests/snapshot.tests.cpp b/tests/tracing.tests/snapshot.tests/snapshot.tests.cpp index 31008be..33277ff 100644 --- a/tests/tracing.tests/snapshot.tests/snapshot.tests.cpp +++ b/tests/tracing.tests/snapshot.tests/snapshot.tests.cpp @@ -98,6 +98,7 @@ TEST_F(SnapshotTest, TestUncompressedSnapshot) }; const auto count = take_snapshot(func, {}, false); ASSERT_TRUE(count); + // NOLINTNEXTLINE(bugprone-unchecked-optional-access): guarded by ASSERT_TRUE above EXPECT_TRUE(count.value()); } TEST_F(SnapshotTest, verbose) @@ -113,6 +114,7 @@ TEST_F(SnapshotTest, verbose) }; const auto count = take_snapshot(func, {}, false, 4094, verbose); ASSERT_TRUE(count); + // NOLINTNEXTLINE(bugprone-unchecked-optional-access): guarded by ASSERT_TRUE above EXPECT_TRUE(count.value()); } @@ -123,6 +125,7 @@ TEST_F(SnapshotTest, TestCompressedSnapshot) }; const auto count = take_snapshot(func, {}, true); ASSERT_TRUE(count); + // NOLINTNEXTLINE(bugprone-unchecked-optional-access): guarded by ASSERT_TRUE above EXPECT_TRUE(count.value()); } @@ -139,6 +142,7 @@ TEST_F(SnapshotTest, AllFileClosedInUnCompressedSnapshot) }; const auto count = take_snapshot(func, {}, false); ASSERT_TRUE(count); + // NOLINTNEXTLINE(bugprone-unchecked-optional-access): guarded by ASSERT_TRUE above EXPECT_TRUE(count.value()); EXPECT_EQ(oldOpenFdCounter, countOpenFileDescriptors()); } @@ -156,6 +160,7 @@ TEST_F(SnapshotTest, AllFileClosedInCompressedSnapshot) }; const auto count = take_snapshot(func, {}, true); ASSERT_TRUE(count); + // NOLINTNEXTLINE(bugprone-unchecked-optional-access): guarded by ASSERT_TRUE above EXPECT_TRUE(count.value()); EXPECT_EQ(oldOpenFdCounter, countOpenFileDescriptors()); } @@ -354,10 +359,11 @@ TEST_F(SnapshotTest, CheckForMissingMUNMAP) }; const auto count = take_snapshot(func, {}, false); ASSERT_TRUE(count); + // NOLINTNEXTLINE(bugprone-unchecked-optional-access): guarded by ASSERT_TRUE above EXPECT_TRUE(count.value()); const std::vector mmappedFilesAfter = mmappedFiles(); - for (auto fileAfter : mmappedFilesAfter) { + for (const auto &fileAfter : mmappedFilesAfter) { if (std::find(mmappedFilesBefore.begin(), mmappedFilesBefore.end(), fileAfter) == mmappedFilesBefore.end()) { if (fileAfter.find(this->temp_dir) != std::string::npos) { diff --git a/tests/tracing.tests/unique_stack.tests/unique_stack.tests.cpp b/tests/tracing.tests/unique_stack.tests/unique_stack.tests.cpp index c85ac1d..5c1af8c 100644 --- a/tests/tracing.tests/unique_stack.tests/unique_stack.tests.cpp +++ b/tests/tracing.tests/unique_stack.tests/unique_stack.tests.cpp @@ -103,6 +103,7 @@ TEST_F(unique_stack_add, simple) std::string in = "A B C D E F G"; uint64_t id = ::unique_stack_add(&uq, in.data(), (uint32_t)in.size()); EXPECT_GT(id, 0); + ::unique_stack_close(&uq); file_drop(&fd); } @@ -115,6 +116,7 @@ TEST_F(unique_stack_add, bad_file_descriptor) std::string in = "A B C D E F G"; uint64_t id = ::unique_stack_add(&uq, in.data(), (uint32_t)in.size()); EXPECT_GT(id, 0); + ::unique_stack_close(&uq); file_drop(&fd); } @@ -127,6 +129,7 @@ TEST_F(unique_stack_add, bigger_than_file) uint64_t id = ::unique_stack_add(&uq, in.data(), (uint32_t)in.size()); EXPECT_GT(id, 0); EXPECT_GT(file_get_size(fd), file_size); + ::unique_stack_close(&uq); file_drop(&fd); } @@ -141,6 +144,7 @@ TEST_F(unique_stack_add, twice_same_data0) EXPECT_GT(id0, 0); EXPECT_GT(id1, 0); EXPECT_EQ(id0, id1); + ::unique_stack_close(&uq); file_drop(&fd); } @@ -154,6 +158,7 @@ TEST_F(unique_stack_add, twice_different_data0) std::string in1 = "G F E D C B A"; uint64_t id1 = ::unique_stack_add(&uq, in1.data(), (uint32_t)in1.size()); EXPECT_NE(id0, id1); + ::unique_stack_close(&uq); file_drop(&fd); } @@ -175,6 +180,7 @@ TEST_F(unique_stack_add, three_different_data0) uint64_t id2 = ::unique_stack_add(&uq, in2.data(), (uint32_t)in2.size()); EXPECT_TRUE(id2); EXPECT_NE(id0, id2); + ::unique_stack_close(&uq); file_drop(&fd); } @@ -192,5 +198,6 @@ TEST_F(unique_stack_add, add_second_page) EXPECT_GT(id, 0); } EXPECT_GT(file_get_size(fd), getpagesize()); + ::unique_stack_close(&uq); file_drop(&fd); } diff --git a/tracing_library/include/CommonLowLevelTracingKit/tracing/_user_tracing.h b/tracing_library/include/CommonLowLevelTracingKit/tracing/_user_tracing.h index 4c03eb2..1e1c09c 100644 --- a/tracing_library/include/CommonLowLevelTracingKit/tracing/_user_tracing.h +++ b/tracing_library/include/CommonLowLevelTracingKit/tracing/_user_tracing.h @@ -152,83 +152,87 @@ _CLLTK_EXTERN_C_END &_META_), \ _CLLTK_ASM_SYM_CONSTRAINT(&_OFFSET_)) -#define _CLLTK_STATIC_TRACEPOINT(_BUFFER_, _FORMAT_, ...) \ - do { \ - /* ------- compile time stuff ------- */ \ - \ - _CLLTK_STATIC_ASSERT(_CLLTK_NARGS(__VA_ARGS__) <= 10, \ - "only supporting up to 10 arguments"); \ - _CLLTK_CHECK_FOR_ARGUMENTS(__VA_ARGS__); \ - \ - /* create meta data for this tracepoint, the per-call-site offset */ \ - /* cache (filled by the startup registration), and a discovery entry */ \ - static _clltk_file_offset_t _clltk_offset = _clltk_file_offset_unset; \ - _CLLTK_CREATE_META_ENTRY_ARGS(_meta, _CLLTK_PLACE_IN(_BUFFER_), _FORMAT_, __VA_ARGS__); \ - _CLLTK_EMIT_META_PTR(_BUFFER_, _meta, _clltk_offset); \ - \ - /* create type information for va_list access at runtime. */ \ - /* it is not possible to use meta data because there is no */ \ - /* common meta data struct usable for all tracepoints. */ \ - static _clltk_argument_types_t _clltk_types = _CLLTK_CREATE_TYPES(__VA_ARGS__); \ - \ - static _clltk_tracebuffer_handler_t *const _tb = &_clltk_##_BUFFER_; \ - \ - /* ------- runtime time stuff ------- */ \ - \ - if ((_tb->runtime.tracebuffer == NULL)) { \ - if (!_clltk_tracebuffer_init(_tb)) { \ - break; \ - } \ - } \ - \ - /* normally already set by the constructor; fallback for call sites */ \ - /* executed before startup registration (constructor priority <= 101) */ \ - if (_clltk_offset == _clltk_file_offset_unset) { \ - _clltk_offset = _clltk_tracebuffer_get_in_file_offset(_tb, &_meta, sizeof(_meta)); \ - } \ - \ - /* at runtime execute trace point */ \ - _clltk_static_tracepoint_with_args(_tb, _clltk_offset, __FILE__, __LINE__, &_clltk_types, \ - _FORMAT_ _CLLTK_CAST(__VA_ARGS__)); \ +#define _CLLTK_STATIC_TRACEPOINT(_BUFFER_, _FORMAT_, ...) \ + do { \ + /* ------- compile time stuff ------- */ \ + \ + _CLLTK_STATIC_ASSERT(_CLLTK_NARGS(__VA_ARGS__) <= 10, \ + "only supporting up to 10 arguments"); \ + _CLLTK_CHECK_FOR_ARGUMENTS(__VA_ARGS__); \ + \ + /* create meta data for this tracepoint, the per-call-site offset */ \ + /* cache (filled by the startup registration), and a discovery entry */ \ + static _clltk_file_offset_t _clltk_offset = _clltk_file_offset_unset; \ + _CLLTK_CREATE_META_ENTRY_ARGS(_meta, _CLLTK_PLACE_IN(_BUFFER_), _FORMAT_, __VA_ARGS__); \ + _CLLTK_EMIT_META_PTR(_BUFFER_, _meta, _clltk_offset); \ + \ + /* create type information for va_list access at runtime. */ \ + /* it is not possible to use meta data because there is no */ \ + /* common meta data struct usable for all tracepoints. */ \ + static _clltk_argument_types_t _clltk_types = _CLLTK_CREATE_TYPES(__VA_ARGS__); \ + \ + static _clltk_tracebuffer_handler_t *const _tb = &_clltk_##_BUFFER_; \ + \ + /* ------- runtime time stuff ------- */ \ + \ + if ((_tb->runtime.tracebuffer == NULL)) { \ + if (!_clltk_tracebuffer_init(_tb)) { \ + break; \ + } \ + } \ + \ + /* normally already set by the constructor; fallback for call sites */ \ + /* executed before startup registration (constructor priority <= 101) */ \ + _clltk_file_offset_t _clltk_off = __atomic_load_n(&_clltk_offset, __ATOMIC_RELAXED); \ + if (_clltk_off == _clltk_file_offset_unset) { \ + _clltk_off = _clltk_tracebuffer_get_in_file_offset(_tb, &_meta, sizeof(_meta)); \ + __atomic_store_n(&_clltk_offset, _clltk_off, __ATOMIC_RELAXED); \ + } \ + \ + /* at runtime execute trace point */ \ + _clltk_static_tracepoint_with_args(_tb, _clltk_off, __FILE__, __LINE__, &_clltk_types, \ + _FORMAT_ _CLLTK_CAST(__VA_ARGS__)); \ } while (0) #if defined(_CLLTK_HAS_FMT) -#define _CLLTK_STATIC_TRACEPOINT_FMT(_BUFFER_, _FORMAT_, ...) \ - do { \ - /* ------- compile time stuff ------- */ \ - if (false) { /* never executed: validates {} format against arg types */ \ - _clltk_fmt_check(_FORMAT_ __VA_OPT__(, ) __VA_ARGS__); \ - } \ - _CLLTK_STATIC_ASSERT(_CLLTK_NARGS(__VA_ARGS__) <= 10, \ - "only supporting up to 10 arguments"); \ - _CLLTK_CHECK_FOR_ARGUMENTS(__VA_ARGS__); \ - \ - /* create meta data for this tracepoint, the per-call-site offset */ \ - /* cache (filled by the startup registration), and a discovery entry */ \ - static _clltk_file_offset_t _clltk_offset = _clltk_file_offset_unset; \ - _CLLTK_CREATE_META_ENTRY_TYPED(_meta, _CLLTK_PLACE_IN(_BUFFER_), \ - _clltk_meta_enty_type_fmt, _FORMAT_, __VA_ARGS__); \ - _CLLTK_EMIT_META_PTR(_BUFFER_, _meta, _clltk_offset); \ - \ - static _clltk_argument_types_t _clltk_types = _CLLTK_CREATE_TYPES(__VA_ARGS__); \ - \ - static _clltk_tracebuffer_handler_t *const _tb = &_clltk_##_BUFFER_; \ - \ - /* ------- runtime time stuff ------- */ \ - \ - if ((_tb->runtime.tracebuffer == NULL)) { \ - if (!_clltk_tracebuffer_init(_tb)) { \ - break; \ - } \ - } \ - \ - if (_clltk_offset == _clltk_file_offset_unset) { \ - _clltk_offset = _clltk_tracebuffer_get_in_file_offset(_tb, &_meta, sizeof(_meta)); \ - } \ - \ - _clltk_static_tracepoint_with_args_unchecked(_tb, _clltk_offset, __FILE__, __LINE__, \ - &_clltk_types, \ - _FORMAT_ _CLLTK_CAST(__VA_ARGS__)); \ +#define _CLLTK_STATIC_TRACEPOINT_FMT(_BUFFER_, _FORMAT_, ...) \ + do { \ + /* ------- compile time stuff ------- */ \ + if (false) { /* never executed: validates {} format against arg types */ \ + _clltk_fmt_check(_FORMAT_ __VA_OPT__(, ) __VA_ARGS__); \ + } \ + _CLLTK_STATIC_ASSERT(_CLLTK_NARGS(__VA_ARGS__) <= 10, \ + "only supporting up to 10 arguments"); \ + _CLLTK_CHECK_FOR_ARGUMENTS(__VA_ARGS__); \ + \ + /* create meta data for this tracepoint, the per-call-site offset */ \ + /* cache (filled by the startup registration), and a discovery entry */ \ + static _clltk_file_offset_t _clltk_offset = _clltk_file_offset_unset; \ + _CLLTK_CREATE_META_ENTRY_TYPED(_meta, _CLLTK_PLACE_IN(_BUFFER_), \ + _clltk_meta_enty_type_fmt, _FORMAT_, __VA_ARGS__); \ + _CLLTK_EMIT_META_PTR(_BUFFER_, _meta, _clltk_offset); \ + \ + static _clltk_argument_types_t _clltk_types = _CLLTK_CREATE_TYPES(__VA_ARGS__); \ + \ + static _clltk_tracebuffer_handler_t *const _tb = &_clltk_##_BUFFER_; \ + \ + /* ------- runtime time stuff ------- */ \ + \ + if ((_tb->runtime.tracebuffer == NULL)) { \ + if (!_clltk_tracebuffer_init(_tb)) { \ + break; \ + } \ + } \ + \ + _clltk_file_offset_t _clltk_off = __atomic_load_n(&_clltk_offset, __ATOMIC_RELAXED); \ + if (_clltk_off == _clltk_file_offset_unset) { \ + _clltk_off = _clltk_tracebuffer_get_in_file_offset(_tb, &_meta, sizeof(_meta)); \ + __atomic_store_n(&_clltk_offset, _clltk_off, __ATOMIC_RELAXED); \ + } \ + \ + _clltk_static_tracepoint_with_args_unchecked(_tb, _clltk_off, __FILE__, __LINE__, \ + &_clltk_types, \ + _FORMAT_ _CLLTK_CAST(__VA_ARGS__)); \ } while (0) #else #define _CLLTK_STATIC_TRACEPOINT_FMT(_BUFFER_, _FORMAT_, ...) \ @@ -241,74 +245,78 @@ _CLLTK_EXTERN_C_END * discovery entry, tracebuffer init, and resolved file offset. The * placeholder arguments only determine the argument type array of the meta * entry; the real values are passed to the runtime call by the caller. */ -#define _CLLTK_STATIC_SPAN_EVENT(_TYPE_, _BUFFER_, _NAME_, _CALL_, ...) \ +#define _CLLTK_STATIC_SPAN_EVENT(_TYPE_, _BUFFER_, _NAME_, _CALL_, ...) \ + do { \ + static _clltk_file_offset_t _clltk_offset = _clltk_file_offset_unset; \ + _CLLTK_CREATE_META_ENTRY_TYPED(_meta, _CLLTK_PLACE_IN(_BUFFER_), _TYPE_, _NAME_, \ + __VA_ARGS__); \ + _CLLTK_EMIT_META_PTR(_BUFFER_, _meta, _clltk_offset); \ + \ + static _clltk_tracebuffer_handler_t *const _tb = &_clltk_##_BUFFER_; \ + if ((_tb->runtime.tracebuffer == NULL)) { \ + if (!_clltk_tracebuffer_init(_tb)) { \ + break; \ + } \ + } \ + _clltk_file_offset_t _clltk_off = __atomic_load_n(&_clltk_offset, __ATOMIC_RELAXED); \ + if (_clltk_off == _clltk_file_offset_unset) { \ + _clltk_off = _clltk_tracebuffer_get_in_file_offset(_tb, &_meta, sizeof(_meta)); \ + __atomic_store_n(&_clltk_offset, _clltk_off, __ATOMIC_RELAXED); \ + } \ + _CALL_; \ + } while (0) + +#define _CLLTK_STATIC_SPAN_BEGIN(_BUFFER_, _PARENT_, _NAME_) \ + ({ \ + const clltk_span_id_t _clltk_span_id = clltk_next_span_id(); \ + const clltk_span_id_t _clltk_span_parent = (_PARENT_); \ + _CLLTK_STATIC_SPAN_EVENT(_clltk_meta_enty_type_span_begin, _BUFFER_, _NAME_, \ + _clltk_static_tracepoint_span_begin( \ + _tb, _clltk_off, _clltk_span_id, _clltk_span_parent), \ + (clltk_span_id_t)0, (clltk_span_id_t)0); \ + _clltk_span_id; \ + }) + +#define _CLLTK_STATIC_SPAN_END(_BUFFER_, _ID_) \ + do { \ + const clltk_span_id_t _clltk_span_id = (_ID_); \ + _CLLTK_STATIC_SPAN_EVENT( \ + _clltk_meta_enty_type_span_end, _BUFFER_, "", \ + _clltk_static_tracepoint_span_end(_tb, _clltk_off, _clltk_span_id), \ + (clltk_span_id_t)0); \ + } while (0) + +#define _CLLTK_STATIC_TRACEPOINT_DUMP(_BUFFER_, _MESSAGE_, _ADDRESS_, _SIZE_) \ do { \ + /* ------- compile time stuff ------- */ \ + \ + /* create meta data for this tracepoint, the per-call-site offset */ \ + /* cache (filled by the startup registration), and a discovery entry */ \ static _clltk_file_offset_t _clltk_offset = _clltk_file_offset_unset; \ - _CLLTK_CREATE_META_ENTRY_TYPED(_meta, _CLLTK_PLACE_IN(_BUFFER_), _TYPE_, _NAME_, \ - __VA_ARGS__); \ + _CLLTK_CREATE_META_ENTRY_DUMP(_meta, _CLLTK_PLACE_IN(_BUFFER_), _MESSAGE_); \ _CLLTK_EMIT_META_PTR(_BUFFER_, _meta, _clltk_offset); \ \ static _clltk_tracebuffer_handler_t *const _tb = &_clltk_##_BUFFER_; \ + \ + /* ------- runtime time stuff ------- */ \ + \ if ((_tb->runtime.tracebuffer == NULL)) { \ if (!_clltk_tracebuffer_init(_tb)) { \ break; \ } \ } \ - if (_clltk_offset == _clltk_file_offset_unset) { \ - _clltk_offset = _clltk_tracebuffer_get_in_file_offset(_tb, &_meta, sizeof(_meta)); \ + \ + /* normally already set by the constructor; fallback for call sites */ \ + /* executed before startup registration (constructor priority <= 101) */ \ + _clltk_file_offset_t _clltk_off = __atomic_load_n(&_clltk_offset, __ATOMIC_RELAXED); \ + if (_clltk_off == _clltk_file_offset_unset) { \ + _clltk_off = _clltk_tracebuffer_get_in_file_offset(_tb, &_meta, sizeof(_meta)); \ + __atomic_store_n(&_clltk_offset, _clltk_off, __ATOMIC_RELAXED); \ } \ - _CALL_; \ - } while (0) - -#define _CLLTK_STATIC_SPAN_BEGIN(_BUFFER_, _PARENT_, _NAME_) \ - ({ \ - const clltk_span_id_t _clltk_span_id = clltk_next_span_id(); \ - const clltk_span_id_t _clltk_span_parent = (_PARENT_); \ - _CLLTK_STATIC_SPAN_EVENT(_clltk_meta_enty_type_span_begin, _BUFFER_, _NAME_, \ - _clltk_static_tracepoint_span_begin( \ - _tb, _clltk_offset, _clltk_span_id, _clltk_span_parent), \ - (clltk_span_id_t)0, (clltk_span_id_t)0); \ - _clltk_span_id; \ - }) - -#define _CLLTK_STATIC_SPAN_END(_BUFFER_, _ID_) \ - do { \ - const clltk_span_id_t _clltk_span_id = (_ID_); \ - _CLLTK_STATIC_SPAN_EVENT( \ - _clltk_meta_enty_type_span_end, _BUFFER_, "", \ - _clltk_static_tracepoint_span_end(_tb, _clltk_offset, _clltk_span_id), \ - (clltk_span_id_t)0); \ - } while (0) - -#define _CLLTK_STATIC_TRACEPOINT_DUMP(_BUFFER_, _MESSAGE_, _ADDRESS_, _SIZE_) \ - do { \ - /* ------- compile time stuff ------- */ \ - \ - /* create meta data for this tracepoint, the per-call-site offset */ \ - /* cache (filled by the startup registration), and a discovery entry */ \ - static _clltk_file_offset_t _clltk_offset = _clltk_file_offset_unset; \ - _CLLTK_CREATE_META_ENTRY_DUMP(_meta, _CLLTK_PLACE_IN(_BUFFER_), _MESSAGE_); \ - _CLLTK_EMIT_META_PTR(_BUFFER_, _meta, _clltk_offset); \ - \ - static _clltk_tracebuffer_handler_t *const _tb = &_clltk_##_BUFFER_; \ - \ - /* ------- runtime time stuff ------- */ \ - \ - if ((_tb->runtime.tracebuffer == NULL)) { \ - if (!_clltk_tracebuffer_init(_tb)) { \ - break; \ - } \ - } \ - \ - /* normally already set by the constructor; fallback for call sites */ \ - /* executed before startup registration (constructor priority <= 101) */ \ - if (_clltk_offset == _clltk_file_offset_unset) { \ - _clltk_offset = _clltk_tracebuffer_get_in_file_offset(_tb, &_meta, sizeof(_meta)); \ - } \ - \ - /* at runtime execute trace point */ \ - _clltk_static_tracepoint_with_dump(_tb, _clltk_offset, _meta.file, _meta.line, _ADDRESS_, \ - _SIZE_); \ + \ + /* at runtime execute trace point */ \ + _clltk_static_tracepoint_with_dump(_tb, _clltk_off, _meta.file, _meta.line, _ADDRESS_, \ + _SIZE_); \ } while (0) #endif diff --git a/tracing_library/source/abstraction/unix_user_space/error.c b/tracing_library/source/abstraction/unix_user_space/error.c index c29f328..2ee70a9 100644 --- a/tracing_library/source/abstraction/unix_user_space/error.c +++ b/tracing_library/source/abstraction/unix_user_space/error.c @@ -32,7 +32,7 @@ void unrecoverable_error(const char *file, size_t line, const char *func, const #define CML_RED "\033[1;31m" #define CML_RESET "\033[0m" snprintf(clltk_format, sizeof(clltk_format), - CML_RED "clltk unrecoverable: %s (%s in %s:%lu)" CML_RESET "\n", format, func, file, + CML_RED "clltk unrecoverable: %s (%s in %s:%zu)" CML_RESET "\n", format, func, file, line); #undef CML_RED #undef CML_RESET @@ -49,7 +49,7 @@ void recoverable_error(const char *file, size_t line, const char *func, const ch #define CML_RED "\033[1;31m" #define CML_RESET "\033[0m" snprintf(clltk_format, sizeof(clltk_format), - CML_RED "clltk recoverable: %s (%s in %s:%lu)" CML_RESET "\n", format, func, file, + CML_RED "clltk recoverable: %s (%s in %s:%zu)" CML_RESET "\n", format, func, file, line); #undef CML_RED #undef CML_RESET diff --git a/tracing_library/source/abstraction/unix_user_space/file.c b/tracing_library/source/abstraction/unix_user_space/file.c index 2ee1ced..5e3e290 100644 --- a/tracing_library/source/abstraction/unix_user_space/file.c +++ b/tracing_library/source/abstraction/unix_user_space/file.c @@ -375,6 +375,8 @@ void file_reset(void) struct stat file_stat; // loop over all files in path while ((iterator = readdir(directory)) != NULL) { + // NOLINTNEXTLINE(clang-analyzer-unix.StdCLibraryFunctions): directory is + // NULL-checked before the loop, so dirfd() returns a valid descriptor if (fstatat(dirfd(directory), iterator->d_name, &file_stat, AT_SYMLINK_NOFOLLOW) == 0) { if (S_ISDIR(file_stat.st_mode)) continue; diff --git a/tracing_library/source/arguments.c b/tracing_library/source/arguments.c index 200a0fa..86080bc 100644 --- a/tracing_library/source/arguments.c +++ b/tracing_library/source/arguments.c @@ -143,7 +143,7 @@ uint32_t get_argument_sizes(const char *const format, uint32_t sizes_out[], return size; } -void get_arguments(void *_buffer, uint32_t sizes[], const _clltk_argument_types_t *types, +void get_arguments(void *_buffer, const uint32_t sizes[], const _clltk_argument_types_t *types, va_list args) { uint64_t buffer = (uint64_t)_buffer; @@ -156,43 +156,43 @@ void get_arguments(void *_buffer, uint32_t sizes[], const _clltk_argument_types_ case _clltk_argument_uint8: case _clltk_argument_sint8: { uint8_t value = (uint8_t)va_arg(args_copy, __typeof__(uint32_t)); - *(uint8_t *)buffer = value; + memcpy((void *)buffer, &value, sizeof(value)); buffer += fix_arg_size; } break; case _clltk_argument_uint16: case _clltk_argument_sint16: { const uint16_t value = (uint16_t)va_arg(args_copy, __typeof__(uint32_t)); - *(uint16_t *)buffer = value; + memcpy((void *)buffer, &value, sizeof(value)); buffer += fix_arg_size; } break; case _clltk_argument_uint32: case _clltk_argument_sint32: { const uint32_t value = va_arg(args_copy, __typeof__(uint32_t)); - *(uint32_t *)buffer = value; + memcpy((void *)buffer, &value, sizeof(value)); buffer += fix_arg_size; } break; case _clltk_argument_uint64: case _clltk_argument_sint64: case _clltk_argument_pointer: { const uint64_t value = va_arg(args_copy, __typeof__(uint64_t)); - *(uint64_t *)buffer = value; + memcpy((void *)buffer, &value, sizeof(value)); buffer += fix_arg_size; } break; case _clltk_argument_uint128: case _clltk_argument_sint128: { const __uint128_t value = va_arg(args_copy, __typeof__(__uint128_t)); - *(__uint128_t *)buffer = value; + memcpy((void *)buffer, &value, sizeof(value)); buffer += fix_arg_size; } break; #if !defined(__KERNEL__) // TODO: create an error? case _clltk_argument_float: { const float value = (float)va_arg(args_copy, __typeof__(double)); - *(float *)buffer = value; + memcpy((void *)buffer, &value, sizeof(value)); buffer += fix_arg_size; } break; case _clltk_argument_double: { const double value = va_arg(args_copy, __typeof__(double)); - *(double *)buffer = value; + memcpy((void *)buffer, &value, sizeof(value)); buffer += fix_arg_size; } break; #endif diff --git a/tracing_library/source/arguments.h b/tracing_library/source/arguments.h index 3911159..905f770 100644 --- a/tracing_library/source/arguments.h +++ b/tracing_library/source/arguments.h @@ -25,7 +25,7 @@ extern "C" { uint32_t get_argument_sizes(const char *const format, uint32_t sizes_out[], _clltk_argument_types_t *types, va_list args); -void get_arguments(void *buffer, uint32_t sizes[], const _clltk_argument_types_t *types, +void get_arguments(void *buffer, const uint32_t sizes[], const _clltk_argument_types_t *types, va_list args); void first_time_check(const char *const format, _clltk_argument_types_t *types); diff --git a/tracing_library/source/crc8/crc8.h b/tracing_library/source/crc8/crc8.h index 930f01b..216bd86 100644 --- a/tracing_library/source/crc8/crc8.h +++ b/tracing_library/source/crc8/crc8.h @@ -73,6 +73,8 @@ crc8_continue(uint8_t crc, const uint8_t *const input_str, size_t input_str_size for (size_t input_str_index = 0; input_str_index < input_str_size; input_str_index++) { const uint8_t current_char = input_str[input_str_index]; const uint8_t table_index = current_char ^ crc; + // NOLINTNEXTLINE(clang-analyzer-core.uninitialized.Assign): table has 256 + // entries, table_index is uint8_t (0-255) - always in bounds crc = sht75_crc_table[table_index]; } } diff --git a/tracing_library/source/md5/md5.c b/tracing_library/source/md5/md5.c index 7478e5b..8028929 100644 --- a/tracing_library/source/md5/md5.c +++ b/tracing_library/source/md5/md5.c @@ -83,10 +83,10 @@ void md5Update(MD5Context *ctx, const uint8_t *input_buffer, size_t input_len) // Convert to little-endian // The local variable `input` our 512-bit chunk separated into 32-bit words // we can use in calculations - input[j] = (uint32_t)(ctx->input[(j * 4) + 3]) << 24 | - (uint32_t)(ctx->input[(j * 4) + 2]) << 16 | - (uint32_t)(ctx->input[(j * 4) + 1]) << 8 | - (uint32_t)(ctx->input[(j * 4)]); + input[j] = (uint32_t)(ctx->input[((size_t)j * 4) + 3]) << 24 | + (uint32_t)(ctx->input[((size_t)j * 4) + 2]) << 16 | + (uint32_t)(ctx->input[((size_t)j * 4) + 1]) << 8 | + (uint32_t)(ctx->input[((size_t)j * 4)]); } md5Step(ctx->buffer, input); offset = 0; @@ -111,9 +111,10 @@ void md5Finalize(MD5Context *ctx) // Do a final update (internal to this function) // Last two 32-bit words are the two halves of the size (converted from bytes to bits) for (unsigned int j = 0; j < 14; ++j) { - input[j] = (uint32_t)(ctx->input[(j * 4) + 3]) << 24 | - (uint32_t)(ctx->input[(j * 4) + 2]) << 16 | - (uint32_t)(ctx->input[(j * 4) + 1]) << 8 | (uint32_t)(ctx->input[(j * 4)]); + input[j] = (uint32_t)(ctx->input[((size_t)j * 4) + 3]) << 24 | + (uint32_t)(ctx->input[((size_t)j * 4) + 2]) << 16 | + (uint32_t)(ctx->input[((size_t)j * 4) + 1]) << 8 | + (uint32_t)(ctx->input[((size_t)j * 4)]); } input[14] = (uint32_t)(ctx->size * 8); input[15] = (uint32_t)((ctx->size * 8) >> 32); diff --git a/tracing_library/source/ringbuffer/ringbuffer.c b/tracing_library/source/ringbuffer/ringbuffer.c index 8a807e7..7f2055f 100644 --- a/tracing_library/source/ringbuffer/ringbuffer.c +++ b/tracing_library/source/ringbuffer/ringbuffer.c @@ -187,7 +187,8 @@ static void drop_oldest_entry(ringbuffer_head_t *rb) if (occupied <= sizeof(ringbuffer_entry_head_t)) { move_last_valid(rb, occupied); return; - } else if (rb->body[rb->last_valid] != ringbuffer_entry_magic) { + } + if (rb->body[rb->last_valid] != ringbuffer_entry_magic) { move_last_valid(rb, 1); found_invalid_data = true; occupied--; @@ -270,7 +271,8 @@ size_t ringbuffer_out(void *destination, size_t max_size, ringbuffer_head_t *sou if (occupied <= sizeof(ringbuffer_entry_head_t)) { // not enough in ringbuffer move_last_valid(source, occupied); return 0; - } else if (source->body[source->last_valid] != ringbuffer_entry_magic) { // wrong magic + } + if (source->body[source->last_valid] != ringbuffer_entry_magic) { // wrong magic move_last_valid(source, 1); occupied--; continue; diff --git a/tracing_library/source/tracebuffer.c b/tracing_library/source/tracebuffer.c index 65de493..e782f19 100644 --- a/tracing_library/source/tracebuffer.c +++ b/tracing_library/source/tracebuffer.c @@ -243,7 +243,7 @@ void _clltk_tracebuffer_register_metaptrs(_clltk_tracebuffer_handler_t *handler, (const _clltk_meta_entry_head_t *)pairs_start[2 * i]; _clltk_file_offset_t *const offset_cache = (_clltk_file_offset_t *)(uintptr_t)pairs_start[2 * i + 1]; - if (*offset_cache != _clltk_file_offset_unset) { + if (__atomic_load_n(offset_cache, __ATOMIC_RELAXED) != _clltk_file_offset_unset) { // already resolved, e.g. by another translation unit's // constructor walking the same merged section continue; @@ -263,9 +263,11 @@ void _clltk_tracebuffer_register_metaptrs(_clltk_tracebuffer_handler_t *handler, } else { unique_stack_add_batch(&buffer->stack, items, todo); for (size_t i = 0; i < todo; i++) { - // leave the cache unset on failure so the lazy path retries + // leave the cache unset on failure so the lazy path retries. + // relaxed atomic store: the lazy runtime path reads the same + // cache atomically from concurrent tracepoints if (_CLLTK_FILE_OFFSET_IS_STATIC(items[i].out_offset)) - *caches[i] = items[i].out_offset; + __atomic_store_n(caches[i], items[i].out_offset, __ATOMIC_RELAXED); } } } diff --git a/tracing_library/source/tracepoint.c b/tracing_library/source/tracepoint.c index 1080cfb..8986e55 100644 --- a/tracing_library/source/tracepoint.c +++ b/tracing_library/source/tracepoint.c @@ -311,10 +311,10 @@ void clltk_dynamic_tracepoint_execution(const char *name, const char *file, cons char *pos = (char *)&entry_buffer->body; pos += (memcpy(pos, file, file_len), file_len); pos += (memcpy(pos, &line, line_len), line_len); - pos += (memcpy(pos, message, message_len), message_len); + memcpy(pos, message, message_len); // add to ringbuffer - _clltk_tracebuffer_handler_t handler = {{name, 10 * 1024}, {NULL, NULL}, {NULL, 0}}; + _clltk_tracebuffer_handler_t handler = {{name, (size_t)10 * 1024}, {NULL, NULL}, {NULL, 0}}; if (_clltk_tracebuffer_init(&handler)) { add_to_ringbuffer(&handler, raw_entry_buffer, raw_entry_size); _clltk_tracebuffer_deinit(&handler); diff --git a/tracing_library/source/unique_stack/unique_stack.c b/tracing_library/source/unique_stack/unique_stack.c index 3e79ff7..08489e0 100644 --- a/tracing_library/source/unique_stack/unique_stack.c +++ b/tracing_library/source/unique_stack/unique_stack.c @@ -57,7 +57,11 @@ unique_stack_handler_t unique_stack_open(file_t *fh, uint64_t file_offset) void unique_stack_close(unique_stack_handler_t *handler) { - RETURN_IF_INVALID(handler); + if (handler == NULL) + return; + + // the in-memory lookup index is owned by the handler; closing releases it + unique_stack_drop_index(handler); handler->valid = 0; handler->file = NULL; @@ -226,8 +230,8 @@ static uint32_t slab_encoded_size(uint32_t logical) static void slab_encode(const uint8_t *in, uint32_t in_size, uint8_t *out) { for (uint32_t i = 0; i < in_size; i++) { - out[2 * i] = (uint8_t)(0x80u | (in[i] >> 4)); - out[2 * i + 1] = (uint8_t)(0x80u | (in[i] & 0x0Fu)); + out[2 * (size_t)i] = (uint8_t)(0x80u | (in[i] >> 4)); + out[2 * (size_t)i + 1] = (uint8_t)(0x80u | (in[i] & 0x0Fu)); } } @@ -341,7 +345,7 @@ static void index_maybe_publish(unique_stack_handler_t *handler, uint64_t stack_ return; const uint32_t logical_size = - sizeof(slab_head_t) + index->used * (uint32_t)sizeof(index_slot_t); + sizeof(slab_head_t) + (uint32_t)((size_t)index->used * sizeof(index_slot_t)); uint8_t *const logical = memory_heap_allocation(logical_size); const slab_head_t slab = { .magic = SLAB_MAGIC,