glim
glim copied to clipboard
koide3
Misc memory error reported by valgrind
When running the offline viewer with valgrind, I get several invalid memory read. For example:
$ ros2 run --prefix="valgrind " glim_ros offline_viewer /tmp/dump/
==351105== Memcheck, a memory error detector
==351105== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==351105== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==351105== Command: /glim_ws/install/glim_ros/lib/glim_ros/offline_viewer /tmp/dump/
==351105==
[2025-03-11 02:47:45.258] [glim] [info] map_path=/tmp/dump/
[2025-03-11 02:47:45.287] [glim] [info] config_path: /tmp/dump//config
[2025-03-11 02:47:52.099] [viewer] [info] Starting interactive viewer
[2025-03-11 02:47:52.419] [viewer] [info] Use config from /tmp/dump//config
[2025-03-11 02:47:52.431] [viewer] [info] Export classes from libgeoref_global.so
[2025-03-11 02:47:53.244] [viewer] [info] Export classes from libcamera_aggregator.so
[2025-03-11 02:48:01.568] [viewer] [info] enable_optimization=false
==351105== Warning: noted but unhandled ioctl 0x30000001 with no size/direction hints.
==351105== This could cause spurious value errors to appear.
==351105== See README_MISSING_SYSCALL_OR_IOCTL for guidance on writing a proper wrapper.
==351105== Warning: noted but unhandled ioctl 0x4b with no size/direction hints.
==351105== This could cause spurious value errors to appear.
==351105== See README_MISSING_SYSCALL_OR_IOCTL for guidance on writing a proper wrapper.
==351105== Warning: noted but unhandled ioctl 0x27 with no size/direction hints.
==351105== This could cause spurious value errors to appear.
==351105== See README_MISSING_SYSCALL_OR_IOCTL for guidance on writing a proper wrapper.
==351105== Warning: noted but unhandled ioctl 0x25 with no size/direction hints.
==351105== This could cause spurious value errors to appear.
==351105== See README_MISSING_SYSCALL_OR_IOCTL for guidance on writing a proper wrapper.
==351105== Warning: noted but unhandled ioctl 0x37 with no size/direction hints.
==351105== This could cause spurious value errors to appear.
==351105== See README_MISSING_SYSCALL_OR_IOCTL for guidance on writing a proper wrapper.
==351105== Warning: noted but unhandled ioctl 0x17 with no size/direction hints.
==351105== This could cause spurious value errors to appear.
==351105== See README_MISSING_SYSCALL_OR_IOCTL for guidance on writing a proper wrapper.
==351105== Warning: set address range perms: large range [0x200000000, 0x300200000) (noaccess)
==351105== Warning: noted but unhandled ioctl 0x19 with no size/direction hints.
==351105== This could cause spurious value errors to appear.
==351105== See README_MISSING_SYSCALL_OR_IOCTL for guidance on writing a proper wrapper.
==351105== Warning: set address range perms: large range [0x10006000000, 0x10106000000) (noaccess)
==351105== Warning: noted but unhandled ioctl 0x49 with no size/direction hints.
==351105== This could cause spurious value errors to appear.
==351105== See README_MISSING_SYSCALL_OR_IOCTL for guidance on writing a proper wrapper.
==351105== Warning: noted but unhandled ioctl 0x21 with no size/direction hints.
==351105== This could cause spurious value errors to appear.
==351105== See README_MISSING_SYSCALL_OR_IOCTL for guidance on writing a proper wrapper.
==351105== Warning: noted but unhandled ioctl 0x1b with no size/direction hints.
==351105== This could cause spurious value errors to appear.
==351105== See README_MISSING_SYSCALL_OR_IOCTL for guidance on writing a proper wrapper.
[2025-03-11 02:48:14.075] [global] [info] Load submaps
==351105== Thread 20:
==351105== Syscall param read(buf) points to unaddressable byte(s)
==351105== at 0x53EE81C: __libc_read (read.c:26)
==351105== by 0x53EE81C: read (read.c:24)
==351105== by 0x515EC8E: std::__basic_file<char>::xsgetn(char*, long) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.30)
==351105== by 0x51A2670: std::basic_filebuf<char, std::char_traits<char> >::xsgetn(char*, long) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.30)
==351105== by 0x51AF8F0: std::istream::read(char*, long) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.30)
==351105== by 0x56B29F5: gtsam_points::PointCloudCPU::load(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (point_cloud_cpu.cpp:267)
==351105== by 0x4E6905E: glim::SubMap::load(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (sub_map.cpp:166)
==351105== by 0x4EA0E94: glim::GlobalMapping::load(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (global_mapping.cpp:736)
==351105== by 0x48F14C4: glim::OfflineViewer::load_map(guik::ProgressInterface&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::shared_ptr<glim::GlobalMapping>) (offline_viewer.cpp:196)
==351105== by 0x48F19DD: operator() (offline_viewer.cpp:130)
==351105== by 0x48F19DD: __invoke_impl<std::shared_ptr<glim::GlobalMapping>, glim::OfflineViewer::main_menu()::<lambda(guik::ProgressInterface&)>&, guik::ProgressInterface&> (invoke.h:61)
==351105== by 0x48F19DD: __invoke_r<std::shared_ptr<glim::GlobalMapping>, glim::OfflineViewer::main_menu()::<lambda(guik::ProgressInterface&)>&, guik::ProgressInterface&> (invoke.h:116)
==351105== by 0x48F19DD: std::_Function_handler<std::shared_ptr<glim::GlobalMapping> (guik::ProgressInterface&), glim::OfflineViewer::main_menu()::{lambda(guik::ProgressInterface&)#1}>::_M_invoke(std::_Any_data const&, guik::ProgressInterface&) (std_function.h:291)
==351105== by 0x48F3E9B: operator() (std_function.h:590)
==351105== by 0x48F3E9B: operator() (progress_modal.hpp:61)
==351105== by 0x48F3E9B: __invoke_impl<void, guik::ProgressModal::open<std::shared_ptr<glim::GlobalMapping> >(const string&, const std::function<std::shared_ptr<glim::GlobalMapping>(guik::ProgressInterface&)>&)::<lambda()> > (invoke.h:61)
==351105== by 0x48F3E9B: __invoke<guik::ProgressModal::open<std::shared_ptr<glim::GlobalMapping> >(const string&, const std::function<std::shared_ptr<glim::GlobalMapping>(guik::ProgressInterface&)>&)::<lambda()> > (invoke.h:96)
==351105== by 0x48F3E9B: _M_invoke<0> (std_thread.h:259)
==351105== by 0x48F3E9B: operator() (std_thread.h:266)
==351105== by 0x48F3E9B: std::thread::_State_impl<std::thread::_Invoker<std::tuple<guik::ProgressModal::open<std::shared_ptr<glim::GlobalMapping> >(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::function<std::shared_ptr<glim::GlobalMapping> (guik::ProgressInterface&)> const&)::{lambda()#1}> > >::_M_run() (std_thread.h:211)
==351105== by 0x516A252: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.30)
==351105== by 0x536EAC2: start_thread (pthread_create.c:442)
==351105== Address 0x2880c060 is 0 bytes after a block of size 200,000 alloc'd
==351105== at 0x4849013: operator new(unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==351105== by 0x56B2910: allocate (new_allocator.h:127)
==351105== by 0x56B2910: allocate (alloc_traits.h:464)
==351105== by 0x56B2910: _M_allocate (stl_vector.h:346)
==351105== by 0x56B2910: _M_create_storage (stl_vector.h:361)
==351105== by 0x56B2910: _Vector_base (stl_vector.h:305)
==351105== by 0x56B2910: vector (stl_vector.h:511)
==351105== by 0x56B2910: gtsam_points::PointCloudCPU::load(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (point_cloud_cpu.cpp:264)
==351105== by 0x4E6905E: glim::SubMap::load(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (sub_map.cpp:166)
==351105== by 0x4EA0E94: glim::GlobalMapping::load(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (global_mapping.cpp:736)
==351105== by 0x48F14C4: glim::OfflineViewer::load_map(guik::ProgressInterface&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::shared_ptr<glim::GlobalMapping>) (offline_viewer.cpp:196)
==351105== by 0x48F19DD: operator() (offline_viewer.cpp:130)
==351105== by 0x48F19DD: __invoke_impl<std::shared_ptr<glim::GlobalMapping>, glim::OfflineViewer::main_menu()::<lambda(guik::ProgressInterface&)>&, guik::ProgressInterface&> (invoke.h:61)
==351105== by 0x48F19DD: __invoke_r<std::shared_ptr<glim::GlobalMapping>, glim::OfflineViewer::main_menu()::<lambda(guik::ProgressInterface&)>&, guik::ProgressInterface&> (invoke.h:116)
==351105== by 0x48F19DD: std::_Function_handler<std::shared_ptr<glim::GlobalMapping> (guik::ProgressInterface&), glim::OfflineViewer::main_menu()::{lambda(guik::ProgressInterface&)#1}>::_M_invoke(std::_Any_data const&, guik::ProgressInterface&) (std_function.h:291)
==351105== by 0x48F3E9B: operator() (std_function.h:590)
==351105== by 0x48F3E9B: operator() (progress_modal.hpp:61)
==351105== by 0x48F3E9B: __invoke_impl<void, guik::ProgressModal::open<std::shared_ptr<glim::GlobalMapping> >(const string&, const std::function<std::shared_ptr<glim::GlobalMapping>(guik::ProgressInterface&)>&)::<lambda()> > (invoke.h:61)
==351105== by 0x48F3E9B: __invoke<guik::ProgressModal::open<std::shared_ptr<glim::GlobalMapping> >(const string&, const std::function<std::shared_ptr<glim::GlobalMapping>(guik::ProgressInterface&)>&)::<lambda()> > (invoke.h:96)
==351105== by 0x48F3E9B: _M_invoke<0> (std_thread.h:259)
==351105== by 0x48F3E9B: operator() (std_thread.h:266)
==351105== by 0x48F3E9B: std::thread::_State_impl<std::thread::_Invoker<std::tuple<guik::ProgressModal::open<std::shared_ptr<glim::GlobalMapping> >(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::function<std::shared_ptr<glim::GlobalMapping> (guik::ProgressInterface&)> const&)::{lambda()#1}> > >::_M_run() (std_thread.h:211)
==351105== by 0x516A252: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.30)
==351105== by 0x536EAC2: start_thread (pthread_create.c:442)
==351105== by 0x53FFA03: clone (clone.S:100)
==351105==
==351105== Warning: set address range perms: large range [0x302000000, 0x6f8000000) (noaccess)
==351105== Thread 2:
==351105== Invalid read of size 8
==351105== at 0x4E16E91: glim::TrajectoryManager::update_anchor(double, Eigen::Transform<double, 3, 1, 0> const&) (trajectory_manager.cpp:32)
==351105== by 0x48AF56F: operator() (interactive_viewer.cpp:487)
==351105== by 0x48AF56F: __invoke_impl<void, glim::InteractiveViewer::globalmap_on_insert_submap(const ConstPtr&)::<lambda()>&> (invoke.h:61)
==351105== by 0x48AF56F: __invoke_r<void, glim::InteractiveViewer::globalmap_on_insert_submap(const ConstPtr&)::<lambda()>&> (invoke.h:111)
==351105== by 0x48AF56F: std::_Function_handler<void (), glim::InteractiveViewer::globalmap_on_insert_submap(std::shared_ptr<glim::SubMap const> const&)::{lambda()#1}>::_M_invoke(std::_Any_data const&) (std_function.h:290)
==351105== by 0x48AA300: operator() (std_function.h:590)
==351105== by 0x48AA300: glim::InteractiveViewer::viewer_loop() (interactive_viewer.cpp:219)
==351105== by 0x516A252: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.30)
==351105== by 0x536EAC2: start_thread (pthread_create.c:442)
==351105== by 0x53FFA03: clone (clone.S:100)
==351105== Address 0x12d4fed8 is 0 bytes after a block of size 8 alloc'd
==351105== at 0x4849013: operator new(unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==351105== by 0x4D54AE2: allocate (new_allocator.h:127)
==351105== by 0x4D54AE2: allocate (alloc_traits.h:464)
==351105== by 0x4D54AE2: _M_allocate (stl_vector.h:346)
==351105== by 0x4D54AE2: void std::vector<double, std::allocator<double> >::_M_realloc_insert<double>(__gnu_cxx::__normal_iterator<double*, std::vector<double, std::allocator<double> > >, double&&) (vector.tcc:440)
==351105== by 0x4E16A0A: emplace_back<double> (vector.tcc:121)
==351105== by 0x4E16A0A: push_back (stl_vector.h:1204)
==351105== by 0x4E16A0A: glim::TrajectoryManager::TrajectoryManager() (trajectory_manager.cpp:7)
==351105== by 0x48B0A9D: glim::InteractiveViewer::InteractiveViewer() (interactive_viewer.cpp:124)
==351105== by 0x48ED5D2: glim::OfflineViewer::OfflineViewer(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (offline_viewer.cpp:21)
==351105== by 0x10D8BC: main (offline_viewer.cpp:27)
==351105==
[2025-03-11 02:57:07.936] [global] [info] deserializing factor graph
[2025-03-11 02:57:08.059] [global] [info] deserializing values
[2025-03-11 02:57:08.089] [global] [info] creating matching cost factors
[2025-03-11 02:57:10.722] [global] [info] optimize
[2025-03-11 02:57:10.827] [viewer] [info] --- smoother_updated ---
count new lin elim gpu_e gpu_l delta time_msec
0 0 0 0 0 0 0 0
[2025-03-11 02:57:10.844] [glim] [info] Found 1101 camera images
[2025-03-11 02:57:11.018] [global] [info] done
The first major issue is Syscall param read(buf) points to unaddressable byte(s) at:
https://github.com/koide3/gtsam_points/blob/7e5b5e188b4fcce482e09cbcf2bd16d4f4bad09b/src/gtsam_points/types/point_cloud_cpu.cpp#L257
std::vector<float> intensities_f(frame->size());
std::ifstream ifs(path + "/intensities_compact.bin", std::ios::binary);
ifs.read(reinterpret_cast<char*>(intensities_f.data()), sizeof(Eigen::Vector4f) * frame->size()); // <-- here
std::copy(intensities_f.begin(), intensities_f.end(), frame->intensities);
intensities_f is a vector of N * float, but N * Eigen::Vector4f are copied into it.
Obviously, sizeof(Eigen::Vector4f) should but sizeof(float).
The second one is Invalid read of size 8 at:
https://github.com/koide3/glim/blob/1fabd5ff2d5aaa4bde0fdfde8e14576c3a50b41c/src/glim/util/trajectory_manager.cpp#L32
const auto found = std::lower_bound(odom_stamps.begin(), odom_stamps.end(), stamp);
const int idx = std::distance(odom_stamps.begin(), found);
if (std::abs(stamp - odom_stamps[idx]) < 1e-6 || idx == 0) { // <--- Here
T_world_odom = T_world_sensor * T_odom_sensor[idx].inverse();
} else {
It seems that snippet assumes the input stamp is found in odom_stamps. But if it is not the case, then std::lower_bound() returns odom_stamps.end(), and odom_stamps[idx] is outside odom_stamps. I guess this is what is reported by valgrind here.
Regarding the odom_stamps issue. The problems is due to the fact trajectory is never initialized properly when the offline viewer is used (odom_stamps = {0}).
In the interactive viewer, trajectory poses are added in InteractiveViewer::odometry_on_new_frame and submap anchor is updated in InteractiveViewer::globalmap_on_insert_submap. When the map is loaded in the offline viewer, the InteractiveViewer::globalmap_on_insert_submap function is called, but InteractiveViewer::odometry_on_new_frame is never called, leaving trajectory->odom_stamps == {0}.
I am not sure what is the proper workaround, but this early return solves the memory issue:
const auto found = std::lower_bound(odom_stamps.begin(), odom_stamps.end(), stamp);
// workaround for offline viewer
if (found == odom_stamps.end()) {
if (!(odom_stamps.size() == 1 && odom_stamps.front() == 0)) {
// trajectory initialized yet anchor stamp does not exist
spdlog::error("anchor stamp not found in trajectory: {}", stamp);
}
return;
}
Thanks for the detailed report and the PR for fixing the intensity read! I'll check it next week and find a way to fix the odom_stamps issue.
