Set symbol visibility to hidden for RMW and RCL packages

[bhatsach]

Hi all,

I am planning to work on a task to set symbol visibility to hidden for some of the RMW and RCL packages. The core rmw package has the header for symbol visibility definitions and has its cmake set visibility to hidden by default in the compile flags [1]. However other packages just have the visibility headers defined but not enabled in compile flags [2]. I am planning to make the changes to enable the same.

Please let me know your thoughts/any concerns about the same. It may happen that some downstream dependency is depending on a hidden symbol, however we do want to catch such cases and have them fixed if possible.

Regards,
Sachin

[1] - https://github.com/ros2/rmw/blob/master/rmw/cmake/configure_rmw_library.cmake#L44-L88

[2] -

RMW packages

• rmw: https://github.com/ros2/rmw/blob/master/rmw/include/rmw/visibility_control.h
• rmw_connext: https://github.com/ros2/rmw_connext/blob/master/rmw_connext_cpp/include/rmw_connext_cpp/visibility_control.h
• rmw_fastrtps: https://github.com/ros2/rmw_fastrtps/blob/master/rmw_fastrtps_cpp/include/rmw_fastrtps_cpp/visibility_control.h

RCL packages:

• rcl: https://github.com/ros2/rcl/blob/master/rcl/include/rcl/visibility_control.h
• rcl_action: https://github.com/ros2/rcl/blob/master/rcl_action/include/rcl_action/visibility_control.h
• rcl_lifecycle: https://github.com/ros2/rcl/blob/master/rcl_lifecycle/include/rcl_lifecycle/visibility_control.h
• rcl_yaml_param_parser: https://github.com/ros2/rcl/blob/master/rcl_yaml_param_parser/include/rcl_yaml_param_parser/visibility_control.h
• rclcpp: https://github.com/ros2/rclcpp/blob/master/rclcpp/include/rclcpp/visibility_control.hpp
• rclcpp_action: https://github.com/ros2/rclcpp/blob/master/rclcpp_action/include/rclcpp_action/visibility_control.hpp
• rclcpp_lifecycle: https://github.com/ros2/rclcpp/blob/master/rclcpp_lifecycle/include/rclcpp_lifecycle/visibility_control.h
• rcutils: https://github.com/ros2/rcutils/blob/master/include/rcutils/visibility_control.h

[wjwwood]

Updating more libraries to hind their symbols by default seems reasonable to me. Any cases where we’re using unintentionally referenced symbols should be addressed, as you said.

@bhatsach

However other packages just have the visibility headers defined but not enabled in compile flags [2].

The symbol visibility headers are used primarily for Windows, where symbols must be declared publicly, and implicitly referencing symbols will not work.

[bhatsach]

Thanks William, I have opened a couple of pull requests for RCL packages to get this started:

Set symbol visibility to hidden for rcl

Set symbol visibility to hidden for rclcpp

[Thomas_Moulard]

Thanks @wjwwood ! Our goal is to catch dllexport issues earlier by enabling this change.

[alsora]

What’s the reason behind setting this in rcl?

rcl_set_symbol_visibility_hidden(${PROJECT_NAME} LANGUAGE "C")

This caused me some troubles since I was trying to use some rcl functions in my application.
The linker immediately told me that some of them were hidden (and indeed I saw the RCL_LOCAL macro).
However, it was also not able to link against other functions marked as RCL_PUBLIC.
Then I noticed this line in the CMakeLists.txt, however the linker was not giving any hints for what concerns the RCL_PUBLIC functions hidden by the aforementioned line.

I opened a question on ROSAnswers about how to set the logger programmatically since all these functions are hidden.
https://answers.ros.org/question/329532/ros2-how-to-programmatically-set-rosout-logging-handler/

[wjwwood]

Functions marked as RCL_PUBLIC should be accessible for other libraries, if not open an issue on the package (I assume rcl) for which this isn’t the case.

Based on your question on ROS answers, I would guess one is just not public but maybe could be (again an issue asking to make it public would be best) and the other might be a bug (using RCL_PUBLIC rather than RCL_LOGGING_PUBLIC or something else).

[alsora]

It was exactly as you said.

The RCL_PUBLIC function was not accessible due to a missing header https://github.com/ros2/rcl/pull/478.
For what concerns the other functions I opened a PR to make them public https://github.com/ros2/rcl/pull/479