Re: [PATCH] warn for strlen of arrays with missing nul (PR 86552)

[Martin Sebor]

On 07/26/2018 02:53 AM, Bernd Edlinger wrote:
@@ -567,13 +597,17 @@ string_length (const void *ptr, unsigned eltsize, 
unsigned maxelts)
    accesses.  Note that this implies the result is not going to be emitted
   into the instruction stream.
+   When ARR is non-null and the string is not properly nul-terminated,
+   set *ARR to the declaration of the outermost constant object whose
+   initializer (or one of its elements) is not nul-terminated.
+
    The value returned is of type `ssizetype'.

    Unfortunately, string_constant can't access the values of const char
    arrays with initializers, so neither can we do so here.  */

Maybe drop that sentence when it is no longer true?

I believe the sentence means is that folding the length of arrays
like this isn't handled:

  const char a[] = { 'a', 'b', '\0' };

because they are represented not as STRING_CST but rather as
aggregate CONSTRUCTORs.

Adding this handling has been something I've been meaning to
do for some time and this seems like a good opportunity to do
it.  I'm testing a patch with this enhancement.

 tree
-c_strlen (tree src, int only_value)
+c_strlen (tree src, int only_value, tree *arr /* = NULL */)
 {
   STRIP_NOPS (src);
   if (TREE_CODE (src) == COND_EXPR
@@ -581,24 +615,31 @@ c_strlen (tree src, int only_value)
     {
       tree len1, len2;

-      len1 = c_strlen (TREE_OPERAND (src, 1), only_value);
-      len2 = c_strlen (TREE_OPERAND (src, 2), only_value);
+      len1 = c_strlen (TREE_OPERAND (src, 1), only_value, arr);
+      len2 = c_strlen (TREE_OPERAND (src, 2), only_value, arr);

Wow, what happens here if the first operand is non-zero terminated and the 
second is zero-terminated?

It depends on the size of the first array and on the length
of the second.  This test case triggers a warning:

  const char a[2][3] = { "123", "12" };

  int f (int i)
  {
    return __builtin_strlen (i < 0 ? a[0] : a[1]);
  }

  warning: ‘__builtin_strlen’ argument missing terminating nul 
[-Wstringop-overflow=]
  note: referenced argument declared here
  const char a[2][3] = { "123", "12" };
             ^

but this one didn't:

  const char a[3] = "123";
  const char b[4] = "123";

  int f (int i)
  {
    return __builtin_strlen (i < 0 ? a : b);
  }

The usual arguments both for and against issuing a diagnostic
here apply but I think it makes more sense to err on the side
of caution and diagnose them than not so I enhanced the patch
to do that, as well as handle a few more strlen contexts.

Martin