Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add AddressSanitizer to CI tests #812

Merged
merged 4 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/build-check.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
8 changes: 7 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -453,6 +454,11 @@ if(BUILD_EXAMPLES)
add_subdirectory(examples)
endif()

if(ASAN)
add_compile_options(-fsanitize=address)
Copy link
Contributor

@DenisBiryukov91 DenisBiryukov91 Nov 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not correct, since -fsanitize is a clang/gcc feature only and thus will fail with other compilers (for example for MSVC it is /fsanitize=address)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't tested it with MSVC, but as far as I see there
https://learn.microsoft.com/en-us/cpp/build/reference/compiler-options?view=msvc-170
and there
https://learn.microsoft.com/en-us/cpp/sanitizers/asan?view=msvc-170
this type of option is also correct.
In addition, this is an optional option that does not have to be enabled for all compilers.

add_link_options(-fsanitize=address)
endif()

if(UNIX OR MSVC)
if(BUILD_TOOLS)
set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/tools)
Expand Down
6 changes: 5 additions & 1 deletion GNUmakefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion src/collections/refcount.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
7 changes: 3 additions & 4 deletions src/protocol/keyexpr.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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: {
Expand Down
1 change: 1 addition & 0 deletions tests/z_api_encoding_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
7 changes: 4 additions & 3 deletions tests/z_channels_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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) \
Expand Down Expand Up @@ -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) {
Expand Down
12 changes: 7 additions & 5 deletions tests/z_collections_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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) \
{ \
Expand Down Expand Up @@ -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
}

Expand Down
6 changes: 6 additions & 0 deletions tests/z_data_struct_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
17 changes: 17 additions & 0 deletions tests/z_endpoint_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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);
Expand All @@ -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;
}
2 changes: 2 additions & 0 deletions tests/z_keyexpr_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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++) {
Expand All @@ -331,6 +332,7 @@ void test_canonize(void) {
assert(strncmp(canonized[i], canon, canon_len) == 0);
}
printf("\n");
free(canon);
}
}

Expand Down
13 changes: 9 additions & 4 deletions tests/z_msgcodec_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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++) {
Expand Down Expand Up @@ -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);
Expand Down
Loading
Loading