From 1a4f48ef67be2b2464d98e8096cc8be9fd097040 Mon Sep 17 00:00:00 2001 From: Sachin Suresh Bhat Date: Tue, 26 Feb 2019 08:51:46 -0800 Subject: [PATCH 1/4] Set symbol visibility to hidden for rcl Enabling symbol visibility feature in gcc and clang compilers. This will hep find symbol export related issues in linux and potentially reduce compile times. Discourse topic link: https://discourse.ros.org/t/set-symbol-visibility-to-hidden-for-rmw-and-rcl-packages/7981 Signed-off-by: Sachin Suresh Bhat --- rcl/CMakeLists.txt | 12 +++-- rcl/cmake/configure_rcl.cmake | 73 ++++++++++++++++++++++++++++ rcl/rcl-extras.cmake | 15 ++++++ rcl_action/CMakeLists.txt | 2 + rcl_lifecycle/CMakeLists.txt | 1 + rcl_yaml_param_parser/CMakeLists.txt | 1 + 6 files changed, 99 insertions(+), 5 deletions(-) create mode 100644 rcl/cmake/configure_rcl.cmake create mode 100644 rcl/rcl-extras.cmake diff --git a/rcl/CMakeLists.txt b/rcl/CMakeLists.txt index cee1035e8..e1b7172ad 100644 --- a/rcl/CMakeLists.txt +++ b/rcl/CMakeLists.txt @@ -12,7 +12,7 @@ find_package(rosidl_generator_c REQUIRED) find_package(tinydir_vendor REQUIRED) include_directories(include) - +include(cmake/configure_rcl.cmake) include(cmake/get_default_rcl_logging_implementation.cmake) get_default_rcl_logging_implementation(RCL_LOGGING_IMPL) @@ -70,9 +70,7 @@ ament_target_dependencies(${PROJECT_NAME} "tinydir_vendor" ) -# Causes the visibility macros to use dllexport rather than dllimport, -# which is appropriate when building the dll but not consuming it. -target_compile_definitions(${PROJECT_NAME} PRIVATE "RCL_BUILDING_DLL") +configure_rcl(${PROJECT_NAME} LANGUAGE "C") install( TARGETS ${PROJECT_NAME} @@ -105,8 +103,12 @@ if(BUILD_TESTING) add_subdirectory(test) endif() -ament_package() +ament_package(CONFIG_EXTRAS "rcl-extras.cmake") +install( + DIRECTORY cmake + DESTINATION share/${PROJECT_NAME} +) install( DIRECTORY include/ DESTINATION include diff --git a/rcl/cmake/configure_rcl.cmake b/rcl/cmake/configure_rcl.cmake new file mode 100644 index 000000000..bf0cc1c40 --- /dev/null +++ b/rcl/cmake/configure_rcl.cmake @@ -0,0 +1,73 @@ +# Copyright 2019 Open Source Robotics Foundation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# Configures ros client library with custom settings. +# The custom settings are all related to library symbol visibility, see: +# https://gcc.gnu.org/wiki/Visibility +# http://www.ibm.com/developerworks/aix/library/au-aix-symbol-visibility/ +# +# Below code is heavily referenced from a similar functionality in rmw: +# https://github.com/ros2/rmw/blob/master/rmw/cmake/configure_rmw_library.cmake +# +# :param library_target: the library target +# :type library_target: string +# :param LANGUAGE: Optional flag for the language of the library. +# Allowed values are "C" and "CXX". The default is "CXX". +# :type LANGUAGE: string +# +# @public +# +macro(configure_rcl library_target) + cmake_parse_arguments(_ARG "" "LANGUAGE" "" ${ARGN}) + if(_ARG_UNPARSED_ARGUMENTS) + message(FATAL_ERROR "configure_rcl() called with unused arguments: ${_ARG_UNPARSED_ARGUMENTS}") + endif() + + if(NOT _ARG_LANGUAGE) + set(_ARG_LANGUAGE "CXX") + endif() + + if(_ARG_LANGUAGE STREQUAL "C") + # Set the visibility to hidden by default if possible + if(CMAKE_C_COMPILER_ID STREQUAL "GNU" OR CMAKE_C_COMPILER_ID MATCHES "Clang") + # Set the visibility of symbols to hidden by default for gcc and clang + # (this is already the default on Windows) + set_target_properties(${library_target} + PROPERTIES + COMPILE_FLAGS "-fvisibility=hidden" + ) + endif() + + elseif(_ARG_LANGUAGE STREQUAL "CXX") + # Set the visibility to hidden by default if possible + if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") + # Set the visibility of symbols to hidden by default for gcc and clang + # (this is already the default on Windows) + set_target_properties(${library_target} + PROPERTIES + COMPILE_FLAGS "-fvisibility=hidden -fvisibility-inlines-hidden" + ) + endif() + + else() + message(FATAL_ERROR "configure_rcl() called with unsupported LANGUAGE: '${_ARG_LANGUAGE}'") + endif() + + if(WIN32) + # Causes the visibility macros to use dllexport rather than dllimport + # which is appropriate when building the dll but not consuming it. + target_compile_definitions(${library_target} + PRIVATE "RCL_BUILDING_DLL") + endif() +endmacro() diff --git a/rcl/rcl-extras.cmake b/rcl/rcl-extras.cmake new file mode 100644 index 000000000..c6fac3d5b --- /dev/null +++ b/rcl/rcl-extras.cmake @@ -0,0 +1,15 @@ +# Copyright 2019 Open Source Robotics Foundation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +include("${rcl_DIR}/configure_rcl.cmake") diff --git a/rcl_action/CMakeLists.txt b/rcl_action/CMakeLists.txt index aa2c0a3dc..7387cd847 100644 --- a/rcl_action/CMakeLists.txt +++ b/rcl_action/CMakeLists.txt @@ -56,6 +56,8 @@ ament_target_dependencies(${PROJECT_NAME} "rmw" "rosidl_generator_c" ) + +configure_rcl(${PROJECT_NAME} LANGUAGE "C") # Causes the visibility macros to use dllexport rather than dllimport, # which is appropriate when building the dll but not consuming it. target_compile_definitions(${PROJECT_NAME} PRIVATE "RCL_ACTION_BUILDING_DLL") diff --git a/rcl_lifecycle/CMakeLists.txt b/rcl_lifecycle/CMakeLists.txt index 1c82a501e..3c04dcffe 100644 --- a/rcl_lifecycle/CMakeLists.txt +++ b/rcl_lifecycle/CMakeLists.txt @@ -48,6 +48,7 @@ ament_target_dependencies(rcl_lifecycle "rcutils" ) +configure_rcl(${PROJECT_NAME} LANGUAGE "C") # Causes the visibility macros to use dllexport rather than dllimport, # which is appropriate when building the dll but not consuming it. target_compile_definitions(rcl_lifecycle PRIVATE "RCL_LIFECYCLE_BUILDING_DLL") diff --git a/rcl_yaml_param_parser/CMakeLists.txt b/rcl_yaml_param_parser/CMakeLists.txt index 0e7b86359..ca433395e 100644 --- a/rcl_yaml_param_parser/CMakeLists.txt +++ b/rcl_yaml_param_parser/CMakeLists.txt @@ -28,6 +28,7 @@ add_library( ${rcl_yaml_parser_sources}) ament_target_dependencies(${PROJECT_NAME} "yaml" "rcutils" "rcl") +configure_rcl(${PROJECT_NAME} LANGUAGE "C") # Causes the visibility macros to use dllexport rather than dllimport, # which is appropriate when building the dll but not consuming it. target_compile_definitions(${PROJECT_NAME} PRIVATE "RCL_YAML_PARAM_PARSER_BUILDING_DLL") From 67e851e1f31db69605db5b9e9a7f371667b39d36 Mon Sep 17 00:00:00 2001 From: Sachin Suresh Bhat Date: Tue, 26 Feb 2019 13:38:31 -0800 Subject: [PATCH 2/4] Remove WIN specific compiler definition in configure_rcl Signed-off-by: Sachin Suresh Bhat --- rcl/CMakeLists.txt | 3 +++ rcl/cmake/configure_rcl.cmake | 7 ------- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/rcl/CMakeLists.txt b/rcl/CMakeLists.txt index e1b7172ad..1c1043b2b 100644 --- a/rcl/CMakeLists.txt +++ b/rcl/CMakeLists.txt @@ -70,6 +70,9 @@ ament_target_dependencies(${PROJECT_NAME} "tinydir_vendor" ) +# Causes the visibility macros to use dllexport rather than dllimport, +# which is appropriate when building the dll but not consuming it. +target_compile_definitions(${PROJECT_NAME} PRIVATE "RCL_BUILDING_DLL") configure_rcl(${PROJECT_NAME} LANGUAGE "C") install( diff --git a/rcl/cmake/configure_rcl.cmake b/rcl/cmake/configure_rcl.cmake index bf0cc1c40..5bcb0983c 100644 --- a/rcl/cmake/configure_rcl.cmake +++ b/rcl/cmake/configure_rcl.cmake @@ -63,11 +63,4 @@ macro(configure_rcl library_target) else() message(FATAL_ERROR "configure_rcl() called with unsupported LANGUAGE: '${_ARG_LANGUAGE}'") endif() - - if(WIN32) - # Causes the visibility macros to use dllexport rather than dllimport - # which is appropriate when building the dll but not consuming it. - target_compile_definitions(${library_target} - PRIVATE "RCL_BUILDING_DLL") - endif() endmacro() From d49a8e99ef13fd64a2d0a3a147a2b97275c3f7ab Mon Sep 17 00:00:00 2001 From: Sachin Suresh Bhat Date: Tue, 26 Feb 2019 23:59:25 -0800 Subject: [PATCH 3/4] Rename macro name rcl_set_symbol_visibility_hidden Signed-off-by: Sachin Suresh Bhat --- rcl/CMakeLists.txt | 4 ++-- ...ure_rcl.cmake => rcl_set_symbol_visibility_hidden.cmake} | 6 +++--- rcl/rcl-extras.cmake | 2 +- rcl_action/CMakeLists.txt | 2 +- rcl_lifecycle/CMakeLists.txt | 2 +- rcl_yaml_param_parser/CMakeLists.txt | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) rename rcl/cmake/{configure_rcl.cmake => rcl_set_symbol_visibility_hidden.cmake} (88%) diff --git a/rcl/CMakeLists.txt b/rcl/CMakeLists.txt index 1c1043b2b..86354b14b 100644 --- a/rcl/CMakeLists.txt +++ b/rcl/CMakeLists.txt @@ -12,7 +12,7 @@ find_package(rosidl_generator_c REQUIRED) find_package(tinydir_vendor REQUIRED) include_directories(include) -include(cmake/configure_rcl.cmake) +include(cmake/rcl_set_symbol_visibility_hidden.cmake) include(cmake/get_default_rcl_logging_implementation.cmake) get_default_rcl_logging_implementation(RCL_LOGGING_IMPL) @@ -73,7 +73,7 @@ ament_target_dependencies(${PROJECT_NAME} # Causes the visibility macros to use dllexport rather than dllimport, # which is appropriate when building the dll but not consuming it. target_compile_definitions(${PROJECT_NAME} PRIVATE "RCL_BUILDING_DLL") -configure_rcl(${PROJECT_NAME} LANGUAGE "C") +rcl_set_symbol_visibility_hidden(${PROJECT_NAME} LANGUAGE "C") install( TARGETS ${PROJECT_NAME} diff --git a/rcl/cmake/configure_rcl.cmake b/rcl/cmake/rcl_set_symbol_visibility_hidden.cmake similarity index 88% rename from rcl/cmake/configure_rcl.cmake rename to rcl/cmake/rcl_set_symbol_visibility_hidden.cmake index 5bcb0983c..ceacbe038 100644 --- a/rcl/cmake/configure_rcl.cmake +++ b/rcl/cmake/rcl_set_symbol_visibility_hidden.cmake @@ -28,10 +28,10 @@ # # @public # -macro(configure_rcl library_target) +macro(rcl_set_symbol_visibility_hidden library_target) cmake_parse_arguments(_ARG "" "LANGUAGE" "" ${ARGN}) if(_ARG_UNPARSED_ARGUMENTS) - message(FATAL_ERROR "configure_rcl() called with unused arguments: ${_ARG_UNPARSED_ARGUMENTS}") + message(FATAL_ERROR "rcl_set_symbol_visibility_hidden() called with unused arguments: ${_ARG_UNPARSED_ARGUMENTS}") endif() if(NOT _ARG_LANGUAGE) @@ -61,6 +61,6 @@ macro(configure_rcl library_target) endif() else() - message(FATAL_ERROR "configure_rcl() called with unsupported LANGUAGE: '${_ARG_LANGUAGE}'") + message(FATAL_ERROR "rcl_set_symbol_visibility_hidden() called with unsupported LANGUAGE: '${_ARG_LANGUAGE}'") endif() endmacro() diff --git a/rcl/rcl-extras.cmake b/rcl/rcl-extras.cmake index c6fac3d5b..24e83b57c 100644 --- a/rcl/rcl-extras.cmake +++ b/rcl/rcl-extras.cmake @@ -12,4 +12,4 @@ # See the License for the specific language governing permissions and # limitations under the License. -include("${rcl_DIR}/configure_rcl.cmake") +include("${rcl_DIR}/rcl_set_symbol_visibility_hidden.cmake") diff --git a/rcl_action/CMakeLists.txt b/rcl_action/CMakeLists.txt index 7387cd847..75541e2fc 100644 --- a/rcl_action/CMakeLists.txt +++ b/rcl_action/CMakeLists.txt @@ -57,7 +57,7 @@ ament_target_dependencies(${PROJECT_NAME} "rosidl_generator_c" ) -configure_rcl(${PROJECT_NAME} LANGUAGE "C") +rcl_set_symbol_visibility_hidden(${PROJECT_NAME} LANGUAGE "C") # Causes the visibility macros to use dllexport rather than dllimport, # which is appropriate when building the dll but not consuming it. target_compile_definitions(${PROJECT_NAME} PRIVATE "RCL_ACTION_BUILDING_DLL") diff --git a/rcl_lifecycle/CMakeLists.txt b/rcl_lifecycle/CMakeLists.txt index 3c04dcffe..3bf53351d 100644 --- a/rcl_lifecycle/CMakeLists.txt +++ b/rcl_lifecycle/CMakeLists.txt @@ -48,7 +48,7 @@ ament_target_dependencies(rcl_lifecycle "rcutils" ) -configure_rcl(${PROJECT_NAME} LANGUAGE "C") +rcl_set_symbol_visibility_hidden(${PROJECT_NAME} LANGUAGE "C") # Causes the visibility macros to use dllexport rather than dllimport, # which is appropriate when building the dll but not consuming it. target_compile_definitions(rcl_lifecycle PRIVATE "RCL_LIFECYCLE_BUILDING_DLL") diff --git a/rcl_yaml_param_parser/CMakeLists.txt b/rcl_yaml_param_parser/CMakeLists.txt index ca433395e..95143c860 100644 --- a/rcl_yaml_param_parser/CMakeLists.txt +++ b/rcl_yaml_param_parser/CMakeLists.txt @@ -28,7 +28,7 @@ add_library( ${rcl_yaml_parser_sources}) ament_target_dependencies(${PROJECT_NAME} "yaml" "rcutils" "rcl") -configure_rcl(${PROJECT_NAME} LANGUAGE "C") +rcl_set_symbol_visibility_hidden(${PROJECT_NAME} LANGUAGE "C") # Causes the visibility macros to use dllexport rather than dllimport, # which is appropriate when building the dll but not consuming it. target_compile_definitions(${PROJECT_NAME} PRIVATE "RCL_YAML_PARAM_PARSER_BUILDING_DLL") From b6e770bf541fe28905157afef1d31cb9262933a1 Mon Sep 17 00:00:00 2001 From: Sachin Suresh Bhat Date: Wed, 27 Feb 2019 12:18:19 -0800 Subject: [PATCH 4/4] Change macro to args for rcl_set_symbol_visibility_hidden Signed-off-by: Sachin Suresh Bhat --- .../rcl_set_symbol_visibility_hidden.cmake | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/rcl/cmake/rcl_set_symbol_visibility_hidden.cmake b/rcl/cmake/rcl_set_symbol_visibility_hidden.cmake index ceacbe038..60c6c77fa 100644 --- a/rcl/cmake/rcl_set_symbol_visibility_hidden.cmake +++ b/rcl/cmake/rcl_set_symbol_visibility_hidden.cmake @@ -11,6 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. + # # Configures ros client library with custom settings. # The custom settings are all related to library symbol visibility, see: @@ -28,17 +29,17 @@ # # @public # -macro(rcl_set_symbol_visibility_hidden library_target) - cmake_parse_arguments(_ARG "" "LANGUAGE" "" ${ARGN}) - if(_ARG_UNPARSED_ARGUMENTS) - message(FATAL_ERROR "rcl_set_symbol_visibility_hidden() called with unused arguments: ${_ARG_UNPARSED_ARGUMENTS}") +function(rcl_set_symbol_visibility_hidden library_target) + cmake_parse_arguments(ARG "" "LANGUAGE" "" ${ARGN}) + if(ARG_UNPARSED_ARGUMENTS) + message(FATAL_ERROR "rcl_set_symbol_visibility_hidden() called with unused arguments: ${ARG_UNPARSED_ARGUMENTS}") endif() - if(NOT _ARG_LANGUAGE) - set(_ARG_LANGUAGE "CXX") + if(NOT ARG_LANGUAGE) + set(ARG_LANGUAGE "CXX") endif() - if(_ARG_LANGUAGE STREQUAL "C") + if(ARG_LANGUAGE STREQUAL "C") # Set the visibility to hidden by default if possible if(CMAKE_C_COMPILER_ID STREQUAL "GNU" OR CMAKE_C_COMPILER_ID MATCHES "Clang") # Set the visibility of symbols to hidden by default for gcc and clang @@ -49,7 +50,7 @@ macro(rcl_set_symbol_visibility_hidden library_target) ) endif() - elseif(_ARG_LANGUAGE STREQUAL "CXX") + elseif(ARG_LANGUAGE STREQUAL "CXX") # Set the visibility to hidden by default if possible if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") # Set the visibility of symbols to hidden by default for gcc and clang @@ -61,6 +62,6 @@ macro(rcl_set_symbol_visibility_hidden library_target) endif() else() - message(FATAL_ERROR "rcl_set_symbol_visibility_hidden() called with unsupported LANGUAGE: '${_ARG_LANGUAGE}'") + message(FATAL_ERROR "rcl_set_symbol_visibility_hidden() called with unsupported LANGUAGE: '${ARG_LANGUAGE}'") endif() -endmacro() +endfunction()