Bug 62181 - [C/C++] Expected new warning: "adding 'char' to a string does not append to the string" [-Wstring-plus-int]
Summary: [C/C++] Expected new warning: "adding 'char' to a string does not append to t...
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 5.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic, easyhack, patch
: 49481 78679 (view as bug list)
Depends on:
Blocks: new-warning
  Show dependency treegraph
 
Reported: 2014-08-19 09:17 UTC by Tobias Burnus
Modified: 2018-09-30 00:44 UTC (History)
9 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2014-10-13 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tobias Burnus 2014-08-19 09:17:44 UTC
CLANG warns for the following code, GCC doesn't (with the options tried):


foo.cc:4:24: warning: adding 'char' to a string does not append to the string [-Wstring-plus-int]
  const char *a = "aa" + 'a';
                  ~~~~~^~~~~
foo.cc:4:24: note: use array indexing to silence this warning
  const char *a = "aa" + 'a';
                       ^
                  &    [    ]
1 warning generated.


Interestingly, the warning does not trigger for integer literals - only for single-character literals and for long/int/char returning functions. That's probably because  "abcd" + 1  is the pointer address shifted by one, i.e. pointing to "bcd". One can also argue whether the note is helpful or not.


In the real-world code, the LHS was a string class and the conversion of "aa" to the string class was missing such that the "operator+" wasn't triggered.


Test case, gives three warnings (and three notes) with CLANG:

#include <stdio.h>

char bar() {
  return 1;
}
int foobar() {
  return 1;
}

int main() {
  const char *a = "aa" + 'c';
  const char *b = "bb" + bar();
  const char *c = "cc" + foobar();
  printf("%s, %s, %s\n", a, b, c);
  return 0;
}
Comment 1 Manuel López-Ibáñez 2014-10-13 00:28:54 UTC
Confirmed. It does warn for "aaa" + x, when x is char/int.
Comment 2 Manuel López-Ibáñez 2014-10-13 00:34:46 UTC
In both FEs there is a function build_binary_op. Put a break-point inside, examine the operands with debug_tree and figure out how to detect this case and when to warn. Add the new warning option to c-family/c.opt and document it in gcc/doc/invoke.texi.

I think the note is useful but we cannot give the fix-it hints yet. Add a FIXME in the code on top of the inform call.
Comment 3 Eric Gallager 2016-08-22 13:40:38 UTC
I think a better fixit would be to suggest using strcat or strncat or strlcat or something, since that actually appends to the string like the programmer probably intended to do. Abusing array indexing like that just looks wrong.
Comment 4 Xi Ruoyao 2017-03-15 06:50:57 UTC
In the recent version of clang this diagnostic was separated to two
warnings, -Wstring-plus-char and -Wstring-plus-int.

-Wstring-plus-char warns

  "adding <char type> to a string pointer does not append to the string"

where <char type> may be char, unsigned char in C/C++, and char16_t,
char32_t, wchar_t in C++ 2011 and above.

-Wstring-plus-int warns

  "adding <int type> to a string does not append to the string"

where the string is a literal and the integer is not a literal (like
Tobias showed in the description).

It's more clear to use two different options.  But the two warnings may
be reported at the same location.  It seems a little redundant.  Should
we silence one while the other has been reported?
Comment 5 Xi Ruoyao 2017-03-15 06:58:14 UTC
Some clang-5.0 warning case:

~~~
Wstring-plus-int.c:20:24: warning: adding 'int' to a string does not append to the string [-Wstring-plus-int]
  const char *a = "aa" + 'a'; /* { dg-warning "does not append" } */
                  ~~~~~^~~~~
Wstring-plus-int.c:20:24: note: use array indexing to silence this warning
  const char *a = "aa" + 'a'; /* { dg-warning "does not append" } */
                       ^
                  &    [    ]
Wstring-plus-int.c:20:24: warning: adding 'char' to a string pointer does not append to the string [-Wstring-plus-char]
  const char *a = "aa" + 'a'; /* { dg-warning "does not append" } */
                  ~~~~~^~~~~
Wstring-plus-int.c:20:24: note: use array indexing to silence this warning
  const char *a = "aa" + 'a'; /* { dg-warning "does not append" } */
                       ^
                  &    [    ]
Wstring-plus-int.c:21:28: warning: adding 'int' to a string does not append to the string [-Wstring-plus-int]
  const wchar_t *b = L"aa" + L'a'; /* { dg-warning "does not append" } */
                     ~~~~~~^~~~~~
Wstring-plus-int.c:21:28: note: use array indexing to silence this warning
  const wchar_t *b = L"aa" + L'a'; /* { dg-warning "does not append" } */
                           ^
                     &     [     ]
Wstring-plus-int.c:22:24: warning: adding 'int' to a string does not append to the string [-Wstring-plus-int]
  const char *e = "aa" + getchar(); /* { dg-warning "does not append" } */
                  ~~~~~^~~~~~~~~~~
Wstring-plus-int.c:22:24: note: use array indexing to silence this warning
  const char *e = "aa" + getchar(); /* { dg-warning "does not append" } */
                       ^
                  &    [          ]
Wstring-plus-int.c:25:21: warning: adding 'char' to a string pointer does not append to the string [-Wstring-plus-char]
  const char *f = a + 'a'; /* { dg-warning "does not append" } */
                  ~~^~~~~
Wstring-plus-int.c:25:21: note: use array indexing to silence this warning
  const char *f = a + 'a'; /* { dg-warning "does not append" } */
                    ^
                  & [    ]
Wstring-plus-int.c:26:23: warning: adding 'char' to a string pointer does not append to the string [-Wstring-plus-char]
  const char *g = 'a' + a; /* { dg-warning "does not append" } */
                  ~~~~^~~
Wstring-plus-int.c:26:23: note: use array indexing to silence this warning
6 warnings generated.
~~~

Note the both warnings are reported at line 22.

By the way clang -Wstring-plus-char doesn't warn for `a += 'a'`.  I think
we should handle it.

I've submitted a series of patches about this PR.  I'll adjust them to be
like clang-5.0 and resubmit them in stage 1.
Comment 6 Martin Sebor 2017-03-15 17:29:16 UTC
With constant arguments (or those whose value or range is known), GCC should warn on the first declaration in comment #0 (copied below) not necessarily because the addition doesn't append 'c' to "aa" (i.e., with -Wstring-plus-char warns) but mainly because it results in an invalid pointer.

  const char *a = "aa" + 'c';

GCC should warn for a constant operand that results in an out-of-bounds pointer regardless of its type, such as the following:

  const char *a = "aa" + 4;

GCC could (and, in my view, should) also warn on the following where the value of i isn't known but where its range is such that the addition will never result in a valid pointer:

  void f (int i)
  {
    if (i < 4) return;
    const char *a = "aa" + i;
    ...
  }
Comment 7 Xi Ruoyao 2017-03-15 18:59:52 UTC
(In reply to Martin Sebor from comment #6)
> With constant arguments (or those whose value or range is known), GCC should
> warn on the first declaration in comment #0 (copied below) not necessarily
> because the addition doesn't append 'c' to "aa" (i.e., with
> -Wstring-plus-char warns) but mainly because it results in an invalid
> pointer.
> 
>   const char *a = "aa" + 'c';
> 
> GCC should warn for a constant operand that results in an out-of-bounds
> pointer regardless of its type, such as the following:
> 
>   const char *a = "aa" + 4;
> 

Clang-5.0 is doing this now for -Wstring-plus-int.  I'll try to do this.

> GCC could (and, in my view, should) also warn on the following where the
> value of i isn't known but where its range is such that the addition will
> never result in a valid pointer:
> 
>   void f (int i)
>   {
>     if (i < 4) return;
>     const char *a = "aa" + i;
>     ...
>   }

We should make a new PR requesting for clang -Warray-bounds as well.  It's
a part of meta-bug PR30334.
Comment 8 Xi Ruoyao 2017-03-15 19:03:05 UTC
(In reply to Xi Ruoyao from comment #7)

> We should make a new PR requesting for clang -Warray-bounds as well.  It's
> a part of meta-bug PR30334.

Sorry. We have -Warray-bounds, but not as well as clang's.  For example clang
warns for "aa"[4], but GCC doesn't.
Comment 9 Martin Sebor 2017-03-15 19:39:35 UTC
(In reply to Xi Ruoyao from comment #8)
> (In reply to Xi Ruoyao from comment #7)
> 
> > We should make a new PR requesting for clang -Warray-bounds as well.  It's
> > a part of meta-bug PR30334.
> 
> Sorry. We have -Warray-bounds, but not as well as clang's.  For example clang
> warns for "aa"[4], but GCC doesn't.

Right.  It only warns on arrays, and only with optimization.  That seems to be because array_at_struct_end_p() in tree.c (called from check_array_ref() in tree-vrp.c) doesn't handle ARRAY_REF with a STRING_CST operand correctly.  It treats it as if it were a reference to an array at the end of a structure.  This only superficially tested change lets GCC warn on that case as well (as long as the result is used).  To detect this without optimization -Warray-bounds would need to also do some checking much earlier.

diff --git a/gcc/tree.c b/gcc/tree.c
index 8f87e7c..5fcbf65 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -13229,6 +13229,9 @@ array_at_struct_end_p (tree ref, bool allow_compref)
       ref = TREE_OPERAND (ref, 0);
     }
 
+  if (TREE_CODE (ref) == STRING_CST)
+    return false;
+
   tree size = NULL;
 
   if (TREE_CODE (ref) == MEM_REF
Comment 10 Xi Ruoyao 2017-06-13 21:27:47 UTC
A series of patch sent to gcc-patches:

<https://gcc.gnu.org/ml/gcc-patches/2017-06/msg00729.html>
Comment 11 Eric Gallager 2018-02-28 07:26:52 UTC
(In reply to Xi Ruoyao from comment #8)
> (In reply to Xi Ruoyao from comment #7)
> 
> > We should make a new PR requesting for clang -Warray-bounds as well.  It's
> > a part of meta-bug PR30334.
> 
> Sorry. We have -Warray-bounds, but not as well as clang's.  For example clang
> warns for "aa"[4], but GCC doesn't.

In that case it's a different meta-bug, PR56456
Comment 12 Manuel López-Ibáñez 2018-09-16 17:33:18 UTC
*** Bug 49481 has been marked as a duplicate of this bug. ***
Comment 13 Manuel López-Ibáñez 2018-09-16 17:35:47 UTC
*** Bug 78679 has been marked as a duplicate of this bug. ***