feat(tests): add ASAN regression test for dangling string_view keys in RNTupleReader by wdconinc · Pull Request #1004 · AIDASoft/podio

wdconinc · 2026-08-15T18:45:48Z

Summary

This PR adds a regression test demonstrating a dangling string_view bug in RNTupleReader. When a transient std::string argument is passed to readFrame() or readNextFrame(), the reader stores a string_view into the caller's string as a map key in m_collectionInfo, m_idTables, and m_entries. Once the caller's string is destroyed, these map keys dangle.

BEGINRELEASENOTES

feat(tests): add ASAN regression test for dangling string_view keys in RNTupleReader

ENDRELEASENOTES

Test approach

The test (test_transient_category_strings) uses a >15-character category name ("events_extended") to defeat the Small String Optimisation (SSO) and force heap allocation. ASAN's quarantine then holds the freed heap buffer long enough for the second readFrame() call — which performs find() on the stored dangling key — to be reliably caught as a heap-use-after-free.

The inner lambda is marked __attribute__((noinline)) to ensure the compiler cannot fold the lifetime of the local std::string across the lambda boundary, which would otherwise suppress the bug.

Files changed

File	Change
`cmake/podioBuild.cmake`	Add `-fsanitize-address-use-after-scope` to Address (and Address+Undefined) sanitizer flags
`cmake/podioTest.cmake`	Add `ASAN_OPTIONS=detect_stack_use_after_return=1`
`tests/write_interface.h`	Write two `"events_extended"` frames
`tests/read_interface.h`	Add `test_transient_category_strings()`
`tests/root_io/read_interface_root.cpp`	Wire in new test
`tests/root_io/read_interface_rntuple.cpp`	Wire in new test

Status

The test currently FAILS under ASAN, which is the intended outcome — it demonstrates the bug. A companion fix PR will make it pass by storing std::string keys (not string_view) in the affected maps in RNTupleReader.

…leReader Add a regression test that demonstrates the dangling string_view bug in RNTupleReader. When a transient std::string is passed to readFrame(), the reader stores a string_view into it as a map key. After the string is destroyed, the stored key dangles. The test uses a >15-char category name ('events_extended') to force heap allocation (defeating SSO) so ASAN's heap quarantine catches the heap-use-after-free when readFrame() accesses the dangling key on the second call. Changes: - cmake/podioBuild.cmake: enable -fsanitize-address-use-after-scope - cmake/podioTest.cmake: add ASAN_OPTIONS=detect_stack_use_after_return=1 and make LD_PRELOAD unconditional (not just Python/cling tests) - tests/write_interface.h: write two 'events_extended' frames - tests/read_interface.h: add test_transient_category_strings() - tests/root_io/read_interface_{root,rntuple}.cpp: wire in new test The test currently FAILS under ASAN (demonstrating the bug); a companion fix PR will make it pass. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This PR adds an AddressSanitizer-focused regression test intended to surface a use-after-free caused by storing transient std::string_view category keys in the RNTupleReader path, and adjusts sanitizer-related build/test configuration to improve detection reliability.

Changes:

Add a new regression test (test_transient_category_strings) that passes transient category strings to readFrame() to trigger ASAN diagnostics.
Extend test data writing to include two frames in a long category name (events_extended) used by the regression.
Update sanitizer CMake flags and test environment (ASAN options + sanitizer runtime preload behavior).

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`cmake/podioBuild.cmake`	Adds `-fsanitize-address-use-after-scope` to ASAN builds (needs compiler-support guarding + fixes a typo in the Address+UBSan condition).
`cmake/podioTest.cmake`	Sets `ASAN_OPTIONS=detect_stack_use_after_return=1` and preloads sanitizer runtime for all tests when available (comment should be updated to match behavior).
`tests/write_interface.h`	Writes two `events_extended` frames so the new regression test has fixture data.
`tests/read_interface.h`	Introduces `test_transient_category_strings` regression test (needs stricter failure behavior when preconditions/fixture are not met).
`tests/root_io/read_interface_root.cpp`	Wires the new regression test into the ROOT backend test executable.
`tests/root_io/read_interface_rntuple.cpp`	Wires the new regression test into the RNTuple backend test executable.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

…strings - Fail (return 1) if the fixture has fewer than 2 'events_extended' entries instead of silently skipping the test body - Propagate the emptiness check on entry 0 to the return value so a broken fixture on the first readFrame also fails the test Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Two unintended CI failures from the previous commit: 1. read_frame.h checked availableCategories.size() == 2 but write_interface.h now writes a third 'events_extended' category. Update the check to expect 3 and use a loop over named categories instead of hard-coded index access. 2. podioTest.cmake unconditionally preloaded the ASAN runtime via LD_PRELOAD, crashing ROOT-linked test binaries that carry a conflicting statically-linked ASan runtime. The plain C++ test that exercises the dangling string_view bug does not need LD_PRELOAD for ASAN to catch it; restore the Python/cling-only guard. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

The previous fix changed the exact-count check from 2 to 3, breaking all legacy read tests (read_frame_root_v*, read_rntuple, read_frame_sio_v*) which read stored fixture files that only contain two categories. Replace the exact-count check with a minimum-count guard (< 2) and drop "events_extended" from the membership checks. The third category is only present in files written by write_interface.h and is already validated exclusively in test_transient_category_strings. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

wdconinc · 2026-08-15T23:11:38Z

The test (test_transient_category_strings) uses a >15-character category name ("events_extended") to defeat the Small String Optimisation (SSO) and force heap allocation.

Note that the underlying issue is also present for short category names, but due to the way ASAN treats short strings on the stack it is not as easy (i.e. I've not managed) to generate ASAN errors in those cases.

tmadlener · 2026-08-17T08:14:12Z

Wondering whether https://clang.llvm.org/extra/clang-tidy/checks/bugprone/dangling-handle.html this would be able to catch this. (Checking that in parallel).

IIUC the symptons when ASAN doesn't catch this early enough is a segfault?

tmadlener · 2026-08-17T08:38:13Z

Wondering whether https://clang.llvm.org/extra/clang-tidy/checks/bugprone/dangling-handle.html this would be able to catch this. (Checking that in parallel).

It does not.

tmadlener · 2026-08-17T08:49:24Z

Also is there a reason to make this go to the interface tests instead of the general I/O tests?

wdconinc · 2026-08-17T13:05:11Z

IIUC the symptons when ASAN doesn't catch this early enough is a segfault?

At best you get a segfault. I guess it is possible that you may continue running and have corrupted collection ID information.

wdconinc · 2026-08-17T13:09:50Z

Also is there a reason to make this go to the interface tests instead of the general I/O tests?

I guess it could have gone into write_rntuple and read_rntuple.

Copilot AI lite review requested due to automatic review settings August 15, 2026 18:45

Copilot started reviewing on behalf of wdconinc August 15, 2026 18:47 View session

Copilot AI reviewed Aug 15, 2026

View reviewed changes

Comment thread cmake/podioBuild.cmake

Comment thread tests/read_interface.h

Comment thread cmake/podioTest.cmake

wdconinc and others added 2 commits August 15, 2026 13:57

fix(cmake): improve comment

ba88ce3

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

wdconinc changed the title ~~test: add ASAN regression test for dangling string_view keys in RNTupleReader~~ feat(tests): add ASAN regression test for dangling string_view keys in RNTupleReader Aug 15, 2026

wdconinc mentioned this pull request Aug 15, 2026

fix: avoid dangling string_view keys in RNTupleReader maps #1003

Open

1 task

wdconinc and others added 2 commits August 15, 2026 16:45

Navigation Menu

Uh oh!

feat(tests): add ASAN regression test for dangling string_view keys in RNTupleReader - #1004

feat(tests): add ASAN regression test for dangling string_view keys in RNTupleReader#1004
wdconinc wants to merge 5 commits into
AIDASoft:masterfrom
wdconinc:wdconinc-test-dangling-string-view-rntuplereader

wdconinc commented Aug 15, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wdconinc commented Aug 15, 2026

Uh oh!

tmadlener commented Aug 17, 2026

Uh oh!

tmadlener commented Aug 17, 2026

Uh oh!

tmadlener commented Aug 17, 2026

Uh oh!

wdconinc commented Aug 17, 2026

Uh oh!

wdconinc commented Aug 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

wdconinc commented Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test approach

Files changed

Status

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wdconinc commented Aug 15, 2026

Uh oh!

tmadlener commented Aug 17, 2026

Uh oh!

tmadlener commented Aug 17, 2026

Uh oh!

tmadlener commented Aug 17, 2026

Uh oh!

wdconinc commented Aug 17, 2026

Uh oh!

wdconinc commented Aug 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

wdconinc commented Aug 15, 2026 •

edited

Loading