Re-enabling ASAN tests - #1617
Merged
Merged
Conversation
* Suppress leaks from Python wrapper / interpreter * New suppressions file testsuite/lsan-suppressions.txt * Enabeling ASAN tests on Jenkins
Member
Author
|
We have a few real memory issues:
|
* Alg loop owns kinsol solver object
Member
Author
|
The remaining issues are basically FMUs leaking memory. We should check if updating the OpenModelica ones improves the situation. Not sure if we can update the remaining FMUs. For those disabling ASAN might be the only option. |
Member
Author
|
Looks like I need to finally do OpenModelica/OpenModelica#14638.... |
* The issues are caused by the FMU binaries
* fmu.size() can't be used before `fmus` is filled * Use unique pointer to clarify ownership
* Equivalent of -Werror flags for GCC/Clang
* It was unused and leaking memory
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
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
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.
Related Issues
Fixes #1615.
Purpose
Approach
Bugs fixed in OMSimulator
Heap-buffer-overflow in
SystemSC(b917d7b)callEventUpdateandterminateSimulationwere allocated in a default member initializer,new bool[fmus.size()](). That runs in the constructor, wherefmusis still empty —initialize()only fills it later — so both arrays were always zero length. Every ME step then wrote one flag per FMU past the end of them. The arrays are nowstd::unique_ptr<bool[]>sized ininitialize()oncefmusis known.unique_ptralso closes two related problems:terminate()delete[]-ed the arrays without clearing the pointers whileinitialize()never re-allocated them (dangling pointer on re-initialize), and~SystemSC()freed nothing at all (leak whenterminate()is never called).KINSOL solver leaked per algebraic loop (01dcf3f)
AlgLoopownedKinsolSolver*as a raw pointer and never deleted it, leaking the KINSOL memory block, fiveN_Vectors, the dense matrix, the linear solver and the user-data struct for every algebraic loop.kinsolDatawas also left uninitialized when the fixed-point solver was selected, and the ~12 error returns inNewKinsolSolver()leaked the half-built object. Now aunique_ptrmember, with the factory holding one until the object is complete.Build configuration
-DASAN=ONon a Clang build silently produced an uninstrumented library — it still reported leaks through the preloaded libasaninterceptors, but detected no overflow or use-after-free at all. A
FATAL_ERRORnow rejects compilers too old to support the sanitizer.-Werror=memory warnings (2193adb): C4700, C4701, C4703, C4715, C4716 and C4172 are raised to the default warning level and turned into errors.Testsuite
testsuite/lsan-suppressions.txt, wired intoruntest.pyviaLSAN_OPTIONS. It covers only non-OMSimulator modules: the uutils coreutils processes theOMSimulatorPython3wrapper starts, CPython's own interpreter state, and lxml/libxml2/libxslt.print_suppressions=0keeps the reports out of the compared test output. The file documents why matching on module paths is safe here and whyfast_unwind_on_malloc=0must not be set.asan: yeson 82 test cases,asan: nowith an explanatory line on 23 (774f903, ab681d0) — see the limitation below.from numpy import uint64from the twoFeedthroughSetValuetests andpython3-numpyfrom the coverage workflow (8cfb221). numpy leaks ~683 bytes registering its ufuncs at import, in stacks too shallow to suppress by interpreter frame.Known limitation
23 tests are marked
asan: no. All of them fail on leaks — and in the twoembracecases a use-after-free — that occur inside the FMU binaries, not in OMSimulator: OpenModelica-generated FMUs leak in<Model>_function_initSynchronous,FMU_16_aero_interface.soleaks ~82 kB duringfmi2Instantiate, and Dymola'sECS_HW.soreads the pointer returned bysetlocale()after a later call has freed it. All of these tests do callterminate()anddelete(), sofmi2FreeInstanceis issued correctly. Suppressions cannot help: once the FMU isdlclosed the stacks are#0 malloc / #1 <unknown module>with nothing left tomatch on. Each disabled test carries a one-line comment naming the reason.