valgrind: get_sparse - move depends on uninitialised value #1442
Description
valgrind memory checker reports some memory error for some of the test cases. I cannot tell if this has some real world consequences, like handing out some bogus location data under very special conditions. At least flagging the issue seemed like a good option.
Apparently, node-ram-cache works with uninitialized memory in several places by using malloc. Let's ignore for a moment, if the underlying assumptions are still valid or might have changed during the last 10 years.
I would assume that the bounds checking in get_sparse only allows reads from those parts of the memory that have been written to beforehand. Somehow test cases trigger some corner cases where this doesn't hold true. That's at least how I read what valgrind is reporting below.
pg_virtualenv valgrind --leak-check=full --show-leak-kinds=all --track-origins=yes --verbose --log-file=valgrind-out.txt tests/test-output-flex-way-change
==27924== Conditional jump or move depends on uninitialised value(s)
==27924== at 0x227405: node_ram_cache::get_sparse(long) const (node-ram-cache.cpp:271)
==27924== by 0x227507: node_ram_cache::get(long) (node-ram-cache.cpp:447)
==27924== by 0x2177C7: middle_query_pgsql_t::get_way_node_locations_db(osmium::WayNodeList*) const (middle-pgsql.cpp:250)
==27924== by 0x24DAEF: output_flex_t::get_way_nodes() (output-flex.cpp:540)
==27924== by 0x24E326: output_flex_t::run_transform[abi:cxx11](geom::osmium_builder_t*, geom_transform_t const*, table_column_type, osmium::Way const&) (output-flex.cpp:994)
==27924== by 0x259E2A: void output_flex_t::add_row<osmium::Way>(table_connection_t*, osmium::Way const&) (output-flex.cpp:1053)
==27924== by 0x255EBE: output_flex_t::table_add_row() (output-flex.cpp:860)
==27924== by 0x255F58: lua_trampoline_table_add_row(lua_State*) (output-flex.cpp:69)
==27924== by 0x4C7BEC1: luaD_precall (ldo.c:319)
==27924== by 0x4C87D46: luaV_execute (lvm.c:709)
==27924== by 0x4C7C27F: luaD_call (ldo.c:402)
==27924== by 0x4C7B7D3: luaD_rawrunprotected (ldo.c:131)
==27924== Uninitialised value was created by a heap allocation
==27924== at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==27924== by 0x2279FF: node_ram_cache::node_ram_cache(int, int) (node-ram-cache.cpp:332)
==27924== by 0x218598: middle_pgsql_t::middle_pgsql_t(options_t const*) (middle-pgsql.cpp:778)
==27924== by 0x1EBBCA: testing::db::import_t::run_import(options_t, std::initializer_list<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (common-import.hpp:145)
==27924== by 0x1EC649: testing::db::import_t::run_import(options_t, char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (common-import.hpp:176)
==27924== by 0x1C5AD2: ____C_A_T_C_H____T_E_S_T____0() (test-output-flex-way-change.cpp:62)
==27924== by 0x2A49E6: invoke (catch.hpp:14100)
==27924== by 0x2A49E6: Catch::RunContext::invokeActiveTestCase() (catch.hpp:12959)
==27924== by 0x2B893F: Catch::RunContext::runCurrentTest(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) (catch.hpp:12932)
==27924== by 0x2CBADF: Catch::RunContext::runTest(Catch::TestCase const&) (catch.hpp:12693)
==27924== by 0x2CE763: execute (catch.hpp:13287)
==27924== by 0x2CE763: Catch::Session::runInternal() (catch.hpp:13493)
==27924== by 0x2CEB3E: Catch::Session::run() (catch.hpp:13449)
==27924== by 0x1BF2DF: run<char> (catch.hpp:13171)
==27924== by 0x1BF2DF: run<char> (catch.hpp:13166)
==27924== by 0x1BF2DF: main (catch.hpp:17448)