[PATCH 0/6] New warnings -Wstring-plus-{char, int} (PR c++/62181)

[PATCH 0/6] New warnings -Wstring-plus-{char, int} (PR c++/62181)

[Xi Ruoyao]

Hi,

I've implemented -Wstring-plus-int and -Wstring-plus-char (like their
counterpart in Clang) for GCC.

This series of patch has been bootstrapped and regtested.  OK for trunk?

Currently these options are not enabled by default like Clang does.
Maybe we could make them enabled by default or by -Wall/-Wextra later.

Xi Ruoyao (6):
  Move char_type_p prototype into c-common.h
  New warning option -Wstring-plus-int
  New warning option -Wstring-plus-char
  New tests for -Wstring-plus-int
  New tests for -Wstring-plus-char
  Document new warning options

 gcc/c-family/c-common.c                        | 25 ++++++++++++++++++++
 gcc/c-family/c-common.h                        |  2 ++
 gcc/c-family/c-warn.c                          | 22 ++++++++++++++++++
 gcc/c-family/c.opt                             | 10 ++++++++
 gcc/c/c-typeck.c                               | 17 +++++++++++++-
 gcc/cp/call.c                                  | 28 ++++++++++++++++++++++
 gcc/cp/cp-tree.h                               |  1 -
 gcc/cp/tree.c                                  |  2 +-
 gcc/doc/invoke.texi                            | 22 +++++++++++++++++-
 gcc/testsuite/c-c++-common/Wstring-plus-char.c | 26 +++++++++++++++++++++
 gcc/testsuite/c-c++-common/Wstring-plus-int.c  | 26 +++++++++++++++++++++
 gcc/testsuite/g++.dg/Wstring-plus-char-1.C     | 16 +++++++++++++
 gcc/testsuite/g++.dg/Wstring-plus-char-2.C     | 26 +++++++++++++++++++++
 gcc/testsuite/g++.dg/Wstring-plus-char-3.C     | 32 ++++++++++++++++++++++++++
 gcc/testsuite/g++.dg/Wstring-plus-int-1.C      |  9 ++++++++
 gcc/testsuite/g++.dg/Wstring-plus-int-2.C      | 10 ++++++++
 16 files changed, 270 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/Wstring-plus-char.c
 create mode 100644 gcc/testsuite/c-c++-common/Wstring-plus-int.c
 create mode 100644 gcc/testsuite/g++.dg/Wstring-plus-char-1.C
 create mode 100644 gcc/testsuite/g++.dg/Wstring-plus-char-2.C
 create mode 100644 gcc/testsuite/g++.dg/Wstring-plus-char-3.C
 create mode 100644 gcc/testsuite/g++.dg/Wstring-plus-int-1.C
 create mode 100644 gcc/testsuite/g++.dg/Wstring-plus-int-2.C


-- 
Xi Ruoyao <ryxi@stu.xidian.edu.cn>
School of Aerospace Science and Technology, Xidian University

[PATCH 1/6] New warnings -Wstring-plus-{char, int} (PR c++/62181)

[Xi Ruoyao]

[-- Attachment #1: Type: text/plain, Size: 767 bytes --]

This patch moves prototype of char_type_p into c-common.h, so we can
use it in c-family/*.

gcc/ChangeLog:

2017-06-12  Xi Ruoyao  <ryxi@stu.xidian.edu.cn>

	* c-family/c-common.h (char_type_p): New prototype.
	* c/c-typeck.c (char_type_p): Make the function extern.
	* cp/cp-tree.h (char_type_p): Remove prototype.
	* cp/tree.c (char_type_p): Change the return type to bool.
---
 gcc/c-family/c-common.h | 1 +
 gcc/c/c-typeck.c        | 3 ++-
 gcc/cp/cp-tree.h        | 1 -
 gcc/cp/tree.c           | 2 +-
 4 files changed, 4 insertions(+), 3 deletions(-)

-- 
Xi Ruoyao <ryxi@stu.xidian.edu.cn>
School of Aerospace Science and Technology, Xidian University

[-- Attachment #2: 0001-Move-char_type_p-prototype-into-c-common.h.patch --]
[-- Type: text/x-patch, Size: 2092 bytes --]

diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 79072e6..82ed4f6 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -962,6 +962,7 @@ extern tree build_real_imag_expr (location_t, enum tree_code, tree);
 extern tree build_unary_op (location_t, enum tree_code, tree, bool);
 extern tree build_binary_op (location_t, enum tree_code, tree, tree, int);
 extern tree perform_integral_promotions (tree);
+extern bool char_type_p (tree);
 
 /* These functions must be defined by each front-end which implements
    a variant of the C language.  They are used by port files.  */
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index ba44406..8adfa9a 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -44,6 +44,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "gimplify.h"
 #include "tree-inline.h"
 #include "omp-general.h"
+#include "c-family/c-common.h"
 #include "c-family/c-objc.h"
 #include "c-family/c-ubsan.h"
 #include "cilk.h"
@@ -3605,7 +3606,7 @@ parser_build_unary_op (location_t loc, enum tree_code code, struct c_expr arg)
 
 /* Returns true if TYPE is a character type, *not* including wchar_t.  */
 
-static bool
+bool
 char_type_p (tree type)
 {
   return (type == char_type_node
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 5dd6023..65c1b82 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6928,7 +6928,6 @@ extern bool cv_qualified_p			(const_tree);
 extern tree cv_unqualified			(tree);
 extern special_function_kind special_function_p (const_tree);
 extern int count_trees				(tree);
-extern int char_type_p				(tree);
 extern void verify_stmt_tree			(tree);
 extern linkage_kind decl_linkage		(tree);
 extern duration_kind decl_storage_duration	(tree);
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index bb17278..d6c1785 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -4855,7 +4855,7 @@ special_function_p (const_tree decl)
 
 /* Returns nonzero if TYPE is a character type, including wchar_t.  */
 
-int
+bool
 char_type_p (tree type)
 {
   return (same_type_p (type, char_type_node)
-- 
2.7.1

[PATCH 2/6] New warnings -Wstring-plus-{char, int} (PR c++/62181)

[Xi Ruoyao]

[-- Attachment #1: Type: text/plain, Size: 528 bytes --]

This patch adds warning option -Wstring-plus-int for C/C++.

gcc/ChangeLog:

2017-06-12  Xi Ruoyao  <ryxi@stu.xidian.edu.cn>

	* c-family/c.opt: New option -Wstring-plus-int.
	* c-family/c-common.c (pointer_int_sum): Checking for
	-Wstring-plus-int.
---
 gcc/c-family/c-common.c | 25 +++++++++++++++++++++++++
 gcc/c-family/c.opt      |  5 +++++
 2 files changed, 30 insertions(+)

-- 
Xi Ruoyao <ryxi@stu.xidian.edu.cn>
School of Aerospace Science and Technology, Xidian University

[-- Attachment #2: 0002-New-warning-option-Wstring-plus-int.patch --]
[-- Type: text/x-patch, Size: 2253 bytes --]

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 4395e51..1eee48f 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -3100,6 +3100,31 @@ pointer_int_sum (location_t loc, enum tree_code resultcode,
   else
     size_exp = size_in_bytes_loc (loc, TREE_TYPE (result_type));
 
+  /* Handle -Wstring-plus-int, warn for adding string literals
+     and an integer which may result in a wild pointer.  */
+  if (warn_string_plus_int
+      && resultcode == PLUS_EXPR
+      && char_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (result_type))))
+    {
+      tree orig_ptrop = tree_strip_nop_conversions(ptrop);
+      if (TREE_CODE (orig_ptrop) == ADDR_EXPR)
+        {
+          tree obj = TREE_OPERAND (orig_ptrop, 0);
+          if (TREE_CODE (obj) == STRING_CST)
+            {
+              tree t = TYPE_DOMAIN (TREE_TYPE (obj));
+              if (TREE_CODE (intop) != INTEGER_CST
+                  || tree_int_cst_lt (intop, TYPE_MIN_VALUE (t))
+                  || int_cst_value (intop)
+                     > int_cst_value (TYPE_MAX_VALUE (t)) + 1)
+                warning_at (loc, OPT_Wstring_plus_int,
+                            "add %qT to string does not append to "
+                            "the string",
+                            TREE_TYPE (intop));
+            }
+        }
+    }
+
   /* We are manipulating pointer values, so we don't need to warn
      about relying on undefined signed overflow.  We disable the
      warning here because we use integer types so fold won't know that
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 37bb236..94ba3eb 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -732,6 +732,11 @@ C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_stringop_overflow) Ini
 Under the control of Object Size type, warn about buffer overflow in string
 manipulation functions like memcpy and strcpy.
 
+Wstring-plus-int
+C ObjC C++ ObjC++ Var(warn_string_plus_int) Warning
+Warn about adding strings and integers, which is likely an ill-formed
+attempt to append the string.
+
 Wsuggest-attribute=format
 C ObjC C++ ObjC++ Var(warn_suggest_attribute_format) Warning
 Warn about functions which might be candidates for format attributes.
-- 
2.7.1

[PATCH 3/6] New warnings -Wstring-plus-{char, int} (PR c++/62181)

[Xi Ruoyao]

[-- Attachment #1: Type: text/plain, Size: 951 bytes --]

This patch adds warning option -Wstring-plus-char for C/C++.

gcc/ChangeLog:

2017-06-12  Xi Ruoyao  <ryxi@stu.xidian.edu.cn>

	* c-family/c-common.h (warn_if_string_plus_char): New prototype.
	* c-family/c-warn.c (warn_if_string_plus_char): New function.
	* c-family/c.opt: New option -Wstring-plus-char.
	* cp/call.c (build_new_op_1): Check for -Wstring-plus-char.
	* c/c-typeck.c (parser_build_binary_op, build_modify_expr):
	Check for -Wstring-plus-char.
---
 gcc/c-family/c-common.h |  1 +
 gcc/c-family/c-warn.c   | 22 ++++++++++++++++++++++
 gcc/c-family/c.opt      |  5 +++++
 gcc/c/c-typeck.c        | 14 ++++++++++++++
 gcc/cp/call.c           | 28 ++++++++++++++++++++++++++++
 5 files changed, 70 insertions(+)

-- 
Xi Ruoyao <ryxi@stu.xidian.edu.cn>
School of Aerospace Science and Technology, Xidian University

[-- Attachment #2: 0003-New-warning-option-Wstring-plus-char.patch --]
[-- Type: text/x-patch, Size: 5908 bytes --]

diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 82ed4f6..82f1c25 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1501,6 +1501,7 @@ extern void c_do_switch_warnings (splay_tree, location_t, tree, tree, bool,
 				  bool);
 extern void warn_for_omitted_condop (location_t, tree);
 extern void warn_for_restrict (unsigned, tree *, unsigned);
+extern void warn_if_string_plus_char (location_t, tree, tree);
 
 /* Places where an lvalue, or modifiable lvalue, may be required.
    Used to select diagnostic messages in lvalue_error and
diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
index 35321a6..d804500 100644
--- a/gcc/c-family/c-warn.c
+++ b/gcc/c-family/c-warn.c
@@ -2332,6 +2332,28 @@ warn_for_restrict (unsigned param_pos, tree *argarray, unsigned nargs)
 			 arg_positions.length ());
 }
 
+/* Like char_type_p, but check the main variant and filter out
+   char16_t (uint_least16_t) and char32_t (uint_least32_t) in C11.  */
+static inline bool
+type_main_variant_is_char (tree type)
+{
+  type = TYPE_MAIN_VARIANT (type);
+  return char_type_p (type)
+         && type != uint_least16_type_node
+         && type != uint_least32_type_node;
+}
+
+void
+warn_if_string_plus_char (location_t loc, tree ptrtype, tree inttype)
+{
+  if (POINTER_TYPE_P (ptrtype)
+      && type_main_variant_is_char (TREE_TYPE (ptrtype))
+      && type_main_variant_is_char (inttype))
+    warning_at (loc, OPT_Wstring_plus_char,
+                "add %qT to string pointer %qT does not append "
+                "to the string", inttype, ptrtype);
+}
+
 /* Callback function to determine whether an expression TP or one of its
    subexpressions comes from macro expansion.  Used to suppress bogus
    warnings.  */
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 94ba3eb..e13cc6c 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -732,6 +732,11 @@ C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_stringop_overflow) Ini
 Under the control of Object Size type, warn about buffer overflow in string
 manipulation functions like memcpy and strcpy.
 
+Wstring-plus-char
+C ObjC C++ ObjC++ Var(warn_string_plus_char) Warning
+Warn about adding string pointers and characters, which is likely an
+ill-formed attempt to append the string.
+
 Wstring-plus-int
 C ObjC C++ ObjC++ Var(warn_string_plus_int) Warning
 Warn about adding strings and integers, which is likely an ill-formed
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 8adfa9a..44e7e02 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -3659,6 +3659,12 @@ parser_build_binary_op (location_t location, enum tree_code code,
 			   arg1.get_start (),
 			   arg2.get_finish ());
 
+  if (warn_string_plus_char && code == PLUS_EXPR)
+    {
+      warn_if_string_plus_char (location, type1, type2);
+      warn_if_string_plus_char (location, type2, type1);
+    }
+
   /* Check for cases such as x+y<<z which users are likely
      to misinterpret.  */
   if (warn_parentheses)
@@ -5788,6 +5794,14 @@ build_modify_expr (location_t location, tree lhs, tree lhs_origtype,
 	  newrhs = build_binary_op (location,
 				    modifycode, lhs, newrhs, 1);
 
+	  if (warn_string_plus_char)
+	    {
+	      tree lt = (lhs_origtype ? lhs_origtype : TREE_TYPE (lhs));
+	      tree rt = (rhs_origtype ? rhs_origtype : TREE_TYPE (rhs));
+	      warn_if_string_plus_char (location, lt, rt);
+	      warn_if_string_plus_char (location, rt, lt);
+	    }
+
 	  /* The original type of the right hand side is no longer
 	     meaningful.  */
 	  rhs_origtype = NULL_TREE;
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index ef99683..8e5ce6a 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -5586,6 +5586,8 @@ build_new_op_1 (location_t loc, enum tree_code code, int flags, tree arg1,
   enum tree_code code2 = NOP_EXPR;
   enum tree_code code_orig_arg1 = ERROR_MARK;
   enum tree_code code_orig_arg2 = ERROR_MARK;
+  tree orig_type1 = error_mark_node;
+  tree orig_type2 = error_mark_node;
   conversion *conv;
   void *p;
   bool strict_p;
@@ -5642,6 +5644,12 @@ build_new_op_1 (location_t loc, enum tree_code code, int flags, tree arg1,
 
       /* =, ->, [], () must be non-static member functions.  */
     case MODIFY_EXPR:
+      if (code2 == PLUS_EXPR)
+        {
+          /* Saved for warn_if_string_plus_char.  */
+          orig_type1 = TREE_TYPE (arg1);
+          orig_type2 = TREE_TYPE (arg2);
+        }
       if (code2 != NOP_EXPR)
 	break;
       /* FALLTHRU */
@@ -5649,6 +5657,11 @@ build_new_op_1 (location_t loc, enum tree_code code, int flags, tree arg1,
     case ARRAY_REF:
       memonly = true;
       break;
+    case PLUS_EXPR:
+      /* Saved for warn_if_string_plus_char.  */
+      orig_type1 = TREE_TYPE (arg1);
+      orig_type2 = TREE_TYPE (arg2);
+      break;
 
     default:
       break;
@@ -5977,6 +5990,13 @@ build_new_op_1 (location_t loc, enum tree_code code, int flags, tree arg1,
   switch (code)
     {
     case MODIFY_EXPR:
+      if (code2 == PLUS_EXPR
+          && (complain & tf_warning)
+          && warn_string_plus_char)
+        {
+          warn_if_string_plus_char (loc, TREE_TYPE (arg1), orig_type2);
+          warn_if_string_plus_char (loc, TREE_TYPE (arg2), orig_type2);
+        }
       return cp_build_modify_expr (loc, arg1, code2, arg2, complain);
 
     case INDIRECT_REF:
@@ -6016,6 +6036,14 @@ build_new_op_1 (location_t loc, enum tree_code code, int flags, tree arg1,
     case BIT_AND_EXPR:
     case BIT_IOR_EXPR:
     case BIT_XOR_EXPR:
+      if ((complain & tf_warning)
+          && warn_string_plus_char
+          && code == PLUS_EXPR)
+        {
+          warn_if_string_plus_char (loc, TREE_TYPE (arg1), orig_type2);
+          warn_if_string_plus_char (loc, TREE_TYPE (arg2), orig_type1);
+        }
+
       return cp_build_binary_op (loc, code, arg1, arg2, complain);
 
     case UNARY_PLUS_EXPR:
-- 
2.7.1

[PATCH 4/6] New warnings -Wstring-plus-{char, int} (PR c++/62181)

[Xi Ruoyao]

[-- Attachment #1: Type: text/plain, Size: 870 bytes --]

This patch adds tests for -Wstring-plus-int.

gcc/ChangeLog:

2017-06-12  Xi Ruoyao  <ryxi@stu.xidian.edu.cn>

	* testsuite/c-c++-common/Wstring-plus-int.c: New test.
	* testsuite/g++.dg/Wstring-plus-int-1.C: Ditto.
	* testsuite/g++.dg/Wstring-plus-int-2.C: Ditto.
---
 gcc/testsuite/c-c++-common/Wstring-plus-int.c | 26 ++++++++++++++++++++++++++
 gcc/testsuite/g++.dg/Wstring-plus-int-1.C     |  9 +++++++++
 gcc/testsuite/g++.dg/Wstring-plus-int-2.C     | 10 ++++++++++
 3 files changed, 45 insertions(+)
 create mode 100644 gcc/testsuite/c-c++-common/Wstring-plus-int.c
 create mode 100644 gcc/testsuite/g++.dg/Wstring-plus-int-1.C
 create mode 100644 gcc/testsuite/g++.dg/Wstring-plus-int-2.C


-- 
Xi Ruoyao <ryxi@stu.xidian.edu.cn>
School of Aerospace Science and Technology, Xidian University

[-- Attachment #2: 0004-New-tests-for-Wstring-plus-int.patch --]
[-- Type: text/x-patch, Size: 2257 bytes --]

diff --git a/gcc/testsuite/c-c++-common/Wstring-plus-int.c b/gcc/testsuite/c-c++-common/Wstring-plus-int.c
new file mode 100644
index 0000000..6172bd0
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wstring-plus-int.c
@@ -0,0 +1,26 @@
+/* Test -Wstring-plus-int.  */
+
+/* { dg-do compile } */
+/* { dg-options "-Wstring-plus-int" } */
+
+extern int getchar();
+extern int offset;
+
+int main(void)
+{
+  const char *a = "aa" + 'a'; /* { dg-warning "does not append" } */
+  const char *b = "aa" + getchar(); /* { dg-warning "does not append" } */
+  const char *c = "aa" + 4; /* { dg-warning "does not append" } */
+  const char *d = "aa" + -1; /* { dg-warning "does not append" } */
+  const char *e = 'x' + "aa"; /* { dg-warning "does not append" } */
+  const char *f = "aa" + offset; /* { dg-warning "does not append" } */
+
+  /* This is legal (at least Clang think it is).  */
+  const char *g = "aa" + 3; /* { dg-bogus "does not append" } */
+
+  /* Although they are strange, still shouldn't
+     be warned by this warning.  Maybe -Warray-bounds.  */
+  const char (*h)[3] = &"aa" + 1; /* { dg-bogus "does not append" } */
+  char i = "aa"[4]; /* { dg-bogus "does not append" } */
+  const char *j = "aa" - 1; /* { dg-bogus "does not append" } */
+}
diff --git a/gcc/testsuite/g++.dg/Wstring-plus-int-1.C b/gcc/testsuite/g++.dg/Wstring-plus-int-1.C
new file mode 100644
index 0000000..fc74428
--- /dev/null
+++ b/gcc/testsuite/g++.dg/Wstring-plus-int-1.C
@@ -0,0 +1,9 @@
+/* Test -Wstring-plus-int for C++ wide char types.  */
+
+/* { dg-do compile } */
+/* { dg-options "-Wstring-plus-int" } */
+
+int main(void)
+{
+  const wchar_t *a = L"aa" + L'a'; /* { dg-warning "does not append" } */
+}
diff --git a/gcc/testsuite/g++.dg/Wstring-plus-int-2.C b/gcc/testsuite/g++.dg/Wstring-plus-int-2.C
new file mode 100644
index 0000000..b69da41
--- /dev/null
+++ b/gcc/testsuite/g++.dg/Wstring-plus-int-2.C
@@ -0,0 +1,10 @@
+/* Test -Wstring-plus-int for C++ 2011 unicode char types.  */
+
+/* { dg-do compile } */
+/* { dg-options "-std=c++11 -Wstring-plus-int" } */
+
+int main(void)
+{
+  const char16_t *a = u"aa" + u'a'; /* { dg-warning "does not append" } */
+  const char32_t *b = U"aa" + U'a'; /* { dg-warning "does not append" } */
+}
-- 
2.7.1

[PATCH 6/6] New warnings -Wstring-plus-{char, int} (PR c++/62181)

[Xi Ruoyao]

[-- Attachment #1: Type: text/plain, Size: 448 bytes --]

This patch adds document of -Wstring-plus-int and -Wstring-plus-char.

gcc/ChangeLog:

2017-06-12  Xi Ruoyao  <ryxi@stu.xidian.edu.cn>

	* doc/invoke.texi (Warning Options): Document -Wstring-plus-int
	and -Wstring-plus-char.
---
 gcc/doc/invoke.texi | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

-- 
Xi Ruoyao <ryxi@stu.xidian.edu.cn>
School of Aerospace Science and Technology, Xidian University

[-- Attachment #2: 0006-Document-new-warning-options.patch --]
[-- Type: text/x-patch, Size: 1709 bytes --]

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 5d41649..4859341 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -311,7 +311,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wsizeof-pointer-memaccess  -Wsizeof-array-argument @gol
 -Wstack-protector  -Wstack-usage=@var{len}  -Wstrict-aliasing @gol
 -Wstrict-aliasing=n  -Wstrict-overflow  -Wstrict-overflow=@var{n} @gol
--Wstringop-overflow=@var{n} @gol
+-Wstringop-overflow=@var{n} @gol -Wstring-plus-char -Wstring-plus-int
 -Wsuggest-attribute=@r{[}pure@r{|}const@r{|}noreturn@r{|}format@r{]} @gol
 -Wsuggest-final-types @gol  -Wsuggest-final-methods  -Wsuggest-override @gol
 -Wmissing-format-attribute  -Wsubobject-linkage @gol
@@ -5125,6 +5125,26 @@ whether to issue a warning.  Similarly to @option{-Wstringop-overflow=3} this
 setting of the option may result in warnings for benign code.
 @end table
 
+@item -Wstring-plus-char
+@opindex Wstring-plus-char
+@opindex Wno-string-plus-char
+Warn for adding a character to a string pointer, which seems like a failed
+attempt to append to the string.  For example, this option will issue a
+warning for the code below.
+
+@smallexample
+const char *p = "abc", *q;
+q = p + 'a';
+@end smallexample
+
+@item -Wstring-plus-int
+@opindex Wstring-plus-int
+@opindex Wno-string-plus-int
+Warn for adding an integer to a string literal, which may forms a pointer
+out of the bound of the string.  The typical examples this warns about are
+@samp{"abc" + 'd'}, @samp{"abc" + getchar()} and @samp{"abc" + 5}, but
+not @samp{"abc" + 1}.
+
 @item -Wsuggest-attribute=@r{[}pure@r{|}const@r{|}noreturn@r{|}format@r{]}
 @opindex Wsuggest-attribute=
 @opindex Wno-suggest-attribute=
-- 
2.7.1

[PATCH 5/6] New warnings -Wstring-plus-{char, int} (PR c++/62181)

[Xi Ruoyao]

[-- Attachment #1: Type: text/plain, Size: 1098 bytes --]

This patch adds tests for -Wstring-plus-char.

gcc/ChangeLog:

2017-06-12  Xi Ruoyao  <ryxi@stu.xidian.edu.cn>

	* testsuite/c-c++-common/Wstring-plus-char.c: New test.
	* testsuite/g++.dg/Wstring-plus-char-1.C: Ditto.
	* testsuite/g++.dg/Wstring-plus-char-2.C: Ditto.
	* testsuite/g++.dg/Wstring-plus-char-3.C: Ditto.
---
 gcc/testsuite/c-c++-common/Wstring-plus-char.c | 26 +++++++++++++++++++++
 gcc/testsuite/g++.dg/Wstring-plus-char-1.C     | 16 +++++++++++++
 gcc/testsuite/g++.dg/Wstring-plus-char-2.C     | 26 +++++++++++++++++++++
 gcc/testsuite/g++.dg/Wstring-plus-char-3.C     | 32 ++++++++++++++++++++++++++
 4 files changed, 100 insertions(+)
 create mode 100644 gcc/testsuite/c-c++-common/Wstring-plus-char.c
 create mode 100644 gcc/testsuite/g++.dg/Wstring-plus-char-1.C
 create mode 100644 gcc/testsuite/g++.dg/Wstring-plus-char-2.C
 create mode 100644 gcc/testsuite/g++.dg/Wstring-plus-char-3.C

-- 
Xi Ruoyao <ryxi@stu.xidian.edu.cn>
School of Aerospace Science and Technology, Xidian University

[-- Attachment #2: 0005-New-tests-for-Wstring-plus-char.patch --]
[-- Type: text/x-patch, Size: 3635 bytes --]

diff --git a/gcc/testsuite/c-c++-common/Wstring-plus-char.c b/gcc/testsuite/c-c++-common/Wstring-plus-char.c
new file mode 100644
index 0000000..6d07750
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wstring-plus-char.c
@@ -0,0 +1,26 @@
+/* Test -Wstring-plus-char.  */
+
+/* { dg-do compile } */
+/* { dg-options "-Wstring-plus-char" } */
+
+typedef __UINT_LEAST16_TYPE__ uint_least16_t;
+
+char *a;
+const char *b;
+char c;
+const char *d;
+uint_least16_t x;
+
+int main(void)
+{
+  d = a + c; /* { dg-warning "does not append" } */
+  d = b + c; /* { dg-warning "does not append" } */
+  d = a + 'a'; /* { dg-warning "does not append" } */
+  d = b + 'a'; /* { dg-warning "does not append" } */
+  d = 'a' + b; /* { dg-warning "does not append" } */
+  d += 'a'; /* { dg-warning "does not append" } */
+  d = a + x; /* { dg-bogus "does not append" } */
+  d = b + x; /* { dg-bogus "does not append" } */
+  d = a + 1u; /* { dg-bogus "does not append" } */
+  d = a + 1; /* { dg-bogus "does not append" } */
+}
diff --git a/gcc/testsuite/g++.dg/Wstring-plus-char-1.C b/gcc/testsuite/g++.dg/Wstring-plus-char-1.C
new file mode 100644
index 0000000..98f6b66
--- /dev/null
+++ b/gcc/testsuite/g++.dg/Wstring-plus-char-1.C
@@ -0,0 +1,16 @@
+/* Test -Wstring-plus-char for C++ wide char types.  */
+
+/* { dg-do compile } */
+/* { dg-options "-Wstring-plus-char" } */
+
+const wchar_t *a, *d;
+wchar_t b;
+int c;
+
+int main(void)
+{
+  d = a + L'a'; /* { dg-warning "does not append" } */
+  d = a + b; /* { dg-warning "does not append" } */
+  d = a + c; /* { dg-bogus "does not append" } */
+  d = a + 1; /* { dg-bogus "does not append" } */
+}
diff --git a/gcc/testsuite/g++.dg/Wstring-plus-char-2.C b/gcc/testsuite/g++.dg/Wstring-plus-char-2.C
new file mode 100644
index 0000000..4bb2ba3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/Wstring-plus-char-2.C
@@ -0,0 +1,26 @@
+/* Test -Wstring-plus-char for C++ 2011 unicode char types.  */
+
+/* { dg-do compile } */
+/* { dg-options "-std=c++11 -Wstring-plus-char" } */
+
+typedef __UINT_LEAST16_TYPE__ uint_least16_t;
+typedef __UINT_LEAST32_TYPE__ uint_least32_t;
+
+const char16_t *a, *d;
+char16_t b;
+uint_least16_t c;
+const char32_t *e, *h;
+char32_t f;
+uint_least32_t g;
+
+int main(void)
+{
+  d = a + u'a'; /* { dg-warning "does not append" } */
+  d = a + b; /* { dg-warning "does not append" } */
+  d = a + c; /* { dg-bogus "does not append" } */
+  d = a + 1; /* { dg-bogus "does not append" } */
+  h = e + U'a'; /* { dg-warning "does not append" } */
+  h = e + f; /* { dg-warning "does not append" } */
+  h = e + g; /* { dg-bogus "does not append" } */
+  h = e + 1; /* { dg-bogus "does not append" } */
+}
diff --git a/gcc/testsuite/g++.dg/Wstring-plus-char-3.C b/gcc/testsuite/g++.dg/Wstring-plus-char-3.C
new file mode 100644
index 0000000..a313059
--- /dev/null
+++ b/gcc/testsuite/g++.dg/Wstring-plus-char-3.C
@@ -0,0 +1,32 @@
+/* Test -Wstring-plus-char.
+   This is a more realistic test case.  */
+
+/* { dg-do compile } */
+/* { dg-options "-Wstring-plus-char" } */
+
+class good_string
+{
+public:
+  good_string(const char *);
+  good_string &operator=(const char *);
+  good_string operator+(char) const;
+  operator const char *(void) const;
+};
+
+/* Someone has forgotten operator+ overload...  */
+class bad_string
+{
+public:
+  bad_string(const char *);
+  bad_string &operator=(const char *);
+  operator const char *(void) const;
+};
+
+int main()
+{
+  good_string good = "aa";
+  good = good + 'a'; /* { dg-bogus "does not append" } */
+  bad_string bad = "bb";
+  bad = bad + 'b'; /* { dg-warning "does not append" } */
+  return 0;
+}
-- 
2.7.1

[PING PATCH 0/6] New warnings -Wstring-plus-{char, int} (PR c++/62181)

[Xi Ruoyao]

On 2017-06-12 09:26 +0800, Xi Ruoyao wrote:
> Hi,
> 
> I've implemented -Wstring-plus-int and -Wstring-plus-char (like their
> counterpart in Clang) for GCC.
> 
> This series of patch has been bootstrapped and regtested.  OK for trunk?
> 
> Currently these options are not enabled by default like Clang does.
> Maybe we could make them enabled by default or by -Wall/-Wextra later.

Ping.
<https://gcc.gnu.org/ml/gcc-patches/2017-06/msg00729.html>
-- 
Xi Ruoyao <ryxi@stu.xidian.edu.cn>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH 0/6] New warnings -Wstring-plus-{char, int} (PR c++/62181)

[Martin Sebor]

On 06/11/2017 07:26 PM, Xi Ruoyao wrote:
> Hi,
>
> I've implemented -Wstring-plus-int and -Wstring-plus-char (like their
> counterpart in Clang) for GCC.

 From the Clang patch(*) it only "warns when a character literal is
added (using '+') to a variable with type 'char *' (or any other
pointer to character type).

Based on the tests in this patch, GCC will warn for non-literal
operands as well.  Is that intentional, and if so, what is your
rationale for the difference?

Martin

[*] 
http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20131021/091671.html

Re: [PATCH 3/6] New warnings -Wstring-plus-{char, int} (PR c++/62181)

[Martin Sebor]

On 06/11/2017 07:34 PM, Xi Ruoyao wrote:
> This patch adds warning option -Wstring-plus-char for C/C++.
>

+void
+warn_if_string_plus_char (location_t loc, tree ptrtype, tree inttype)
+{
+  if (POINTER_TYPE_P (ptrtype)
+      && type_main_variant_is_char (TREE_TYPE (ptrtype))
+      && type_main_variant_is_char (inttype))
+    warning_at (loc, OPT_Wstring_plus_char,
+                "add %qT to string pointer %qT does not append "
+                "to the string", inttype, ptrtype);

The text of the warning doesn't read like a grammatically correct
sentence.  ("Adding a to b" would be correct.)

That said, I wonder if it should also be made more accurate.
Based on c-c++-common/Wstring-plus-char.c for the snippet below

   char *a;
   const char *b;
   const char c = 'c';
   const char *d = a + c;

it will print

   warning: add 'char' to 'char *' does not append to the string

even though no string is apparent or need to exist in the program
(a could point to an array of chars with no terminating NUL).
I see Clang prints something similar (modulo the bad grammar) but
I think it might be clearer if the warning instead read something
like:

   adding 'char' to 'char *' does not append to a string

or (if the warning were to trigger only for character constants
like in Clang):

   adding 'char' to 'char *' does not append 'c' to the first operand

i.e., if the warning also included the value of the character
constant.

Martin

Re: [PATCH 2/6] New warnings -Wstring-plus-{char, int} (PR c++/62181)

[Martin Sebor]

On 06/11/2017 07:32 PM, Xi Ruoyao wrote:
> This patch adds warning option -Wstring-plus-int for C/C++.
>
> gcc/ChangeLog:
>
> 2017-06-12  Xi Ruoyao  <ryxi@stu.xidian.edu.cn>
>
> 	* c-family/c.opt: New option -Wstring-plus-int.
> 	* c-family/c-common.c (pointer_int_sum): Checking for
> 	-Wstring-plus-int.

This is a very useful warning but I would suggest to word it
in terms of what it actually does rather than what it might be
intended to do.  E.g., for

   const char *p = "123" + 7;

issue

   warning: offset 7 exceeds the upper bound 3 of the array

rather than

   warning: adding 'int' to a string does not append to the string

(I have trouble envisioning on what grounds someone might expect
the addition to have this effect.)

Given that the warning only triggers when the upper bound of
an array is exceeded I would also suggest to consider including
the warning in -Warray-bounds.  (With that, it would be useful
to also detect exceeding the upper bound of non-literal arrays
as well.)

Martin

Re: [PATCH 6/6] New warnings -Wstring-plus-{char, int} (PR c++/62181)

[Martin Sebor]

On 06/11/2017 07:39 PM, Xi Ruoyao wrote:
> This patch adds document of -Wstring-plus-int and -Wstring-plus-char.

+@item -Wstring-plus-char
+@opindex Wstring-plus-char
+@opindex Wno-string-plus-char
+Warn for adding a character to a string pointer, which seems like a failed
+attempt to append to the string.  For example, this option will issue a
+warning for the code below.

The text above should be corrected for grammar:

   Warn when a character is added to a character pointer.  Such
   addition it may be an incorrect attempt to append the character
   to a string.

Similarly, the text below should be corrected (though as I mentioned
in my earlier response to one of the prior patches, I would prefer
to see the out-of-bounds warning(s) phrased in terms the (undefined)
effects of the addition and included in -Warray-bounds rather than
adding a new option based on assumptions about the intended effects,
and extended to all arrays of known bound rather than applied only
to string literals).

+@item -Wstring-plus-int
+@opindex Wstring-plus-int
+@opindex Wno-string-plus-int
+Warn for adding an integer to a string literal, which may forms a pointer
+out of the bound of the string.  The typical examples this warns about are
+@samp{"abc" + 'd'}, @samp{"abc" + getchar()} and @samp{"abc" + 5}, but
+not @samp{"abc" + 1}.

   Warn when an integer constant in excess of its upper bound is
   added to a string literal.

Martin

Re: [PATCH 2/6] New warnings -Wstring-plus-{char, int} (PR c++/62181)

[Xi Ruoyao]

On 2017-06-19 10:51 -0600, Martin Sebor wrote:
> On 06/11/2017 07:32 PM, Xi Ruoyao wrote:
> > This patch adds warning option -Wstring-plus-int for C/C++.
> > 
> > gcc/ChangeLog:
> > 
> > 2017-06-12  Xi Ruoyao  <ryxi@stu.xidian.edu.cn>
> > 
> > 	* c-family/c.opt: New option -Wstring-plus-int.
> > 	* c-family/c-common.c (pointer_int_sum): Checking for
> > 	-Wstring-plus-int.
> 
> This is a very useful warning but I would suggest to word it
> in terms of what it actually does rather than what it might be
> intended to do.  E.g., for
> 
>    const char *p = "123" + 7;
> 
> issue
> 
>    warning: offset 7 exceeds the upper bound 3 of the array
> 
> rather than
> 
>    warning: adding 'int' to a string does not append to the string
> 
> (I have trouble envisioning on what grounds someone might expect
> the addition to have this effect.)

How about something like `const char *p = "123" + getchar();` ?

I'd like this for -Wstring-plus-int=1:

    warning: adding 'int' to a string does not append to the string
    [-Wstring-plus-int=]
        const char *p = "123" + 7;
                              ^
    note: offset 7 exceeds the size 4 of the string, using the result
    may lead to undefined behaviour.

(Clang permits "123" + 4 since its result is well defined in standard.
Maybe we could permit "123" + 3 only.)

For level 1 we only warn for such obvious mistakes. And for
-Wstring-plus-int=2:

    warning: adding 'int' to a string does not append to the string
    [-Wstring-plus-int=]
        const char *p = "123" + getchar();
                              ^
    note: the offset may exceed the size of the string.

(Clang also warn while it's impossible to know whether the offset
exceeds.  It seems aggressively so we can make it level 2.)

> Given that the warning only triggers when the upper bound of
> an array is exceeded I would also suggest to consider including
> the warning in -Warray-bounds.  (With that, it would be useful
> to also detect exceeding the upper bound of non-literal arrays
> as well.)

We can let -Warray-bounds enable -Wstring-plus-int=1, but not =2.
-- 
Xi Ruoyao <ryxi@stu.xidian.edu.cn>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH 3/6] New warnings -Wstring-plus-{char, int} (PR c++/62181)

[Xi Ruoyao]

On 2017-06-19 10:30 -0600, Martin Sebor wrote:
> On 06/11/2017 07:34 PM, Xi Ruoyao wrote:
> > This patch adds warning option -Wstring-plus-char for C/C++.
> > 
> 
> +void
> +warn_if_string_plus_char (location_t loc, tree ptrtype, tree inttype)
> +{
> +  if (POINTER_TYPE_P (ptrtype)
> +      && type_main_variant_is_char (TREE_TYPE (ptrtype))
> +      && type_main_variant_is_char (inttype))
> +    warning_at (loc, OPT_Wstring_plus_char,
> +                "add %qT to string pointer %qT does not append "
> +                "to the string", inttype, ptrtype);
> 
> The text of the warning doesn't read like a grammatically correct
> sentence.  ("Adding a to b" would be correct.)

Yes.  It's a typo.

> That said, I wonder if it should also be made more accurate.
> Based on c-c++-common/Wstring-plus-char.c for the snippet below
> 
>    char *a;
>    const char *b;
>    const char c = 'c';
>    const char *d = a + c;
> 
> it will print
> 
>    warning: add 'char' to 'char *' does not append to the string
> 
> even though no string is apparent or need to exist in the program
> (a could point to an array of chars with no terminating NUL).
> I see Clang prints something similar (modulo the bad grammar) but
> I think it might be clearer if the warning instead read something
> like:
> 
>    adding 'char' to 'char *' does not append to a string
> 
> or (if the warning were to trigger only for character constants
> like in Clang):
> 
>    adding 'char' to 'char *' does not append 'c' to the first operand

Clang 4.0 only warns for character constants.  But Clang 5.0
(pre-release) also warns for variables with type char.
Which option should we take?  Maybe -Wstring-plus-char={1,2} ?

> i.e., if the warning also included the value of the character
> constant.
> 
> Martin
-- 
Xi Ruoyao <ryxi@stu.xidian.edu.cn>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH 2/6] New warnings -Wstring-plus-{char, int} (PR c++/62181)

[Martin Sebor]

On 06/19/2017 11:28 AM, Xi Ruoyao wrote:
> On 2017-06-19 10:51 -0600, Martin Sebor wrote:
>> On 06/11/2017 07:32 PM, Xi Ruoyao wrote:
>>> This patch adds warning option -Wstring-plus-int for C/C++.
>>>
>>> gcc/ChangeLog:
>>>
>>> 2017-06-12  Xi Ruoyao  <ryxi@stu.xidian.edu.cn>
>>>
>>> 	* c-family/c.opt: New option -Wstring-plus-int.
>>> 	* c-family/c-common.c (pointer_int_sum): Checking for
>>> 	-Wstring-plus-int.
>>
>> This is a very useful warning but I would suggest to word it
>> in terms of what it actually does rather than what it might be
>> intended to do.  E.g., for
>>
>>    const char *p = "123" + 7;
>>
>> issue
>>
>>    warning: offset 7 exceeds the upper bound 3 of the array
>>
>> rather than
>>
>>    warning: adding 'int' to a string does not append to the string
>>
>> (I have trouble envisioning on what grounds someone might expect
>> the addition to have this effect.)
>
> How about something like `const char *p = "123" + getchar();` ?

I'm not sure I correctly understand the question (or whether
it's meant in response to my comment in parentheses) but let
me clarify what I meant.

In my view, the group of C++ (and certainly C) programmers who
might expect "123" + i to append the string representation of
the integer result to a string literal isn't significant enough
to focus the warning on.

Whether or not the addition is valid depends on the value of
the integer operand.  There are three sets of cases where the
addition is or may be invalid:

1) the integer operand is an out of bounds constant
2) the integer operand's non-constant value or the lower bound
    of its range is known to be out of bounds,
3) the lower bound of the operand's range is in bounds but
    the upper bound is out of bounds (as in the getchar example).

(1) can be handled with lexical analysis alone (as in you parch)
but it's prone to a high rate of false negatives.  (3) can also
be handled by lexical analysis alone but it's prone to a high
rate of false positives.  (2) has no false positives but some
false negatives.  It can only be detected with optimization.

With that in mind the warning would serve a greater purpose
by being aimed more broadly and describing the nature of the
error: forming an invalid pointer.  I believe it would best
be implemented analogously to or even integrated into
-Warray-bounds.  I.e., I suggest covering set (2) above.

>
> I'd like this for -Wstring-plus-int=1:
>
>     warning: adding 'int' to a string does not append to the string
>     [-Wstring-plus-int=]
>         const char *p = "123" + 7;
>                               ^
>     note: offset 7 exceeds the size 4 of the string, using the result
>     may lead to undefined behaviour.

The addition itself is undefined, regardless of whether or not
the result is used.

>
> (Clang permits "123" + 4 since its result is well defined in standard.
> Maybe we could permit "123" + 3 only.)

"123" is an array of 4 elements, with "123" + 4 pointing just past
the last (NUL) element.  It's valid to form a pointer past the last
element of an array and warning about it would likely be viewed as
a false positive (certainly if it were an out-of-bounds type of
warning).

Martin

Re: [PATCH 2/6] New warnings -Wstring-plus-{char, int} (PR c++/62181)

[Xi Ruoyao]

On 2017-06-19 12:44 -0600, Martin Sebor wrote:
> On 06/19/2017 11:28 AM, Xi Ruoyao wrote:
> > On 2017-06-19 10:51 -0600, Martin Sebor wrote:
> > > On 06/11/2017 07:32 PM, Xi Ruoyao wrote:
> > > > This patch adds warning option -Wstring-plus-int for C/C++.
> > > > 
> > > > gcc/ChangeLog:
> > > > 
> > > > 2017-06-12  Xi Ruoyao  <ryxi@stu.xidian.edu.cn>
> > > > 
> > > > 	* c-family/c.opt: New option -Wstring-plus-int.
> > > > 	* c-family/c-common.c (pointer_int_sum): Checking for
> > > > 	-Wstring-plus-int.
> > > 
> > > This is a very useful warning but I would suggest to word it
> > > in terms of what it actually does rather than what it might be
> > > intended to do.  E.g., for
> > > 
> > >    const char *p = "123" + 7;
> > > 
> > > issue
> > > 
> > >    warning: offset 7 exceeds the upper bound 3 of the array
> > > 
> > > rather than
> > > 
> > >    warning: adding 'int' to a string does not append to the string
> > > 
> > > (I have trouble envisioning on what grounds someone might expect
> > > the addition to have this effect.)
> > 
> > How about something like `const char *p = "123" + getchar();` ?
> 
> I'm not sure I correctly understand the question (or whether
> it's meant in response to my comment in parentheses) but let
> me clarify what I meant.
> 
> In my view, the group of C++ (and certainly C) programmers who
> might expect "123" + i to append the string representation of
> the integer result to a string literal isn't significant enough
> to focus the warning on.
> 
> Whether or not the addition is valid depends on the value of
> the integer operand.  There are three sets of cases where the
> addition is or may be invalid:
> 
> 1) the integer operand is an out of bounds constant
> 2) the integer operand's non-constant value or the lower bound
>     of its range is known to be out of bounds,
> 3) the lower bound of the operand's range is in bounds but
>     the upper bound is out of bounds (as in the getchar example).
> 
> (1) can be handled with lexical analysis alone (as in you parch)
> but it's prone to a high rate of false negatives.  (3) can also
> be handled by lexical analysis alone but it's prone to a high
> rate of false positives.  (2) has no false positives but some
> false negatives.  It can only be detected with optimization.
> 
> With that in mind the warning would serve a greater purpose
> by being aimed more broadly and describing the nature of the
> error: forming an invalid pointer.  I believe it would best
> be implemented analogously to or even integrated into
> -Warray-bounds.  I.e., I suggest covering set (2) above.

Now I think I've been cheat by GCC wiki, which states PR 62181
is an "easy-hack" :)

I'll try to improve -Warray-bounds.

> > 
> > I'd like this for -Wstring-plus-int=1:
> > 
> >     warning: adding 'int' to a string does not append to the string
> >     [-Wstring-plus-int=]
> >         const char *p = "123" + 7;
> >                               ^
> >     note: offset 7 exceeds the size 4 of the string, using the result
> >     may lead to undefined behaviour.
> 
> The addition itself is undefined, regardless of whether or not
> the result is used.
> 
> > 
> > (Clang permits "123" + 4 since its result is well defined in standard.
> > Maybe we could permit "123" + 3 only.)
> 
> "123" is an array of 4 elements, with "123" + 4 pointing just past
> the last (NUL) element.  It's valid to form a pointer past the last
> element of an array and warning about it would likely be viewed as
> a false positive (certainly if it were an out-of-bounds type of
> warning).
> 
> Martin
-- 
Xi Ruoyao <ryxi@stu.xidian.edu.cn>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH 2/6] New warnings -Wstring-plus-{char, int} (PR c++/62181)

[Xi Ruoyao]

On 2017-06-19 12:44 -0600, Martin Sebor wrote:
> On 06/19/2017 11:28 AM, Xi Ruoyao wrote:
> > On 2017-06-19 10:51 -0600, Martin Sebor wrote:
> > > On 06/11/2017 07:32 PM, Xi Ruoyao wrote:
> > > > This patch adds warning option -Wstring-plus-int for C/C++.
> > > > 
> > > > gcc/ChangeLog:
> > > > 
> > > > 2017-06-12  Xi Ruoyao  <ryxi@stu.xidian.edu.cn>
> > > > 
> > > > 	* c-family/c.opt: New option -Wstring-plus-int.
> > > > 	* c-family/c-common.c (pointer_int_sum): Checking for
> > > > 	-Wstring-plus-int.
> > > 
> > > This is a very useful warning but I would suggest to word it
> > > in terms of what it actually does rather than what it might be
> > > intended to do.  E.g., for
> > > 
> > >    const char *p = "123" + 7;
> > > 
> > > issue
> > > 
> > >    warning: offset 7 exceeds the upper bound 3 of the array
> > > 
> > > rather than
> > > 
> > >    warning: adding 'int' to a string does not append to the string
> > > 
> > > (I have trouble envisioning on what grounds someone might expect
> > > the addition to have this effect.)
> > 
> > How about something like `const char *p = "123" + getchar();` ?
> 
> I'm not sure I correctly understand the question (or whether
> it's meant in response to my comment in parentheses) but let
> me clarify what I meant.
> 
> In my view, the group of C++ (and certainly C) programmers who
> might expect "123" + i to append the string representation of
> the integer result to a string literal isn't significant enough
> to focus the warning on.
> 
> Whether or not the addition is valid depends on the value of
> the integer operand.  There are three sets of cases where the
> addition is or may be invalid:
> 
> 1) the integer operand is an out of bounds constant
> 2) the integer operand's non-constant value or the lower bound
>     of its range is known to be out of bounds,
> 3) the lower bound of the operand's range is in bounds but
>     the upper bound is out of bounds (as in the getchar example).
> 
> (1) can be handled with lexical analysis alone (as in you parch)
> but it's prone to a high rate of false negatives.  (3) can also
> be handled by lexical analysis alone but it's prone to a high
> rate of false positives.  (2) has no false positives but some
> false negatives.  It can only be detected with optimization.
> 
> With that in mind the warning would serve a greater purpose
> by being aimed more broadly and describing the nature of the
> error: forming an invalid pointer.  I believe it would best
> be implemented analogously to or even integrated into
> -Warray-bounds.  I.e., I suggest covering set (2) above.

I created PR 81172.  For const char *p = "123" + 'c' we should have:

    warning: offset 99 is above array bounds, the behaviour is
    undefined [-Warray-bounds]
        const char *p = "123" + 'c';

and perhaps (for the case the pointer operand is a string or a string
pointer):
    note: adding integer to a string does not append to the string.

> > 
> > I'd like this for -Wstring-plus-int=1:
> > 
> >     warning: adding 'int' to a string does not append to the string
> >     [-Wstring-plus-int=]
> >         const char *p = "123" + 7;
> >                               ^
> >     note: offset 7 exceeds the size 4 of the string, using the result
> >     may lead to undefined behaviour.
> 
> The addition itself is undefined, regardless of whether or not
> the result is used.
> 
> > 
> > (Clang permits "123" + 4 since its result is well defined in standard.
> > Maybe we could permit "123" + 3 only.)
> 
> "123" is an array of 4 elements, with "123" + 4 pointing just past
> the last (NUL) element.  It's valid to form a pointer past the last
> element of an array and warning about it would likely be viewed as
> a false positive (certainly if it were an out-of-bounds type of
> warning).
> 
> Martin
-- 
Xi Ruoyao <ryxi@stu.xidian.edu.cn>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH 2/6] New warnings -Wstring-plus-{char, int} (PR c++/62181)

[Gerald Pfeifer]

On Thu, 22 Jun 2017, Xi Ruoyao wrote:
> I created PR 81172.  For const char *p = "123" + 'c' we should have:
> 
>     warning: offset 99 is above array bounds, the behaviour is
>     undefined [-Warray-bounds]
>         const char *p = "123" + 'c';
> 
> and perhaps (for the case the pointer operand is a string or a string
> pointer):
>     note: adding integer to a string does not append to the string.

I do think this makes sense.  I'm really not convinced there is a 
lot of code out there that uses the "123" + i idiom to create a pointer, 
even if it is legitimate code, so a warning makes sense to me.

Gerald