Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

valgrind error in testAHRSFactor #55

Closed
jlblancoc opened this issue Jun 10, 2019 · 13 comments
Closed

valgrind error in testAHRSFactor #55

jlblancoc opened this issue Jun 10, 2019 · 13 comments
Milestone
GTSAM 4.1

Comments

@jlblancoc
Copy link
Member

jlblancoc commented Jun 10, 2019 •
edited
Loading

Just wanted to report it here for giving it a closer look. Perhaps it's a false positive.

Reproduce with: make testAHRSFactor.run.valgrind

Please, refer to the attached valgrind log.

log.txt
@dellaert
Copy link
Member

It seems limited to one test, but I can’t see what’s wrong with it.

@ProfFan ProfFan added this to the GTSAM 4.1 milestone Sep 23, 2019
@ProfFan
Copy link
Collaborator

ProfFan commented Sep 23, 2019

Will take a look at this.

@ProfFan
Copy link
Collaborator

ProfFan commented Sep 23, 2019

#121

@ProfFan
Copy link
Collaborator

ProfFan commented Dec 31, 2019

I did not get any errors with AddressSanitizer, @jlblancoc could you run this again on develop?

@jlblancoc
Copy link
Member Author

jlblancoc commented Jan 7, 2020 via email

@jlblancoc
Copy link
Member Author

Valgrind keeps complaining for the current "develop".
It looks like "theta" inside SO3 is uninitialized.

@ProfFan
Copy link
Collaborator

ProfFan commented Jan 12, 2020

Can you post a log?

@dellaert
Copy link
Member

Looking at the earlier posted log, it seems it seems to involve DexpFunctor and ExpmapFunctor. But, both ExpmapFunctor constructors initialize theta just fine... Also, I checked the order of initialization (see https://en.cppreference.com/w/cpp/language/initializer_list) and as I read it, theta2 is always initialized before theta, as intended. Puzzled...

@ProfFan
Copy link
Collaborator

ProfFan commented Jan 12, 2020

I tested in Debug, which does not exhibit the error. Could be a problem with the optimization eliminating some "Dead" code.

@jlblancoc
Copy link
Member Author

Hi,

Yep, I also tested and in a Debug build, valgrind is happy.
I'm sure the problem is not in ExpmapFunctor's constructors, but in some vector/matrix that is somehow left uninitialized (at least one coefficient), anywhere up the callstack:

==9141== Conditional jump or move depends on uninitialised value(s)
==9141==    at 0x4FB4DA9: gtsam::so3::ExpmapFunctor::ExpmapFunctor(Eigen::Matrix<double, 3, 1, 0, 3, 1> const&, bool) (in /home/jlblanco/code/gtsam-jlblancoc/build/gtsam/libgtsam.so.4.0.0)
==9141==    by 0x4FB54C3: gtsam::so3::DexpFunctor::DexpFunctor(Eigen::Matrix<double, 3, 1, 0, 3, 1> const&, bool) (in /home/jlblanco/code/gtsam-jlblancoc/build/gtsam/libgtsam.so.4.0.0)
==9141==    by 0x4FB6547: gtsam::SO3::ExpmapDerivative(Eigen::Matrix<double, 3, 1, 0, 3, 1> const&) (in /home/jlblanco/code/gtsam-jlblancoc/build/gtsam/libgtsam.so.4.0.0)
==9141==    by 0x4FB177C: gtsam::Rot3::ExpmapDerivative(Eigen::Matrix<double, 3, 1, 0, 3, 1> const&) (in /home/jlblanco/code/gtsam-jlblancoc/build/gtsam/libgtsam.so.4.0.0)
==9141==    by 0x5172524: gtsam::PreintegratedRotation::incrementalRotation(Eigen::Matrix<double, 3, 1, 0, 3, 1> const&, Eigen::Matrix<double, 3, 1, 0, 3, 1> const&, double, gtsam::OptionalJacobian<3, 3>) const (in /home/jlblanco/code/gtsam-jlblancoc/build/gtsam/libgtsam.so.4.0.0)
==9141==    by 0x517271A: gtsam::PreintegratedRotation::integrateMeasurement(Eigen::Matrix<double, 3, 1, 0, 3, 1> const&, Eigen::Matrix<double, 3, 1, 0, 3, 1> const&, double, gtsam::OptionalJacobian<3, 3>, gtsam::OptionalJacobian<3, 3>) (in /home/jlblanco/code/gtsam-jlblancoc/build/gtsam/libgtsam.so.4.0.0)
==9141==    by 0x51537B1: gtsam::PreintegratedAhrsMeasurements::integrateMeasurement(Eigen::Matrix<double, 3, 1, 0, 3, 1> const&, double) (in /home/jlblanco/code/gtsam-jlblancoc/build/gtsam/libgtsam.so.4.0.0)
==9141==    by 0x123721: AHRSFactorFirstOrderPreIntegratedMeasurementsTest::run(TestResult&) (in /home/jlblanco/code/gtsam-jlblancoc/build/gtsam/navigation/tests/testAHRSFactor)
==9141==    by 0x138561: TestRegistry::run(TestResult&) (in /home/jlblanco/code/gtsam-jlblancoc/build/gtsam/navigation/tests/testAHRSFactor)

@ProfFan
Copy link
Collaborator

ProfFan commented Jan 13, 2020 

[1/1] cd /home/fan/Projects/CV/SLAM/gtsam_build/gtsam/navigation/tests && /home/fan/Projects/CV/SLAM/gtsam_build/gtsam/navigation/tests/testAHRSFactor
FAILED: gtsam/navigation/tests/CMakeFiles/testAHRSFactor.run 
cd /home/fan/Projects/CV/SLAM/gtsam_build/gtsam/navigation/tests && /home/fan/Projects/CV/SLAM/gtsam_build/gtsam/navigation/tests/testAHRSFactor
==143729==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x559177864888 in (anonymous namespace)::evaluatePreintegratedMeasurements(Eigen::Matrix<double, 3, 1, 0, 3, 1> const&, std::__cxx11::list<Eigen::Matrix<double, 3, 1, 0, 3, 1>, std::allocator<Eigen::Matrix<double, 3, 1, 0, 3, 1> > > const&, std::__cxx11::list<double, std::allocator<double> > const&, Eigen::Matrix<double, 3, 1, 0, 3, 1> const&) /home/fan/Projects/CV/SLAM/gtsam/gtsam/navigation/tests/testAHRSFactor.cpp:64:3
    #1 0x559177862fad in AHRSFactorFirstOrderPreIntegratedMeasurementsTest::run(TestResult&) /home/fan/Projects/CV/SLAM/gtsam/gtsam/navigation/tests/testAHRSFactor.cpp:363:7
    #2 0x559177bd2f9c in TestRegistry::run(TestResult&) /home/fan/Projects/CV/SLAM/gtsam/CppUnitLite/TestRegistry.cpp:62:15
    #3 0x559177bd22ad in TestRegistry::runAllTests(TestResult&) /home/fan/Projects/CV/SLAM/gtsam/CppUnitLite/TestRegistry.cpp:29:22
    #4 0x559177872679 in main /home/fan/Projects/CV/SLAM/gtsam/gtsam/navigation/tests/testAHRSFactor.cpp:510:10
    #5 0x7f3a2d37e152 in __libc_start_main (/usr/lib/libc.so.6+0x27152)
    #6 0x5591777c4acd in _start (/home/fan/Projects/CV/SLAM/gtsam_build/gtsam/navigation/tests/testAHRSFactor+0xe4acd)

  Uninitialized value was stored to memory at
    #0 0x55917793f5c6 in std::_List_const_iterator<Eigen::Matrix<double, 3, 1, 0, 3, 1> >::operator++() /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/9.2.0/../../../../include/c++/9.2.0/bits/stl_list.h:303:10
    #1 0x559177864a2d in (anonymous namespace)::evaluatePreintegratedMeasurements(Eigen::Matrix<double, 3, 1, 0, 3, 1> const&, std::__cxx11::list<Eigen::Matrix<double, 3, 1, 0, 3, 1>, std::allocator<Eigen::Matrix<double, 3, 1, 0, 3, 1> > > const&, std::__cxx11::list<double, std::allocator<double> > const&, Eigen::Matrix<double, 3, 1, 0, 3, 1> const&) /home/fan/Projects/CV/SLAM/gtsam/gtsam/navigation/tests/testAHRSFactor.cpp:64:43
    #2 0x559177862fad in AHRSFactorFirstOrderPreIntegratedMeasurementsTest::run(TestResult&) /home/fan/Projects/CV/SLAM/gtsam/gtsam/navigation/tests/testAHRSFactor.cpp:363:7
    #3 0x559177bd2f9c in TestRegistry::run(TestResult&) /home/fan/Projects/CV/SLAM/gtsam/CppUnitLite/TestRegistry.cpp:62:15
    #4 0x559177bd22ad in TestRegistry::runAllTests(TestResult&) /home/fan/Projects/CV/SLAM/gtsam/CppUnitLite/TestRegistry.cpp:29:22
    #5 0x559177872679 in main /home/fan/Projects/CV/SLAM/gtsam/gtsam/navigation/tests/testAHRSFactor.cpp:510:10
    #6 0x7f3a2d37e152 in __libc_start_main (/usr/lib/libc.so.6+0x27152)

  Uninitialized value was created by a heap allocation
    #0 0x55917784191b in operator new(unsigned long) (/home/fan/Projects/CV/SLAM/gtsam_build/gtsam/navigation/tests/testAHRSFactor+0x16191b)
    #1 0x559177b6be19 in __gnu_cxx::new_allocator<std::_List_node<Eigen::Matrix<double, 3, 1, 0, 3, 1> > >::allocate(unsigned long, void const*) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/9.2.0/../../../../include/c++/9.2.0/ext/new_allocator.h:114:27
    #2 0x559177b6b9e5 in std::allocator_traits<std::allocator<std::_List_node<Eigen::Matrix<double, 3, 1, 0, 3, 1> > > >::allocate(std::allocator<std::_List_node<Eigen::Matrix<double, 3, 1, 0, 3, 1> > >&, unsigned long) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/9.2.0/../../../../include/c++/9.2.0/bits/alloc_traits.h:444:20
    #3 0x559177b6a7bd in std::__cxx11::_List_base<Eigen::Matrix<double, 3, 1, 0, 3, 1>, std::allocator<Eigen::Matrix<double, 3, 1, 0, 3, 1> > >::_M_get_node() /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/9.2.0/../../../../include/c++/9.2.0/bits/stl_list.h:438:16
    #4 0x559177b69d54 in std::_List_node<Eigen::Matrix<double, 3, 1, 0, 3, 1> >* std::__cxx11::list<Eigen::Matrix<double, 3, 1, 0, 3, 1>, std::allocator<Eigen::Matrix<double, 3, 1, 0, 3, 1> > >::_M_create_node<Eigen::Matrix<double, 3, 1, 0, 3, 1> >(Eigen::Matrix<double, 3, 1, 0, 3, 1>&&) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/9.2.0/../../../../include/c++/9.2.0/bits/stl_list.h:630:21
    #5 0x559177b69492 in void std::__cxx11::list<Eigen::Matrix<double, 3, 1, 0, 3, 1>, std::allocator<Eigen::Matrix<double, 3, 1, 0, 3, 1> > >::_M_insert<Eigen::Matrix<double, 3, 1, 0, 3, 1> >(std::_List_iterator<Eigen::Matrix<double, 3, 1, 0, 3, 1> >, Eigen::Matrix<double, 3, 1, 0, 3, 1>&&) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/9.2.0/../../../../include/c++/9.2.0/bits/stl_list.h:1907:18
    #6 0x5591778a76b2 in std::__cxx11::list<Eigen::Matrix<double, 3, 1, 0, 3, 1>, std::allocator<Eigen::Matrix<double, 3, 1, 0, 3, 1> > >::push_back(Eigen::Matrix<double, 3, 1, 0, 3, 1>&&) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/9.2.0/../../../../include/c++/9.2.0/bits/stl_list.h:1213:15
    #7 0x5591778629ef in AHRSFactorFirstOrderPreIntegratedMeasurementsTest::run(TestResult&) /home/fan/Projects/CV/SLAM/gtsam/gtsam/navigation/tests/testAHRSFactor.cpp:351:18
    #8 0x559177bd2f9c in TestRegistry::run(TestResult&) /home/fan/Projects/CV/SLAM/gtsam/CppUnitLite/TestRegistry.cpp:62:15
    #9 0x559177bd22ad in TestRegistry::runAllTests(TestResult&) /home/fan/Projects/CV/SLAM/gtsam/CppUnitLite/TestRegistry.cpp:29:22
    #10 0x559177872679 in main /home/fan/Projects/CV/SLAM/gtsam/gtsam/navigation/tests/testAHRSFactor.cpp:510:10
    #11 0x7f3a2d37e152 in __libc_start_main (/usr/lib/libc.so.6+0x27152)

SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/fan/Projects/CV/SLAM/gtsam/gtsam/navigation/tests/testAHRSFactor.cpp:64:3 in (anonymous namespace)::evaluatePreintegratedMeasurements(Eigen::Matrix<double, 3, 1, 0, 3, 1> const&, std::__cxx11::list<Eigen::Matrix<double, 3, 1, 0, 3, 1>, std::allocator<Eigen::Matrix<double, 3, 1, 0, 3, 1> > > const&, std::__cxx11::list<double, std::allocator<double> > const&, Eigen::Matrix<double, 3, 1, 0, 3, 1> const&)
Exiting
ninja: build stopped: subcommand failed.

I think this is a common false positive as Eigen does not initialize the object from the default constructor, and std::list uses the copy constructor (correct me if not). I'll do some PoCs to confirm.

@ProfFan
Copy link
Collaborator

ProfFan commented Jul 11, 2020

I believe this is already fixed, if not please feel free to reopen :) @jlblancoc

@ProfFan ProfFan closed this as completed Jul 11, 2020
@jlblancoc
Copy link
Member Author

@ProfFan yes, I just tested with the current develop and the error is not there anymore... 👍

varunagrawal added a commit that referenced this issue Mar 25, 2021
@varunagrawal
Squashed 'wrap/' changes from 548e61b1f..29628426d
a62a986 
29628426d Merge pull request #59 from borglab/fixes
a95429ea0 Merge pull request #56 from borglab/fix/this-instantiation
3e22d6516 more documenatation and some formatting
526301499 updated the test to test for non-templated This
cdb75f36f Merge branch 'master' into fix/this-instantiation
0f5ae3b7f moved example pybind template to templates directory
d55f5db38 remove extra whitespace
21891ad3d skip tests until we figure out what's going on
2ea6307c3 better way of handling the matlab includes in the matlab wrapper
d0f8a392c Merge pull request #55 from borglab/feature/refactor3
57d47cbd9 create directories to store generated output
4788a1e37 fixed This instantiation
61d2cbfc4 add namespace test to matlab wrapper
ec39023e6 added more, smaller tests for Python wrapper
19c35b857 test for matlab class inheritance and some clean up
06ca5da13 test for matlab functions
cb05d7379 minor clean up and separate tests for geometry and class
8d8145cc4 break down test interface file into smaller files that can be easily debugged
97328f057 restructured test files and added dedicated fixtures directory

git-subtree-dir: wrap
git-subtree-split: 29628426d2c1a7bb728e40307c0f25cb468cd1bc
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
GTSAM 4.1
Development

No branches or pull requests
3 participants
@ProfFan @jlblancoc @dellaert