@cryptotaxi247 / netdata-1 / commits / e2c7888d6

Address sanitizer through CMake and use it for unit tests. (#16748)

* Disable address sanitizer for some functions. These functions report some issues when running the address sanitizer with `-W unittest`. We want to run the sanitized binary on Github PRs to catch newly-introduced issues. FIXMEs were added so that we know which ones already existed prior to this change. * Add cmake option to use address sanitizer * Run unit tests with address sanitizer. * Specify attribute before the function declaration. * Disable hardening flags.

vkalintiris committed Jan 10, 2024 at 17:07 UTC e2c7888d6f228acd6813a7c57fadd355a073b780
6 files changed +62 -6
CMakeLists.txt
+13 -1
@@ -71,6 +71,12 @@ if(NOT CMAKE_BUILD_TYPE)
71 set(CMAKE_BUILD_TYPE "Release")
72 endif()
73
74 +option(ENABLE_ADDRESS_SANITIZER "enable address sanitizer" False)
75 +
76 +if(ENABLE_ADDRESS_SANITIZER)
77 + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=address")
78 +endif()
79 +
80 set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fexceptions")
81 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${CMAKE_C_FLAGS}")
82
@@ -114,7 +120,13 @@ option(ENABLE_LOGS_MANAGEMENT_TESTS "enable logs management tests" True)
120
121 include(CheckCCompilerFlag)
122
117 -option(DISABLE_HARDENING "disable adding extra compiler flags for hardening" False)
123 +# Disable hardening for debug builds by default.
124 +if(CMAKE_BUILD_TYPE STREQUAL "Debug")
125 + option(DISABLE_HARDENING "disable adding extra compiler flags for hardening" TRUE)
126 +else()
127 + # FIXME: Until https://github.com/netdata/netdata/pull/16731 resolves the issue
128 + option(DISABLE_HARDENING "disable adding extra compiler flags for hardening" TRUE)
129 +endif()
130
131 set(EXTRA_HARDENING_FLAGS "")
132
database/rrdlabels.c
+5
@@ -694,6 +694,11 @@ static RRDLABEL *rrdlabels_find_label_with_key_unsafe(RRDLABELS *labels, RRDLABE
694 // ----------------------------------------------------------------------------
695 // rrdlabels_add()
696
697 +/*
698 + * FIXME: Attribute added because address sanitizer reports an issue when
699 + * running the agent with `-W unittest`.
700 +*/
701 +__attribute__((no_sanitize("address")))
702 static void labels_add_already_sanitized(RRDLABELS *labels, const char *key, const char *value, RRDLABEL_SRC ls)
703 {
704 RRDLABEL *new_label = add_label_name_value(key, value);
libnetdata/buffer/buffer.h
+7 -1
@@ -307,7 +307,13 @@ static inline void buffer_memcat(BUFFER *wb, const void *mem, size_t bytes) {
307 buffer_overflow_check(wb);
308 }
309
310 -static inline void buffer_json_strcat(BUFFER *wb, const char *txt) {
310 +/*
311 + * FIXME: Attribute added because address sanitizer reports a log-related
312 + * issue when running the agent with `-W unittest`.
313 +*/
314 +__attribute__((no_sanitize("address")))
315 +static inline void buffer_json_strcat(BUFFER *wb, const char *txt)
316 +{
317 if(unlikely(!txt || !*txt)) return;
318
319 const unsigned char *t = (const unsigned char *)txt;
libnetdata/dictionary/dictionary.c
+5
@@ -3643,6 +3643,11 @@ size_t dictionary_unittest_views(void) {
3643 return errors;
3644 }
3645
3646 +/*
3647 + * FIXME: a dictionary-related leak is reported when running the address
3648 + * sanitizer. Need to investigate if it's introduced by the unit-test itself,
3649 + * or the dictionary implementation.
3650 +*/
3651 int dictionary_unittest(size_t entries) {
3652 if(entries < 10) entries = 10;
3653
libnetdata/log/log.c
+28 -4
@@ -1563,7 +1563,13 @@ static void priority_annotator(BUFFER *wb, const char *key, struct log_field *lf
1563 buffer_strcat(wb, nd_log_id2priority(pri));
1564 }
1565
1566 -static bool needs_quotes_for_logfmt(const char *s) {
1566 +/*
1567 + * FIXME: Attribute added because address sanitizer reports a log-related
1568 + * issue when running the agent with `-W unittest`.
1569 +*/
1570 +__attribute__((no_sanitize("address")))
1571 +static bool needs_quotes_for_logfmt(const char *s)
1572 +{
1573 static bool safe_for_logfmt[256] = {
1574 [' '] = true, ['!'] = true, ['"'] = false, ['#'] = true, ['$'] = true, ['%'] = true, ['&'] = true,
1575 ['\''] = true, ['('] = true, [')'] = true, ['*'] = true, ['+'] = true, [','] = true, ['-'] = true,
@@ -1594,7 +1600,13 @@ static bool needs_quotes_for_logfmt(const char *s) {
1600 return false;
1601 }
1602
1597 -static void string_to_logfmt(BUFFER *wb, const char *s) {
1603 +/*
1604 + * FIXME: Attribute added because address sanitizer reports a log-related
1605 + * issue when running the agent with `-W unittest`.
1606 +*/
1607 +__attribute__((no_sanitize("address")))
1608 +static void string_to_logfmt(BUFFER *wb, const char *s)
1609 +{
1610 bool spaces = needs_quotes_for_logfmt(s);
1611
1612 if(spaces)
@@ -1606,7 +1618,13 @@ static void string_to_logfmt(BUFFER *wb, const char *s) {
1618 buffer_fast_strcat(wb, "\"", 1);
1619 }
1620
1609 -static void nd_logger_logfmt(BUFFER *wb, struct log_field *fields, size_t fields_max) {
1621 +/*
1622 + * FIXME: Attribute added because address sanitizer reports a log-related
1623 + * issue when running the agent with `-W unittest`.
1624 +*/
1625 +__attribute__((no_sanitize("address")))
1626 +static void nd_logger_logfmt(BUFFER *wb, struct log_field *fields, size_t fields_max)
1627 +{
1628
1629 // --- FIELD_PARSER_VERSIONS ---
1630 //
@@ -2226,7 +2244,13 @@ static ND_LOG_SOURCES nd_log_validate_source(ND_LOG_SOURCES source) {
2244 // ----------------------------------------------------------------------------
2245 // public API for loggers
2246
2229 -void netdata_logger(ND_LOG_SOURCES source, ND_LOG_FIELD_PRIORITY priority, const char *file, const char *function, unsigned long line, const char *fmt, ... ) {
2247 +/*
2248 + * FIXME: Attribute added because address sanitizer reports a log-related
2249 + * issue when running the agent with `-W unittest`.
2250 +*/
2251 +__attribute__((no_sanitize("address")))
2252 +void netdata_logger(ND_LOG_SOURCES source, ND_LOG_FIELD_PRIORITY priority, const char *file, const char *function, unsigned long line, const char *fmt, ... )
2253 +{
2254 int saved_errno = errno;
2255 source = nd_log_validate_source(source);
2256
tests/run-unit-tests.sh
+4
@@ -20,6 +20,8 @@
20
21 install_netdata() {
22 echo "Installing Netdata"
23 +
24 + NETDATA_CMAKE_OPTIONS="-DCMAKE_BUILD_TYPE=Debug -DENABLE_ADDRESS_SANITIZER=On" \
25 fakeroot ./netdata-installer.sh \
26 --install-prefix "$HOME" \
27 --dont-wait \
@@ -32,6 +34,8 @@ install_netdata() {
34
35 c_unit_tests() {
36 echo "Running C code unit tests"
37 +
38 + ASAN_OPTIONS=detect_leaks=0 \
39 "$HOME"/netdata/usr/sbin/netdata -W unittest
40 }
41