configure: enable -Wformat=2

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 <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

diff --git a/configure.ac b/configure.ac
index 083ec34f3cd3023b7e93fbf4be42705aa16f4d21..ae4374dca6188789550573a29978989e905b53d7 100644 (file)
--- 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 4bb681e09c1a9dca8850242dfc156c98beb8e62e..28063ebc9cfdc631084621cc6fa25c4492b55913 100644 (file)
--- 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 99d4fb31f600fc2776348623e1821b0f76fba1b9..af9cb7e19bca6ac8eb632e2a5b6941d04aff285c 100644 (file)
--- 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 8aa4ff9f91efab9155858559f2685bf6cfd9d835..93014627fe5162bf8d49c4813074debe3edf1ca7 100644 (file)
--- 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 a3250639becd448d38a10e61ef47287709256204..beca1fb4b9bb9069a93f144a01afd769ee11a9f5 100644 (file)
--- 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 2a992830d5a41209cb99c162b71c6d2a4e38f57c..8a703b019e14b04a8536aa26dc9d35c28c98fb66 100644 (file)
--- 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 bf0a517ad3b33b48ca69ff17b9a91a527dafb868..424afd1c5776a8cc89bec902d701f6ecf66c0a9e 100644 (file)
--- 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 1c79b8931012b8135ee4d890bd0a9b66faf0a33d..53565f5eb58db06345c7de63edd8d558ecc61083 100644 (file)
--- 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);