container
container copied to clipboard
[1.85] flat_map/vector crashes on appends (memory corruption)
Consider this test code:
#include <cstdint>
#include <string>
#include <boost/container/flat_map.hpp>
typedef boost::container::flat_map< std::uint8_t, std::string > my_map;
void add_element(std::string& str, char elem)
{
str.push_back(elem);
}
int main()
{
my_map m;
add_element(m[static_cast< std::uint8_t >(96u)], 'a');
add_element(m[static_cast< std::uint8_t >(102u)], 'a');
add_element(m[static_cast< std::uint8_t >(104u)], 'a');
}
g++ -O2 -I. -std=gnu++17 -o test_flat_map test_flat_map.cpp
This crashes with:
free(): invalid pointer
Aborted (core dumped)
valgrind
also shows a number of invalid memory accesses, see the attached log:
This code works correctly in Boost 1.84.0. It also doesn't crash if compiled with -O0
.
gcc 11.4.0, Kubuntu 22.04.
Bisect shows that the first bad commit is 1a4a205ea6ef7b4e67a2faab7c7d745711807695. Reverting this commit on 1.85 (with resolved conflicts) fixes the crash.
Thanks for the report!
I think the issue is produced because flat_map used UB in the implementation, long ago, when C++03 compilers had no movable std::pair type and the class was designed to achieve move emulation in those compilers. The following commit should fix the issue, your example seems to work after the commit, but I didn't want to close the issue without having your feedback:
https://github.com/boostorg/container/commit/20ad12f20e661978e90dc7f36d8ab8ac05e5a5a9
Thanks, the commit does fix the problem.
It is probably worth attaching this patch to the 1.85.0 release notes.
Closing this issue as fixed.
@igaztanaga How come this was not caught by a unit test?
I think it was optimization-level and context code (inlining decisions) dependent and Boost.Container tests only failed consistently with latest GCC versions (>=11, I think). MSVC and clang showed no failure.
Once latest GCC versions were mainstream, boost.Container tests were failing and I figure out that UB could be the cause.
FYI Similar bug (UB => -O2
code elision) occurs in this example, ~~still in process of checking if the mentioned commit fixes the problem~~ the mentioned commit fixes this (near-)minimal reproducible example: https://gcc.godbolt.org/z/qM5befxeY
#include <boost/container/flat_map.hpp>
struct A {
int i;
auto operator<=>(const A&) const = default;
};
int main() {
boost::container::flat_map<A, A> map;
map.emplace(A{1}, A{});
for (auto const& [_1, _2] : map)
throw 123; // this line is never executed, UB!
}
Might be worth getting a patch posted on https://www.boost.org/patches/ and https://www.boost.org/users/history/version_1_85_0.html as a known issue (e.g. how it was done for the 1.83.0 and earlier releases).
E: So, fun. This wasn't caught internally to my org because
- We use the cmake build, as-part-of-source-tree.
- The cmake build would never end up building any tests: the referenced file is just non-existent.
-
-DNDEBUG
also hides the issue (at least the one that I'm referring to, and the original one reported) from occurring / "elides" the UB. - Even if all that was not at issue, the current Jamfile's format does not appear to be supported by Dimov's
boost_test_jamfile
converter / translator since it doesn't explicitly specify the tests.
Does boost-1.86.0-beta1 include the fix for this?
Yes. Release notes were not updated for this beta but the fix is there.
Trying to build a patch for 1.85 I could not reproduce the bug with Boost.Build. It turns out that Build defines -DNDEBUG and that avoids @Lastique's example's crash...
Anyway, here's the first patch for 1.85 if you want to test it:
The patch is better placed on the website and linked in 1.85 release notes. See an example of a PR doing this here.
Many thanks for your example Andrey. PR:
https://github.com/boostorg/website/pull/862