[BUG] Msgpack may clean up data twice #404
Labels
bug
Something isn't working
Comments
mgovers
changed the title
|
It appears that the root cause is the fact that Minimal repro case (using Debug configuration without additional optimizations): // SPDX-FileCopyrightText: 2022 Contributors to the Power Grid Model project <[email protected]>
//
// SPDX-License-Identifier: MPL-2.0
#include <msgpack.hpp>
namespace {
struct Foo {
std::unique_ptr<msgpack::zone> zone;
};
} // namespace
int main() {
Foo const foo{};
return 0;
}
[build] "<...>/cmake.exe" -E __run_co_compile --tidy=clang-tidy.exe;--extra-arg=/EHsc;--extra-arg-before=--driver-mode=cl --source=<source_dir>/source.cpp -- <...>/clang-cl.exe /nologo -TP -imsvc<std_lib> /DWIN32 /D_WINDOWS /EHsc /Zi /Ob0 /Od /RTC1 -std:c++20 -MDd /showIncludes /Fo<build_dir>/test_tmp.cpp.obj /Fd<build_dir>\ -c -- <source_dir>/source.cpp
[build] <msgpack_dir>/include/msgpack/v1/detail/cpp11_zone.hpp:197:9: error: Attempt to free released memory [clang-analyzer-cplusplus.NewDelete,-warnings-as-errors]
[build] ::free(p);
[build] ^
[build] <source_dir>/source.cpp:15:12: note: Calling implicit destructor for 'Foo'
[build] return 0;
[build] ^
[build] <source_dir>/source.cpp:15:12: note: Calling '~unique_ptr'
[build] <std_lib>/memory:3289:13: note: Assuming field '_Myval2' is non-null
[build] if (_Mypair._Myval2) {
[build] ^~~~~~~~~~~~~~~
[build] <std_lib>/memory:3289:9: note: Taking true branch
[build] if (_Mypair._Myval2) {
[build] ^
[build] <std_lib>/memory:3290:13: note: Calling 'default_delete::operator()'
[build] _Mypair._Get_first()(_Mypair._Myval2);
[build] ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[build] <std_lib>/memory:3180:9: note: Memory is released
[build] delete _Ptr;
[build] ^~~~~~~~~~~
[build] <std_lib>/memory:3180:9: note: Calling 'zone::operator delete'
[build] delete _Ptr;
[build] ^~~~~~~~~~~
[build] <msgpack_dir>/include/msgpack/v1/detail/cpp11_zone.hpp:197:9: note: Attempt to free released memory
[build] ::free(p);
[build] ^~~~~~~~~ |
|
Bug reported to |
|
Full clang-tidy run passed. See also #420 (review) ✅ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
According to
clang-tidy(full run on all targets, e.g. https://github.com/PowerGridModel/power-grid-model/actions/runs/6529848243/job/17728134248 ),msgpackcleans up data twicestd::unique_ptr::~unique_ptrstd::freeThis is very dangerous, because it may clean up memory that another (part of the) program may already be using
Data Dump
To Reproduce
Run
clang-tidyon all targets, e.g. in CI (manual workflow dispatch). See e.g. https://github.com/PowerGridModel/power-grid-model/actions/runs/6529848243/job/17728134248Expected behavior
clang-tidyrun on all targets passes as expected.Additional context
It looks like the root cause of the issue lies in a cleanup within
msgpackwhen moving objects. Moving ofmsgpack::object_handlealready cleans up the data, meaning that calling the destructor of themsgpack::object_handlewill attempt to cleanup the data again, which is a bug.We may want to move to the
visitorpattern implementation ofmsgpackto work around this. That way, there is nounpackerobject with extended lifetime (no member variables) and no moving needs to happen.We already seek to do this because of the better memory performance
The text was updated successfully, but these errors were encountered: