fix: sanitizer + memory fixes - #14
Open
ZocoLini wants to merge 19 commits into
Open
Conversation
ZocoLini
force-pushed
the
build/sanitizers
branch
2 times, most recently
from
September 9, 2026 09:07
93cfcec to
31dda31
Compare
CUQDYN_SANITIZE builds our own modules with AddressSanitizer, LeakSanitizer and UndefinedBehaviorSanitizer, reached through 'scripts/build.sh serial asan'. The flags go on after deps.cmake so the FetchContent dependencies keep out of it: hdf5 generates H5Tinit.c by running H5detect, which probes the platform with deliberately misaligned stores and aborts under UBSan. scripts/test.sh takes a build directory prefix and passes --output-on-failure, without which ctest swallows the report the sanitizer writes. MemorySanitizer is not usable here: clang-only, no Fortran, and it needs every linked object instrumented, which the prebuilt misqp and xml2 cannot be. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B7RAcSVsC6c8r9E4u7W61f
chargelocaloptions_ takes them as double* and writes eight bytes to each, while problem_specifications declared them REAL(C_FLOAT), four bytes on the stack. ASan reports the overflow; the values the local solver then read back were whatever the first half of a double decodes to as a single. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B7RAcSVsC6c8r9E4u7W61f
Five of its cases never released their inputs, which LeakSanitizer reports as 2808 bytes over 31 allocations. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B7RAcSVsC6c8r9E4u7W61f
CVodeSVtolerances clones what it is given into cv_Vabstol, and CVodeFree destroys that clone rather than ours, so the vector leaked on every call. solve_ode runs once per objective evaluation, which made this the leak that grows with maxevaluation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B7RAcSVsC6c8r9E4u7W61f
matrix_array_set_index copies into a matrix of its own rather than taking the one it is handed, so both the array and the four inputs are the test's to free. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B7RAcSVsC6c8r9E4u7W61f
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B7RAcSVsC6c8r9E4u7W61f
The output_function obj_func returns goes back through the deallocator sacess itself uses, exp_total was malloc'd by the test and destroyexp does not free the struct it is handed, and data keeps its own initial_values and observed_idx after create_expetiment_struct has taken the rest. What load_configuration_XML allocates still leaks; that one is destroyexp's. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B7RAcSVsC6c8r9E4u7W61f
load_configuration_XML only reached xmlFreeDoc on the error path, so every successful load abandoned the whole document. The configuration keeps copies rather than pointers into it, so the tree is done with by then. It accounted for 19354 of the 20986 bytes test_ess_obj_function leaked. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B7RAcSVsC6c8r9E4u7W61f
Nearly every one of its 141 call sites feeds the result straight into atoi, atof or strcmp and drops it, so the copy it mallocs leaked. The allocations are now tracked and released when load_configuration_XML returns; the three that outlive the load are strdup'd into their structs first, and the four sites that already freed their own no longer do it twice. test_ess_obj_function goes from 20986 bytes in 378 allocations to 1263 in 13. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B7RAcSVsC6c8r9E4u7W61f
destroyexp released the experiment but not what load_configuration_XML hung off it: nameMatlab, execution.file, bench.type, bench.X0 and bench.F0, and the user, global and local option structs with the two strings the last one owns. vector_char had its free commented out, count_element_multi asked libxml2 for a string it only counted, and eSSversion was callocated 500 bytes that every branch immediately overwrote with a literal. test_ess_obj_function now leaks nothing, down from 23182 bytes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B7RAcSVsC6c8r9E4u7W61f
problem_specifications allocates F0 with only the initial points that carry an objective value, so a configuration whose <point> has no fx attribute leaves it allocated but empty. minloc of an empty array is 0, and X0(:,0) reads the column before the array, which ASan reports as a use-after-free or a heap-buffer-overflow depending on what sits there. Clamping the index keeps the first initial point, which is what the configuration asked to start from. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B7RAcSVsC6c8r9E4u7W61f
Fortran character constants are passed as a pointer and a length and carry no terminator, so when libgfortran hands a WRITE format string to strndup the interceptor reads one byte past it and reports a global-buffer-overflow. The constants are ours, the call is libgfortran's, and the report is neither. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B7RAcSVsC6c8r9E4u7W61f
The MPI2 variant is gone, and the leak that grew with maxevaluation was solve_ode releasing nothing for the tolerance vector it handed CVODES. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B7RAcSVsC6c8r9E4u7W61f
localoptions%tol selects one of three tolerance levels, the way local.tol does in MEIGO: ssm_localsolver.m switches on 1, 2 and 3, and ssm_defaults.m documents 2 as the default. The Fortran port copied that switch including its lack of a final branch, so any other value left thres and acc unassigned and the local solvers read whatever the stack held. dhc takes thres as its stopping threshold. Read as a large number it returns after a single evaluation, which is how the local search has been behaving all along; read as zero it cannot return at all, because the budget check leaves through vr = thres/10 and ABS(0) .GE. 0 stays true. Under the sanitizers the stack holds the second kind of garbage: test_ess_solver ran 22 minutes on the logistic model alone, at 100% CPU, without finishing. The same seed took 2 seconds without them. Both fall-throughs now take the documented default. The whole asan suite runs in 45 seconds, and test_cuqdyn_algo goes from 42 to 77 seconds in debug because the local search finally does the work it was meant to. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lxq4dki1a1bvYLrx7KrMXe
updateresultsess_ declares its seventh parameter double *totaliter, and the serial path handed it common_vars%iter, an INTEGER. There is no interface block for the call, so nothing checked it: the C side loaded eight bytes from a four-byte-aligned slot to build the iteration count in the result struct. UBSan reports it as a misaligned load at output.c:2384. The other four callers all pass results%totaliter, which is the REAL the type declares for this, so the serial one now fills it and does the same. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lxq4dki1a1bvYLrx7KrMXe
…figs TEMPORARY, DROP THIS COMMIT. Borja reviews this part himself; the two fixes under it stand on their own and do not need it. Every sacess-*.xml under example-files/ asked for <tol>0</tol>, which is neither a level nor the sentinel for "not given": the parser takes -1 for that, and ssm_optset keeps the default when tol is below zero. So 0 read as a value the user chose and reached the local solvers with no branch to take. The parser now checks the level the way it already checks solver, and the sixteen configurations ask for the default level 2. Behaviour does not move: <tol>2</tol>, <tol>default</tol> and the guard in call_dhc all give 1d-8. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lxq4dki1a1bvYLrx7KrMXe
destroyexp left three groups behind. logandtranslation_ allocates logindex always and transconst, log_max_dom and log_min_dom on some paths; initrngrandomserial_ allocates the mt19937 generator and the seed array. Those frees existed, but in destroySystemBiology, which nothing calls, so on every real path they outlived the struct. destroySystemBiology is left as it is: destroyexp now leaves the pointers NULL, so its frees are no-ops if it is ever reached. create_expetiment_struct already opens with a block that nulls exactly the fields destroyexp frees, and these six were missing from it. They have to be there rather than left to the caller's malloc: neither logandtranslation_ nor initrngrandomserial_ is on every path, and destroyexp cannot tell an allocation from a stale stack value. Without it destroyexp segfaults on the first free. destroy_result_data is also declared in the header now; it was defined but unreachable from outside. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lxq4dki1a1bvYLrx7KrMXe
Three leaks at the end of the function. The experiment_total array was malloc'd and never freed: destroyexp empties the struct but the block holding it belongs to whoever allocated it, which is how sacess's own main_file.c and test_ess_obj_function already treat it. init_result_data's bestx_value was never released either. And N_VSetArrayPointer swapped the returned vector's data for that buffer, leaving the array New_Serial had just allocated with nothing pointing at it, while handing the caller a block the solver had made. The values are copied into the vector instead, which is a handful of doubles, and then the result data goes back with destroy_result_data. The vector the caller gets owns its own array, as everything else that comes out of this library does. test_cuqdyn_algo drops from 1460508 bytes in 2016 allocations to none, together with the destroyexp fix under this one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lxq4dki1a1bvYLrx7KrMXe
The scenarios leaked the vector execute_ess_solver returns, plus the initial values and the observability indices read_data_file builds. That is 3160 bytes in 27 allocations across the three. Not destroy_cuqdyn_data, which would double free: execute_ess_solver keeps texp, yexp and initial_values, and destroyexp releases all three. Only the two pieces the solver never sees are ours, which is the same split test_ess_obj_function already makes. The asan suite is now clean, 8 of 8. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lxq4dki1a1bvYLrx7KrMXe
ZocoLini
force-pushed
the
build/sanitizers
branch
from
September 9, 2026 10:07
31dda31 to
3bfa8aa
Compare
ZocoLini
marked this pull request as ready for review
September 9, 2026 10:10
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.