https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88251

eggert at cs dot ucla.edu changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |UNCONFIRMED
         Resolution|INVALID                     |---

--- Comment #3 from eggert at cs dot ucla.edu ---
(In reply to Martin Sebor from comment #1)
> The warning in this case is by design.

Then I’m afraid the design will have to be changed somewhat, as GCC is
generating false alarms about carefully-written code that is correctly
detecting buffer overflow, and this problem is causing GNU applications to
avoid using -Wformat-truncation=2 in code that would otherwise benefit from it.

The example I gave previously was deliberately stripped down and so perhaps
illustrates the issue poorly. Here is a less stripped-down example, taken from
an experimental version of Emacs. The original code uses repeated calls to
snprintf to gradually append to a buffer, each time using the count of the
previous snprintf to figure out where the next snprintf should start, and
returning 0 or 1 depending on whether the string fits. This stripped-down
version shows two iterations of this:

  typedef unsigned long size_t;
  typedef unsigned uid_t;
  typedef unsigned long uintmax_t;

  struct sockaddr_un
    {
      unsigned short sun_family;
      char sun_path[108];
    };
  int snprintf (char *__restrict, size_t, const char *__restrict, ...)
    __attribute__ ((__nothrow__))
    __attribute__ ((__format__ (__printf__, 3, 4)));

  int
  set_socket_name (struct sockaddr_un *server, char const *server_name,
                   char const *tmpdir, uid_t u)
  {
    char *sun_path = server->sun_path;
    enum { sun_pathsize = sizeof server->sun_path };
    int tmpdirlen = (tmpdir
                     ? snprintf (sun_path, sun_pathsize, "%s", tmpdir)
                     : snprintf (sun_path, sun_pathsize, "/tmp/u%u", u));
    if (! (0 <= tmpdirlen && tmpdirlen < sun_pathsize))
      return 0;
    uintmax_t uid = u;
    int remaining = sun_pathsize - tmpdirlen;
    int n = snprintf (&sun_path[tmpdirlen], remaining, "/emacs%lu/%s",
                      uid, server_name);
    if (! (0 <= n && n < remaining))
      return 0;
    return 1;
  }

Compile this with ‘gcc -O2 -S -Wall -Wformat-truncation=2 emacsclient2.i’ and
the output is:

  emacsclient2.i: In function ‘set_socket_name’:
  emacsclient2.i:27:56: warning: ‘/emacs’ directive output truncated writing 6
bytes into a region of size 1 [-Wformat-truncation=]
     int n = snprintf (&sun_path[tmpdirlen], remaining, "/emacs%lu/%s",
                                                         ~^~~~~
  emacsclient2.i:27:54: note: directive argument in the range [0, 4294967295]
     int n = snprintf (&sun_path[tmpdirlen], remaining, "/emacs%lu/%s",
                                                        ^~~~~~~~~~~~~~
  emacsclient2.i:27:54: note: assuming directive output of 1 byte
  emacsclient2.i:27:11: note: ‘snprintf’ output 9 or more bytes (assuming 19)
into a destination of size 1
     int n = snprintf (&sun_path[tmpdirlen], remaining, "/emacs%lu/%s",
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         uid, server_name);
         ~~~~~~~~~~~~~~~~~

Here GCC complains about the second snprintf call, apparently under the
assumption that the caller should first check that ‘remaining’ is at least 9
before the second call to snprintf. But such a check would be silly and hard to
maintain; the code is properly designed to let snprintf do the buffer overflow
check, and there should be no warning here.

Here’s another example, taken from Gnulib. This is a longer version of the
example I started off this bug report with (though it is still stripped down).

  typedef unsigned long size_t;
  extern int snprintf (char *__restrict, size_t, const char *__restrict, ...)
    __attribute__ ((__nothrow__))
    __attribute__ ((__format__ (__printf__, 3, 4)));
  extern int __xpg_strerror_r (int, char *, size_t);
  int
  rpl_strerror_r (int errnum, char *buf, size_t buflen)
  {
    if (buflen <= 1)
      {
        if (buflen)
          *buf = '\0';
        return 34;
      }
    *buf = '\0';
    int ret = __xpg_strerror_r (errnum, buf, buflen);
    if (!*buf && ret == 22)
      snprintf (buf, buflen, "Unknown error %d", errnum);
    return ret;
  }

This general-purpose function should work for any buffer length. It operates by
calling other general-purpose functions (__xpg_strerror_r, snprintf) that are
similar. The ‘buflen <= 1’ business works around a bug in AIX, where
__xpg_strerror_r function mishandles buffer lengths of 0 or 1. The ‘(!*buf &&
ret == 22)’ business works around an incompatibility with some other systems,
where __xpg_strerror_r does not properly fill in the buffer for unknown errors.
Compile this with ‘gcc -O2 -S -Wall -Wformat-truncation=2 strerror_r4.i’ and
the output is:

  strerror_r4.i: In function ‘rpl_strerror_r’:
  strerror_r4.i:18:31: warning: ‘Unknown error ’ directive output truncated
writing 14 bytes into a region of size 2 [-Wformat-truncation=]
       snprintf (buf, buflen, "Unknown error %d", errnum);
                               ~~^~~~~~~~~~~~
  strerror_r4.i:18:5: note: ‘snprintf’ output between 16 and 26 bytes into a
destination of size 2
       snprintf (buf, buflen, "Unknown error %d", errnum);
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Here the message is a false alarm because the snprintf is doing what it is
designed to do (write into a buffer that is possibly too small) and is
obviously safe.

> The warning can also be avoided by using the snprintf return value and taking 
> some action based on it

That would slow the code down, would make it harder to maintain, and would not
make it any safer. Instead of doing this, common practice in GNU applications
is to avoid using -Wformat-truncation=2.

Reply via email to