Dangling std::string pointers in mesh class

[StefanBruens]

The boundarylayer python test crash reproducibly. According to valgrind the mesh class contains quite some problematic code for the bcnames array. Typical valgrind report:

test_boundarylayer.py ==409999== Invalid read of size 8
==409999==    at 0x9FF0ABB: UnknownInlinedFun (basic_string.h:228)
==409999==    by 0x9FF0ABB: UnknownInlinedFun (basic_string.h:556)
==409999==    by 0x9FF0ABB: netgen::Mesh::SetBCName(int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (meshclass.cpp:7333)
==409999==    by 0x9F60529: netgen::BoundaryLayerTool::CreateFaceDescriptorsSides() (boundarylayer.cpp:725)
==409999==    by 0x9F6337C: netgen::BoundaryLayerTool::Perform() (boundarylayer.cpp:1523)
==409999==    by 0x9F6824F: netgen::GenerateBoundaryLayer(netgen::Mesh&, netgen::BoundaryLayerParameters const&) (boundarylayer.cpp:1552)
==409999==    by 0xA33094B: UnknownInlinedFun (python_mesh.cpp:1502)
==409999==    by 0xA33094B: void pybind11::detail::argument_loader...(cast.h:1631)
==409999==    by 0xA098C90: UnknownInlinedFun (cast.h:1605)
==409999==    by 0xA098C90: UnknownInlinedFun (pybind11.h:279)
==409999==    by 0xA098C90: pybind11::cpp_function::initialize<ExportNetgenMeshing(pybind11::module_&)::...(pybind11.h:249)
==409999==    by 0x9F39704: pybind11::cpp_function::dispatcher(_object*, _object*, _object*) (pybind11.h:971)
==409999==    by 0x4A3346C: ??? (in /usr/lib64/libpython3.11.so.1.0)
==409999==    by 0x4A1790E: _PyObject_MakeTpCall (in /usr/lib64/libpython3.11.so.1.0)
==409999==    by 0x4A20272: _PyEval_EvalFrameDefault (in /usr/lib64/libpython3.11.so.1.0)
==409999==    by 0x4A1C69E: ??? (in /usr/lib64/libpython3.11.so.1.0)
==409999==    by 0x4A4492A: ??? (in /usr/lib64/libpython3.11.so.1.0)
==409999==  Address 0x6f9c9e0 is 512 bytes inside a block of size 1,800 free'd
==409999==    at 0x484C579: operator delete[](void*, unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==409999==    by 0x9F5E0BA: UnknownInlinedFun (array.hpp:1136)
==409999==    by 0x9F5E0BA: UnknownInlinedFun (array.hpp:883)
==409999==    by 0x9F5E0BA: netgen::Mesh::AddFaceDescriptor(netgen::FaceDescriptor const&) (meshclass.hpp:699)
==409999==    by 0x9F60506: netgen::BoundaryLayerTool::CreateFaceDescriptorsSides() (boundarylayer.cpp:723)
==409999==    by 0x9F6337C: netgen::BoundaryLayerTool::Perform() (boundarylayer.cpp:1523)
==409999==    by 0x9F6824F: netgen::GenerateBoundaryLayer(netgen::Mesh&, netgen::BoundaryLayerParameters const&) (boundarylayer.cpp:1552)
==409999==    by 0xA33094B: UnknownInlinedFun (python_mesh.cpp:1502)
==409999==    by 0xA33094B: void pybind11::detail::argument_loader<netgen::Mesh&, std::variant... (cast.h:1631)
==409999==    by 0xA098C90: UnknownInlinedFun (cast.h:1605)
==409999==    by 0xA098C90: UnknownInlinedFun (pybind11.h:279)
==409999==    by 0xA098C90: pybind11::cpp_function::initialize<ExportNetgenMeshing(pybind11::module_&)::...(pybind11.h:249)
==409999==    by 0x9F39704: pybind11::cpp_function::dispatcher(_object*, _object*, _object*) (pybind11.h:971)
==409999==    by 0x4A3346C: ??? (in /usr/lib64/libpython3.11.so.1.0)
==409999==    by 0x4A1790E: _PyObject_MakeTpCall (in /usr/lib64/libpython3.11.so.1.0)
==409999==    by 0x4A20272: _PyEval_EvalFrameDefault (in /usr/lib64/libpython3.11.so.1.0)
==409999==    by 0x4A1C69E: ??? (in /usr/lib64/libpython3.11.so.1.0)
==409999==  Block was alloc'd at
==409999==    at 0x484851F: operator new[](unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==409999==    by 0x9F5DF2E: UnknownInlinedFun (array.hpp:1120)
==409999==    by 0x9F5DF2E: UnknownInlinedFun (array.hpp:883)
==409999==    by 0x9F5DF2E: netgen::Mesh::AddFaceDescriptor(netgen::FaceDescriptor const&) (meshclass.hpp:699)
==409999==    by 0x9F60124: netgen::BoundaryLayerTool::CreateNewFaceDescriptors() (boundarylayer.cpp:673)
==409999==    by 0x9F6336C: netgen::BoundaryLayerTool::Perform() (boundarylayer.cpp:1521)
==409999==    by 0x9F6824F: netgen::GenerateBoundaryLayer(netgen::Mesh&, netgen::BoundaryLayerParameters const&) (boundarylayer.cpp:1552)

As can be seen the problematic memory block is from Array<FaceDecoding> Mesh::facedecoding, which has been grown (reallocated) by Mesh::AddFaceDescriptor. The FaceDescriptor::bcname is a member, thus growing the facedecoding array invalidates any pointers to the bcname.

Unfortunately, Mesh::operator=(const Mesh& mesh2) ignores this, and assigns the raw pointer &facedecoding[fi].bcname to bcnames:

netgen/libsrc/meshing/meshclass.cpp

Lines 324 to 349 in 3bfa6c1

	// Remap string* values to new mesh
	std::map<const string*, string*> names_map;
	for (auto fi : Range(facedecoding))
	names_map[&mesh2.facedecoding[fi].bcname] = &facedecoding[fi].bcname;
	auto get_name = [&](const string *old_name) -> string* {
	if (!old_name) return nullptr;
	if (names_map.count(old_name)) return names_map[old_name];
	return new string(*old_name);
	};
	materials.SetSize( mesh2.materials.Size() );
	for ( int i = 0; i < mesh2.materials.Size(); i++ )
	{
	const string * old_name = mesh2.materials[i];
	if ( old_name ) materials[i] = dimension == 2 ? get_name(old_name) : new string ( *old_name );
	else materials[i] = 0;
	}
	bcnames.SetSize( mesh2.bcnames.Size() );
	for ( int i = 0; i < mesh2.bcnames.Size(); i++ )
	{
	const string * old_name = mesh2.bcnames[i];
	if ( old_name ) bcnames[i] = dimension == 3 ? get_name(old_name) : new string ( *old_name );
	else bcnames[i] = 0;
	}

[StefanBruens]

Analysis for this case is not fully correct, the problem found by valgrind comes from some invalid code in BoundaryLayerTool::CreateFaceDescriptorSides():

netgen/libsrc/meshing/boundarylayer.cpp

Lines 716 to 728 in 975414c

	const auto& fd = mesh.GetFaceDescriptor(facei);
	// auto isIn = domains.Test(fd.DomainIn());
	// auto isOut = domains.Test(fd.DomainOut());
	int si = params.sides_keep_surfaceindex ? facei : -1;
	// domin and domout can only be set later
	FaceDescriptor new_fd(si, -1,
	-1, si);
	new_fd.SetBCProperty(new_si);
	mesh.AddFaceDescriptor(new_fd);
	si_map[facei] = new_si;
	mesh.SetBCName(new_si-1, fd.GetBCName());
	face_done.SetBit(facei);
	}

mesh.AddFaceDescriptor(new_fd); may reallocate and thus invalidate fd. fd has to be refetched.