This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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

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.

-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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]