UBSAN: runtime error publisher_options.hpp:67:62: runtime error: reference binding to null pointer of type 'std::__1::shared_ptr<std::__1::allocator<void> >::element_type' (aka 'std::__1::allocator<void>')

[oneattosecond]

Bug report

Required Info:

• Operating System:
  • MacOS 10.14.6
• Installation type:
  • compiled from source
• Version or commit hash:
  • dashing repos file
• DDS implementation:
  • Fast-RTPS
• Client library (if applicable):
  • rclcpp, cartographer, navigation, map_server

Steps to reproduce issue

1. Enable UBSAN by adding below lines to various CMakeLists.txt incl. cartographer & navigation2 modules

   set (CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -fno-omit-frame-pointer -fsanitize=undefined")
   set (CMAKE_LINKER_FLAGS_DEBUG "${CMAKE_LINKER_FLAGS_DEBUG} -fno-omit-frame-pointer -fsanitize=undefined")

2. Rebuild all TB3 and related NAV packages with

   colcon build --cmake-args -DBUILD_TESTING=OFF -DCMAKE_BUILD_TYPE=Debug -DFORCE_DEBUG_BUILD=True --symlink-install

3. Run cartographer and navigation and map server logic as per the usual, notice a bunch of runtime errors (UBSAN warnings) that follow same pattern

Showing a sample from cartographer, but same exact pattern happens in occupancy grid, world model, dwb_controller, navfn_planner, etc. It's either worth fixing or just white noise, I'm not sure so I figured it's worth filing

[cartographer_node-1] ~/ros2_osx_dashing/install/rclcpp/include/rclcpp/publisher_options.hpp:67:62: runtime error: reference binding to null pointer of type 'std::__1::shared_ptr<std::__1::allocator >::element_type' (aka 'std::__1::allocator')
[cartographer_node-1] xcode_path/usr/include/c++/v1/memory:4706:63: runtime error: reference binding to null pointer of type 'std::__1::allocator'
[cartographer_node-1] xcode_path/usr/include/c++/v1/type_traits:2299:12: runtime error: reference binding to null pointer of type 'std::__1::allocator'
[cartographer_node-1] xcode_path/usr/include/c++/v1/memory:4706:41: runtime error: reference binding to null pointer of type 'std::__1::allocator'
[cartographer_node-1] xcode_path/usr/include/c++/v1/memory:4327:64: runtime error: reference binding to null pointer of type 'std::__1::allocator'
[cartographer_node-1] xcode_path/usr/include/c++/v1/memory:4327:42: runtime error: reference binding to null pointer of type 'std::__1::allocator'
[cartographer_node-1] xcode_path/usr/include/c++/v1/memory:3669:66: runtime error: reference binding to null pointer of type 'std::__1::allocator'
[cartographer_node-1] xcode_path/usr/include/c++/v1/memory:3669:44: runtime error: reference binding to null pointer of type 'std::__1::allocator'
[cartographer_node-1] xcode_path/usr/include/c++/v1/tuple:1111:48: runtime error: reference binding to null pointer of type 'std::__1::allocator'
[cartographer_node-1] xcode_path/usr/include/c++/v1/tuple:1111:28: runtime error: reference binding to null pointer of type 'std::__1::allocator'
[cartographer_node-1] xcode_path/usr/include/c++/v1/tuple:646:17: runtime error: reference binding to null pointer of type 'std::__1::allocator'
[cartographer_node-1] xcode_path/usr/include/c++/v1/tuple:380:56: runtime error: reference binding to null pointer of type 'std::__1::allocator'
[cartographer_node-1] xcode_path/usr/include/c++/v1/tuple:380:36: runtime error: reference binding to null pointer of type 'std::__1::allocator'
[cartographer_node-1] xcode_path/usr/include/c++/v1/tuple:221:44: runtime error: reference binding to null pointer of type 'std::__1::allocator'
[cartographer_node-1] xcode_path/usr/include/c++/v1/tuple:221:24: runtime error: reference binding to null pointer of type 'std::__1::allocator'
[cartographer_node-1] xcode_path/usr/include/c++/v1/tuple:265:102: runtime error: reference binding to null pointer of type 'std::__1::allocator'
[cartographer_node-1] xcode_path/usr/include/c++/v1/tuple:956:63: runtime error: reference binding to null pointer of type 'std::__1::allocator'
[cartographer_node-1] xcode_path/usr/include/c++/v1/memory:2190:44: runtime error: reference binding to null pointer of type 'std::__1::allocator'
[cartographer_node-1] xcode_path/usr/include/c++/v1/memory:2190:22: runtime error: reference binding to null pointer of type 'const std::__1::allocator'

Expected behavior

No warnings from UBSAN

Actual behavior

Saw warnings from UBSAN

Additional information

[dirk-thomas]

It's either worth fixing or just white noise, I'm not sure so I figured it's worth filing

Please consider continuing your investigation to determine which of the two cases it actually is. If it is the first a pull request would be highly appreciates. If it is the second case the ticket can be closed (if there is no way to suppress this warning programmatically).

[oneattosecond]

I'm leaning toward this might be an issue worth fixing.

If you look at the context of the failing line:

struct PublisherOptionsWithAllocator : public PublisherOptionsBase
...
std::shared_ptr allocator = nullptr;
...
auto message_alloc = std::make_shared(*allocator.get());

It appears to be de-ref'ing a null pointer, thought I'm not familiar enough with the surrounding code to suggest a useful fix at this point. Thinking.

[oneattosecond]

tried removing the .get(), but this doesn't remove the UBSAN warning... maybe I need to recompile ROS and then recompile cartographer/nav?

[mabelzhang]

To make reproducing this easier, could you please provide instructions for reproducing this on a minimal example? E.g. without cartographer, which would make it a lot less overhead. Thanks!

[wjwwood]

I think this is worth fixing, and in general we shouldn't be using shared_ptr to allocators, as allocators are required to work as instances and in most cases are required to be CopyAssignable:

A a1(a) A a1 = a | Copy-constructs a1 such that a1 == a. Does not throw exceptions. (Note: every Allocator also satisfies CopyConstructible) |  
-- | -- | --
A a(b) | Constructs a such that B(a)==b and A(b)==a.  Does not throw exceptions. (Note: this implies that all allocators  related by rebind maintain each other's resources, such as memory pools)

(from: https://en.cppreference.com/w/cpp/named_req/Allocator#Requirements)

Among other requirements.

Also we should just emulate the interfaces uses by std containers like vector:

explicit vector( const Allocator& alloc ) noexcept;

Taking it by const & and storing an instance if needed. Which we can imagine vector may do since it has a method:

https://en.cppreference.com/w/cpp/container/vector/get_allocator

---

I'm working on a pr right now to change this across the entire code base.

[nnmm]

This bug still exists – @wjwwood is the linked PR addressing this?

[wjwwood]

@nnmm No, unfortunately I got hung up trying to figure out how to do it in a deprecation cycle and never got around to it.

[clalancette]

Running the rclcpp tests, I don't see any instances of this particular UBSAN problem anymore. Thus I'm going to close this PR out. If this is still a problem, please feel free to reopen and we can look at it again.