ASan error reported in the AmclNode constructor

[agalbachicar]

Bug description

I've run ASan reports via colcon sanitizer reports over beluga and beluga_amcl. You will be happy to hear that beluga is free of detectable address bugs but there was one caught in beluga_amcl. In particular, I think this is not a problem of AmclNode itself, but a problem in rclcpp that I've also seen in other distros using normal Nodes. The problem involves NodeParameters and other classes... It could be well very well a valid issue or a false positive detection. I've spent some time not so long ago debugging a similar problem in rcl and rclcpp with @ivanpauno, who may remember our interaction, which was also detected by ASan tests.

It could be a good thing to atomize the case to a do-nothing node in a simple test that reproduces the error to make sure it is not related to our code (which I believe it is not related).

Platform (please complete the following information):

• OS: docker, jammy, humble
• Beluga version: 3144527

How to reproduce

List steps to reproduce the issue:

1. Follow the steps here to install the mixins.
2. Compile beluga and beluga_amcl with ASan.

$ colcon build --build-base=build-asan --install-base=install-asan --cmake-args -DCMAKE_BUILD_TYPE=Debug --mixin asan-gcc --packages-up-to beluga beluga_amcl --symlink-install --event-handlers console_direct+

3. Run tests for each package:

$ colcon test --build-base=build-asan --install-base=install-asan --event-handlers sanitizer_report+ --packages-up-to beluga --event-handlers console_direct+

and

$ colcon test --build-base=build-asan --install-base=install-asan --event-handlers sanitizer_report+ --packages-select beluga_amcl --event-handlers console_direct+

The last one produces the following relevant output:

test 1                                                                                                   
    Start 1: test_amcl_node                                                                              
                                                                                                         
1: Test command: /usr/bin/python3.10 "-u" "/opt/ros/humble/share/ament_cmake_test/cmake/run_test.py" "/ws
/build-asan/beluga_amcl/test_results/beluga_amcl/test_amcl_node.gtest.xml" "--package-name" "beluga_amcl"
 "--output-file" "/ws/build-asan/beluga_amcl/ament_cmake_gmock/test_amcl_node.txt" "--command" "/ws/build
-asan/beluga_amcl/test/test_amcl_node" "--gtest_output=xml:/ws/build-asan/beluga_amcl/test_results/beluga
_amcl/test_amcl_node.gtest.xml"                                                                          
1: Test timeout computed to be: 60                                                                       
1: -- run_test.py: invoking following command in '/ws/build-asan/beluga_amcl/test':                      
1:  - /ws/build-asan/beluga_amcl/test/test_amcl_node --gtest_output=xml:/ws/build-asan/beluga_amcl/test_r
esults/beluga_amcl/test_amcl_node.gtest.xml                                                              
1: Running main() from gmock_main.cc                                                                     
1: [==========] Running 68 tests from 4 test suites.                                                     
1: [----------] Global test environment set-up.                                                          
1: [----------] 7 tests from TestLifecycle                                                               
1: [ RUN      ] TestLifecycle.FullSpin                                                                   
1: =================================================================                                     
1: ==9720==ERROR: AddressSanitizer: new-delete-type-mismatch on 0x607000012bd0 in thread T0:             
1:   object passed to delete has wrong type:                                                             
1:   size of the allocated type:   72 bytes; 
1:   size of the deallocated type: 1 bytes.                                                    [219/1805]
1:     #0 0x7f5e77d9c24f in operator delete(void*, unsigned long) ../../../../src/libsanitizer/asan/asan_
new_delete.cpp:172                                                                                       
1:     #1 0x55b19386995c in __gnu_cxx::new_allocator<char>::deallocate(char*, unsigned long) /usr/include
/c++/11/ext/new_allocator.h:145                                                                          
1:     #2 0x55b1938568d7 in std::allocator_traits<std::allocator<char> >::deallocate(std::allocator<char>
&, char*, unsigned long) /usr/include/c++/11/bits/alloc_traits.h:496                                     
1:     #3 0x55b19390a1c1 in void rclcpp::allocator::retyped_deallocate<char, std::allocator<char> >(void*
, void*) (/ws/build-asan/beluga_amcl/test/test_amcl_node+0x5351c1)                                       
1:     #4 0x7f5e74cc3dae in rcutils_string_map_fini (/opt/ros/humble/lib/librcutils.so+0x9dae)           
1:     #5 0x7f5e74cf4d05  (/opt/ros/humble/lib/librcl.so+0x22d05)                                        
1:     #6 0x7f5e74cf5027 in rcl_node_resolve_name (/opt/ros/humble/lib/librcl.so+0x23027)                
1:     #7 0x7f5e74cf51f8 in rcl_publisher_init (/opt/ros/humble/lib/librcl.so+0x231f8)                   
1:     #8 0x7f5e751b2a6d in rclcpp::PublisherBase::PublisherBase(rclcpp::node_interfaces::NodeBaseInterfa
ce*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, rosidl_messa
ge_type_support_t const&, rcl_publisher_options_s const&) (/opt/ros/humble/lib/librclcpp.so+0x158a6d)    
1:     #9 0x7f5e751773bb  (/opt/ros/humble/lib/librclcpp.so+0x11d3bb)                                    
1:     #10 0x7f5e75177af0 in std::_Function_handler<std::shared_ptr<rclcpp::PublisherBase> (rclcpp::node_
interfaces::NodeBaseInterface*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<c
har> > const&, rclcpp::QoS const&), rclcpp::create_publisher_factory<rcl_interfaces::msg::ParameterEvent_
<std::allocator<void> >, std::allocator<void>, rclcpp::Publisher<rcl_interfaces::msg::ParameterEvent_<std
::allocator<void> >, std::allocator<void> > >(rclcpp::PublisherOptionsWithAllocator<std::allocator<void> 
> const&)::{lambda(rclcpp::node_interfaces::NodeBaseInterface*, std::__cxx11::basic_string<char, std::cha
r_traits<char>, std::allocator<char> > const&, rclcpp::QoS const&)#1}>::_M_invoke(std::_Any_data const&, 
rclcpp::node_interfaces::NodeBaseInterface*&&, std::__cxx11::basic_string<char, std::char_traits<char>, s
td::allocator<char> > const&, rclcpp::QoS const&) (/opt/ros/humble/lib/librclcpp.so+0x11daf0)            
1:     #11 0x7f5e7516c404 in rclcpp::node_interfaces::NodeParameters::NodeParameters(std::shared_ptr<rclc
pp::node_interfaces::NodeBaseInterface>, std::shared_ptr<rclcpp::node_interfaces::NodeLoggingInterface>, 
std::shared_ptr<rclcpp::node_interfaces::NodeTopicsInterface>, std::shared_ptr<rclcpp::node_interfaces::N
odeServicesInterface>, std::shared_ptr<rclcpp::node_interfaces::NodeClockInterface>, std::vector<rclcpp::
Parameter, std::allocator<rclcpp::Parameter> > const&, bool, bool, rclcpp::QoS const&, rclcpp::PublisherO
ptionsBase const&, bool, bool) (/opt/ros/humble/lib/librclcpp.so+0x112404)                               
1:     #12 0x7f5e7527a761 in rclcpp_lifecycle::LifecycleNode::LifecycleNode(std::__cxx11::basic_string<ch
ar, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_tra
its<char>, std::allocator<char> > const&, rclcpp::NodeOptions const&, bool) (/opt/ros/humble/lib/librclcp
p_lifecycle.so+0x29761)                                                                                  
1:     #13 0x7f5e7742a769 in beluga_amcl::AmclNode::AmclNode(rclcpp::NodeOptions const&) /ws/src/beluga/b
eluga_amcl/src/amcl_node.cpp:56                                                                          
1:     #14 0x55b19381dd8f in AmclNodeUnderTest /ws/src/beluga/beluga_amcl/test/test_amcl_node.cpp:73     
1:     #15 0x55b19381dfbd in construct<(anonymous namespace)::AmclNodeUnderTest> /usr/include/c++/11/ext/
new_allocator.h:162                                                                                      
1:     #16 0x55b19381d95a in construct<(anonymous namespace)::AmclNodeUnderTest> /usr/include/c++/11/bits
/alloc_traits.h:516                                                                                      
1:     #17 0x55b19381d2d6 in _Sp_counted_ptr_inplace<> /usr/include/c++/11/bits/shared_ptr_base.h:519    
1:     #18 0x55b19381ca1c in __shared_count<(anonymous namespace)::AmclNodeUnderTest, std::allocator<(ano
nymous namespace)::AmclNodeUnderTest> > /usr/include/c++/11/bits/shared_ptr_base.h:650                   
1:     #19 0x55b19381c27c in __shared_ptr<std::allocator<(anonymous namespace)::AmclNodeUnderTest> > /usr
/include/c++/11/bits/shared_ptr_base.h:1342                                                              
1:     #20 0x55b19381bd61 in shared_ptr<std::allocator<(anonymous namespace)::AmclNodeUnderTest> > /usr/i
nclude/c++/11/bits/shared_ptr.h:409
1:     #21 0x55b19381b923 in allocate_shared<(anonymous namespace)::AmclNodeUnderTest, std::all[169/1805$
onymous namespace)::AmclNodeUnderTest> > /usr/include/c++/11/bits/shared_ptr.h:863                       
1:     #22 0x55b19381a503 in make_shared<(anonymous namespace)::AmclNodeUnderTest> /usr/include/c++/11/bi
ts/shared_ptr.h:879                                                                                      
1:     #23 0x55b193815f7d in SetUp /ws/src/beluga/beluga_amcl/test/test_amcl_node.cpp:296                
1:     #24 0x55b193a8d3cf in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test
, void>(testing::Test*, void (testing::Test::*)(), char const*) /opt/ros/humble/src/gtest_vendor/src/gtes
t.cc:2433                                                                                                
1:     #25 0x55b193a7e5eb in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, v
oid>(testing::Test*, void (testing::Test::*)(), char const*) /opt/ros/humble/src/gtest_vendor/src/gtest.c
c:2469                                                                                                   
1:     #26 0x55b193a26c34 in testing::Test::Run() /opt/ros/humble/src/gtest_vendor/src/gtest.cc:2503     
1:     #27 0x55b193a281c6 in testing::TestInfo::Run() /opt/ros/humble/src/gtest_vendor/src/gtest.cc:2684 
1:     #28 0x55b193a28efe in testing::TestSuite::Run() /opt/ros/humble/src/gtest_vendor/src/gtest.cc:2816
1:     #29 0x55b193a45e3f in testing::internal::UnitTestImpl::RunAllTests() /opt/ros/humble/src/gtest_ven
dor/src/gtest.cc:5338                                                                                    
1:     #30 0x55b193a90059 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::inte
rnal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), 
char const*) /opt/ros/humble/src/gtest_vendor/src/gtest.cc:2433                                          
1:     #31 0x55b193a811ac in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::interna
l::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), cha
r const*) (/ws/build-asan/beluga_amcl/test/test_amcl_node+0x6ac1ac)                                      
1:     #32 0x55b193a428d3 in testing::UnitTest::Run() /opt/ros/humble/src/gtest_vendor/src/gtest.cc:4925 
1:     #33 0x55b193a12a35 in RUN_ALL_TESTS() /opt/ros/humble/src/gtest_vendor/include/gtest/gtest.h:2473 
1:     #34 0x55b193a128fe in main /opt/ros/humble/src/gmock_vendor/src/gmock_main.cc:63                  
1:     #35 0x7f5e73fa7d8f  (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f)                                     
1:     #36 0x7f5e73fa7e3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f)                 
1:     #37 0x55b1937a3c74 in _start (/ws/build-asan/beluga_amcl/test/test_amcl_node+0x3cec74)            
1:                                                                                                       
1: 0x607000012bd0 is located 0 bytes inside of 72-byte region [0x607000012bd0,0x607000012c18)            
1: allocated by thread T0 here:                                                                          
1:     #0 0x7f5e77d9b1e7 in operator new(unsigned long) ../../../../src/libsanitizer/asan/asan_new_delete
.cpp:99                                                                                                  
1:     #1 0x55b193867985 in __gnu_cxx::new_allocator<char>::allocate(unsigned long, void const*) (/ws/bui
ld-asan/beluga_amcl/test/test_amcl_node+0x492985)                                                        
1:     #2 0x55b1938543b5 in std::allocator_traits<std::allocator<char> >::allocate(std::allocator<char>&,
 unsigned long) (/ws/build-asan/beluga_amcl/test/test_amcl_node+0x47f3b5)                                
1:     #3 0x55b19390a039 in void* rclcpp::allocator::retyped_allocate<std::allocator<char> >(unsigned lon
g, void*) (/ws/build-asan/beluga_amcl/test/test_amcl_node+0x535039)                                      
1:     #4 0x7f5e74cc3bb2 in rcutils_string_map_init (/opt/ros/humble/lib/librcutils.so+0x9bb2)           
1:                                                                                                       
1: SUMMARY: AddressSanitizer: new-delete-type-mismatch ../../../../src/libsanitizer/asan/asan_new_delete.
cpp:172 in operator delete(void*, unsigned long)                                                         
1: ==9720==HINT: if you don't care about these errors you may set ASAN_OPTIONS=new_delete_type_mismatch=0
1: ==9720==ABORTING                                                                                      
1: -- run_test.py: return code 1                                                                         
1: -- run_test.py: generate result file '/ws/build-asan/beluga_amcl/test_results/beluga_amcl/test_amcl_no
de.gtest.xml' with failed test                                                                           
1: -- run_test.py: verify result file '/ws/build-asan/beluga_amcl/test_results/beluga_amcl/test_amcl_node
.gtest.xml'                                                                                              
1/3 Test #1: test_amcl_node .......................***Failed    0.48 sec 

Other tests are successful.

Expected behavior

Everything should pass.

Actual behavior

There is an error.

Additional context

N/A

[hidmic]

This is not on us. rclcpp's use of std::allocator for C allocations down in rcl and below is broken. rclcpp::allocator::retyped_allocate specifies a size, but rclcpp::allocator::retyped_deallocate doesn't (because it can't, because a C++ allocator is being used to implement a C malloc / free API). That's what ASan is complaining about.

I would just report upstream and suppress the warning here.

[ivanpauno]

This is not on us. rclcpp's use of std::allocator for C allocations down in rcl and below is broken. rclcpp::allocator::retyped_allocate specifies a size, but rclcpp::allocator::retyped_deallocate doesn't (because it can't, because a C++ allocator is being used to implement a C malloc / free API). That's what ASan is complaining about.

Lol, I forgot that was a thing.
Yes, that's completely broken, it doesn't really break because std::allocator ignores the size when you dealloc AFAIK.
But it can break with custom allocators.