Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

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>') #849

Closed
oneattosecond opened this issue Sep 10, 2019 · 8 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@oneattosecond
Copy link

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
Copy link
Member

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).

@dirk-thomas dirk-thomas added the more-information-needed Further information is required label Sep 10, 2019
@oneattosecond
Copy link
Author

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
Copy link
Author

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

@mabelzhang
Copy link

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
Copy link
Member

wjwwood commented Oct 9, 2019

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
Copy link
Contributor

nnmm commented Dec 17, 2020

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

@wjwwood
Copy link
Member

wjwwood commented Dec 17, 2020

@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.

@wjwwood wjwwood added help wanted Extra attention is needed bug Something isn't working and removed more-information-needed Further information is required labels Dec 17, 2020
@clalancette
Copy link
Contributor

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.

nnmm pushed a commit to ApexAI/rclcpp that referenced this issue Jul 9, 2022
This just clears out some warnings from clang static analysis.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
DensoADAS pushed a commit to DensoADAS/rclcpp that referenced this issue Aug 5, 2022
…os2#849)

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

6 participants