[Bug tree-optimization/96951] New: strncpy truncation warning does not recognize truncation check

[Bug tree-optimization/96951] New: strncpy truncation warning does not recognize truncation check

[fw at gcc dot gnu.org]

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

            Bug ID: 96951
           Summary: strncpy truncation warning does not recognize
                    truncation check
           Product: gcc
           Version: 10.2.1
            Status: UNCONFIRMED
          Keywords: diagnostic
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: fw at gcc dot gnu.org
  Target Milestone: ---

This code example produces a warning:

#include <string.h>

struct buffer {
  char string[10];
};

int
f (struct buffer *p, const char *s)
{
  strncpy (p->string, s, sizeof (p->string));
  if (p->string[sizeof (p->string) - 1] != '\0')
    return -1;
  return 0;
}

t.c: In function ‘f’:
t.c:10:3: warning: ‘strncpy’ specified bound 10 equals destination size
[-Wstringop-truncation]
   10 |   strncpy (p->string, s, sizeof (p->string));
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

There is an explicit truncation check, however, so the warning does not apply.

Suggested by Kim Barrett here:
https://mail.openjdk.java.net/pipermail/hotspot-dev/2020-September/042890.html

-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

[Bug tree-optimization/96951] strncpy truncation warning does not recognize truncation check

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
With the exception of code that needs to ensure the rest of the buffer is
filled with zeros (typically code dealing with passwords etc.), strncpy is
pretty much never something one should use.  Even the above example, if it is
only meant to truncate and return -1 if it doesn't fit, but otherwise just copy
the string, is wasting time on the extra clearing (sure, when it is just 10
bytes that isn't that big deal).

[Bug tree-optimization/96951] strncpy truncation warning does not recognize truncation check

[fw at gcc dot gnu.org]

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

--- Comment #2 from Florian Weimer <fw at gcc dot gnu.org> ---
Then the warning should recommend to use memccpy, perhaps?

  if (memccpy (p->string, s, '\0', sizeof (p->string)) == NULL)
    return -1;
  return 0;

-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

[Bug tree-optimization/96951] strncpy truncation warning does not recognize truncation check

[msebor at gcc dot gnu.org]

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

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
           Severity|normal                      |enhancement
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2020-09-07
             Blocks|                            |88781

--- Comment #3 from Martin Sebor <msebor at gcc dot gnu.org> ---
The warning suppression only considers an explicit nul assignment, not a test
for one.  If in the code the test case was derived from the string member is
not necessarily meant to be a string then declaring it with attribute nonstring
avoids the warning:

struct buffer {
  __attribute__((nonstring)) char string[10];
};

The attribute will trigger other warnings in uses of the member that aren't
clearly nul-terminated.

There are different ways to write the code to avoid the warning and I'm not
sure that any one of them is necessarily the best.  I'd rather users read up on
the problem with strncpy (and strncat) and choose what works best for them.  I
wrote a blog post about this some time ago:
https://developers.redhat.com/blog/2018/05/24/detecting-string-truncation-with-gcc-8/

As for memccpy, doesn't know about it yet but it's on my to-do list to add it
to resolve pr88814.  Ideally, I'd also like to improve Glibc's memccpy to make
it one-pass unless someone beats me to it (hint, hint ;-)


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88781
[Bug 88781] [meta-bug] bogus/missing -Wstringop-truncation warnings

[Bug tree-optimization/96951] strncpy truncation warning does not recognize truncation check

[fw at gcc dot gnu.org]

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

--- Comment #4 from Florian Weimer <fw at gcc dot gnu.org> ---
Thanks.

Checking for the null byte at the end (as in comment 0) currently seems the
most convenient way. Writing those additional null bytes is not entirely free.
As an industry standard, the strlcpy function has emerged as an alternative:

  if (strlcpy (p->string, s, sizeof (p->string)) >= sizeof (p->string))
    return -1;

But as you might recall, glibc consensus procedures currently prevent me from
adding support for strlcpy support to the GNU toolchain.

memccpy is already there, but *very* obscure, so I do not think that is a good
alternative.

Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

[Bug tree-optimization/96951] strncpy truncation warning does not recognize truncation check

[kab at acm dot org]

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

--- Comment #5 from kab at acm dot org ---
(In reply to Martin Sebor from comment #3)
> If in the code the test case was derived from the string
> member is not necessarily meant to be a string then declaring it with
> attribute nonstring avoids the warning:

In the original code the string is intended to be a nul-terminated string. 
Also, its in a struct from a system header, so not modifiable.

> As for memccpy...

memccpy isn't so portable (think Windows, which deprecates it, though under the
control of a macro).  The code from the discussion that led here could cope
with that, but that's not universally true.