From 02887229857ed345f7a565344090c01e091c8385 Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Tue, 2 Jul 2019 14:35:24 +0200 Subject: [PATCH 01/13] Add tests --- tracetools/CMakeLists.txt | 7 +++ tracetools/include/tracetools/tp_call.h | 4 +- tracetools/include/tracetools/tracetools.h | 3 +- tracetools/package.xml | 1 + tracetools/src/tracetools.c | 6 +- tracetools/src/utils.cpp | 4 ++ tracetools/test/test_utils.cpp | 67 ++++++++++++++++++++++ tracetools/test/test_utils.hpp | 66 +++++++++++++++++++++ 8 files changed, 154 insertions(+), 4 deletions(-) create mode 100644 tracetools/test/test_utils.cpp create mode 100644 tracetools/test/test_utils.hpp diff --git a/tracetools/CMakeLists.txt b/tracetools/CMakeLists.txt index a6327a5..2263184 100644 --- a/tracetools/CMakeLists.txt +++ b/tracetools/CMakeLists.txt @@ -87,8 +87,15 @@ else() endif() if(BUILD_TESTING) + find_package(ament_cmake_gtest REQUIRED) find_package(ament_lint_auto REQUIRED) ament_lint_auto_find_test_dependencies() + + ament_add_gtest(test_utils test/test_utils.cpp) + if(TARGET test_utils) + target_link_libraries(test_utils ${PROJECT_NAME} -rdynamic) + target_include_directories(test_utils PRIVATE test/) + endif() endif() ament_package() diff --git a/tracetools/include/tracetools/tp_call.h b/tracetools/include/tracetools/tp_call.h index a1aaa13..1facd35 100644 --- a/tracetools/include/tracetools/tp_call.h +++ b/tracetools/include/tracetools/tp_call.h @@ -189,11 +189,13 @@ TRACEPOINT_EVENT( rclcpp_callback_register, TP_ARGS( const void *, callback_arg, - const char *, symbol_arg + const char *, symbol_arg, + const void *, address_arg ), TP_FIELDS( ctf_integer_hex(const void *, callback, callback_arg) ctf_string(symbol, symbol_arg) + ctf_integer_hex(const void *, address, address_arg) ) ) diff --git a/tracetools/include/tracetools/tracetools.h b/tracetools/include/tracetools/tracetools.h index 2bf082d..b6c38f6 100644 --- a/tracetools/include/tracetools/tracetools.h +++ b/tracetools/include/tracetools/tracetools.h @@ -129,7 +129,8 @@ void TRACEPOINT( void TRACEPOINT( rclcpp_callback_register, const void * callback, - const char * function_symbol); + const char * function_symbol, + const void * address); /** * tp: callback_start diff --git a/tracetools/package.xml b/tracetools/package.xml index a3cf0c5..45e574a 100644 --- a/tracetools/package.xml +++ b/tracetools/package.xml @@ -15,6 +15,7 @@ liblttng-ust-dev + ament_cmake_gtest ament_lint_auto ament_lint_common diff --git a/tracetools/src/tracetools.c b/tracetools/src/tracetools.c index fc2b996..36103d1 100644 --- a/tracetools/src/tracetools.c +++ b/tracetools/src/tracetools.c @@ -182,13 +182,15 @@ void TRACEPOINT( void TRACEPOINT( rclcpp_callback_register, const void * callback, - const char * function_symbol) + const char * function_symbol, + const void * address) { CONDITIONAL_TP( ros2, rclcpp_callback_register, callback, - function_symbol); + function_symbol, + address); } void TRACEPOINT( diff --git a/tracetools/src/utils.cpp b/tracetools/src/utils.cpp index acb49cb..2190df9 100644 --- a/tracetools/src/utils.cpp +++ b/tracetools/src/utils.cpp @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include + #if defined(TRACETOOLS_LTTNG_ENABLED) && !defined(_WIN32) #include #include @@ -24,11 +26,13 @@ const char * get_symbol(void * funptr) #if defined(TRACETOOLS_LTTNG_ENABLED) && !defined(_WIN32) #define SYMBOL_LAMBDA "[lambda]" if (funptr == 0) { + std::cout << "lamba!" << std::endl; return SYMBOL_LAMBDA; } Dl_info info; if (dladdr(funptr, &info) == 0) { + std::cout << "unknown!" << std::endl; return SYMBOL_UNKNOWN; } diff --git a/tracetools/test/test_utils.cpp b/tracetools/test/test_utils.cpp new file mode 100644 index 0000000..d60a3a0 --- /dev/null +++ b/tracetools/test/test_utils.cpp @@ -0,0 +1,67 @@ +// Copyright 2019 Robert Bosch GmbH +// +// 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 + +#include +#include + +#include "tracetools/utils.hpp" +#include "test_utils.hpp" + +int function_int_int(int num) +{ + return num + 1; +} + +void function_generic_shared(const std::shared_ptr p) +{ + (void)p; +} + +void function_generic_unique(const std::unique_ptr p) +{ + (void)p; +} + +/* + Testing address and symbol resolution for std::function objects. + */ +TEST(TestUtils, valid_address_symbol) { + std::function f = &function_int_int; + std::function lambda = [](int num) {return num + 1;}; + std::function l = lambda; + + ASSERT_EQ(f(69), l(69)); + + // Address for an std::function with an underlying lambda should be nullptr + ASSERT_EQ(get_address(l), nullptr) << "get_address() for lambda std::function not 0"; + // But address for one with an actual underlying function should be non-zero + ASSERT_GT(get_address(f), (void *)0) << "get_address() for function not valid"; + + ASSERT_STREQ(get_symbol(get_address(f)), "function_int_int(int)") << "invalid function name"; + + // Generic + std::function)> fg_shared = &function_generic_shared; + SomeGenericClass gc_shared; + gc_shared.set(fg_shared); + ASSERT_GT(gc_shared.get_address_(), (void *)0) << "generic -- address invalid"; + ASSERT_STREQ(gc_shared.get_symbol_(), "function_generic_shared(std::shared_ptr)") << "generic -- symbol invalid"; + + std::function)> fg_unique = &function_generic_unique; + SomeGenericClass gc_unique; + gc_unique.set(fg_unique); + ASSERT_GT(gc_unique.get_address_(), (void *)0) << "generic -- address invalid"; + ASSERT_STREQ(gc_unique.get_symbol_(), "function_generic_unique(std::unique_ptr >)") << "generic -- symbol invalid"; +} diff --git a/tracetools/test/test_utils.hpp b/tracetools/test/test_utils.hpp new file mode 100644 index 0000000..33e9605 --- /dev/null +++ b/tracetools/test/test_utils.hpp @@ -0,0 +1,66 @@ +#include +#include + +#include "tracetools/utils.hpp" +#include "function_traits.hpp" + + +template +class SomeGenericClass +{ +using SharedPtrCallback = std::function)>; +using UniquePtrCallback = std::function)>; + +public: + SomeGenericClass() + : my_callback_shared_(nullptr), my_callback_unique_(nullptr) + {} + + template< + typename TheType, + typename std::enable_if< + rclcpp::function_traits::same_arguments< + TheType, + SharedPtrCallback + >::value + >::type * = nullptr + > + void set(TheType callback) + { + my_callback_shared_ = callback; + } + + template< + typename TheType, + typename std::enable_if< + rclcpp::function_traits::same_arguments< + TheType, + UniquePtrCallback + >::value + >::type * = nullptr + > + void set(TheType callback) + { + my_callback_unique_ = callback; + } + + void * get_address_() + { + if (my_callback_shared_) { + return get_address(my_callback_shared_); + } else if (my_callback_unique_) { + return get_address(my_callback_unique_); + } else { + return (void *)0; + } + } + + const char * get_symbol_() + { + return get_symbol(get_address_()); + } + +private: + SharedPtrCallback my_callback_shared_; + UniquePtrCallback my_callback_unique_; +}; From 5c329059440afbbfd4ca9a84289aa35965a9910b Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Wed, 3 Jul 2019 10:25:15 +0200 Subject: [PATCH 02/13] Simplify tests --- tracetools/CMakeLists.txt | 1 - tracetools/test/test_utils.cpp | 57 +++++++++++++++-------------- tracetools/test/test_utils.hpp | 66 ---------------------------------- 3 files changed, 28 insertions(+), 96 deletions(-) delete mode 100644 tracetools/test/test_utils.hpp diff --git a/tracetools/CMakeLists.txt b/tracetools/CMakeLists.txt index 2263184..def6f95 100644 --- a/tracetools/CMakeLists.txt +++ b/tracetools/CMakeLists.txt @@ -94,7 +94,6 @@ if(BUILD_TESTING) ament_add_gtest(test_utils test/test_utils.cpp) if(TARGET test_utils) target_link_libraries(test_utils ${PROJECT_NAME} -rdynamic) - target_include_directories(test_utils PRIVATE test/) endif() endif() diff --git a/tracetools/test/test_utils.cpp b/tracetools/test/test_utils.cpp index d60a3a0..a0fb10d 100644 --- a/tracetools/test/test_utils.cpp +++ b/tracetools/test/test_utils.cpp @@ -20,17 +20,19 @@ #include "tracetools/utils.hpp" #include "test_utils.hpp" -int function_int_int(int num) +class SomeClassWithCallback { - return num + 1; -} +public: + SomeClassWithCallback() {} -void function_generic_shared(const std::shared_ptr p) -{ - (void)p; -} + void my_callback(int some_number, std::string some_string) + { + (void)some_number; + (void)some_string; + } +}; -void function_generic_unique(const std::unique_ptr p) +void function_shared(const std::shared_ptr p) { (void)p; } @@ -39,29 +41,26 @@ void function_generic_unique(const std::unique_ptr p) Testing address and symbol resolution for std::function objects. */ TEST(TestUtils, valid_address_symbol) { - std::function f = &function_int_int; - std::function lambda = [](int num) {return num + 1;}; - std::function l = lambda; - - ASSERT_EQ(f(69), l(69)); + // Function pointer + std::function)> f = &function_shared; + // Address for one with an actual underlying function should be non-zero + ASSERT_GT(get_address(f), (void *)0) << "get_address() for function not valid"; + ASSERT_STREQ(get_symbol(get_address(f)), "function_shared(std::shared_ptr)") << "invalid function name"; + // Lambda + std::function l = [](int num) {return num + 1;}; // Address for an std::function with an underlying lambda should be nullptr ASSERT_EQ(get_address(l), nullptr) << "get_address() for lambda std::function not 0"; - // But address for one with an actual underlying function should be non-zero - ASSERT_GT(get_address(f), (void *)0) << "get_address() for function not valid"; + // TODO symbol - ASSERT_STREQ(get_symbol(get_address(f)), "function_int_int(int)") << "invalid function name"; - - // Generic - std::function)> fg_shared = &function_generic_shared; - SomeGenericClass gc_shared; - gc_shared.set(fg_shared); - ASSERT_GT(gc_shared.get_address_(), (void *)0) << "generic -- address invalid"; - ASSERT_STREQ(gc_shared.get_symbol_(), "function_generic_shared(std::shared_ptr)") << "generic -- symbol invalid"; - - std::function)> fg_unique = &function_generic_unique; - SomeGenericClass gc_unique; - gc_unique.set(fg_unique); - ASSERT_GT(gc_unique.get_address_(), (void *)0) << "generic -- address invalid"; - ASSERT_STREQ(gc_unique.get_symbol_(), "function_generic_unique(std::unique_ptr >)") << "generic -- symbol invalid"; + // Bind (to member function) + SomeClassWithCallback scwc; + std::function fscwc = std::bind( + &SomeClassWithCallback::my_callback, + &scwc, + std::placeholders::_1, + std::placeholders::_2 + ); + ASSERT_EQ(get_address(fscwc), nullptr) << "get_address() for std::bind std::function not 0"; + // TODO symbol } diff --git a/tracetools/test/test_utils.hpp b/tracetools/test/test_utils.hpp deleted file mode 100644 index 33e9605..0000000 --- a/tracetools/test/test_utils.hpp +++ /dev/null @@ -1,66 +0,0 @@ -#include -#include - -#include "tracetools/utils.hpp" -#include "function_traits.hpp" - - -template -class SomeGenericClass -{ -using SharedPtrCallback = std::function)>; -using UniquePtrCallback = std::function)>; - -public: - SomeGenericClass() - : my_callback_shared_(nullptr), my_callback_unique_(nullptr) - {} - - template< - typename TheType, - typename std::enable_if< - rclcpp::function_traits::same_arguments< - TheType, - SharedPtrCallback - >::value - >::type * = nullptr - > - void set(TheType callback) - { - my_callback_shared_ = callback; - } - - template< - typename TheType, - typename std::enable_if< - rclcpp::function_traits::same_arguments< - TheType, - UniquePtrCallback - >::value - >::type * = nullptr - > - void set(TheType callback) - { - my_callback_unique_ = callback; - } - - void * get_address_() - { - if (my_callback_shared_) { - return get_address(my_callback_shared_); - } else if (my_callback_unique_) { - return get_address(my_callback_unique_); - } else { - return (void *)0; - } - } - - const char * get_symbol_() - { - return get_symbol(get_address_()); - } - -private: - SharedPtrCallback my_callback_shared_; - UniquePtrCallback my_callback_unique_; -}; From 52e04a395b01bc5b01213f1e7e2333e25f19c62e Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Wed, 3 Jul 2019 10:37:08 +0200 Subject: [PATCH 03/13] Fix lint errors --- tracetools/test/test_utils.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tracetools/test/test_utils.cpp b/tracetools/test/test_utils.cpp index a0fb10d..455b0a4 100644 --- a/tracetools/test/test_utils.cpp +++ b/tracetools/test/test_utils.cpp @@ -16,9 +16,9 @@ #include #include +#include #include "tracetools/utils.hpp" -#include "test_utils.hpp" class SomeClassWithCallback { @@ -45,13 +45,14 @@ TEST(TestUtils, valid_address_symbol) { std::function)> f = &function_shared; // Address for one with an actual underlying function should be non-zero ASSERT_GT(get_address(f), (void *)0) << "get_address() for function not valid"; - ASSERT_STREQ(get_symbol(get_address(f)), "function_shared(std::shared_ptr)") << "invalid function name"; + ASSERT_STREQ(get_symbol(get_address(f)), "function_shared(std::shared_ptr)") << + "invalid function name"; // Lambda std::function l = [](int num) {return num + 1;}; // Address for an std::function with an underlying lambda should be nullptr ASSERT_EQ(get_address(l), nullptr) << "get_address() for lambda std::function not 0"; - // TODO symbol + // TODO(christophebedard) check symbol // Bind (to member function) SomeClassWithCallback scwc; @@ -62,5 +63,5 @@ TEST(TestUtils, valid_address_symbol) { std::placeholders::_2 ); ASSERT_EQ(get_address(fscwc), nullptr) << "get_address() for std::bind std::function not 0"; - // TODO symbol + // TODO(christophebedard) check symbol } From 64cb3a29521f0ed075ac423b0ea74585fe256f03 Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Wed, 3 Jul 2019 13:51:59 +0200 Subject: [PATCH 04/13] Remove address arg for register_callback tracepoint --- tracetools/include/tracetools/tp_call.h | 4 +--- tracetools/include/tracetools/tracetools.h | 3 +-- tracetools/src/tracetools.c | 6 ++---- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/tracetools/include/tracetools/tp_call.h b/tracetools/include/tracetools/tp_call.h index 1facd35..a1aaa13 100644 --- a/tracetools/include/tracetools/tp_call.h +++ b/tracetools/include/tracetools/tp_call.h @@ -189,13 +189,11 @@ TRACEPOINT_EVENT( rclcpp_callback_register, TP_ARGS( const void *, callback_arg, - const char *, symbol_arg, - const void *, address_arg + const char *, symbol_arg ), TP_FIELDS( ctf_integer_hex(const void *, callback, callback_arg) ctf_string(symbol, symbol_arg) - ctf_integer_hex(const void *, address, address_arg) ) ) diff --git a/tracetools/include/tracetools/tracetools.h b/tracetools/include/tracetools/tracetools.h index b6c38f6..2bf082d 100644 --- a/tracetools/include/tracetools/tracetools.h +++ b/tracetools/include/tracetools/tracetools.h @@ -129,8 +129,7 @@ void TRACEPOINT( void TRACEPOINT( rclcpp_callback_register, const void * callback, - const char * function_symbol, - const void * address); + const char * function_symbol); /** * tp: callback_start diff --git a/tracetools/src/tracetools.c b/tracetools/src/tracetools.c index 36103d1..fc2b996 100644 --- a/tracetools/src/tracetools.c +++ b/tracetools/src/tracetools.c @@ -182,15 +182,13 @@ void TRACEPOINT( void TRACEPOINT( rclcpp_callback_register, const void * callback, - const char * function_symbol, - const void * address) + const char * function_symbol) { CONDITIONAL_TP( ros2, rclcpp_callback_register, callback, - function_symbol, - address); + function_symbol); } void TRACEPOINT( From f489192a7c2396e64b7a6deb2e8d7a2768b492de Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Wed, 3 Jul 2019 13:53:08 +0200 Subject: [PATCH 05/13] Implement symbol resolution for non-funct ptr std::function objects --- tracetools/include/tracetools/utils.hpp | 40 +++++++++++++++++++------ tracetools/src/utils.cpp | 37 +++++++++++------------ tracetools/test/test_utils.cpp | 6 +--- 3 files changed, 50 insertions(+), 33 deletions(-) diff --git a/tracetools/include/tracetools/utils.hpp b/tracetools/include/tracetools/utils.hpp index e73030b..5a0d995 100644 --- a/tracetools/include/tracetools/utils.hpp +++ b/tracetools/include/tracetools/utils.hpp @@ -18,18 +18,40 @@ #include #include +#define SYMBOL_UNKNOWN "UNKNOWN" + +const char * _demangle_symbol(const char * mangled); + +const char * _get_symbol_funcptr(void * funcptr); + template -void * get_address(std::function f) +const char * _get_symbol_non_funcptr(std::function f) { - typedef T (fnType)(U...); - fnType ** fnPointer = f.template target(); - // Might be a lambda - if (fnPointer == nullptr) { - return 0; - } - return reinterpret_cast(*fnPointer); +#if defined(TRACETOOLS_LTTNG_ENABLED) && !defined(_WIN32) + return _demangle_symbol(f.target_type().name()); +#else + (void)f; + return SYMBOL_UNKNOWN; +#endif } -const char * get_symbol(void * funptr); +template +const char * get_symbol(std::function f) +{ +#if defined(TRACETOOLS_LTTNG_ENABLED) && !defined(_WIN32) + typedef T (fnType)(U...); + fnType ** fnPointer = f.template target(); + // If we get an actual address + if (fnPointer != nullptr) { + void * funcptr = reinterpret_cast(*fnPointer); + return _get_symbol_funcptr(funcptr); + } + // Otherwise we have to go through target_type() + return _get_symbol_non_funcptr(f); +#else + (void)f; + return SYMBOL_UNKNOWN; +#endif +} #endif // TRACETOOLS__UTILS_HPP_ diff --git a/tracetools/src/utils.cpp b/tracetools/src/utils.cpp index 2190df9..e9667b1 100644 --- a/tracetools/src/utils.cpp +++ b/tracetools/src/utils.cpp @@ -12,38 +12,37 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include - #if defined(TRACETOOLS_LTTNG_ENABLED) && !defined(_WIN32) #include #include #endif #include "tracetools/utils.hpp" -const char * get_symbol(void * funptr) +const char * _demangle_symbol(const char * mangled) { -#define SYMBOL_UNKNOWN "UNKNOWN" #if defined(TRACETOOLS_LTTNG_ENABLED) && !defined(_WIN32) -#define SYMBOL_LAMBDA "[lambda]" - if (funptr == 0) { - std::cout << "lamba!" << std::endl; - return SYMBOL_LAMBDA; - } - - Dl_info info; - if (dladdr(funptr, &info) == 0) { - std::cout << "unknown!" << std::endl; - return SYMBOL_UNKNOWN; - } - char * demangled = nullptr; int status; - demangled = abi::__cxa_demangle(info.dli_sname, NULL, 0, &status); + demangled = abi::__cxa_demangle(mangled, NULL, 0, &status); // Use demangled symbol if possible - const char * demangled_val = (status == 0 ? demangled : info.dli_sname); + const char * demangled_val = (status == 0 ? demangled : mangled); return demangled_val != 0 ? demangled_val : SYMBOL_UNKNOWN; #else - (void)funptr; + (void)mangled; + return SYMBOL_UNKNOWN; +#endif +} + +const char * _get_symbol_funcptr(void * funcptr) +{ +#if defined(TRACETOOLS_LTTNG_ENABLED) && !defined(_WIN32) + Dl_info info; + if (dladdr(funcptr, &info) == 0) { + return SYMBOL_UNKNOWN; + } + return _demangle_symbol(info.dli_sname); +#else + (void)funcptr; return SYMBOL_UNKNOWN; #endif } diff --git a/tracetools/test/test_utils.cpp b/tracetools/test/test_utils.cpp index 455b0a4..d75233c 100644 --- a/tracetools/test/test_utils.cpp +++ b/tracetools/test/test_utils.cpp @@ -44,14 +44,11 @@ TEST(TestUtils, valid_address_symbol) { // Function pointer std::function)> f = &function_shared; // Address for one with an actual underlying function should be non-zero - ASSERT_GT(get_address(f), (void *)0) << "get_address() for function not valid"; - ASSERT_STREQ(get_symbol(get_address(f)), "function_shared(std::shared_ptr)") << + ASSERT_STREQ(get_symbol(f), "function_shared(std::shared_ptr)") << "invalid function name"; // Lambda std::function l = [](int num) {return num + 1;}; - // Address for an std::function with an underlying lambda should be nullptr - ASSERT_EQ(get_address(l), nullptr) << "get_address() for lambda std::function not 0"; // TODO(christophebedard) check symbol // Bind (to member function) @@ -62,6 +59,5 @@ TEST(TestUtils, valid_address_symbol) { std::placeholders::_1, std::placeholders::_2 ); - ASSERT_EQ(get_address(fscwc), nullptr) << "get_address() for std::bind std::function not 0"; // TODO(christophebedard) check symbol } From 5bd910a32223537b3f3370ec01d2851729dfa556 Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Wed, 3 Jul 2019 15:21:27 +0200 Subject: [PATCH 06/13] Simplify utils --- tracetools/include/tracetools/utils.hpp | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/tracetools/include/tracetools/utils.hpp b/tracetools/include/tracetools/utils.hpp index 5a0d995..57d1ff6 100644 --- a/tracetools/include/tracetools/utils.hpp +++ b/tracetools/include/tracetools/utils.hpp @@ -24,17 +24,6 @@ const char * _demangle_symbol(const char * mangled); const char * _get_symbol_funcptr(void * funcptr); -template -const char * _get_symbol_non_funcptr(std::function f) -{ -#if defined(TRACETOOLS_LTTNG_ENABLED) && !defined(_WIN32) - return _demangle_symbol(f.target_type().name()); -#else - (void)f; - return SYMBOL_UNKNOWN; -#endif -} - template const char * get_symbol(std::function f) { @@ -47,7 +36,7 @@ const char * get_symbol(std::function f) return _get_symbol_funcptr(funcptr); } // Otherwise we have to go through target_type() - return _get_symbol_non_funcptr(f); + return _demangle_symbol(f.target_type().name()); #else (void)f; return SYMBOL_UNKNOWN; From 4c2fdd8689736d584306adc2f7d37484953a2acb Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Wed, 3 Jul 2019 15:21:45 +0200 Subject: [PATCH 07/13] Split symbols test into 3 tests --- tracetools/test/test_utils.cpp | 43 +++++++++++++++++++++++----------- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/tracetools/test/test_utils.cpp b/tracetools/test/test_utils.cpp index d75233c..09bc8a2 100644 --- a/tracetools/test/test_utils.cpp +++ b/tracetools/test/test_utils.cpp @@ -20,6 +20,11 @@ #include "tracetools/utils.hpp" +void function_shared(const std::shared_ptr p) +{ + (void)p; +} + class SomeClassWithCallback { public: @@ -32,25 +37,30 @@ public: } }; -void function_shared(const std::shared_ptr p) -{ - (void)p; +/* + Testing symbol resolution for std::function object created from a function pointer. + */ +TEST(TestUtils, valid_symbol_funcptr) { + std::function)> f = &function_shared; + EXPECT_STREQ(get_symbol(f), "function_shared(std::shared_ptr)") << + "invalid symbol"; } /* - Testing address and symbol resolution for std::function objects. + Testing symbol resolution for std::function object created from a lambda. */ -TEST(TestUtils, valid_address_symbol) { - // Function pointer - std::function)> f = &function_shared; - // Address for one with an actual underlying function should be non-zero - ASSERT_STREQ(get_symbol(f), "function_shared(std::shared_ptr)") << - "invalid function name"; - - // Lambda +TEST(TestUtils, valid_symbol_lambda) { std::function l = [](int num) {return num + 1;}; - // TODO(christophebedard) check symbol + EXPECT_STREQ( + get_symbol(l), + "TestUtils_valid_symbol_lambda_Test::TestBody()::{lambda(int)#1}") << + "invalid symbol"; +} +/* + Testing symbol resolution for std::function object created from std::bind. + */ +TEST(TestUtils, valid_symbol_bind) { // Bind (to member function) SomeClassWithCallback scwc; std::function fscwc = std::bind( @@ -59,5 +69,10 @@ TEST(TestUtils, valid_address_symbol) { std::placeholders::_1, std::placeholders::_2 ); - // TODO(christophebedard) check symbol + EXPECT_STREQ( + get_symbol( + fscwc), + "std::_Bind, std::_Placeholder<2>))(int, std::__cxx11::basic_string, std::allocator >)>") + << + "invalid symbol"; } From eefb9ff4392d344b638f13999f7d20426b494aa9 Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Wed, 3 Jul 2019 15:22:04 +0200 Subject: [PATCH 08/13] Add -rdynamic to tests --- tracetools_test/CMakeLists.txt | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tracetools_test/CMakeLists.txt b/tracetools_test/CMakeLists.txt index 142ad6f..b09e2a0 100644 --- a/tracetools_test/CMakeLists.txt +++ b/tracetools_test/CMakeLists.txt @@ -25,62 +25,80 @@ if(BUILD_TESTING) ament_target_dependencies(test_publisher rclcpp std_msgs + tracetools ) + target_link_libraries(test_publisher -rdynamic) add_executable(test_subscription src/test_subscription.cpp ) ament_target_dependencies(test_subscription rclcpp std_msgs + tracetools ) + target_link_libraries(test_subscription -rdynamic) add_executable(test_intra src/test_intra.cpp ) ament_target_dependencies(test_intra rclcpp std_msgs + tracetools ) + target_link_libraries(test_intra -rdynamic) add_executable(test_ping src/test_ping.cpp ) ament_target_dependencies(test_ping rclcpp std_msgs + tracetools ) + target_link_libraries(test_ping -rdynamic) add_executable(test_pong src/test_pong.cpp ) ament_target_dependencies(test_pong rclcpp std_msgs + tracetools ) + target_link_libraries(test_pong -rdynamic) add_executable(test_timer src/test_timer.cpp ) ament_target_dependencies(test_timer rclcpp + tracetools ) + target_link_libraries(test_timer -rdynamic) add_executable(test_service src/test_service.cpp ) ament_target_dependencies(test_service rclcpp std_srvs + tracetools ) + target_link_libraries(test_service -rdynamic) add_executable(test_service_ping src/test_service_ping.cpp ) ament_target_dependencies(test_service_ping rclcpp std_srvs + tracetools ) + target_link_libraries(test_service_ping -rdynamic) add_executable(test_service_pong src/test_service_pong.cpp ) ament_target_dependencies(test_service_pong rclcpp std_srvs + tracetools ) + target_link_libraries(test_service_pong -rdynamic) install(TARGETS test_intra From e4dcc9878be665b5c450e4d398579d5c27aee255 Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Wed, 3 Jul 2019 15:43:58 +0200 Subject: [PATCH 09/13] Split big string --- tracetools/test/test_utils.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tracetools/test/test_utils.cpp b/tracetools/test/test_utils.cpp index 09bc8a2..8541eb9 100644 --- a/tracetools/test/test_utils.cpp +++ b/tracetools/test/test_utils.cpp @@ -72,7 +72,9 @@ TEST(TestUtils, valid_symbol_bind) { EXPECT_STREQ( get_symbol( fscwc), - "std::_Bind, std::_Placeholder<2>))(int, std::__cxx11::basic_string, std::allocator >)>") + "std::_Bind, std::_Placeholder<2>))(int, std::__cxx11::basic_string" + ", std::allocator >)>") << "invalid symbol"; } From 6e58ffc372ee5af915462b4b5a7f51cc02c4b1e4 Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Thu, 4 Jul 2019 11:21:33 +0200 Subject: [PATCH 10/13] Return distinctive error messages as symbols instead of a generic one --- tracetools/include/tracetools/utils.hpp | 2 +- tracetools/src/utils.cpp | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tracetools/include/tracetools/utils.hpp b/tracetools/include/tracetools/utils.hpp index 57d1ff6..955c303 100644 --- a/tracetools/include/tracetools/utils.hpp +++ b/tracetools/include/tracetools/utils.hpp @@ -39,7 +39,7 @@ const char * get_symbol(std::function f) return _demangle_symbol(f.target_type().name()); #else (void)f; - return SYMBOL_UNKNOWN; + return "DISABLED__get_symbol"; #endif } diff --git a/tracetools/src/utils.cpp b/tracetools/src/utils.cpp index e9667b1..807fe87 100644 --- a/tracetools/src/utils.cpp +++ b/tracetools/src/utils.cpp @@ -26,10 +26,10 @@ const char * _demangle_symbol(const char * mangled) demangled = abi::__cxa_demangle(mangled, NULL, 0, &status); // Use demangled symbol if possible const char * demangled_val = (status == 0 ? demangled : mangled); - return demangled_val != 0 ? demangled_val : SYMBOL_UNKNOWN; + return demangled_val != 0 ? demangled_val : "UNKNOWN_demangling_failed"; #else (void)mangled; - return SYMBOL_UNKNOWN; + return "DISABLED__demangle_symbol"; #endif } @@ -43,6 +43,6 @@ const char * _get_symbol_funcptr(void * funcptr) return _demangle_symbol(info.dli_sname); #else (void)funcptr; - return SYMBOL_UNKNOWN; + return "DISABLED__get_symbol_funcptr"; #endif } From 04adadf5083fed0e2ff69aa1e6eadf0fa091e9e4 Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Thu, 4 Jul 2019 11:21:53 +0200 Subject: [PATCH 11/13] Make tracetools a shared lib explicitly --- tracetools/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tracetools/CMakeLists.txt b/tracetools/CMakeLists.txt index def6f95..ca6062a 100644 --- a/tracetools/CMakeLists.txt +++ b/tracetools/CMakeLists.txt @@ -57,7 +57,7 @@ if(TRACING_ENABLED) list(APPEND SOURCES ${LTTNG_TP_FILES}) endif() -add_library(${PROJECT_NAME} ${SOURCES}) +add_library(${PROJECT_NAME} SHARED ${SOURCES}) if(TRACING_ENABLED) target_compile_definitions(${PROJECT_NAME} PUBLIC TRACETOOLS_LTTNG_ENABLED) target_link_libraries(${PROJECT_NAME} ${LTTNG_LIBRARIES} -ldl) From baf90e00e9fabdac42398eff20af1c670527cef2 Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Thu, 4 Jul 2019 16:33:49 +0200 Subject: [PATCH 12/13] Remove check for TRACETOOLS_LTTNG_ENABLED in header file --- tracetools/include/tracetools/utils.hpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tracetools/include/tracetools/utils.hpp b/tracetools/include/tracetools/utils.hpp index 955c303..2e882bc 100644 --- a/tracetools/include/tracetools/utils.hpp +++ b/tracetools/include/tracetools/utils.hpp @@ -27,7 +27,6 @@ const char * _get_symbol_funcptr(void * funcptr); template const char * get_symbol(std::function f) { -#if defined(TRACETOOLS_LTTNG_ENABLED) && !defined(_WIN32) typedef T (fnType)(U...); fnType ** fnPointer = f.template target(); // If we get an actual address @@ -37,10 +36,6 @@ const char * get_symbol(std::function f) } // Otherwise we have to go through target_type() return _demangle_symbol(f.target_type().name()); -#else - (void)f; - return "DISABLED__get_symbol"; -#endif } #endif // TRACETOOLS__UTILS_HPP_ From 6e820b70a3448ef6480dd1f9d320201ae016f553 Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Thu, 4 Jul 2019 16:51:21 +0200 Subject: [PATCH 13/13] Remove unnecessary comment in test_utils --- tracetools/test/test_utils.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/tracetools/test/test_utils.cpp b/tracetools/test/test_utils.cpp index 8541eb9..ab94037 100644 --- a/tracetools/test/test_utils.cpp +++ b/tracetools/test/test_utils.cpp @@ -61,7 +61,6 @@ TEST(TestUtils, valid_symbol_lambda) { Testing symbol resolution for std::function object created from std::bind. */ TEST(TestUtils, valid_symbol_bind) { - // Bind (to member function) SomeClassWithCallback scwc; std::function fscwc = std::bind( &SomeClassWithCallback::my_callback,