Bug 89678 - Bogus -Wstringop-truncation on strncat with bound that depends on strlen of source
Summary: Bogus -Wstringop-truncation on strncat with bound that depends on strlen of s...
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 9.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks: Wstringop-truncation
  Show dependency treegraph
 
Reported: 2019-03-12 11:12 UTC by Martin Liška
Modified: 2022-03-17 20:02 UTC (History)
0 users

See Also:
Host:
Target:
Build:
Known to work:
Known to fail: 8.3.0, 9.0
Last reconfirmed: 2019-03-12 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Liška 2019-03-12 11:12:30 UTC
I see following isolated from bctoolbox package:

$ cat log.c
#include <stdlib.h>
#include <string.h>

char *bctbx_strcat_vprintf(char *dst, char *ret) {
  size_t dstlen, retlen;

  if (!dst)
    return ret;

  dstlen = strlen(dst);
  retlen = strlen(ret);

  if ((dst = __builtin_realloc(dst, dstlen + retlen + 1)) != 0) {
    strncat(dst, ret, retlen);
    dst[dstlen + retlen] = '\0';
    return dst;
  } else {
    return 0;
  }
}

char *a;
char *b;

int main(int argc, char **argv) {
  a = malloc(10);
  b = malloc(10);

  if (argc) {
    strcpy(a, "one");
    strcpy(b, "two");
  }

  __builtin_printf(bctbx_strcat_vprintf(a, b));
  return 0;
}

$ gcc log.c -c -O2 -Wstringop-truncation
log.c: In function ‘bctbx_strcat_vprintf.part.0’:
log.c:14:5: warning: ‘strncat’ output truncated before terminating nul copying as many bytes from a string as its length [-Wstringop-truncation]
   14 |     strncat(dst, ret, retlen);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~
log.c:11:12: note: length computed here
   11 |   retlen = strlen(ret);
      |            ^~~~~~~~~~~
Comment 1 Martin Sebor 2019-03-12 18:11:14 UTC
The test case boils down to this:

  void f (char *p, char *q)
  {
    unsigned m = __builtin_strlen (p);
    unsigned n = __builtin_strlen (q);

    __builtin_strncat (p, q, n);
    p[m + n] = '\0';
  }

The warning in these cases is by design: it points out that the bound in the strncat call depends on the length of the source string (which is usually a bug).  The use case the warning is designed not to trigger for is the one described in the CERT strncpy and strncat article: https://www.us-cert.gov/bsi/articles/knowledge/coding-practices/strncpy-and-strncat, i.e.,

  strncat (dest, source, dest_size - strlen (dest) - 1);

In general, the warning assumes that strncpy and strncat are used in meaningful (though perhaps incorrect) ways.  In the test case in comment #0, the use of strncat is correct but pointless.  It's clear that the size of the buffer is sufficient for the concatenation of the two strings, so there is no point in using strncat to constrain the copy.  The code would be more clearly and also more efficiently written like so:

  if ((dst = __builtin_realloc(dst, dstlen + retlen + 1)) != 0) {
    strcpy(dst + dstlen, ret);

That said, a patch I'm testing for pr88780 relaxes the warning to avoid triggering for code like this.
Comment 2 Martin Sebor 2022-03-17 20:02:35 UTC
I'm no longer working on this.