test: runtime sanitizers + static-analysis gate, and the bugs they found

.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,

.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)
   # ============================================

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)
 

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 @@
             ]
         }
     ]
-}
+}

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

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()

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;

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

decoder_tool/cpp/include/CommonLowLevelTracingKit/decoder/Inline.hpp

@@ -58,6 +58,8 @@ template <typename To, typename From> CONST_INLINE constexpr To safe_cast(From v
 	} else {
 		// both signed or both unsigned
 		using Common = std::common_type_t<From, To>;
+		// 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<Common>(value);
 		const Common lo = static_cast<Common>(std::numeric_limits<To>::min());
 		const Common hi = static_cast<Common>(std::numeric_limits<To>::max());

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<std::string> error;
 
 		bool valid() const noexcept { return !error.has_value(); }

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();

decoder_tool/cpp/source/Tracepoint.cpp

@@ -12,6 +12,7 @@
 #include "file.hpp"
 #include "formatter.hpp"
 #include "ringbuffer.hpp"
+#include <cstring>
 
 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<const size_t *>(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;

decoder_tool/cpp/source/TracepointInternal.hpp

@@ -5,6 +5,7 @@
 #include "CommonLowLevelTracingKit/decoder/Tracepoint.hpp"
 #include "file.hpp"
 #include "ringbuffer.hpp"
+#include <cstring>
 #include <new>
 #include <ranges>
 #include <type_traits>
@@ -16,8 +17,10 @@ template <PODType T, std::ranges::contiguous_range R>
 INLINE static constexpr T get(R r, size_t offset = 0, bool foreign_endian = false) {
 	const uintptr_t base = std::bit_cast<uintptr_t>(r.data());
 	const uintptr_t position = base + offset;
-	const T *const ptr = reinterpret_cast<T *>(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<const void *>(position), sizeof(T));
 	if constexpr (CommonLowLevelTracingKit::decoder::source::internal::ByteSwappable<T>) {
 		if (foreign_endian) {
 			value = CommonLowLevelTracingKit::decoder::source::internal::byteswapValue(value);

decoder_tool/cpp/source/low_level/file.hpp

@@ -60,7 +60,11 @@ namespace CommonLowLevelTracingKit::decoder::source {
 			return value;
 		}
 		template <typename T = FilePart> 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<const void *>(getPtr(offset, sizeof(T))),
+						sizeof(T));
 			if constexpr (internal::ByteSwappable<T>) {
 				if (m_foreign_endian) { value = internal::byteswapValue(value); }
 			}

decoder_tool/cpp/source/low_level/formatter.cpp

@@ -454,7 +454,8 @@ std::string formatter::dump(const std::string_view format, const std::span<const
 	const size_t format_size = format.size();
 	if (args_raw.size() < sizeof(uint32_t))
 		CLLTK_DECODER_THROW(FormattingFailed, "args_raw too small for dump size");
-	uint32_t dump_size = (*std::bit_cast<const uint32_t *>(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);
 	}

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++) {