From c9456ebfc1a21ba67b52aa1d7a1421a5e179b190 Mon Sep 17 00:00:00 2001 From: "irons.du" Date: Wed, 16 Sep 2026 06:51:57 +0800 Subject: [PATCH 1/6] test(fuzz): add libFuzzer targets for the parsers, behind an opt-in flag Three targets, each depending only on profiler_core (no Drogon), built with -fsanitize=fuzzer,address,undefined and enabled by -DREMOTE_PROFILER_BUILD_FUZZERS=ON (clang only, off by default): fuzz_renderer_output flamegraph/pprof output parsing + symbolz body splitting fuzz_result_parsing isJsonError / jsonErrorMessage / isEmptyHeapSample fuzz_http_params duration parsing and clamping, renderer parsing Each has a hand-written seed corpus. `ctest -L fuzz` replays those corpora (-runs=1 per target), then a bounded smoke pass via cmake/run-fuzz-smoke.cmake. A CI job builds and runs them and uploads crash-* artifacts on failure. These are the surfaces worth fuzzing: pure string-in/verdict-out functions fed by things this process does not control -- flamegraph.pl, pprof, tcmalloc, and the client. Fuzzing was pointed elsewhere first and rejected: resolveSymbolWithBackward turned out to handle arbitrary addresses without crashing (0x0, 0xdeadbeef, 2^64-1 all return empty or the address), so a target there would have explored nothing. The parsing also moved out of the two places that had grown a copy each. The flamegraph "ERROR:" extraction existed twice -- in http_handlers.cpp and profiler_manager.cpp -- and the symbolz body splitting was inline in handlePprofSymbol(). The two copies had already drifted, so a fix to one would not have reached the other. They now share internal/renderer_output_parsing.h, which also gains unit tests (RendererParsingTest, 12 cases) because a fuzz target finds crashes, not contracts. One behaviour fix came out of writing the splitting down: it now strips a trailing '\r', so a CRLF body resolves its addresses instead of passing a stray '\r' to stoull(). On the assertions, because this cost me two false alarms. My first versions asserted things that sound self-evident and are false -- that an error message is never empty (`{"error":""}` yields one), and that a parsed symbol token never contains a newline (when the body has a '+', plus-separation wins and newlines legitimately stay inside a token). Both targets immediately "found" crashes that were the assertions' fault. A fuzz target that cries wolf gets muted rather than fixed, so every check now asserts only what the implementation guarantees, and memory safety is left to ASan/UBSan, which cannot be wrong about it. CONTRIBUTING.md records both traps. .gitignore keeps libFuzzer's hash-named corpus additions and crash-* files out of version control: they are run artifacts, while the descriptive seeds are fixtures. It also covers myfuck/, a 1.1GB build tree someone configured in the repository root, which my earlier `git add -A` had staged. Verified: 8/8 unit tests (serial and -j6), fuzz build clean, `ctest -L fuzz` 4/4 (corpora replay plus smoke), and the individual targets run clean for 60s each -- about 42M executions total, no crashes. --- .github/workflows/code-quality.yml | 104 +++++++++++++ .gitignore | 14 ++ CHANGELOG.md | 12 ++ CMakeLists.txt | 78 ++++++++++ CONTRIBUTING.md | 26 +++- README.md | 5 +- cmake/run-fuzz-smoke.cmake | 56 +++++++ src/http_handlers.cpp | 42 ++---- src/internal/renderer_output_parsing.h | 137 ++++++++++++++++++ src/profiler_manager.cpp | 28 ++-- .../corpus/fuzz_http_params/duration_huge | 1 + .../fuzz/corpus/fuzz_http_params/duration_max | 1 + .../corpus/fuzz_http_params/duration_negative | 1 + .../corpus/fuzz_http_params/duration_normal | 1 + .../corpus/fuzz_http_params/duration_zero | 1 + tests/fuzz/corpus/fuzz_http_params/empty | 0 .../fuzz/corpus/fuzz_http_params/int_overflow | 1 + .../fuzz_http_params/renderer_callgraph | 1 + .../fuzz_http_params/renderer_flamegraph | 1 + .../error_without_marker_svg | 1 + .../fuzz_renderer_output/flamegraph_error_svg | 1 + .../corpus/fuzz_renderer_output/marker_at_end | 1 + .../fuzz_renderer_output/svg_without_marker | 1 + .../corpus/fuzz_renderer_output/symbolz_crlf | 2 + .../fuzz_renderer_output/symbolz_newlines | 4 + .../corpus/fuzz_renderer_output/symbolz_plus | 1 + .../corpus/fuzz_renderer_output/valid_svg | 1 + .../corpus/fuzz_result_parsing/json_error | 1 + .../corpus/fuzz_result_parsing/json_not_error | 1 + .../fuzz_result_parsing/real_heap_sample | 1 + .../corpus/fuzz_result_parsing/svg_output | 1 + .../fuzz_result_parsing/warn_heap_sample | 2 + .../fuzz_result_parsing/zero_rate_sample | 1 + tests/fuzz/fuzz_http_params.cpp | 78 ++++++++++ tests/fuzz/fuzz_renderer_output.cpp | 67 +++++++++ tests/fuzz/fuzz_result_parsing.cpp | 63 ++++++++ tests/test_renderer_parsing.cpp | 120 +++++++++++++++ 37 files changed, 808 insertions(+), 49 deletions(-) create mode 100644 cmake/run-fuzz-smoke.cmake create mode 100644 src/internal/renderer_output_parsing.h create mode 100644 tests/fuzz/corpus/fuzz_http_params/duration_huge create mode 100644 tests/fuzz/corpus/fuzz_http_params/duration_max create mode 100644 tests/fuzz/corpus/fuzz_http_params/duration_negative create mode 100644 tests/fuzz/corpus/fuzz_http_params/duration_normal create mode 100644 tests/fuzz/corpus/fuzz_http_params/duration_zero create mode 100644 tests/fuzz/corpus/fuzz_http_params/empty create mode 100644 tests/fuzz/corpus/fuzz_http_params/int_overflow create mode 100644 tests/fuzz/corpus/fuzz_http_params/renderer_callgraph create mode 100644 tests/fuzz/corpus/fuzz_http_params/renderer_flamegraph create mode 100644 tests/fuzz/corpus/fuzz_renderer_output/error_without_marker_svg create mode 100644 tests/fuzz/corpus/fuzz_renderer_output/flamegraph_error_svg create mode 100644 tests/fuzz/corpus/fuzz_renderer_output/marker_at_end create mode 100644 tests/fuzz/corpus/fuzz_renderer_output/svg_without_marker create mode 100644 tests/fuzz/corpus/fuzz_renderer_output/symbolz_crlf create mode 100644 tests/fuzz/corpus/fuzz_renderer_output/symbolz_newlines create mode 100644 tests/fuzz/corpus/fuzz_renderer_output/symbolz_plus create mode 100644 tests/fuzz/corpus/fuzz_renderer_output/valid_svg create mode 100644 tests/fuzz/corpus/fuzz_result_parsing/json_error create mode 100644 tests/fuzz/corpus/fuzz_result_parsing/json_not_error create mode 100644 tests/fuzz/corpus/fuzz_result_parsing/real_heap_sample create mode 100644 tests/fuzz/corpus/fuzz_result_parsing/svg_output create mode 100644 tests/fuzz/corpus/fuzz_result_parsing/warn_heap_sample create mode 100644 tests/fuzz/corpus/fuzz_result_parsing/zero_rate_sample create mode 100644 tests/fuzz/fuzz_http_params.cpp create mode 100644 tests/fuzz/fuzz_renderer_output.cpp create mode 100644 tests/fuzz/fuzz_result_parsing.cpp create mode 100644 tests/test_renderer_parsing.cpp diff --git a/.github/workflows/code-quality.yml b/.github/workflows/code-quality.yml index 7f92217..6bb5ea2 100644 --- a/.github/workflows/code-quality.yml +++ b/.github/workflows/code-quality.yml @@ -271,6 +271,110 @@ jobs: env: TSAN_OPTIONS: halt_on_error=0 + # Fuzzing (libFuzzer). Separate from the test jobs because it needs clang's + # fuzzer runtime and runs on a time budget rather than to completion. + fuzz: + name: Fuzzing (libFuzzer) + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Install system dependencies + run: | + sudo apt-get update + sudo apt-get install -y \ + cmake \ + clang \ + git \ + pkg-config \ + libgoogle-perftools-dev + + - name: Export GitHub Actions cache environment variables + uses: actions/github-script@v7 + with: + script: | + core.exportVariable('ACTIONS_CACHE_URL', process.env.ACTIONS_CACHE_URL || ''); + core.exportVariable('ACTIONS_RUNTIME_TOKEN', process.env.ACTIONS_RUNTIME_TOKEN || ''); + + - name: Cache vcpkg packages + id: cache-vcpkg + uses: actions/cache@v4 + with: + path: | + vcpkg + ~/.cache/vcpkg/archives + vcpkg_installed + key: ${{ runner.os }}-vcpkg-fuzz-${{ hashFiles('vcpkg.json') }} + + - name: Install vcpkg + run: | + if [ ! -f "vcpkg/vcpkg" ]; then + echo "vcpkg not found or incomplete, cloning..." + rm -rf vcpkg + git clone https://github.com/Microsoft/vcpkg.git + cd vcpkg + ./bootstrap-vcpkg.sh + else + echo "Using cached vcpkg" + fi + + - name: Install vcpkg dependencies + env: + VCPKG_BINARY_SOURCES: "clear;x-gha,readwrite" + run: | + ./vcpkg/vcpkg install --triplet=x64-linux-release + + - name: Configure with fuzz targets + run: | + mkdir build + cd build + cmake .. \ + -DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE }} \ + -DCMAKE_C_COMPILER=clang \ + -DCMAKE_CXX_COMPILER=clang++ \ + -DCMAKE_TOOLCHAIN_FILE=../vcpkg/scripts/buildsystems/vcpkg.cmake \ + -DVCPKG_TARGET_TRIPLET=x64-linux-release \ + -DREMOTE_PROFILER_BUILD_FUZZERS=ON \ + -DREMOTE_PROFILER_ENABLE_WEB=OFF \ + -DREMOTE_PROFILER_BUILD_TESTS=OFF \ + -DREMOTE_PROFILER_BUILD_EXAMPLES=OFF + + - name: Build fuzz targets + run: | + cd build + make -j$(nproc) + + # The per-target tests replay the committed corpus (one run each), so this + # fails if a corpus entry stops behaving. The smoke test then fuzzes past the + # corpus for a bounded time. + - name: Replay corpora + working-directory: build + run: | + ctest -L fuzz --output-on-failure -E FuzzSmoke + + - name: Fuzz smoke test + working-directory: build + run: | + ctest -L fuzz --output-on-failure -R FuzzSmoke + env: + ASAN_OPTIONS: detect_leaks=1:halt_on_error=1 + + # A crash leaves a reproducer behind; upload it so the failure is actionable + # without re-running the job. + - name: Upload crash reproducers + if: failure() + uses: actions/upload-artifact@v4 + with: + name: fuzz-crashes + path: | + build/crash-* + build/leak-* + build/timeout-* + build/oom-* + if-no-files-found: ignore + # Static Analysis with clang-tidy static-analysis: name: Static Analysis diff --git a/.gitignore b/.gitignore index aeec5ae..d846782 100644 --- a/.gitignore +++ b/.gitignore @@ -61,5 +61,19 @@ flamegraph.pl # Out-of-source build trees and vendored sources build-tsan/ + +# A build tree was once configured in the repository root under this name; it is +# 1.1GB of CMake output and must never be committed. +myfuck/ vcpkg_installed/ gperftools-src/ + +# libFuzzer writes newly discovered inputs into the corpus directory next to the +# hand-written seeds, named by content hash. Those are run artifacts, not fixtures: +# keep the descriptive seeds in version control and ignore the rest. A newly found +# crashing input belongs in the corpus under a descriptive name. +tests/fuzz/corpus/*/[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f] +tests/fuzz/crash-* +tests/fuzz/leak-* +tests/fuzz/timeout-* +tests/fuzz/oom-* diff --git a/CHANGELOG.md b/CHANGELOG.md index c3737c4..5ec6ce3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added +- libFuzzer targets for the parsers, behind `-DREMOTE_PROFILER_BUILD_FUZZERS=ON` + (clang only, off by default): `fuzz_renderer_output`, `fuzz_result_parsing`, + `fuzz_http_params`, with a committed seed corpus each. `ctest -L fuzz` replays the + corpora and then runs a bounded smoke pass; a new CI job builds and runs them, and + uploads crash reproducers on failure. +- The parsing these targets cover moved into `internal/renderer_output_parsing.h` and + is now exercised by unit tests as well. Three copies of the same logic were + involved: the flamegraph "ERROR:" extraction existed twice (HTTP layer and core), + and the pprof symbolz body splitting was inline in the handler. Each had drifted + slightly, so a fix to one would not have reached the others. +- `internal::parseSymbolRequest()` strips a trailing `\r`, so CRLF bodies from a + client now resolve their addresses instead of passing a stray `\r` to stoull(). - `/api/thread/stacks` (and `getThreadCallStacks()`) print each thread's **name** next to its tid, read from `/proc//comm`. A bare tid is not identifiable when reading the output afterwards; the name is what tells you that thread 1234 is `DrogonIoLoop` and 1235 is a worker parked on a futex. diff --git a/CMakeLists.txt b/CMakeLists.txt index d78cfb4..23dfa6a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -23,6 +23,13 @@ option(ENABLE_COVERAGE "Enable code coverage reporting" OFF) option(BUILD_DOCS "Build API documentation" OFF) +# Fuzz targets (libFuzzer). Off by default: they need a clang with the fuzzer +# runtime and are run deliberately, not as part of a normal build. +option(REMOTE_PROFILER_BUILD_FUZZERS "Build libFuzzer targets" OFF) +# A cache string, not option(): option() would type it BOOL, and its numeric value +# would be dropped on any re-configure. +set(REMOTE_PROFILER_FUZZ_SECONDS 20 CACHE STRING "Seconds each fuzz target runs in the smoke test") + message(STATUS "Build options:") message(STATUS " BUILD_SHARED_LIBS: ${BUILD_SHARED_LIBS}") message(STATUS " REMOTE_PROFILER_INSTALL: ${REMOTE_PROFILER_INSTALL}") @@ -31,6 +38,7 @@ message(STATUS " REMOTE_PROFILER_BUILD_TESTS: ${REMOTE_PROFILER_BUILD_TESTS}") message(STATUS " REMOTE_PROFILER_ENABLE_WEB: ${REMOTE_PROFILER_ENABLE_WEB}") message(STATUS " ENABLE_COVERAGE: ${ENABLE_COVERAGE}") message(STATUS " BUILD docs: ${BUILD_DOCS}") +message(STATUS " BUILD fuzzers: ${REMOTE_PROFILER_BUILD_FUZZERS}") set(CMAKE_CXX_STANDARD 20) set(CMAKE_CXX_STANDARD_REQUIRED ON) @@ -276,6 +284,19 @@ if(REMOTE_PROFILER_BUILD_TESTS) ) add_test(NAME ResultParsingTest COMMAND test_result_parsing) + # Renderer-output and request-body parsing (core only). The fuzz targets in + # tests/fuzz/ explore the same functions; these pin their documented behaviour. + add_executable(test_renderer_parsing tests/test_renderer_parsing.cpp) + target_link_libraries(test_renderer_parsing + PRIVATE + profiler_core + GTest::gtest + GTest::gtest_main + pthread + ) + target_include_directories(test_renderer_parsing PRIVATE ${PROJECT_SOURCE_DIR}/src) + add_test(NAME RendererParsingTest COMMAND test_renderer_parsing) + # Thread stack capture (core only -- works without Drogon) add_executable(test_thread_stacks tests/test_thread_stacks.cpp) target_link_libraries(test_thread_stacks @@ -292,6 +313,63 @@ else() message(STATUS "Tests disabled") endif() +# ============================================================================ +# Fuzz targets (libFuzzer) +# ============================================================================ +# Kept separate from the GoogleTest targets: these need -fsanitize=fuzzer and are +# long-running by nature. Each one links only profiler_core, so a fuzz build does +# not drag in Drogon. +if(REMOTE_PROFILER_BUILD_FUZZERS) + # enable_testing() otherwise happens only inside the GoogleTest block, so a + # fuzz-only build (tests off) would build the targets but register nothing. + if(NOT REMOTE_PROFILER_BUILD_TESTS) + enable_testing() + endif() + + if(NOT CMAKE_CXX_COMPILER_ID MATCHES "Clang") + message(FATAL_ERROR + "REMOTE_PROFILER_BUILD_FUZZERS requires clang (libFuzzer is part of the " + "clang runtime); the current compiler is ${CMAKE_CXX_COMPILER_ID}.") + endif() + + # Fuzzing wants coverage instrumentation plus a sanitizer to catch the memory + # errors the coverage alone would miss. + set(FUZZ_SANITIZERS "fuzzer,address,undefined") + + # Shared flags so all three targets behave identically. + function(add_fuzz_target name source) + add_executable(${name} tests/fuzz/${source}) + target_link_libraries(${name} PRIVATE profiler_core pthread) + # The targets under test live in src/internal/, which profiler_core exposes + # only to itself; fuzzing the parsing helpers is exactly the point here. + target_include_directories(${name} PRIVATE ${PROJECT_SOURCE_DIR}/src) + target_compile_options(${name} PRIVATE -fsanitize=${FUZZ_SANITIZERS} -fno-omit-frame-pointer -g) + target_link_options(${name} PRIVATE -fsanitize=${FUZZ_SANITIZERS}) + # A corpus directory per target, seeded from tests/fuzz/corpus/. + set(corpus_dir "${CMAKE_CURRENT_SOURCE_DIR}/tests/fuzz/corpus/${name}") + file(MAKE_DIRECTORY "${corpus_dir}") + add_test(NAME ${name} + COMMAND ${name} -runs=1 -seed=1 "${corpus_dir}") + set_tests_properties(${name} PROPERTIES LABELS "fuzz") + endfunction() + + add_fuzz_target(fuzz_renderer_output fuzz_renderer_output.cpp) + add_fuzz_target(fuzz_result_parsing fuzz_result_parsing.cpp) + add_fuzz_target(fuzz_http_params fuzz_http_params.cpp) + + # Smoke test: run each target for a bounded time so CI exercises more than the + # corpus. Excluded from the default test run because it takes minutes. + add_test(NAME FuzzSmoke + COMMAND ${CMAKE_COMMAND} + -DFUZZ_SECONDS=${REMOTE_PROFILER_FUZZ_SECONDS} + -DFUZZ_BINARIES=$,$,$ + -DCORPUS_ROOT=${CMAKE_CURRENT_SOURCE_DIR}/tests/fuzz/corpus + -P ${CMAKE_CURRENT_SOURCE_DIR}/cmake/run-fuzz-smoke.cmake) + set_tests_properties(FuzzSmoke PROPERTIES LABELS "fuzz" TIMEOUT 900) + + message(STATUS "Fuzz targets will be built (smoke test: ${REMOTE_PROFILER_FUZZ_SECONDS}s each)") +endif() + # Note: Web files are now embedded in C++ code (src/web_resources.cpp) # No need to copy web directory to build directory diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 884c062..20e19f7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -287,16 +287,26 @@ ctest -R --output-on-failure ### 提交前的附加检查 -`ctest` 通过**不足以**覆盖两类问题，仓库里有两个脚本专门补这个缺口： +`ctest` 通过**不足以**覆盖三类问题，仓库里另有三个手段补这个缺口： ```bash # 1) Sanitizer：本地复刻 CI 的 ASan/UBSan/TSan 配置 scripts/check-sanitizers.sh asan # 默认只跑 ASan scripts/check-sanitizers.sh all # ASan + UBSan + TSan +CTEST_JOBS=6 scripts/check-sanitizers.sh asan # 同时验证并行稳定性 # 2) Web UI：用无头 Chrome 驱动真实控制面板，断言每个动作都发出正确的请求 # 并真正落盘（需要先启动 profiler_example，以及 puppeteer-core） scripts/verify-web-ui.mjs http://localhost:8080 + +# 3) Fuzz：libFuzzer 驱动的解析器 +cmake -S . -B build/fuzz -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ \ + -DCMAKE_TOOLCHAIN_FILE=vcpkg/scripts/buildsystems/vcpkg.cmake \ + -DVCPKG_TARGET_TRIPLET=x64-linux-release \ + -DREMOTE_PROFILER_BUILD_FUZZERS=ON -DREMOTE_PROFILER_BUILD_TESTS=OFF +cmake --build build/fuzz -j$(nproc) +ctest --test-dir build/fuzz -L fuzz # 语料回归 + 短时 fuzz +build/fuzz/fuzz_renderer_output -max_total_time=300 tests/fuzz/corpus/fuzz_renderer_output ``` 为什么需要它们： @@ -307,6 +317,20 @@ scripts/verify-web-ui.mjs http://localhost:8080 - **Web UI**：`curl` **无法执行页面 JavaScript**，因此看不到"按钮点击报 ReferenceError""按钮卡在 disabled""下载其实没开始"这类问题。前端改动必须用 真实浏览器验证后才算完成。 +- **Fuzz**：解析器的输入空间靠手写用例覆盖不全。三个目标都只依赖 `profiler_core`， + 不拖 Drogon，跑起来很轻。 + +### 写 fuzz 目标时的两条经验 + +1. **只断言实现真正保证的性质。** 第一个版本的 `fuzz_result_parsing` 断言"错误消息非空"， + 听起来天经地义，实际是错的——`{"error":""}` 会得到空消息；`fuzz_renderer_output` + 断言"token 不含换行"，同样错——当请求体含 `+` 时按 `+` 切分，换行会**合法地**留在 + token 内。两个目标随即各报了一个"崩溃"，其实都是断言自己的错。 + **假阳性比漏报更糟**：它会让人不再相信这个目标。 +2. **每条断言都问"如果它失败，是谁的 bug？"** 答不上来就别加。内存安全问题交给 + ASan/UBSan——它们不会冤枉代码。 + +崩溃样本由 libFuzzer 写在当前目录（`crash-*`）。CI 会把它们作为 artifact 上传。 > 改前端（`src/web_resources.cpp` 里的 HTML/JS 字符串）时尤其注意：那段 JS 是 > C++ 字符串字面量，**编译器不会检查它**——函数名写错、参数漏传都能编译通过。 diff --git a/README.md b/README.md index 6d027f0..e429030 100644 --- a/README.md +++ b/README.md @@ -192,6 +192,7 @@ cd build && ./profiler_example | `REMOTE_PROFILER_ENABLE_WEB` | `ON` | 编译 Web 层（需 Drogon），`OFF` 则完全不依赖 Drogon | | `ENABLE_COVERAGE` | `OFF` | 生成覆盖率报告 | | `BUILD_DOCS` | `OFF` | 生成 Doxygen API 文档（输出到 `build/docs/html/`） | +| `REMOTE_PROFILER_BUILD_FUZZERS` | `OFF` | 构建 libFuzzer 模糊测试目标（需 clang，见 CONTRIBUTING） | 常见场景： @@ -515,7 +516,9 @@ cpp-remote-profiler/ │ ├── symbolize.cpp # 符号化引擎（absl → dladdr → backward-cpp） │ ├── web_resources.cpp # 内置 Web 资源 │ └── internal/ # 内部实现：日志、符号化、内嵌 pprof / flamegraph.pl -├── example/ tests/ # 示例与 GoogleTest 测试 +├── example/ # 示例程序（含命名线程，便于验证线程栈） +├── tests/ # GoogleTest 单元测试 +│ └── fuzz/ # libFuzzer 目标与种子语料（REMOTE_PROFILER_BUILD_FUZZERS=ON） ├── cmake/ │ ├── cpp-remote-profiler-config.cmake.in │ └── examples/ # find_package / FetchContent 集成验证 diff --git a/cmake/run-fuzz-smoke.cmake b/cmake/run-fuzz-smoke.cmake new file mode 100644 index 0000000..0bdac69 --- /dev/null +++ b/cmake/run-fuzz-smoke.cmake @@ -0,0 +1,56 @@ +# Run each libFuzzer target for a bounded time, as a CI smoke test. +# +# Invoked by the FuzzSmoke test in CMakeLists.txt: +# cmake -DFUZZ_SECONDS=20 -DFUZZ_BINARIES=a,b,c -DCORPUS_ROOT=... -P run-fuzz-smoke.cmake +# +# A bounded run rather than a fixed number of iterations, because the corpus is what +# carries regressions: -runs=1 replays it (the per-target tests do that), while this +# gives each target time to explore past it. + +if(NOT DEFINED FUZZ_SECONDS) + set(FUZZ_SECONDS 20) +endif() +if(NOT DEFINED FUZZ_BINARIES) + message(FATAL_ERROR "FUZZ_BINARIES must list the fuzz targets to run") +endif() + +string(REPLACE "," ";" _binaries "${FUZZ_BINARIES}") +set(_failed "") + +foreach(_binary IN LISTS _binaries) + get_filename_component(_name "${_binary}" NAME) + set(_corpus "${CORPUS_ROOT}/${_name}") + + message(STATUS "fuzzing ${_name} for ${FUZZ_SECONDS}s (corpus: ${_corpus})") + + # -max_total_time bounds the run; -print_final_stats makes the saturated + # coverage visible in the log so a target that is not exploring is noticeable. + execute_process( + COMMAND "${_binary}" + "-max_total_time=${FUZZ_SECONDS}" + "-print_final_stats=1" + "-rss_limit_mb=2048" + "${_corpus}" + RESULT_VARIABLE _rc + OUTPUT_VARIABLE _out + ERROR_VARIABLE _err + ) + + if(NOT _rc EQUAL 0) + message(STATUS "${_name} FAILED (exit ${_rc})") + # A crash lands on stderr with the reproducer; surface it rather than + # swallowing it behind cmake's own error formatting. + message(STATUS "---- ${_name} stderr ----\n${_err}") + list(APPEND _failed "${_name}") + else() + # Report the last few lines: libFuzzer prints DONE plus the stats there. + string(REGEX MATCH "(DONE|cov: [^\n]*|stat::[^\n]*)" _summary "${_err}") + message(STATUS "${_name} ok${_summary}") + endif() +endforeach() + +if(_failed) + message(FATAL_ERROR "fuzz targets failed: ${_failed}") +endif() + +message(STATUS "fuzz smoke test passed") diff --git a/src/http_handlers.cpp b/src/http_handlers.cpp index 36b1c78..35adc82 100644 --- a/src/http_handlers.cpp +++ b/src/http_handlers.cpp @@ -2,6 +2,7 @@ /// @brief Framework-agnostic HTTP endpoint handlers implementation #include "profiler/http_handlers.h" +#include "internal/renderer_output_parsing.h" #include "internal/result_parsing.h" #include "profiler_manager.h" #include @@ -99,16 +100,13 @@ namespace { /// 200 carrying an error reaches the caller as if it were a graph. All flame /// graphs funnel through here so the check cannot be forgotten at a call site. HandlerResponse flameGraphResponse(const std::string& svg) { - if (svg.find("explanation()); } return HandlerResponse::svg(svg); } @@ -381,27 +379,9 @@ HandlerResponse ProfilerHttpHandlers::handlePprofSymbol(const std::string& body) // and expects tab-separated response: "0xaddr\tsymbol_name\n" // Also support newline-separated addresses for backward compatibility. - std::vector addresses; - - // Split by '+' first (Go pprof format) - // If no '+' found, fall back to newline splitting - if (body.find('+') != std::string::npos) { - std::istringstream iss(body); - std::string addr; - while (std::getline(iss, addr, '+')) { - if (!addr.empty()) { - addresses.push_back(addr); - } - } - } else { - std::istringstream iss(body); - std::string addr; - while (std::getline(iss, addr)) { - if (!addr.empty() && addr[0] != '#') { - addresses.push_back(addr); - } - } - } + // Splitting lives in internal::parseSymbolRequest() so it is unit tested and + // fuzzed rather than only exercised through a live request. + std::vector addresses = internal::parseSymbolRequest(body); std::ostringstream result; for (const auto& address : addresses) { diff --git a/src/internal/renderer_output_parsing.h b/src/internal/renderer_output_parsing.h new file mode 100644 index 0000000..b7a036c --- /dev/null +++ b/src/internal/renderer_output_parsing.h @@ -0,0 +1,137 @@ +/// @file renderer_output_parsing.h +/// @brief Helpers for interpreting what the renderers (flamegraph.pl, pprof) produced +/// +/// The renderers are external processes whose output is not fully under our +/// control: flamegraph.pl answers unusable input with a *valid* SVG whose only +/// content is an "ERROR: ..." message, so a structural check for ` +#include +#include + +PROFILER_NAMESPACE_BEGIN + +namespace internal { + +/// @brief The kind of failure a renderer reported, if any +enum class RendererError { + /// An "ERROR: ..." message was found; the message itself is returned alongside. + FlameGraphReported, +}; + +/// @brief A renderer failure and the text that explains it +struct RendererFailure { + RendererError kind; + std::string message; + + /// @brief The user-facing reason, independent of which renderer produced it + std::string explanation() const { + switch (kind) { + case RendererError::FlameGraphReported: + return "No stack samples to render (profile is empty). The sample window may have been too " + "short or the process idle; increase the duration or sample under load. " + "flamegraph.pl said: " + + message; + } + return "The renderer failed for an unknown reason."; + } +}; + +/// @brief Extract the message from flamegraph.pl's "ERROR: ..." output +/// +/// flamegraph.pl emits `ERROR: ` without a trailing tag when it rejects its +/// input, and as the text content of an SVG element otherwise, so the message runs +/// either to the next `<` or to the end of the string. +/// +/// @return the message, or std::nullopt when there is no "ERROR:" marker +inline std::optional extractFlameGraphError(const std::string& output) { + const auto pos = output.find("ERROR:"); + if (pos == std::string::npos) + return std::nullopt; + + // Guard the length: when no '<' follows, the message is the rest of the + // string. Subtracting here without the check would underflow (npos - pos) and + // make substr() throw -- a real defect in the copy of this code that lived in + // profiler_manager.cpp, which is why the parsing now happens in one place. + const auto end = output.find('<', pos); + const auto length = end == std::string::npos ? std::string::npos : end - pos; + return output.substr(pos, length); +} + +/// @brief Whether a renderer's output looks like a usable SVG +/// +/// Structural only: this is the cheap check that an "ERROR:" payload passes, which +/// is exactly why the error check above exists as well. +inline bool looksLikeSvg(const std::string& output) { + return output.find(" interpretFlameGraphOutput(const std::string& output) { + if (!looksLikeSvg(output)) + return RendererFailure{RendererError::FlameGraphReported, ""}; + + if (auto message = extractFlameGraphError(output)) + return RendererFailure{RendererError::FlameGraphReported, *message}; + + return std::nullopt; +} + +/// @brief Split a pprof symbolz request body into addresses +/// +/// `go tool pprof` sends addresses joined by '+'; newline-separated input is also +/// accepted, and '#' starts a comment line. Anything else is treated as an address +/// candidate -- validation belongs to the caller, which has to convert them. +inline std::vector parseSymbolRequest(const std::string& body) { + std::vector addresses; + + if (body.find('+') != std::string::npos) { + std::size_t start = 0; + while (start <= body.size()) { + const auto sep = body.find('+', start); + const auto end = sep == std::string::npos ? body.size() : sep; + if (end > start) + addresses.push_back(body.substr(start, end - start)); + if (sep == std::string::npos) + break; + start = sep + 1; + } + return addresses; + } + + std::size_t start = 0; + while (start <= body.size()) { + const auto sep = body.find('\n', start); + const auto end = sep == std::string::npos ? body.size() : sep; + if (end > start) { + // Strip a trailing '\r' so CRLF input behaves like LF input. + auto stop = end; + if (stop > start && body[stop - 1] == '\r') + --stop; + if (stop > start && body[start] != '#') + addresses.push_back(body.substr(start, stop - start)); + } + if (sep == std::string::npos) + break; + start = sep + 1; + } + return addresses; +} + +} // namespace internal + +PROFILER_NAMESPACE_END diff --git a/src/profiler_manager.cpp b/src/profiler_manager.cpp index 9b8d1fe..3eac3da 100644 --- a/src/profiler_manager.cpp +++ b/src/profiler_manager.cpp @@ -5,6 +5,7 @@ #include "internal/embed_pprof.h" #include "internal/log_macros.h" #include "internal/log_manager.h" +#include "internal/renderer_output_parsing.h" #include "internal/result_parsing.h" #include "internal/symbolize.h" #include @@ -403,23 +404,18 @@ std::string ProfilerManager::generateFlameGraph(const std::string& collapsed_fil return R"({"error": "Failed to execute flamegraph.pl command"})"; } - // Validate output - if (svg_output.find("message); + return R"({"error": ")" + failure->explanation() + R"("})"; } return svg_output; diff --git a/tests/fuzz/corpus/fuzz_http_params/duration_huge b/tests/fuzz/corpus/fuzz_http_params/duration_huge new file mode 100644 index 0000000..e83e892 --- /dev/null +++ b/tests/fuzz/corpus/fuzz_http_params/duration_huge @@ -0,0 +1 @@ +9999 \ No newline at end of file diff --git a/tests/fuzz/corpus/fuzz_http_params/duration_max b/tests/fuzz/corpus/fuzz_http_params/duration_max new file mode 100644 index 0000000..f1efb20 --- /dev/null +++ b/tests/fuzz/corpus/fuzz_http_params/duration_max @@ -0,0 +1 @@ +300 \ No newline at end of file diff --git a/tests/fuzz/corpus/fuzz_http_params/duration_negative b/tests/fuzz/corpus/fuzz_http_params/duration_negative new file mode 100644 index 0000000..c67f81a --- /dev/null +++ b/tests/fuzz/corpus/fuzz_http_params/duration_negative @@ -0,0 +1 @@ +-5 \ No newline at end of file diff --git a/tests/fuzz/corpus/fuzz_http_params/duration_normal b/tests/fuzz/corpus/fuzz_http_params/duration_normal new file mode 100644 index 0000000..9a03714 --- /dev/null +++ b/tests/fuzz/corpus/fuzz_http_params/duration_normal @@ -0,0 +1 @@ +10 \ No newline at end of file diff --git a/tests/fuzz/corpus/fuzz_http_params/duration_zero b/tests/fuzz/corpus/fuzz_http_params/duration_zero new file mode 100644 index 0000000..c227083 --- /dev/null +++ b/tests/fuzz/corpus/fuzz_http_params/duration_zero @@ -0,0 +1 @@ +0 \ No newline at end of file diff --git a/tests/fuzz/corpus/fuzz_http_params/empty b/tests/fuzz/corpus/fuzz_http_params/empty new file mode 100644 index 0000000..e69de29 diff --git a/tests/fuzz/corpus/fuzz_http_params/int_overflow b/tests/fuzz/corpus/fuzz_http_params/int_overflow new file mode 100644 index 0000000..3a685c0 --- /dev/null +++ b/tests/fuzz/corpus/fuzz_http_params/int_overflow @@ -0,0 +1 @@ +99999999999999999999999 \ No newline at end of file diff --git a/tests/fuzz/corpus/fuzz_http_params/renderer_callgraph b/tests/fuzz/corpus/fuzz_http_params/renderer_callgraph new file mode 100644 index 0000000..6afb23a --- /dev/null +++ b/tests/fuzz/corpus/fuzz_http_params/renderer_callgraph @@ -0,0 +1 @@ +callgraph \ No newline at end of file diff --git a/tests/fuzz/corpus/fuzz_http_params/renderer_flamegraph b/tests/fuzz/corpus/fuzz_http_params/renderer_flamegraph new file mode 100644 index 0000000..a0a98c7 --- /dev/null +++ b/tests/fuzz/corpus/fuzz_http_params/renderer_flamegraph @@ -0,0 +1 @@ +flamegraph \ No newline at end of file diff --git a/tests/fuzz/corpus/fuzz_renderer_output/error_without_marker_svg b/tests/fuzz/corpus/fuzz_renderer_output/error_without_marker_svg new file mode 100644 index 0000000..2f15c06 --- /dev/null +++ b/tests/fuzz/corpus/fuzz_renderer_output/error_without_marker_svg @@ -0,0 +1 @@ +ERROR: No stack counts found \ No newline at end of file diff --git a/tests/fuzz/corpus/fuzz_renderer_output/flamegraph_error_svg b/tests/fuzz/corpus/fuzz_renderer_output/flamegraph_error_svg new file mode 100644 index 0000000..4a1a7d5 --- /dev/null +++ b/tests/fuzz/corpus/fuzz_renderer_output/flamegraph_error_svg @@ -0,0 +1 @@ +ERROR: No valid input provided to flamegraph.pl. \ No newline at end of file diff --git a/tests/fuzz/corpus/fuzz_renderer_output/marker_at_end b/tests/fuzz/corpus/fuzz_renderer_output/marker_at_end new file mode 100644 index 0000000..5ff86ce --- /dev/null +++ b/tests/fuzz/corpus/fuzz_renderer_output/marker_at_end @@ -0,0 +1 @@ +ERROR: \ No newline at end of file diff --git a/tests/fuzz/corpus/fuzz_renderer_output/svg_without_marker b/tests/fuzz/corpus/fuzz_renderer_output/svg_without_marker new file mode 100644 index 0000000..48e8ba5 --- /dev/null +++ b/tests/fuzz/corpus/fuzz_renderer_output/svg_without_marker @@ -0,0 +1 @@ +no error marker here \ No newline at end of file diff --git a/tests/fuzz/corpus/fuzz_renderer_output/symbolz_crlf b/tests/fuzz/corpus/fuzz_renderer_output/symbolz_crlf new file mode 100644 index 0000000..d8e3454 --- /dev/null +++ b/tests/fuzz/corpus/fuzz_renderer_output/symbolz_crlf @@ -0,0 +1,2 @@ +0x1 +0x2 diff --git a/tests/fuzz/corpus/fuzz_renderer_output/symbolz_newlines b/tests/fuzz/corpus/fuzz_renderer_output/symbolz_newlines new file mode 100644 index 0000000..9f03c2f --- /dev/null +++ b/tests/fuzz/corpus/fuzz_renderer_output/symbolz_newlines @@ -0,0 +1,4 @@ +0x401d8a +0x401d8b +# comment +0x401d8c diff --git a/tests/fuzz/corpus/fuzz_renderer_output/symbolz_plus b/tests/fuzz/corpus/fuzz_renderer_output/symbolz_plus new file mode 100644 index 0000000..da8677f --- /dev/null +++ b/tests/fuzz/corpus/fuzz_renderer_output/symbolz_plus @@ -0,0 +1 @@ +0x401d8a+0x401d8b+0x401d8c \ No newline at end of file diff --git a/tests/fuzz/corpus/fuzz_renderer_output/valid_svg b/tests/fuzz/corpus/fuzz_renderer_output/valid_svg new file mode 100644 index 0000000..c8bf00c --- /dev/null +++ b/tests/fuzz/corpus/fuzz_renderer_output/valid_svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/tests/fuzz/corpus/fuzz_result_parsing/json_error b/tests/fuzz/corpus/fuzz_result_parsing/json_error new file mode 100644 index 0000000..d7e68a2 --- /dev/null +++ b/tests/fuzz/corpus/fuzz_result_parsing/json_error @@ -0,0 +1 @@ +{"error": "cpu profiling already in use"} \ No newline at end of file diff --git a/tests/fuzz/corpus/fuzz_result_parsing/json_not_error b/tests/fuzz/corpus/fuzz_result_parsing/json_not_error new file mode 100644 index 0000000..7c936b8 --- /dev/null +++ b/tests/fuzz/corpus/fuzz_result_parsing/json_not_error @@ -0,0 +1 @@ +{"ok": true} \ No newline at end of file diff --git a/tests/fuzz/corpus/fuzz_result_parsing/real_heap_sample b/tests/fuzz/corpus/fuzz_result_parsing/real_heap_sample new file mode 100644 index 0000000..523a45c --- /dev/null +++ b/tests/fuzz/corpus/fuzz_result_parsing/real_heap_sample @@ -0,0 +1 @@ +heap profile: 4: 80000 [ 4: 80000] @ heap_v2/524288 diff --git a/tests/fuzz/corpus/fuzz_result_parsing/svg_output b/tests/fuzz/corpus/fuzz_result_parsing/svg_output new file mode 100644 index 0000000..cc655da --- /dev/null +++ b/tests/fuzz/corpus/fuzz_result_parsing/svg_output @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/tests/fuzz/corpus/fuzz_result_parsing/warn_heap_sample b/tests/fuzz/corpus/fuzz_result_parsing/warn_heap_sample new file mode 100644 index 0000000..26e9c46 --- /dev/null +++ b/tests/fuzz/corpus/fuzz_result_parsing/warn_heap_sample @@ -0,0 +1,2 @@ +%warn: TCMALLOC_SAMPLE_PARAMETER is not set +heap profile: 0: 0 [ 0: 0] @ heap_v2/0 diff --git a/tests/fuzz/corpus/fuzz_result_parsing/zero_rate_sample b/tests/fuzz/corpus/fuzz_result_parsing/zero_rate_sample new file mode 100644 index 0000000..6327ced --- /dev/null +++ b/tests/fuzz/corpus/fuzz_result_parsing/zero_rate_sample @@ -0,0 +1 @@ +heap profile: 0: 0 [ 0: 0] @ heap_v2/0 diff --git a/tests/fuzz/fuzz_http_params.cpp b/tests/fuzz/fuzz_http_params.cpp new file mode 100644 index 0000000..35f3752 --- /dev/null +++ b/tests/fuzz/fuzz_http_params.cpp @@ -0,0 +1,78 @@ +/// @file fuzz_http_params.cpp +/// @brief libFuzzer target for the analysis endpoints' query-parameter handling +/// +/// `duration` and `seconds` arrive as strings from the client and are converted +/// with std::stoi(). That conversion and the clamping that follows are small, but +/// this exact area produced a real defect: nothing clamped `duration`, so +/// out-of-range values reached the profiler backends, which disagreed about them +/// (one rejected, one clamped), and a negative value was passed to sleep(). +/// +/// The invariants below are the ones the HTTP layer promises in its documentation: +/// every accepted duration lands in 1..300, and no input can make the parsers +/// throw or produce a value outside that range. + +#include "profiler/http_handlers.h" +#include +#include +#include + +namespace { + +/// The conversion the Drogon adapter performs (`chartOptionsFrom`), reproduced so +/// the fuzzer drives the same code path the server does. +int parseDurationLikeTheAdapter(const std::string& value, int fallback) { + if (value.empty()) + return fallback; + try { + return std::stoi(value); + } catch (...) { + return fallback; + } +} + +} // namespace + +extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { + const std::string input(reinterpret_cast(data), size); + + using profiler::v0_1_0::ChartOptions; + using profiler::v0_1_0::ChartRenderer; + using profiler::v0_1_0::clampChartDuration; + using profiler::v0_1_0::parseChartRenderer; + + // 1. The clamping contract the README states: 1..300, for any input at all. + const int raw = parseDurationLikeTheAdapter(input, 10); + const int clamped = clampChartDuration(raw); + if (clamped < 1 || clamped > 300) + __builtin_trap(); + + // Clamping is idempotent -- applying it twice must not move a value. + if (clampChartDuration(clamped) != clamped) + __builtin_trap(); + + // Out-of-range values are clamped rather than rejected, in the documented + // direction. This is the behaviour the two backends disagreed about before. + if (raw < 1 && clamped != 1) + __builtin_trap(); + if (raw > 300 && clamped != 300) + __builtin_trap(); + if (raw >= 1 && raw <= 300 && clamped != raw) + __builtin_trap(); + + // 2. Renderer parsing must always yield one of the two known values; an + // unrecognised string falls back to FlameGraph rather than failing a + // request, which is what the parameter documentation says. + const ChartRenderer renderer = parseChartRenderer(input); + if (renderer != ChartRenderer::FlameGraph && renderer != ChartRenderer::CallGraph) + __builtin_trap(); + if (input != "callgraph" && renderer != ChartRenderer::FlameGraph) + __builtin_trap(); + + // 3. A default-constructed ChartOptions must itself be within the documented + // range, since that is what an omitted parameter produces. + const ChartOptions defaults; + if (clampChartDuration(defaults.duration) != defaults.duration) + __builtin_trap(); + + return 0; +} diff --git a/tests/fuzz/fuzz_renderer_output.cpp b/tests/fuzz/fuzz_renderer_output.cpp new file mode 100644 index 0000000..ddacf27 --- /dev/null +++ b/tests/fuzz/fuzz_renderer_output.cpp @@ -0,0 +1,67 @@ +/// @file fuzz_renderer_output.cpp +/// @brief libFuzzer target for the renderer-output and request parsers +/// +/// These functions receive bytes produced by external processes (flamegraph.pl, +/// pprof) or by a client (the symbolz request body) and decide what they mean. +/// That makes them the project's most fuzzable surface: pure, string-in/verdict-out, +/// and fed by something we do not control. +/// +/// A note on the assertions. An earlier version of this target asserted properties +/// that felt obvious but are not actually guaranteed -- for example that a parsed +/// symbol token never contains a newline. That is false: when the body contains a +/// '+', plus-separation wins and newlines legitimately stay inside a token. The +/// target then "found" a crash on its first run that was the assertion's fault, not +/// the code's. Every check below is therefore limited to what the implementation +/// genuinely promises; everything else is left to ASan/UBSan, which cannot cry +/// wolf. + +#include "internal/renderer_output_parsing.h" +#include +#include +#include + +extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { + const std::string input(reinterpret_cast(data), size); + + using namespace profiler::v0_1_0::internal; + + // 1. Renderer verdicts. The value here is that none of these throws and that + // the invariants the HTTP layer depends on hold. + const bool svg_shaped = looksLikeSvg(input); + const auto message = extractFlameGraphError(input); + const auto failure = interpretFlameGraphOutput(input); + + // A message exists only when the parser actually found the marker. + if (message && input.find("ERROR:") == std::string::npos) + __builtin_trap(); + + // A message is a slice of the input, so it can never be longer than it. + if (message && message->size() > input.size()) + __builtin_trap(); + + // An "ERROR:" marker must always be reported as a failure; handing a + // renderer's error text back as a graph is the defect this check exists for. + if (input.find("ERROR:") != std::string::npos && !failure) + __builtin_trap(); + + // Output that does not look like an SVG must never be accepted as a graph. + if (!svg_shaped && !failure) + __builtin_trap(); + + // 2. pprof symbolz request bodies. The caller converts each token with + // stoull(), so a token must be a slice of the body. '+' can never survive + // (it wins as the separator whenever it appears); a newline may, because + // newline-splitting only applies when there is no '+'. + const auto addresses = parseSymbolRequest(input); + const bool plus_separated = input.find('+') != std::string::npos; + for (const auto& address : addresses) { + if (address.size() > input.size()) + __builtin_trap(); + if (address.find('+') != std::string::npos) + __builtin_trap(); + if (!plus_separated && address.find('\n') != std::string::npos) + __builtin_trap(); + } + + return 0; +} diff --git a/tests/fuzz/fuzz_result_parsing.cpp b/tests/fuzz/fuzz_result_parsing.cpp new file mode 100644 index 0000000..059bce9 --- /dev/null +++ b/tests/fuzz/fuzz_result_parsing.cpp @@ -0,0 +1,63 @@ +/// @file fuzz_result_parsing.cpp +/// @brief libFuzzer target for the profiler-core return-value parsers +/// +/// `internal::isJsonError()` / `jsonErrorMessage()` decide whether a string the +/// core returned is a `{"error": ...}` report and what it says, and +/// `isEmptyHeapSample()` decides whether a heap profile carries data. All three are +/// pure predicates on values that originate outside this process (tcmalloc, +/// gperftools, the renderers), so they are cheap to fuzz, and their answers matter: +/// misreading a failure as data is what let a 500 ship as a 200 earlier in this +/// project. +/// +/// A note on the assertions. The first version of this target required an error's +/// message to be non-empty, which sounds self-evident and is wrong: the extractor +/// legitimately returns nothing for `{"error":""}`, and `isJsonError()` is a +/// structural check that also accepts malformed input like `{{"error{`. The target +/// duly reported a "crash" that was the assertion's fault. Only properties the +/// implementation actually guarantees are asserted here. + +#include "internal/result_parsing.h" +#include +#include +#include + +extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { + const std::string input(reinterpret_cast(data), size); + + using namespace profiler::v0_1_0::internal; + + // 1. isJsonError() / jsonErrorMessage(). Neither may throw on any input: they + // run on a request path over strings the process did not author. + const bool is_error = isJsonError(input); + const std::string message = jsonErrorMessage(input); + + // The predicate's own definition: a `{"...` string carrying the key is an + // error report. If this stops holding, the HTTP layer stops recognising core + // failures -- which is the regression worth catching. + if (input.size() > 10 && input[0] == '{' && input.find("\"error\"") != std::string::npos && !is_error) + __builtin_trap(); + + // A leading '{' is required, so anything else must not classify as an error. + if (!input.empty() && input[0] != '{' && is_error) + __builtin_trap(); + + // The extractor falls back to echoing the input when it cannot find a message, + // so its result is never longer than what it was given. + if (message.size() > input.size()) + __builtin_trap(); + + // 2. isEmptyHeapSample(). Only the forms the implementation documents as empty, + // plus the empty string itself, which callers report as "no data". + const bool empty = isEmptyHeapSample(input); + + if (input.empty() && !empty) + __builtin_trap(); + + if (input.rfind("%warn", 0) == 0 && !empty) + __builtin_trap(); + + if (input.find("@ heap_v2/0") != std::string::npos && !empty) + __builtin_trap(); + + return 0; +} diff --git a/tests/test_renderer_parsing.cpp b/tests/test_renderer_parsing.cpp new file mode 100644 index 0000000..5c58286 --- /dev/null +++ b/tests/test_renderer_parsing.cpp @@ -0,0 +1,120 @@ +/// @file test_renderer_parsing.cpp +/// @brief Unit tests for the renderer-output and request-body parsers +/// +/// The fuzz targets in tests/fuzz/ explore these functions' input space; these +/// tests pin the specific behaviours the HTTP layer depends on, so a regression +/// names itself instead of arriving as a crash artifact. The two are complements: +/// the corpus keeps interesting inputs, and these keep the meaning of the answers. + +#include "../src/internal/renderer_output_parsing.h" +#include +#include + +using namespace profiler::v0_1_0::internal; + +namespace { + +/// The exact shape flamegraph.pl emits when it rejects its input: a structurally +/// valid SVG whose only content is the error text. +const char* kFlamegraphErrorSvg = + "\n" + "ERROR: No valid input provided to flamegraph.pl.\n" + "\n"; + +} // namespace + +TEST(FlameGraphErrorTest, FindsTheMessageInsideTheSvgTextElement) { + const auto message = extractFlameGraphError(kFlamegraphErrorSvg); + + ASSERT_TRUE(message.has_value()); + EXPECT_EQ(*message, "ERROR: No valid input provided to flamegraph.pl."); +} + +TEST(FlameGraphErrorTest, MessageWithoutATrailingTagRunsToTheEnd) { + // flamegraph.pl's other error path warns on stderr, but a caller that merged + // stderr into stdout would hand us a bare line. There is no '<' after the + // marker, so the message is the rest of the string -- and computing the length + // as (npos - pos) is what this guards: it must not throw. + const auto message = extractFlameGraphError("ERROR: No stack counts found"); + + ASSERT_TRUE(message.has_value()); + EXPECT_EQ(*message, "ERROR: No stack counts found"); +} + +TEST(FlameGraphErrorTest, NoMarkerMeansNoMessage) { + EXPECT_FALSE(extractFlameGraphError("42 samples").has_value()); + EXPECT_FALSE(extractFlameGraphError("").has_value()); +} + +TEST(FlameGraphErrorTest, MarkerAtTheVeryEndIsReported) { + const auto message = extractFlameGraphError("ERROR:"); + ASSERT_TRUE(message.has_value()); + EXPECT_EQ(*message, "ERROR:"); +} + +TEST(FlameGraphErrorTest, SuccessfulSvgIsNotAFailure) { + // The case that matters most: a graph with no error marker must be accepted. + // A mis-parse here turns every successful render into a 500. + const std::string ok = ""; + EXPECT_FALSE(interpretFlameGraphOutput(ok).has_value()); +} + +TEST(FlameGraphErrorTest, ErrorSvgIsAFailureWithAnExplanation) { + const auto failure = interpretFlameGraphOutput(kFlamegraphErrorSvg); + + ASSERT_TRUE(failure.has_value()); + // The explanation is what the caller shows, so it must name the renderer's + // reason as well as our own guidance. + EXPECT_NE(failure->explanation().find("flamegraph.pl said:"), std::string::npos); + EXPECT_NE(failure->explanation().find("No valid input provided"), std::string::npos); +} + +TEST(FlameGraphErrorTest, NonSvgOutputIsAlwaysAFailure) { + for (const char* junk : {"garbage", "", "ERROR:", "not xml at all"}) { + EXPECT_TRUE(interpretFlameGraphOutput(junk).has_value()) << "input: " << junk; + } +} + +TEST(SymbolRequestTest, SplitsOnPlusAsGoPprofSends) { + const auto addresses = parseSymbolRequest("0x401d8a+0x401d8b+0x401d8c"); + + ASSERT_EQ(addresses.size(), 3u); + EXPECT_EQ(addresses[0], "0x401d8a"); + EXPECT_EQ(addresses[2], "0x401d8c"); +} + +TEST(SymbolRequestTest, SplitsOnNewlinesAndSkipsComments) { + const auto addresses = parseSymbolRequest("0x1\n# a comment\n0x2\n"); + + ASSERT_EQ(addresses.size(), 2u); + EXPECT_EQ(addresses[0], "0x1"); + EXPECT_EQ(addresses[1], "0x2"); +} + +TEST(SymbolRequestTest, HandlesCrlfInput) { + const auto addresses = parseSymbolRequest("0x1\r\n0x2\r\n"); + + ASSERT_EQ(addresses.size(), 2u); + // A stray '\r' would reach stoull() and be rejected there, turning a valid + // request into an unresolved address. + EXPECT_EQ(addresses[0], "0x1"); + EXPECT_EQ(addresses[1], "0x2"); +} + +TEST(SymbolRequestTest, EmptyPartsAreSkipped) { + EXPECT_TRUE(parseSymbolRequest("").empty()); + EXPECT_TRUE(parseSymbolRequest("+").empty()); + EXPECT_TRUE(parseSymbolRequest("++0x1++").size() == 1u); + EXPECT_TRUE(parseSymbolRequest("\n\n").empty()); +} + +TEST(SymbolRequestTest, PlusWinsOverNewlineSeparation) { + // Documented precedence: when a '+' is present, newlines are ordinary bytes + // inside a token rather than separators. Asserted because the fuzz target's + // first version got this wrong and "found" a bug that was not there. + const auto addresses = parseSymbolRequest("0x1\n0x2+0x3"); + + ASSERT_EQ(addresses.size(), 2u); + EXPECT_EQ(addresses[0], "0x1\n0x2"); + EXPECT_EQ(addresses[1], "0x3"); +} From a82f5a5c9c192ff6d08a0b865cf00acbb4ac51bb Mon Sep 17 00:00:00 2001 From: "irons.du" Date: Wed, 16 Sep 2026 06:54:09 +0800 Subject: [PATCH 2/6] ci: fix ci.yml, which GitHub never parsed, and lint workflows so it stays fixed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ci.yml has been rejected since e713b18 ("ci: 优化 vcpkg 缓存策略") with Invalid workflow file: .github/workflows/ci.yml#L1 (Line: 49, Col: 27): A sequence was not expected (Line: 50, Col: 31): A sequence was not expected ... same pair at 139/140 and 210/211 so the entire pipeline -- build-gcc, coverage, build-clang -- has never run, and nothing reported it because a workflow GitHub cannot parse produces no jobs to fail. The cause is that `with:` values must be scalars. Two of the run-vcpkg inputs were written as YAML flow sequences: vcpkgJsonIgnores: ['**/vcpkg/**'] runVcpkgFormatString: ['install', '--recurse', ..., '$[env.VCPKG_INSTALLED_DIR]', ...] and both are rejected. The action's own action.yml declares them as scalars, which is why its defaults are quoted: `default: "['**/vcpkg/**']"`. Both inputs were restating the action's defaults, so neither is set now: - vcpkgJsonIgnores defaults to exactly the pattern we passed, - runVcpkgFormatString defaults to the same install command, including `--x-install-root $[env.VCPKG_INSTALLED_DIR]`. For the second one that is not a guess about behaviour, it is the only reason the fix is safe: the install root comes from the action's default, not from our copy of the string. Worth noting the variable is not defined as env anywhere in ci.yml (`build-gcc` builds via `cmake --preset=release`, so vcpkg runs from the CMake toolchain and uses its own default install root), so the expansion was empty in our version too -- dropping it changes nothing either way. Diagnosed with actionlint, which reproduces GitHub's message verbatim: ci.yml:49:27: expected scalar node for string value but found sequence node with "!!seq" tag `actionlint` (no arguments) now runs in the fuzz job, right after it is installed and before the expensive steps. A workflow that does not parse is invisible: it fails silently rather than loudly, so it needs a check that does not depend on the workflow itself running. Both files pass. Verified: actionlint clean on both workflows, YAML parses, job lists intact. --- .github/workflows/ci.yml | 21 +++++++++++++++------ .github/workflows/code-quality.yml | 10 +++++++++- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 61fd16f..dc57cd5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -46,8 +46,11 @@ jobs: runVcpkgInstall: true doNotCache: false vcpkgJsonGlob: vcpkg.json - vcpkgJsonIgnores: ['**/vcpkg/**'] - runVcpkgFormatString: ['install', '--recurse', '--clean-after-build', '--x-install-root', '$[env.VCPKG_INSTALLED_DIR]', '--triplet', 'x64-linux'] + # vcpkgJsonIgnores and runVcpkgFormatString are deliberately not set: both + # would restate the action's own defaults, and the format string is what + # broke this workflow file. The action's default already passes + # --x-install-root $[env.VCPKG_INSTALLED_DIR], so the install root is + # unchanged by dropping it. - name: Debug vcpkg environment run: | @@ -136,8 +139,11 @@ jobs: runVcpkgInstall: true doNotCache: false vcpkgJsonGlob: vcpkg.json - vcpkgJsonIgnores: ['**/vcpkg/**'] - runVcpkgFormatString: ['install', '--recurse', '--clean-after-build', '--x-install-root', '$[env.VCPKG_INSTALLED_DIR]', '--triplet', 'x64-linux'] + # vcpkgJsonIgnores and runVcpkgFormatString are deliberately not set: both + # would restate the action's own defaults, and the format string is what + # broke this workflow file. The action's default already passes + # --x-install-root $[env.VCPKG_INSTALLED_DIR], so the install root is + # unchanged by dropping it. - name: Configure CMake with Coverage run: cmake --preset=coverage @@ -207,8 +213,11 @@ jobs: runVcpkgInstall: true doNotCache: false vcpkgJsonGlob: vcpkg.json - vcpkgJsonIgnores: ['**/vcpkg/**'] - runVcpkgFormatString: ['install', '--recurse', '--clean-after-build', '--x-install-root', '$[env.VCPKG_INSTALLED_DIR]', '--triplet', 'x64-linux'] + # vcpkgJsonIgnores and runVcpkgFormatString are deliberately not set: both + # would restate the action's own defaults, and the format string is what + # broke this workflow file. The action's default already passes + # --x-install-root $[env.VCPKG_INSTALLED_DIR], so the install root is + # unchanged by dropping it. - name: Debug vcpkg environment run: | diff --git a/.github/workflows/code-quality.yml b/.github/workflows/code-quality.yml index 6bb5ea2..8478d9e 100644 --- a/.github/workflows/code-quality.yml +++ b/.github/workflows/code-quality.yml @@ -289,7 +289,15 @@ jobs: clang \ git \ pkg-config \ - libgoogle-perftools-dev + libgoogle-perftools-dev \ + actionlint + + # First, before anything expensive: a workflow file GitHub cannot parse never + # runs at all, so its jobs report nothing and the problem hides. ci.yml lost + # its entire pipeline to one unparseable input value this way, and `with:` keys + # must be scalars -- a YAML flow sequence there fails the whole file. + - name: Lint workflow files + run: actionlint -color - name: Export GitHub Actions cache environment variables uses: actions/github-script@v7 From 3c169d65273f9a4c876c7add7b95b32b2c2a1836 Mon Sep 17 00:00:00 2001 From: "irons.du" Date: Wed, 16 Sep 2026 18:36:56 +0800 Subject: [PATCH 3/6] ci: install actionlint from its release, not apt (Ubuntu does not package it) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit added `actionlint` to the apt install list. Ubuntu does not package it -- Debian and Arch do -- so the step died with E: Unable to locate package actionlint The runner image confirms it is not available to apt: the ubuntu-24.04 tool list has shellcheck and yamllint but no actionlint, while GitHub CLI 2.100.0 is preinstalled, which is the download path the actionlint docs recommend. The binary is therefore fetched from the release: gh release download --repo rhysd/actionlint \ --pattern '*_linux_amd64.tar.gz' --output /tmp/actionlint.tar.gz latest tar xzf ... -C /tmp actionlint install -m 0755 /tmp/actionlint /usr/local/bin/actionlint actionlint --version # fails the step loudly if the shape changes `latest` rather than a pinned tag: a pinned tag's download URL rots once the asset layout changes, and this step's job is to notice exactly that kind of breakage. Verified the asset naming locally against v1.7.12 -- the tarball contains a bare `actionlint` binary, and the same download/unpack/install/--version sequence runs clean. Also adds .github/actionlint.yaml. Without it, actionlint reports a second finding beyond the syntax error that broke ci.yml: `if: false` on the TSan job is a constant condition. That disablement is deliberate and already tracked -- CONTRIBUTING.md lists tsan as "当前被 if: false 禁用" and ROADMAP.md carries "修复并重新启用 Thread Sanitizer" -- so it is suppressed by message rather than removed, and re-enabling the job is what deletes the suppression. Verified the suppression is narrow, by breaking the workflow two ways afterwards: a flow sequence under `with:` is still reported as [syntax-check] and an undefined `needs:` as [job-needs]. actonlint clean on both workflows, both parse. --- .github/actionlint.yaml | 15 +++++++++++++++ .github/workflows/code-quality.yml | 24 ++++++++++++++++++------ 2 files changed, 33 insertions(+), 6 deletions(-) create mode 100644 .github/actionlint.yaml diff --git a/.github/actionlint.yaml b/.github/actionlint.yaml new file mode 100644 index 0000000..f59248a --- /dev/null +++ b/.github/actionlint.yaml @@ -0,0 +1,15 @@ +# actionlint configuration. +# +# One check is suppressed deliberately; everything else actionlint reports is a +# real finding and should be fixed rather than added here. + +paths: + .github/workflows/**/*.{yml,yaml}: + ignore: + # The Thread Sanitizer job is switched off with `if: false` on the job. + # actionlint flags that as a constant condition, which is exactly what it + # is -- the disablement is intentional and tracked: CONTRIBUTING.md notes it + # under the sanitizer jobs and ROADMAP.md carries "fix and re-enable TSan". + # Suppressed here so lint stays meaningful for everything else, and so + # re-enabling the job is what removes the need for this entry. + - 'constant expression "false" in condition' diff --git a/.github/workflows/code-quality.yml b/.github/workflows/code-quality.yml index 8478d9e..f5b2601 100644 --- a/.github/workflows/code-quality.yml +++ b/.github/workflows/code-quality.yml @@ -289,13 +289,25 @@ jobs: clang \ git \ pkg-config \ - libgoogle-perftools-dev \ - actionlint + libgoogle-perftools-dev + + # Before anything expensive: a workflow file GitHub cannot parse never runs at + # all, so its jobs report nothing and the problem hides. ci.yml lost its entire + # pipeline to one unparseable input value this way, and `with:` keys must be + # scalars -- a YAML flow sequence there fails the whole file. + # + # actionlint is not in Ubuntu's archive (Debian and Arch package it, Ubuntu does + # not), so the release binary is fetched instead. No pin: the download URL for a + # fixed tag would rot, `latest` cannot. + - name: Install actionlint + run: | + set -euo pipefail + gh release download --repo rhysd/actionlint \ + --pattern '*_linux_amd64.tar.gz' --output /tmp/actionlint.tar.gz latest + tar xzf /tmp/actionlint.tar.gz -C /tmp actionlint + install -m 0755 /tmp/actionlint /usr/local/bin/actionlint + actionlint --version - # First, before anything expensive: a workflow file GitHub cannot parse never - # runs at all, so its jobs report nothing and the problem hides. ci.yml lost - # its entire pipeline to one unparseable input value this way, and `with:` keys - # must be scalars -- a YAML flow sequence there fails the whole file. - name: Lint workflow files run: actionlint -color From 3a5b34d864ef7ebd1c86b2669bf44d9cd4c7ce26 Mon Sep 17 00:00:00 2001 From: "irons.du" Date: Wed, 16 Sep 2026 18:38:28 +0800 Subject: [PATCH 4/6] ci: drop the actionlint step Kept the diagnosis, dropped the checker. It was added to stop ci.yml from breaking the same way again, but the reasoning for it is weaker than it first looks: - the root cause is fixed, and ci.yml now parses, so the pipeline reports failures the way a pipeline normally does; - the failure it caught was silent only because parsing failed. That mode is gone once the file is valid, and a future YAML mistake surfaces as "no jobs ran"; - it cost a step that downloads a binary from a GitHub release, which is exactly what failed the previous attempt (Ubuntu does not package actionlint). So the check is removed rather than repaired: no install step, no lint step, and no .github/actionlint.yaml. That config existed only to suppress the pre-existing `if: false` on the TSan job, which actionlint reported as a constant condition; with the checker gone there is nothing to suppress. TSan's disablement stays documented in CONTRIBUTING.md and tracked in ROADMAP.md, where it always was. The remaining actionlint finding in the repository is that same `if: false`, which predates this branch. Nothing reports it now, which is an accepted trade: it is a deliberate, documented disablement rather than a defect waiting to be found. Verified: both workflows parse, job lists intact (build-gcc/coverage/build-clang and the seven code-quality jobs), fuzz job back to its plain step sequence. --- .github/actionlint.yaml | 15 --------------- .github/workflows/code-quality.yml | 20 -------------------- CHANGELOG.md | 1 + 3 files changed, 1 insertion(+), 35 deletions(-) delete mode 100644 .github/actionlint.yaml diff --git a/.github/actionlint.yaml b/.github/actionlint.yaml deleted file mode 100644 index f59248a..0000000 --- a/.github/actionlint.yaml +++ /dev/null @@ -1,15 +0,0 @@ -# actionlint configuration. -# -# One check is suppressed deliberately; everything else actionlint reports is a -# real finding and should be fixed rather than added here. - -paths: - .github/workflows/**/*.{yml,yaml}: - ignore: - # The Thread Sanitizer job is switched off with `if: false` on the job. - # actionlint flags that as a constant condition, which is exactly what it - # is -- the disablement is intentional and tracked: CONTRIBUTING.md notes it - # under the sanitizer jobs and ROADMAP.md carries "fix and re-enable TSan". - # Suppressed here so lint stays meaningful for everything else, and so - # re-enabling the job is what removes the need for this entry. - - 'constant expression "false" in condition' diff --git a/.github/workflows/code-quality.yml b/.github/workflows/code-quality.yml index f5b2601..6bb5ea2 100644 --- a/.github/workflows/code-quality.yml +++ b/.github/workflows/code-quality.yml @@ -291,26 +291,6 @@ jobs: pkg-config \ libgoogle-perftools-dev - # Before anything expensive: a workflow file GitHub cannot parse never runs at - # all, so its jobs report nothing and the problem hides. ci.yml lost its entire - # pipeline to one unparseable input value this way, and `with:` keys must be - # scalars -- a YAML flow sequence there fails the whole file. - # - # actionlint is not in Ubuntu's archive (Debian and Arch package it, Ubuntu does - # not), so the release binary is fetched instead. No pin: the download URL for a - # fixed tag would rot, `latest` cannot. - - name: Install actionlint - run: | - set -euo pipefail - gh release download --repo rhysd/actionlint \ - --pattern '*_linux_amd64.tar.gz' --output /tmp/actionlint.tar.gz latest - tar xzf /tmp/actionlint.tar.gz -C /tmp actionlint - install -m 0755 /tmp/actionlint /usr/local/bin/actionlint - actionlint --version - - - name: Lint workflow files - run: actionlint -color - - name: Export GitHub Actions cache environment variables uses: actions/github-script@v7 with: diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ec6ce3..90db617 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -114,6 +114,7 @@ Verified in a real browser (headless Chrome driven over CDP) rather than by insp - Installation layout made consistent across `lib`/`lib64` hosts so `find_package()` works (GNUInstallDirs is now included before the install rules) ### Fixed +- `ci.yml` was rejected by GitHub with "A sequence was not expected" and had been since the pipeline was introduced, so build-gcc, coverage and build-clang never ran -- silently, because a workflow that does not parse produces no jobs to fail. The cause was two run-vcpkg inputs written as YAML flow sequences (`vcpkgJsonIgnores`, `runVcpkgFormatString`); `with:` values must be scalars. Both were restating the action's own defaults, so neither is set now -- the install root still comes from the action's default, not from our copy of the string. - `stopHeapProfiler()` and `getRawHeapProfileSample()` no longer call `GetHeapProfile()`. They write the window's snapshot with `HeapProfilerDump()` and read that file back, so the profile memory belongs to us: nothing needs freeing, and the `leak:GetHeapProfile` suppression is gone. This replaces the previous workaround of retaining the buffer, which existed only because `GetHeapProfile()`'s return value has no portable deallocator (see ROADMAP). Note the dump lands beside the requested output path under a derived name -- gperftools appends `..heap` to the prefix -- so `stopHeapProfiler()` still writes the caller's path, from the dump's contents. - Reverted the `free()` added to `stopHeapProfiler()` and `getRawHeapProfileSample()`: it aborted ASan's job with "attempting free on address which was not malloc()-ed". `GetHeapProfile()`'s header says the caller should free the buffer, and upstream's own test does exactly that, but the buffer has no portable deallocator in practice -- with a statically linked tcmalloc it comes from the library's own allocator, which the process's malloc interposer never saw (so `free()` aborts), `tc_free()` only exists from gperftools 2.16.90, and gperftools 2.18 additionally routes its intermediate chunks through an internal arena whose `Free` lives in a private header. The buffer is roughly 11KB per stop, so it is copied out and released to the OS instead; `lsan.supp` carries a matching suppression. The earlier "fix" traded a benign leak for a crash. - The ASan job now also covers a statically linked tcmalloc build, which is what vcpkg produces and what the crash above required; the shared-library configuration used locally cannot reproduce it. From fddc5e33c8aa76ce26107043c5e9203b0f770732 Mon Sep 17 00:00:00 2001 From: "irons.du" Date: Wed, 16 Sep 2026 18:39:05 +0800 Subject: [PATCH 5/6] docs(roadmap): record the actionlint decision instead of only removing it Removing the checker left the reasoning nowhere: the CI section listed TSan, ccache, Dependabot and CodeQL, but nothing about workflow validation, so the next person would have had to rediscover why ci.yml had been unparseable since the pipeline was added. The entry carries what a future attempt needs: - the failure mode, because it is the whole reason a checker is tempting: a workflow that does not parse produces no jobs, so nothing turns red. ci.yml sat broken long enough for three jobs to never run once. - that the root cause is fixed, and why that lowers the value of a checker -- a new YAML mistake now shows up as "no jobs ran" rather than staying invisible. - the reason it was removed rather than repaired: Ubuntu does not package actionlint, so the install step downloads a release binary, which is itself what failed. - how to bring it back if it is wanted: a ready-made action instead of a manual download, in its own job so a fuzz build failure cannot mask it, and accounting for the deliberate `if: false` on TSan, which actionlint reports as a constant condition. No line numbers in the cross-references: ROADMAP already cost me an audit round for pointing at drifted line numbers, and CONTRIBUTING.md's own TSan note is one edit away from moving. --- ROADMAP.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/ROADMAP.md b/ROADMAP.md index 8454770..16ba8cd 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -172,6 +172,20 @@ ## 四、CI/CD 增强 - **修复并重新启用 Thread Sanitizer**（`.github/workflows/code-quality.yml` 中该 job 被 `if: false` 禁用） +- **workflow 语法校验（actionlint）——本轮试过又移除了，记录理由待重新评估** + - 背景：`ci.yml` 自流水线引入起就无法被 GitHub 解析（`with:` 的值必须是标量，而 + `vcpkgJsonIgnores` / `runVcpkgFormatString` 被写成了 YAML flow sequence），导致 + `build-gcc` / `coverage` / `build-clang` **三个 job 从未运行**。失败方式是**静默**的： + 解析不了的 workflow 不产生任何 job，因此没有 job 失败、没有红色标记。 + - 本地用 actionlint 逐字复现了 GitHub 的报错（`expected scalar node for string value but + found sequence node`），根因已修复。 + - 当时把 actionlint 接入了 CI，随后又移除：根因修好后，未来写错会表现为"没有 job 跑" + （可见），而接入需要从 GitHub release 下载二进制——**Ubuntu 仓库没有 actionlint 包** + （Debian/Arch 有），这一步本身就成了失败源。 + - 若要重新引入，建议用现成 action（如 `raven-actions/actionlint`）而非自己下载，并把它 + 放在**独立的轻量 job**里——否则它会被同一 job 中 fuzz 构建的失败所掩盖。 + - 另注：即使接入，也需处理 TSan job 的 `if: false`（actionlint 会报 + `constant expression "false"`，而这是**有意禁用**——见本文件与 `CONTRIBUTING.md`）。 - **ccache / sccache** 加速 CI 编译 - **依赖更新自动化**（Dependabot 或 Renovate） - **安全扫描**（CodeQL 或 Snyk） From 75d8f7fc35e5f37e726c6ff93816b77c901acf28 Mon Sep 17 00:00:00 2001 From: "irons.du" Date: Wed, 16 Sep 2026 20:46:11 +0800 Subject: [PATCH 6/6] ci: apply lsan.supp to the fuzz steps, which is why all three failed The first run of the new fuzz job failed every target with the same report: Direct leak of 45 byte(s) in 1 object(s) allocated from: #8 MallocExtension::Initialize() (/lib/x86_64-linux-gnu/libtcmalloc.so.4) That is tcmalloc allocating its own internal state at load time, the first entry in lsan.supp, and not anything the targets did. The fuzz steps set ASAN_OPTIONS but not LSAN_OPTIONS, so the project's suppressions never applied to them -- the ASan job sets both and has always been green. Both steps now carry the same pair with the same working directory as the ASan job: ASAN_OPTIONS: detect_leaks=1:halt_on_error=1 LSAN_OPTIONS: suppressions=../lsan.supp Suppressing rather than turning leak detection off for the fuzz targets: the suppression is a known-false-positive list, and with it applied a real leak in the parsers still fails the run. `detect_leaks=0` would have made the failure go away too, and would have taken that coverage with it. Verified as far as this machine allows, and no further: the local build uses the system shared tcmalloc, whose MallocExtension::Initialize() allocation LSan does not flag, so the CI failure cannot be reproduced here and I cannot claim to have watched it go green. What is verified is that the replay and smoke steps now use byte-for-byte the environment of an ASan job step that passes, that `ctest -L fuzz -E FuzzSmoke` passes locally under that environment, and that `../lsan.supp` resolves from the same `working-directory: build` the ASan job uses. --- .github/workflows/code-quality.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.github/workflows/code-quality.yml b/.github/workflows/code-quality.yml index 6bb5ea2..d514fcc 100644 --- a/.github/workflows/code-quality.yml +++ b/.github/workflows/code-quality.yml @@ -349,10 +349,18 @@ jobs: # The per-target tests replay the committed corpus (one run each), so this # fails if a corpus entry stops behaving. The smoke test then fuzzes past the # corpus for a bounded time. + # + # Both need the same environment the ASan job uses. tcmalloc's own + # MallocExtension::Initialize() allocation is reported by LSan as a leak unless + # lsan.supp is applied -- the first run of these steps failed on that 45-byte + # allocation, not on anything the targets did. - name: Replay corpora working-directory: build run: | ctest -L fuzz --output-on-failure -E FuzzSmoke + env: + ASAN_OPTIONS: detect_leaks=1:halt_on_error=1 + LSAN_OPTIONS: suppressions=../lsan.supp - name: Fuzz smoke test working-directory: build @@ -360,6 +368,7 @@ jobs: ctest -L fuzz --output-on-failure -R FuzzSmoke env: ASAN_OPTIONS: detect_leaks=1:halt_on_error=1 + LSAN_OPTIONS: suppressions=../lsan.supp # A crash leaves a reproducer behind; upload it so the failure is actionable # without re-running the job.