From 411b31544f22b773b4aad6cdb81faa81dc05e641 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Mon, 23 Aug 2021 14:32:51 -0400 Subject: [PATCH] configure: enable -Wformat=2 MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The -Wformat=2 diagnostic flag on GCC enables the -Wformat-nonliteral -Wformat-security diagnostics, which are useful to catch some format string mistakes. -Wformat-security is also enabled by default with Clang, meaning that there were some warnings only appearing with Clang. Try to enabled the -Wformat=2 flag to make things more consistent across compilers and catch more mistakes. The only issues are these, in tests/regression/ust/linking: CC demo_builtin-demo.o In file included from /usr/include/stdio.h:866, from /home/simark/src/lttng-tools/tests/regression/ust/linking/demo.c:9: /usr/include/bits/stdio2.h: In function ‘sprintf’: /usr/include/bits/stdio2.h:40:35: warning: format not a string literal, argument types not checked [-Wformat-nonliteral] 40 | __va_arg_pack ()); | ^~~~~~~~~~~~~ The reason this appears is that this directory uses -Wsystem-headers, making the compiler show diagnostics in headers considered "system headers". Manually silence those warnings by disabling -Wformat-nonliteral in that specific directory. Change-Id: I4c7991e76b2f5405f3b3397348adb9134de37d41 Signed-off-by: Simon Marchi Signed-off-by: Jérémie Galarneau --- configure.ac | 1 + src/bin/lttng/commands/list.cpp | 19 ++++--------------- src/common/macros.h | 8 ++++++++ src/common/utils.cpp | 3 +++ src/common/utils.h | 5 ++++- src/lib/lttng-ctl/lttng-ctl-health.cpp | 3 +++ tests/regression/ust/linking/Makefile.am | 5 ++++- tests/unit/test_payload.cpp | 12 ++++++------ 8 files changed, 33 insertions(+), 23 deletions(-) diff --git a/configure.ac b/configure.ac index 083ec34f3..ae4374dca 100644 --- a/configure.ac +++ b/configure.ac @@ -72,6 +72,7 @@ m4_define([WARN_FLAGS_LIST], [ dnl -Wshadow dnl -Wno-gnu-folding-constant dnl -Wsuggest-attribute=format dnl + -Wformat=2 dnl dnl GCC enables this with -Wall in C++, and that generates a dnl lot of warnings that have on average a low value to fix. -Wno-sign-compare dnl diff --git a/src/bin/lttng/commands/list.cpp b/src/bin/lttng/commands/list.cpp index 4bb681e09..28063ebc9 100644 --- a/src/bin/lttng/commands/list.cpp +++ b/src/bin/lttng/commands/list.cpp @@ -338,13 +338,8 @@ static void print_events(struct lttng_event *event) if (ret) { filter_msg = strdup(" [failed to retrieve filter]"); } else if (filter_str) { - const char * const filter_fmt = " [filter: '%s']"; - - filter_msg = (char *) malloc(strlen(filter_str) + - strlen(filter_fmt) + 1); - if (filter_msg) { - sprintf(filter_msg, filter_fmt, - filter_str); + if (asprintf(&filter_msg, " [filter: '%s']", filter_str) == -1) { + filter_msg = NULL; } } @@ -1151,14 +1146,8 @@ static int list_session_agent_events(void) if (ret) { filter_msg = strdup(" [failed to retrieve filter]"); } else if (filter_str) { - const char * const filter_fmt = - " [filter: '%s']"; - - filter_msg = (char *) malloc(strlen(filter_str) + - strlen(filter_fmt) + 1); - if (filter_msg) { - sprintf(filter_msg, filter_fmt, - filter_str); + if (asprintf(&filter_msg, " [filter: '%s']", filter_str) == -1) { + filter_msg = NULL; } } diff --git a/src/common/macros.h b/src/common/macros.h index 99d4fb31f..af9cb7e19 100644 --- a/src/common/macros.h +++ b/src/common/macros.h @@ -79,6 +79,10 @@ void *zmalloc(size_t len) #define ATTR_FORMAT_PRINTF(_string_index, _first_to_check) \ __attribute__((format(printf, _string_index, _first_to_check))) +/* Attribute suitable to tag functions as having strftime()-like arguments. */ +#define ATTR_FORMAT_STRFTIME(_string_index) \ + __attribute__((format(strftime, _string_index, 0))) + /* Macros used to ignore specific compiler diagnostics. */ #define DIAGNOSTIC_PUSH _Pragma("GCC diagnostic push") @@ -87,10 +91,14 @@ void *zmalloc(size_t len) #if defined(__clang__) /* Clang */ # define DIAGNOSTIC_IGNORE_SUGGEST_ATTRIBUTE_FORMAT +# define DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL \ + _Pragma("GCC diagnostic ignored \"-Wformat-nonliteral\"") #else /* GCC */ # define DIAGNOSTIC_IGNORE_SUGGEST_ATTRIBUTE_FORMAT \ _Pragma("GCC diagnostic ignored \"-Wsuggest-attribute=format\"") +# define DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL \ + _Pragma("GCC diagnostic ignored \"-Wformat-nonliteral\"") #endif /* diff --git a/src/common/utils.cpp b/src/common/utils.cpp index 8aa4ff9f9..93014627f 100644 --- a/src/common/utils.cpp +++ b/src/common/utils.cpp @@ -1194,7 +1194,10 @@ size_t utils_get_current_time_str(const char *format, char *dst, size_t len) /* Get date and time for session path */ time(&rawtime); timeinfo = localtime(&rawtime); + DIAGNOSTIC_PUSH + DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL ret = strftime(dst, len, format, timeinfo); + DIAGNOSTIC_POP if (ret == 0) { ERR("Unable to strftime with format %s at dst %p of len %zu", format, dst, len); diff --git a/src/common/utils.h b/src/common/utils.h index a3250639b..beca1fb4b 100644 --- a/src/common/utils.h +++ b/src/common/utils.h @@ -41,7 +41,10 @@ int utils_get_count_order_u32(uint32_t x); int utils_get_count_order_u64(uint64_t x); const char *utils_get_home_dir(void); char *utils_get_user_home_dir(uid_t uid); -size_t utils_get_current_time_str(const char *format, char *dst, size_t len); + +size_t utils_get_current_time_str(const char *format, char *dst, size_t len) + ATTR_FORMAT_STRFTIME(1); + int utils_get_group_id(const char *name, bool warn, gid_t *gid); char *utils_generate_optstring(const struct option *long_options, size_t opt_count); diff --git a/src/lib/lttng-ctl/lttng-ctl-health.cpp b/src/lib/lttng-ctl/lttng-ctl-health.cpp index 2a992830d..8a703b019 100644 --- a/src/lib/lttng-ctl/lttng-ctl-health.cpp +++ b/src/lib/lttng-ctl/lttng-ctl-health.cpp @@ -214,8 +214,11 @@ int set_health_socket_path(struct lttng_health *lh, home = "/tmp"; } + DIAGNOSTIC_PUSH + DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL ret = snprintf(lh->health_sock_path, sizeof(lh->health_sock_path), home_str, home); + DIAGNOSTIC_POP if ((ret < 0) || (ret >= sizeof(lh->health_sock_path))) { return -ENOMEM; } diff --git a/tests/regression/ust/linking/Makefile.am b/tests/regression/ust/linking/Makefile.am index bf0a517ad..424afd1c5 100644 --- a/tests/regression/ust/linking/Makefile.am +++ b/tests/regression/ust/linking/Makefile.am @@ -2,7 +2,10 @@ # -Wsystem-headers is needed to print warnings in the tracepoint # description file. -AM_CPPFLAGS += -I$(srcdir) -Wsystem-headers +# +# However, we see some -Wformat-nonliteral warnings in some system headers, +# so disable that. +AM_CPPFLAGS += -I$(srcdir) -Wsystem-headers -Wno-format-nonliteral # Set LIBS to nothing so the application does not link on useless # libraries. diff --git a/tests/unit/test_payload.cpp b/tests/unit/test_payload.cpp index 1c79b8931..53565f5eb 100644 --- a/tests/unit/test_payload.cpp +++ b/tests/unit/test_payload.cpp @@ -107,14 +107,14 @@ static void test_fd_push_pop_imbalance(void) } handle = lttng_payload_view_pop_fd_handle(&view); - ok(!handle, test_description); + ok(!handle, "%s", test_description); fd_handle_put(handle); } lttng_payload_reset(&payload); return; fail: - fail(test_description); + fail("%s", test_description); lttng_payload_reset(&payload); } @@ -158,12 +158,12 @@ static void test_fd_pop_fd_root_views(void) } lttng_payload_reset(&payload); - pass(test_description); + pass("%s", test_description); fd_handle_put(handle); return; fail: lttng_payload_reset(&payload); - fail(test_description); + fail("%s", test_description); fd_handle_put(handle); } @@ -212,7 +212,7 @@ static void test_fd_pop_fd_descendant_views(void) } lttng_payload_reset(&payload); - pass(test_description); + pass("%s", test_description); fd_handle_put(handle1); fd_handle_put(handle2); fd_handle_put(view_handle1); @@ -220,7 +220,7 @@ static void test_fd_pop_fd_descendant_views(void) return; fail: lttng_payload_reset(&payload); - fail(test_description); + fail("%s", test_description); fd_handle_put(handle1); fd_handle_put(handle2); fd_handle_put(view_handle1); -- 2.34.1