Skip to content

CI: add sanitizer and -Werror jobs - #513

Draft
stevekennedyuk wants to merge 7 commits into
openstreetmap:masterfrom
stevekennedyuk:pr/sanitizer-ci
Draft

stevekennedyuk wants to merge 7 commits into
openstreetmap:masterfrom
stevekennedyuk:pr/sanitizer-ci

Conversation

@stevekennedyuk

@stevekennedyuk stevekennedyuk commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Adds a Sanitizers & Warnings workflow so the bugs fixed in this series don't creep back.

Draft — stacked on #511 and #512, which are still open; #502–#510 and #528 are now merged. Its own change is the last commit, and I'll rebase it when those two are merged. Restacked on #512 after #508 merged.

  • Warnings: GCC and Clang with ENABLE_WARNINGS + ENABLE_WERROR, running the full test suite.
  • ASan + UBSan: the full suite including the Apache/renderd integration tests; UBSan halts on error. Services report sanitizer errors in their own logs and may still exit successfully, so a final step fails the job if any report appears in build/tests/logs.
  • TSan: the unit test executables, including the new unit_tests (TSan can't be injected into the system httpd).
  • The sanitizer jobs run on the runner host rather than in a container, so vm.mmap_rnd_bits can be lowered for TSan. The apache2/memcached services that installing the packages starts are stopped, because memcached's default port clashes with the memcached_default tests.
  • The CMake build action accepts EXTRA_CMAKE_OPTIONS.

🤖 Generated with Claude Code

https://claude.ai/code/session_01YForD7qSL9LT6FhMuAoKPV

@codecov-commenter

codecov-commenter commented Oct 10, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.80%. Comparing base (c4e3e62) to head (2fd3941).
⚠️ Report is 60 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #513      +/-   ##
==========================================
+ Coverage   81.54%   81.80%   +0.26%     
==========================================
  Files          24       24              
  Lines        5645     5453     -192     
==========================================
- Hits         4603     4461     -142     
+ Misses       1042      992      -50     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread src/renderd.c Outdated
while ((ret_size < sizeof(struct protocol)) && (retry > 0)) {
ret_size = recv(pfd, resp + ret_size, (sizeof(struct protocol)
- ret_size), 0);
ssize_t n = recv(pfd, (char *)resp + ret_size,
Comment thread src/store_file.c Outdated
size_t file_offset, tile_size;

if (m == NULL) {
snprintf(log_msg, PATH_MAX - 1, "Failed to allocate memory for metatile header\n");
Comment thread src/store_memcached.c Outdated
char * buf_raw;

if (m == NULL) {
snprintf(log_msg, 1024, "Failed to allocate memory for metatile header\n");
Comment thread src/store_memcached.c Outdated
rc = memcached_cas(store->storage_ctx, meta_path, strlen(meta_path), buf, len, 0, flags, cas);
// memcached_get() does not return a CAS value, so a compare-and-swap cannot be used here
// (the previous code passed an uninitialised CAS token). Replace the existing item instead.
rc = memcached_replace(store->storage_ctx, meta_path, strlen(meta_path), buf, len, 0, flags);
Comment thread src/store_rados.c Outdated
char * buf_raw;

if (m == NULL) {
snprintf(log_msg, 1024, "Failed to allocate memory for metatile header\n");
Comment thread src/store_ro_http_proxy.c Outdated
ctx->cache.y = y;
ctx->cache.z = z;
strcpy(ctx->cache.xmlname, xmlconfig);
snprintf(ctx->cache.xmlname, sizeof(ctx->cache.xmlname), "%s", xmlconfig);
stevekennedyuk and others added 6 commits October 22, 2026 10:42
- Compile sources shared between targets once via internal OBJECT
  libraries (PIC so they also link into mod_tile.so). A full build with
  tests goes from 104 to 37 compile steps.
- Generate config.h into the build tree instead of includes/, so the
  source tree stays clean and several build dirs can coexist.
- Bump cmake_minimum_required to 3.15 (list(PREPEND) needs it).
- find_package(PkgConfig) explicitly (pkg_get_variable is used directly).
- Drop link_directories(${CMAKE_LIBRARY_PATH}).
- Use option() for boolean switches.
- New options: ENABLE_WARNINGS, ENABLE_WERROR, ENABLE_SANITIZERS.
- TILE_LOAD_DIRECTORY / TILE_LOAD_FILENAME can be set to override the
  build-host /etc/os-release detection (packaging, cross builds).
- Print a feature summary of which optional backends were found.
- Add CMakePresets.json: default, dev, asan, tsan.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YForD7qSL9LT6FhMuAoKPV
Log messages and storage IDs are deliberately truncated to
PATH_MAX-sized buffers, so this heuristic only produces false
positives here. Only added when the compiler supports the flag.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YForD7qSL9LT6FhMuAoKPV
- configure.ac reads the version from CMakeLists.txt so it is only
  defined in one place (and fails loudly if it cannot).
- Bug report URL points at GitHub issues rather than the old trac.
- Use pkg-config for iniparser when a .pc file is available, falling
  back to the previous AC_CHECK_LIB probe.
- configure prints a deprecation warning pointing at CMake; README
  marks Autotools as deprecated.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YForD7qSL9LT6FhMuAoKPV
- With ENABLE_SANITIZERS containing "address", tests run with the ASan
  runtime in LD_PRELOAD, so the system (uninstrumented) httpd can load
  the instrumented mod_tile.so. Without this every Apache-based test
  fails under ASan.
- TSan cannot be preloaded into uninstrumented processes, so the tsan
  test preset only runs the unit test executables.
- tests/tsan.supp suppresses a lock-order report inside GDAL's plugin
  initialisation (on renderd's main thread, before any rendering).
- The asan test preset halts on UBSan errors instead of only printing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YForD7qSL9LT6FhMuAoKPV
unit_tests compiles metatile.cpp and parameterize_style.cpp, which use
ICU (via Mapnik) directly. At -O0 GCC does not inline those calls away,
so a Debug build failed to link:

  undefined reference to symbol '_ZNK6icu_7413UnicodeString10doHashCodeEv'
  libicuuc.so.74: error adding symbols: DSO missing from command line

Add ${ICU_LIBRARIES}, as renderd and gen_tile_test already do.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YForD7qSL9LT6FhMuAoKPV
Catch2 v2 is end-of-life. Vendor the official v3.16.0 amalgamated
build (tests/catch/catch_amalgamated.{hpp,cpp}, with LICENSE and a
README recording the upstream tag/commit) in place of catch.hpp.

- Catch's implementation is compiled once (catch2_o, with
  CATCH_AMALGAMATED_CUSTOM_MAIN and third-party warnings suppressed)
  instead of in every test translation unit.
- catch_main.cpp now defines main() explicitly; gen_tile_test keeps
  its own main() and drops CATCH_CONFIG_RUNNER.
- Catch::Matchers::Contains -> ContainsSubstring (v3 rename), including the
  new unit_test_*.cpp suites.
- Autotools: build the Catch2 implementation once (catch2.o) and link it
  into every test binary.
- Explicit #include <fstream> previously pulled in by catch.hpp.
- Default CMAKE_CXX_STANDARD is now 14 (v3 minimum; mapnik-config
  already reports -std=c++14 for Mapnik 3), with a guard that bumps an
  old cached value of 11 rather than failing.

Every suite runs the same number of test cases and assertions as
under v2.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YForD7qSL9LT6FhMuAoKPV
@stevekennedyuk
stevekennedyuk force-pushed the pr/sanitizer-ci branch 2 times, most recently from a1c3172 to 2b27a26 Compare October 22, 2026 10:00
- Warnings: GCC and Clang builds with ENABLE_WARNINGS and ENABLE_WERROR,
  running the full test suite.
- ASan + UBSan: Debug build with ENABLE_SANITIZERS=address,undefined,
  running the full suite including the Apache/renderd integration
  tests; UBSan halts on error.
- TSan: unit test executables (TSan cannot be injected into the
  system httpd).
- The sanitizer jobs run on the host rather than in a container so
  vm.mmap_rnd_bits can be lowered for TSan; the apache2/memcached
  system services started by installing the packages are stopped, as
  memcached's default port collides with the memcached_default tests.
- Services log sanitizer errors to their own logs and may still exit
  successfully, so a final step fails the job if any report appears
  in build/tests/logs.
- The CMake build action accepts EXTRA_CMAKE_OPTIONS.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YForD7qSL9LT6FhMuAoKPV
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants