[1.85] flat_map/vector crashes on appends (memory corruption)

[Lastique]

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:

test_flat_map.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.

[Lastique]

Bisect shows that the first bad commit is 1a4a205. Reverting this commit on 1.85 (with resolved conflicts) fixes the crash.

container_revert_inline_conversion.patch.gz

[igaztanaga]

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:

20ad12f

[Lastique]

Thanks, the commit does fix the problem.

It is probably worth attaching this patch to the 1.85.0 release notes.

[igaztanaga]

Closing this issue as fixed.

[haferburg]

@igaztanaga How come this was not caught by a unit test?

[igaztanaga]

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.

[13steinj]

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.

[justusranvier]

Does boost-1.86.0-beta1 include the fix for this?

[igaztanaga]

Yes. Release notes were not updated for this beta but the fix is there.

[igaztanaga]

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:

0001-container-fix-flat_map.patch

[Lastique]

The patch is better placed on the website and linked in 1.85 release notes. See an example of a PR doing this here.

[igaztanaga]

Many thanks for your example Andrey. PR:

boostorg/website#862