Add a cmake option to activate address sanitizers (#146)

* Add a cmake option to activate address sanitizers

* Fix unique_ptr type for comm buffer to array

We keep a pointer to the raw data buffer received from the comm interfaces
in our packages. This was held as a unique_ptr<uint8_t> while we were storing
a uint8_t type inside. This commit changes the pointer's type to the correct
one.

* Fix memory leak in tests

test_tcp_server had a memory leak by not deleting manually allocated clients.

Author: Felix Exner (fexner)

.github/workflows/ci.yml

@@ -32,7 +32,7 @@ jobs:
       - name: install boost
         run: sudo apt-get install -y libboost-dev
       - name: configure
-        run: mkdir build && cd build && cmake .. -DBUILDING_TESTS=1 -DINTEGRATION_TESTS=1
+        run: mkdir build && cd build && cmake .. -DBUILDING_TESTS=1 -DINTEGRATION_TESTS=1 -DWITH_ASAN=ON
         env:
             CXXFLAGS: -g -O2  -fprofile-arcs -ftest-coverage
             CFLAGS: -g -O2 -fprofile-arcs -ftest-coverage

CMakeLists.txt

@@ -8,6 +8,7 @@ if(NOT CMAKE_CONFIGURATION_TYPES AND NOT CMAKE_BUILD_TYPE)
   set(CMAKE_BUILD_TYPE RelWithDebInfo)
 endif()
 
+option(WITH_ASAN "Compile with address sanitizer support" OFF)
 
 ##
 ## Check C++11 support / enable global pedantic and Wall
@@ -51,6 +52,10 @@ add_library(urcl SHARED
 add_library(ur_client_library::urcl ALIAS urcl)
 target_compile_options(urcl PRIVATE -Wall -Wextra -Wno-unused-parameter)
 target_compile_options(urcl PUBLIC ${CXX17_FLAG})
+if(WITH_ASAN)
+  target_compile_options(urcl PUBLIC -fsanitize=address)
+  target_link_options(urcl PUBLIC -fsanitize=address)
+endif()
 target_include_directories( urcl PUBLIC
   $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
   $<INSTALL_INTERFACE:include>

include/ur_client_library/comm/bin_parser.h

@@ -239,7 +239,7 @@ class BinParser
    * \param buffer The buffer to copy the remaining bytes to.
    * \param buffer_length Reference to write the number of remaining bytes to.
    */
-  void rawData(std::unique_ptr<uint8_t>& buffer, size_t& buffer_length)
+  void rawData(std::unique_ptr<uint8_t[]>& buffer, size_t& buffer_length)
   {
     buffer_length = buf_end_ - buf_pos_;
     buffer.reset(new uint8_t[buffer_length]);

include/ur_client_library/primary/primary_package.h

@@ -82,7 +82,7 @@ class PrimaryPackage : public comm::URPackage<PackageHeader>
   virtual std::string toString() const;
 
 protected:
-  std::unique_ptr<uint8_t> buffer_;
+  std::unique_ptr<uint8_t[]> buffer_;
   size_t buffer_length_;
 };
 

include/ur_client_library/rtde/rtde_package.h

@@ -69,7 +69,7 @@ class RTDEPackage : public comm::URPackage<PackageHeader>
   virtual std::string toString() const;
 
 protected:
-  std::unique_ptr<uint8_t> buffer_;
+  std::unique_ptr<uint8_t[]> buffer_;
   size_t buffer_length_;
   PackageType type_;
 };

tests/test_bin_parser.cpp

@@ -230,7 +230,7 @@ TEST(bin_parser, raw_data_parser)
                        0x62, 0x65, 0x20, 0x70, 0x61, 0x72, 0x73, 0x65, 0x64 };
   comm::BinParser bp(buffer, sizeof(buffer));
 
-  std::unique_ptr<uint8_t> raw_data;
+  std::unique_ptr<uint8_t[]> raw_data;
   size_t size;
   bp.rawData(raw_data, size);
 

tests/test_tcp_server.cpp

@@ -31,6 +31,7 @@
 #include <gtest/gtest.h>
 #include <condition_variable>
 #include <chrono>
+#include <memory>
 
 #include <ur_client_library/comm/tcp_server.h>
 #include <ur_client_library/comm/tcp_socket.h>
@@ -225,13 +226,12 @@ TEST_F(TCPServerTest, unlimited_clients_allowed)
   server.start();
 
   // Test that a large number of clients can connect to the server
-  std::vector<Client*> clients;
-  Client* client;
+  std::vector<std::unique_ptr<Client>> clients;
+  std::unique_ptr<Client> client;
   for (unsigned int i = 0; i < 100; ++i)
   {
-    client = new Client(port_);
+    clients.push_back(std::make_unique<Client>(port_));
     ASSERT_TRUE(waitForConnectionCallback());
-    clients.push_back(client);
   }
 }