Merge pull request #812 from ZettaScaleLabs/asan_ci_tests

Add AddressSanitizer to CI tests

.github/workflows/build-check.yaml

@@ -30,7 +30,7 @@ jobs:
       - name: Build & run tests
         run: |
           sudo apt install -y ninja-build
-          CMAKE_GENERATOR=Ninja make test
+          CMAKE_GENERATOR=Ninja ASAN=ON make test
 
   check_format:
     name: Check codebase format with clang-format
@@ -290,6 +290,6 @@ jobs:
       - name: Build & test pico
         run: |
           sudo apt install -y ninja-build
-          CMAKE_GENERATOR=Ninja make
+          CMAKE_GENERATOR=Ninja ASAN=ON make
           python3 ./build/tests/no_router.py
         timeout-minutes: 5

CMakeLists.txt

@@ -97,7 +97,6 @@ else()
       add_compile_options(-Wpedantic)
     endif()
     # add_compile_options(-Wconversion)
-    # add_link_options(-fsanitize=address)
   elseif(MSVC)
     add_compile_options(/W4 /WX /Od /wd4127)
   elseif(CMAKE_SYSTEM_NAME MATCHES "Generic")
@@ -379,12 +378,14 @@ option(BUILD_EXAMPLES "Use this to also build the examples." ON)
 option(BUILD_TOOLS "Use this to also build the tools." OFF)
 option(BUILD_TESTING "Use this to also build tests." ON)
 option(BUILD_INTEGRATION "Use this to also build integration tests." OFF)
+option(ASAN "Enable AddressSanitizer." OFF)
 
 message(STATUS "Produce Debian and RPM packages: ${PACKAGING}")
 message(STATUS "Build examples: ${BUILD_EXAMPLES}")
 message(STATUS "Build tools: ${BUILD_TOOLS}")
 message(STATUS "Build tests: ${BUILD_TESTING}")
 message(STATUS "Build integration: ${BUILD_INTEGRATION}")
+message(STATUS "AddressSanitizer: ${ASAN}")
 
 set(PICO_LIBS "")
 if(PICO_STATIC)
@@ -453,6 +454,11 @@ if(BUILD_EXAMPLES)
   add_subdirectory(examples)
 endif()
 
+if(ASAN)
+  add_compile_options(-fsanitize=address)
+  add_link_options(-fsanitize=address)
+endif()
+
 if(UNIX OR MSVC)
   if(BUILD_TOOLS)
     set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/tools)

GNUmakefile

@@ -41,6 +41,10 @@ BUILD_TOOLS?=OFF
 # Accepted values: ON, OFF
 FORCE_C99?=OFF
 
+# Enable AddressSanitizer.
+# Accepted values: ON, OFF
+ASAN?=OFF
+
 # Debug level. This sets the ZENOH_DEBUG variable.
 # Accepted values:
 #  0: NONE
@@ -82,7 +86,7 @@ CMAKE_OPT=-DZENOH_DEBUG=$(ZENOH_DEBUG) -DBUILD_EXAMPLES=$(BUILD_EXAMPLES) -DCMAK
  -DZ_FEATURE_MULTI_THREAD=$(Z_FEATURE_MULTI_THREAD) -DZ_FEATURE_INTEREST=$(Z_FEATURE_INTEREST) -DZ_FEATURE_UNSTABLE_API=$(Z_FEATURE_UNSTABLE_API)\
  -DZ_FEATURE_PUBLICATION=$(Z_FEATURE_PUBLICATION) -DZ_FEATURE_SUBSCRIPTION=$(Z_FEATURE_SUBSCRIPTION) -DZ_FEATURE_QUERY=$(Z_FEATURE_QUERY) -DZ_FEATURE_QUERYABLE=$(Z_FEATURE_QUERYABLE)\
  -DZ_FEATURE_RAWETH_TRANSPORT=$(Z_FEATURE_RAWETH_TRANSPORT) -DFRAG_MAX_SIZE=$(FRAG_MAX_SIZE) -DBATCH_UNICAST_SIZE=$(BATCH_UNICAST_SIZE) -DBATCH_MULTICAST_SIZE=$(BATCH_MULTICAST_SIZE)\
- -DBUILD_INTEGRATION=$(BUILD_INTEGRATION) -DBUILD_TOOLS=$(BUILD_TOOLS) -DBUILD_SHARED_LIBS=$(BUILD_SHARED_LIBS) -H.
+ -DASAN=$(ASAN) -DBUILD_INTEGRATION=$(BUILD_INTEGRATION) -DBUILD_TOOLS=$(BUILD_TOOLS) -DBUILD_SHARED_LIBS=$(BUILD_SHARED_LIBS) -H.
 
 ifeq ($(FORCE_C99), ON)
 	CMAKE_OPT += -DCMAKE_C_STANDARD=99

src/collections/refcount.c

@@ -198,7 +198,8 @@ bool _z_rc_decrease_strong(void** cnt) {
     if (_ZP_RC_OP_DECR_AND_CMP_STRONG(c, 1)) {
         return _z_rc_decrease_weak(cnt);
     }
-    return _z_rc_decrease_weak(cnt);
+    _z_rc_decrease_weak(cnt);
+    return true;
 }
 
 bool _z_rc_decrease_weak(void** cnt) {

src/protocol/keyexpr.c

@@ -146,7 +146,7 @@ zp_keyexpr_canon_status_t __zp_canon_prefix(const char *start, size_t *len) {
     char const *next_slash;
 
     do {
-        next_slash = strchr(chunk_start, '/');
+        next_slash = memchr(chunk_start, '/', _z_ptr_char_diff(end, chunk_start));
         const char *chunk_end = next_slash ? next_slash : end;
         size_t chunk_len = _z_ptr_char_diff(chunk_end, chunk_start);
         switch (chunk_len) {
@@ -291,7 +291,7 @@ void __zp_ke_write_chunk(char **writer, const char *chunk, size_t len, const cha
         writer[0] = _z_ptr_char_offset(writer[0], 1);
     }
 
-    (void)memcpy(writer[0], chunk, len);
+    (void)memmove(writer[0], chunk, len);
     writer[0] = _z_ptr_char_offset(writer[0], (ptrdiff_t)len);
 }
 
@@ -766,10 +766,9 @@ zp_keyexpr_canon_status_t _z_keyexpr_canonize(char *start, size_t *len) {
         } else {
             assert(false);  // anything before "$*" or "**" must be part of the canon prefix
         }
-
         while (next_slash != NULL) {
             reader = _z_ptr_char_offset(next_slash, 1);
-            next_slash = strchr(reader, '/');
+            next_slash = memchr(reader, '/', _z_ptr_char_diff(end, reader));
             chunk_end = next_slash ? next_slash : end;
             switch (_z_ptr_char_diff(chunk_end, reader)) {
                 case 0: {

tests/z_api_encoding_test.c

@@ -85,6 +85,7 @@ void test_with_schema(void) {
     z_encoding_to_string(z_encoding_loan_mut(&e), &s);
     assert(strncmp("zenoh/bytes;my_schema", z_string_data(z_string_loan(&s)), z_string_len(z_string_loan(&s))) == 0);
     z_encoding_drop(z_encoding_move(&e));
+    z_string_drop(z_string_move(&s));
 
     z_encoding_from_str(&e, "zenoh/string;");
     z_encoding_set_schema_from_substr(z_encoding_loan_mut(&e), "my_schema", 3);

tests/z_channels_test.c

@@ -13,12 +13,10 @@
 //
 #include <assert.h>
 #include <stddef.h>
-#include <stdio.h>
-#include <stdlib.h>
 
 #include "zenoh-pico/api/handlers.h"
 #include "zenoh-pico/api/macros.h"
-#include "zenoh-pico/net/sample.h"
+#include "zenoh-pico/collections/bytes.h"
 
 #undef NDEBUG
 #include <assert.h>
@@ -37,6 +35,7 @@
             .attachment = _z_bytes_null(),                                                          \
         };                                                                                          \
         z_call(*z_loan(closure), &sample);                                                          \
+        _z_bytes_drop(&payload);                                                                    \
     } while (0);
 
 #define _RECV(handler, method, buf)                                             \
@@ -192,11 +191,13 @@ void zero_size_test(void) {
     assert(z_fifo_channel_sample_new(&closure, &fifo_handler, 0) != Z_OK);
     assert(z_fifo_channel_sample_new(&closure, &fifo_handler, 1) == Z_OK);
     z_drop(z_move(fifo_handler));
+    z_drop(z_move(closure));
 
     z_owned_ring_handler_sample_t ring_handler;
     assert(z_ring_channel_sample_new(&closure, &ring_handler, 0) != Z_OK);
     assert(z_ring_channel_sample_new(&closure, &ring_handler, 1) == Z_OK);
     z_drop(z_move(ring_handler));
+    z_drop(z_move(closure));
 }
 
 int main(void) {

tests/z_collections_test.c

@@ -18,7 +18,6 @@
 
 #include "zenoh-pico/collections/fifo.h"
 #include "zenoh-pico/collections/lifo.h"
-#include "zenoh-pico/collections/list.h"
 #include "zenoh-pico/collections/ring.h"
 #include "zenoh-pico/collections/string.h"
 
@@ -313,10 +312,10 @@ void int_map_iterator_test(void) {
     _z_str_intmap_t map;
 
     map = _z_str_intmap_make();
-    _z_str_intmap_insert(&map, 10, "A");
-    _z_str_intmap_insert(&map, 20, "B");
-    _z_str_intmap_insert(&map, 30, "C");
-    _z_str_intmap_insert(&map, 40, "D");
+    _z_str_intmap_insert(&map, 10, _z_str_clone("A"));
+    _z_str_intmap_insert(&map, 20, _z_str_clone("B"));
+    _z_str_intmap_insert(&map, 30, _z_str_clone("C"));
+    _z_str_intmap_insert(&map, 40, _z_str_clone("D"));
 
 #define TEST_MAP(map)                                                      \
     {                                                                      \
@@ -346,6 +345,9 @@ void int_map_iterator_test(void) {
 
     TEST_MAP(map2);
 
+    _z_str_intmap_clear(&map);
+    _z_str_intmap_clear(&map2);
+
 #undef TEST_MAP
 }
 

tests/z_data_struct_test.c

@@ -15,6 +15,7 @@
 #include <stddef.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 
 #include "zenoh-pico/api/primitives.h"
 #include "zenoh-pico/api/types.h"
@@ -29,6 +30,7 @@ void entry_list_test(void) {
     _z_transport_peer_entry_list_t *root = _z_transport_peer_entry_list_new();
     for (int i = 0; i < 10; i++) {
         _z_transport_peer_entry_t *entry = (_z_transport_peer_entry_t *)z_malloc(sizeof(_z_transport_peer_entry_t));
+        memset(entry, 0, sizeof(_z_transport_peer_entry_t));
         root = _z_transport_peer_entry_list_insert(root, entry);
     }
     _z_transport_peer_entry_list_t *list = root;
@@ -39,6 +41,7 @@ void entry_list_test(void) {
 
     for (int i = 0; i < 11; i++) {
         _z_transport_peer_entry_t *entry = (_z_transport_peer_entry_t *)z_malloc(sizeof(_z_transport_peer_entry_t));
+        memset(entry, 0, sizeof(_z_transport_peer_entry_t));
         root = _z_transport_peer_entry_list_insert(root, entry);
     }
     assert(_z_transport_peer_entry_list_head(root)->_peer_id == _Z_KEYEXPR_MAPPING_UNKNOWN_REMOTE - 1);
@@ -140,6 +143,8 @@ void str_vec_list_intmap_test(void) {
 
     _z_str_intmap_clear(&map);
     assert(_z_str_intmap_is_empty(&map) == true);
+
+    z_free(s);
 }
 
 void _z_slice_custom_deleter(void *data, void *context) {
@@ -243,6 +248,7 @@ void z_id_to_string_test(void) {
     assert(z_string_len(z_string_loan(&id_str)) == 32);
     assert(strncmp("0f0e0d0c0b0a09080706050403020100", z_string_data(z_string_loan(&id_str)),
                    z_string_len(z_string_loan(&id_str))) == 0);
+    z_string_drop(z_string_move(&id_str));
 }
 
 int main(void) {

tests/z_endpoint_test.c

@@ -42,25 +42,32 @@ int main(void) {
 
     str = _z_string_alias_str("");
     assert(_z_locator_from_string(&lc, &str) == _Z_ERR_CONFIG_LOCATOR_INVALID);
+    _z_locator_clear(&lc);
 
     str = _z_string_alias_str("/");
     assert(_z_locator_from_string(&lc, &str) == _Z_ERR_CONFIG_LOCATOR_INVALID);
+    _z_locator_clear(&lc);
 
     str = _z_string_alias_str("tcp");
     assert(_z_locator_from_string(&lc, &str) == _Z_ERR_CONFIG_LOCATOR_INVALID);
+    _z_locator_clear(&lc);
 
     str = _z_string_alias_str("tcp/");
     assert(_z_locator_from_string(&lc, &str) == _Z_ERR_CONFIG_LOCATOR_INVALID);
+    _z_locator_clear(&lc);
 
     str = _z_string_alias_str("127.0.0.1:7447");
     assert(_z_locator_from_string(&lc, &str) == _Z_ERR_CONFIG_LOCATOR_INVALID);
+    _z_locator_clear(&lc);
 
     str = _z_string_alias_str("tcp/127.0.0.1:7447?");
     assert(_z_locator_from_string(&lc, &str) == _Z_RES_OK);
+    _z_locator_clear(&lc);
 
     // No metadata defined so far... but this is a valid syntax in principle
     str = _z_string_alias_str("tcp/127.0.0.1:7447?invalid=ctrl");
     assert(_z_locator_from_string(&lc, &str) == _Z_RES_OK);
+    _z_locator_clear(&lc);
 
     // Endpoint
     printf(">>> Testing endpoints...\n");
@@ -80,25 +87,32 @@ int main(void) {
 
     str = _z_string_alias_str("");
     assert(_z_endpoint_from_string(&ep, &str) == _Z_ERR_CONFIG_LOCATOR_INVALID);
+    _z_endpoint_clear(&ep);
 
     str = _z_string_alias_str("/");
     assert(_z_endpoint_from_string(&ep, &str) == _Z_ERR_CONFIG_LOCATOR_INVALID);
+    _z_endpoint_clear(&ep);
 
     str = _z_string_alias_str("tcp");
     assert(_z_endpoint_from_string(&ep, &str) == _Z_ERR_CONFIG_LOCATOR_INVALID);
+    _z_endpoint_clear(&ep);
 
     str = _z_string_alias_str("tcp/");
     assert(_z_endpoint_from_string(&ep, &str) == _Z_ERR_CONFIG_LOCATOR_INVALID);
+    _z_endpoint_clear(&ep);
 
     str = _z_string_alias_str("127.0.0.1:7447");
     assert(_z_endpoint_from_string(&ep, &str) == _Z_ERR_CONFIG_LOCATOR_INVALID);
+    _z_endpoint_clear(&ep);
 
     str = _z_string_alias_str("tcp/127.0.0.1:7447?");
     assert(_z_endpoint_from_string(&ep, &str) == _Z_RES_OK);
+    _z_endpoint_clear(&ep);
 
     // No metadata defined so far... but this is a valid syntax in principle
     str = _z_string_alias_str("tcp/127.0.0.1:7447?invalid=ctrl");
     assert(_z_endpoint_from_string(&ep, &str) == _Z_RES_OK);
+    _z_endpoint_clear(&ep);
 
     str = _z_string_alias_str("udp/127.0.0.1:7447#iface=eth0");
     assert(_z_endpoint_from_string(&ep, &str) == _Z_RES_OK);
@@ -116,12 +130,15 @@ int main(void) {
 
     str = _z_string_alias_str("udp/127.0.0.1:7447#invalid=eth0");
     assert(_z_endpoint_from_string(&ep, &str) == _Z_RES_OK);
+    _z_endpoint_clear(&ep);
 
     str = _z_string_alias_str("udp/127.0.0.1:7447?invalid=ctrl#iface=eth0");
     assert(_z_endpoint_from_string(&ep, &str) == _Z_RES_OK);
+    _z_endpoint_clear(&ep);
 
     str = _z_string_alias_str("udp/127.0.0.1:7447?invalid=ctrl#invalid=eth0");
     assert(_z_endpoint_from_string(&ep, &str) == _Z_RES_OK);
+    _z_endpoint_clear(&ep);
 
     return 0;
 }

tests/z_keyexpr_test.c

@@ -314,6 +314,7 @@ void test_canonize(void) {
             printf("  Match: %.*s : %s\n", (int)canon_len, canon, canonized[i]);
             assert(strncmp(canonized[i], canon, canon_len) == 0);
         }
+        free(canon);
     }
 
     for (int i = 0; i < N; i++) {
@@ -331,6 +332,7 @@ void test_canonize(void) {
             assert(strncmp(canonized[i], canon, canon_len) == 0);
         }
         printf("\n");
+        free(canon);
     }
 }
 

tests/z_msgcodec_test.c

@@ -221,18 +221,23 @@ char *gen_str(size_t size) {
     return str;
 }
 
+_z_string_t gen_string(size_t len) {
+    char *str = gen_str(len);
+    _z_string_t ret = _z_string_copy_from_str(str);
+    z_free(str);
+    return ret;
+}
+
 _z_string_svec_t gen_str_array(size_t size) {
     _z_string_svec_t sa = _z_string_svec_make(size);
     for (size_t i = 0; i < size; i++) {
-        _z_string_t s = _z_string_copy_from_str(gen_str(16));
+        _z_string_t s = gen_string(16);
         _z_string_svec_append(&sa, &s);
     }
 
     return sa;
 }
 
-_z_string_t gen_string(size_t len) { return _z_string_alias_str(gen_str(len)); }
-
 _z_locator_array_t gen_locator_array(size_t size) {
     _z_locator_array_t la = _z_locator_array_make(size);
     for (size_t i = 0; i < size; i++) {
@@ -341,7 +346,7 @@ void assert_eq_locator_array(const _z_locator_array_t *left, const _z_locator_ar
         _z_string_t ls = _z_locator_to_string(l);
         _z_string_t rs = _z_locator_to_string(r);
 
-        printf("%s:%s", _z_string_data(&ls), _z_string_data(&rs));
+        printf("%.*s:%.*s", (int)_z_string_len(&ls), _z_string_data(&ls), (int)_z_string_len(&rs), _z_string_data(&rs));
         if (i < left->_len - 1) printf(" ");
 
         _z_string_clear(&ls);