build warning in tests/libgearman-1.0/worker_test.cc

[esabol]

Testing master branch build on Ubuntu 18.04 and saw this:

tests/libgearman-1.0/worker_test.cc: In function 'test_return_t gearman_worker_add_options_GEARMAN_WORKER
_GRAB_UNIQ_worker_work(void*)':
tests/libgearman-1.0/worker_test.cc:1642:22: warning: '%d' directive output truncated writing between 1 and 11 bytes into a region of size 0 [-Wformat-truncation=]
 static test_return_t gearman_worker_add_options_GEARMAN_WORKER_GRAB_UNIQ_worker_work(void *)
                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tests/libgearman-1.0/worker_test.cc:1642:22: note: using the range [-2147483648, 2147483647] for directive argument
In file included from /usr/include/stdio.h:862:0,
                 from /usr/include/c++/7/cstdio:42,
                 from ./libtest/test.hpp:60,
                 from tests/libgearman-1.0/worker_test.cc:41:
/usr/include/x86_64-linux-gnu/bits/stdio2.h:65:44: note: '__builtin___snprintf_chk' output between 66 and 76 bytes into a destination of size 64
        __bos (__s), __fmt, __va_arg_pack ());
                                            ^

[esabol]

# gcc --version
gcc (Ubuntu 7.4.0-1ubuntu1~18.04.1) 7.4.0

Refer to issue #258 for the Dockerfile used.

[esabol]

This warning was added in gcc 7.1. See https://gcc.gnu.org/gcc-7/changes.html.

Some discussion of this warning and how to circumvent it can be found here: https://stackoverflow.com/questions/51534284/how-to-circumvent-format-truncation-warning-in-gcc https://stackoverflow.com/questions/57780368/how-to-stop-gcc-complaining-about-directive-output-may-be-truncated-in-snprint

This patch eliminates the warning:

--- /path/to/modified/gearmand/tests/libgearman-1.0/worker_test.cc     2019-11-27 17:02:46.027204000 -0500
+++ ./worker_test.cc    2019-12-15 00:36:46.081880000 -0500
@@ -1640 +1640,2 @@
-  #pragma GCC diagnostic warning "-Wformat-truncation"
+  #pragma GCC diagnostic push
+  #pragma GCC diagnostic ignored "-Wformat-truncation"
@@ -1675,0 +1677,3 @@
+#if __GNUC__ >= 7
+  #pragma GCC diagnostic pop
+#endif

I'm guessing the code is assuming that random() returns an int between 0 and RAND_MAX, which is 32767 on most platforms? I'm not sure that's a strictly safe assumption, but it's probably safe enough. If that's the case, then another option is to add "% 100000u" after random(), like so:

--- /path/to/modified/gearmand/tests/libgearman-1.0/worker_test.cc     2019-11-27 17:02:46.027204000 -0500
+++ ./worker_test.cc    2019-12-15 00:47:42.170765000 -0500
@@ -1648 +1648 @@
-  snprintf(function_name, GEARMAN_FUNCTION_MAX_SIZE, "_%s%d", __func__, int(random())); 
+  snprintf(function_name, GEARMAN_FUNCTION_MAX_SIZE, "_%s%d", __func__, int(random()) % 100000u);
@@ -1651 +1651 @@
-  snprintf(unique_name, GEARMAN_MAX_UNIQUE_SIZE, "_%s%d", __func__, int(random())); 
+  snprintf(unique_name, GEARMAN_MAX_UNIQUE_SIZE, "_%s%d", __func__, int(random()) % 100000u);

That doesn't seem to work though. I just get the following warning instead:

tests/libgearman-1.0/worker_test.cc:1642:22: warning: '%d' directive output truncated writing between 1 and 5 bytes into a region of size 0 [-Wformat-truncation=]
 static test_return_t gearman_worker_add_options_GEARMAN_WORKER_GRAB_UNIQ_worker_work(void *)
                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tests/libgearman-1.0/worker_test.cc:1642:22: note: directive argument in the range [0, 99999]
In file included from /usr/include/stdio.h:862:0,
                 from /usr/include/c++/7/cstdio:42,
                 from ./libtest/test.hpp:60,
                 from tests/libgearman-1.0/worker_test.cc:41:
/usr/include/x86_64-linux-gnu/bits/stdio2.h:65:44: note: '__builtin___snprintf_chk' output between 66 and 70 bytes into a destination of size 64
        __bos (__s), __fmt, __va_arg_pack ());
                                            ^

So we still seem to be 6 bytes off of what the compiler thinks the maximum size of the string could be (70). Does this indicate an actual bug?

If not, should I just submit a PR with the first patch above and just silent the compiler?

[esabol]

@p-alik or @SpamapS : Any thoughts on this?

[p-alik]

since https://github.com/gearman/gearmand/blob/daab15fe0fc8da1eb2653c039ff8a1ff2df8692c/libgearman-1.0/limits.h#L55 and

$ perl -E 'say length "gearman_worker_add_options_GEARMAN_WORKER_GRAB_UNIQ_worker_work"'
63

We could solve the issue in this or similar way:

diff --git a/tests/libgearman-1.0/worker_test.cc b/tests/libgearman-1.0/worker_test.cc
index 2839a6e5..2e92e3a9 100644
--- a/tests/libgearman-1.0/worker_test.cc
+++ b/tests/libgearman-1.0/worker_test.cc
@@ -1636,19 +1636,27 @@ static test_return_t gearman_worker_set_identifier_TEST(void *)
   return TEST_SUCCESS;
 }

-#if __GNUC__ >= 7
-  #pragma GCC diagnostic warning "-Wformat-truncation"
-#endif
 static test_return_t gearman_worker_add_options_GEARMAN_WORKER_GRAB_UNIQ_worker_work(void *)
 { 
   libgearman::Worker worker(libtest::default_port());
   ASSERT_EQ(GEARMAN_SUCCESS, gearman_worker_add_server(&worker, NULL, second_port));

   char function_name[GEARMAN_FUNCTION_MAX_SIZE];
-  snprintf(function_name, GEARMAN_FUNCTION_MAX_SIZE, "_%s%d", __func__, int(random())); 
+  snprintf(function_name, int(GEARMAN_FUNCTION_MAX_SIZE), "_%s%d", __func__, int(random()));

   char unique_name[GEARMAN_MAX_UNIQUE_SIZE];
-  snprintf(unique_name, GEARMAN_MAX_UNIQUE_SIZE, "_%s%d", __func__, int(random())); 
+  int n = snprintf(unique_name, sizeof unique_name, "_%s%06d", std::string(__func__).substr(0, GEARMAN_MAX_UNIQUE_SIZE-7).c_str(), int(random()) % 100000u);
+  if (n < 0)
+  {
+    perror ("snprintf failed");
+    abort ();
+  }
+
+  if ((size_t)n > sizeof unique_name)
+  {
+    perror ("unique_name too long");
+    abort ();
+  }

[esabol]

Yeah, that looks good to me. Do you want to do the PR, @p-alik ?

[p-alik]

Yes, I'll do.