* [PATCH 2/5] c++: New warning option -Wstring-plus-int
@ 2017-02-22 11:10 Xi Ruoyao
2017-04-28 20:45 ` Jeff Law
0 siblings, 1 reply; 3+ messages in thread
From: Xi Ruoyao @ 2017-02-22 11:10 UTC (permalink / raw)
To: gcc-patches; +Cc: ryxi
This patch adds warning option -Wstring-plus-int for C++.
gcc/ChangeLog:
2017-02-22  Xi Ruoyao  <ryxi@stu.xidian.edu.cn>
* c-family/c-common.h (maybe_warn_string_plus_int): new prototype
* c-family/c-warn.h (maybe_warn_string_plus_int): new function
* c-family/c-opt: new option -Wstring-plus-int
* cp/call.c (build_new_op_1): Checking for -Wstring-plus-int
---
 gcc/c-family/c-common.h |  2 ++
 gcc/c-family/c-warn.c   | 34 ++++++++++++++++++++++++++++++++++
 gcc/c-family/c.opt      |  5 +++++
 gcc/cp/call.c           | 35 +++++++++++++++++++++++++++++++++++
 4 files changed, 76 insertions(+)
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 72fba56..8c748e9 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1503,6 +1503,8 @@ 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, vec<tree, va_gc> *);
+extern void maybe_warn_string_plus_int (location_t, tree, tree,
+Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â enum tree_code, enum tree_code);
Â
 /* 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 09c5760..0b038a7 100644
--- a/gcc/c-family/c-warn.c
+++ b/gcc/c-family/c-warn.c
@@ -2290,3 +2290,37 @@ do_warn_duplicated_branches_r (tree *tp, int *, void *)
     do_warn_duplicated_branches (*tp);
   return NULL_TREE;
 }
+
+static inline bool
+char_type_cv_p (tree type)
+{
+Â Â return char_type_p (build_qualified_type (type, TYPE_UNQUALIFIED));
+}
+
+/* Warn about adding string literals/pointers and chars.
+Â Â Â Produce a warning when:
+Â Â Â Â Â 1) arg1 is a string literal, and arg2 is a non-literal integer;
+     2) arg1 is a string pointer, and arg2 is a character.  */
+
+void
+maybe_warn_string_plus_int (location_t loc, tree type1, tree type2,
+Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â enum tree_code code1, enum tree_code code2)
+{
+Â Â bool arg2_is_char = char_type_cv_p (type2);
+Â Â /* In C11, char16_t and char32_t are typedefs. Prevent false positives
+     about uint_least16_t and uint_least32_t.  */
+Â Â if (type2 == uint_least16_type_node ||
+Â Â Â Â type2 == uint_least32_type_node)
+Â Â arg2_is_char = false;
+
+Â Â if (code1 == STRING_CST && TREE_CODE (type2) == INTEGER_TYPE &&
+Â Â Â Â Â Â (code2 != INTEGER_CST || arg2_is_char))
+Â Â Â Â warning_at (loc, OPT_Wstring_plus_int,
+Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "adding %qT to string literal does not append to "
+Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "the string", type2);
+Â Â else if (TREE_CODE (type1) == POINTER_TYPE &&
+Â Â Â Â Â Â Â Â Â char_type_cv_p (TREE_TYPE (type1)) && arg2_is_char)
+Â Â Â Â warning_at (loc, OPT_Wstring_plus_int,
+Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "adding %qT to string pointer %qT does not append "
+Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "to the string", type2, type1);
+}
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index a1b3ae5..f0b482f 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -716,6 +716,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 string pointers 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.
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 93fae0d..95bda1b 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -5555,6 +5555,8 @@ build_new_op_1 (location_t loc, enum tree_code code, int flags, tree arg1,
   void *p;
   bool strict_p;
   bool any_viable_p;
+Â Â tree orig_arg1 = error_mark_node;
+Â Â tree orig_arg2 = error_mark_node;
Â
   if (error_operand_p (arg1)
       || error_operand_p (arg2)
@@ -5605,8 +5607,20 @@ build_new_op_1 (location_t loc, enum tree_code code, int flags, tree arg1,
       code_orig_arg2 = TREE_CODE (TREE_TYPE (arg2));
       break;
Â
+Â Â Â Â case PLUS_EXPR:
+      /* These are saved for the sake of warn_string_plus_int.  */
+Â Â Â Â Â Â orig_arg1 = arg1;
+Â Â Â Â Â Â orig_arg2 = arg2;
+Â Â Â Â Â Â break;
+
       /* =, ->, [], () must be non-static member functions.  */
     case MODIFY_EXPR:
+Â Â Â Â Â Â if (code2 == PLUS_EXPR)
+Â Â Â Â Â Â Â Â {
+        /* Like PLUS_EXPR.  */
+Â Â Â Â Â Â Â Â orig_arg1 = arg1;
+ Â Â orig_arg2 = arg2;
+Â Â Â Â Â Â Â Â }
       if (code2 != NOP_EXPR)
 break;
       /* FALLTHRU */
@@ -5937,9 +5951,30 @@ build_new_op_1 (location_t loc, enum tree_code code, int flags, tree arg1,
     return result;
Â
 builtin:
+  /* Warn about adding string literals/pointers and chars.  */
+Â Â if (code == PLUS_EXPR && (complain & tf_warning) &&
+Â Â Â Â Â Â warn_string_plus_int)
+Â Â Â Â {
+Â Â Â Â Â Â maybe_warn_string_plus_int (loc, TREE_TYPE (arg1),
+ Â Â TREE_TYPE (orig_arg2),
+ Â Â TREE_CODE (arg1),
+ Â Â TREE_CODE (orig_arg2));
+Â Â Â Â Â Â maybe_warn_string_plus_int (loc, TREE_TYPE (arg2),
+ Â Â TREE_TYPE (orig_arg1),
+ Â Â TREE_CODE (arg2),
+ Â Â TREE_CODE (orig_arg1));
+Â Â Â Â }
+
   switch (code)
     {
     case MODIFY_EXPR:
+      /* Warn about adding string literals/pointers and chars.  */
+Â Â Â Â Â Â if (code2 == PLUS_EXPR && (complain & tf_warning) &&
+Â Â Â Â Â Â Â Â warn_string_plus_int)
+Â Â Â Â Â Â maybe_warn_string_plus_int (loc, TREE_TYPE (arg1),
+Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â TREE_TYPE (orig_arg2),
+Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â TREE_CODE (arg1),
+Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â TREE_CODE (orig_arg2));
       return cp_build_modify_expr (loc, arg1, code2, arg2, complain);
Â
     case INDIRECT_REF:
--
Xi Ruoyao <ryxi@stu.xidian.edu.cn>
School of Aerospace Science and Technology, Xidian University
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 2/5] c++: New warning option -Wstring-plus-int
2017-02-22 11:10 [PATCH 2/5] c++: New warning option -Wstring-plus-int Xi Ruoyao
@ 2017-04-28 20:45 ` Jeff Law
2017-04-29 7:17 ` Xi Ruoyao
0 siblings, 1 reply; 3+ messages in thread
From: Jeff Law @ 2017-04-28 20:45 UTC (permalink / raw)
To: Xi Ruoyao, gcc-patches
On 02/22/2017 04:01 AM, Xi Ruoyao wrote:
> This patch adds warning option -Wstring-plus-int for C++.
>
> gcc/ChangeLog:
>
> 2017-02-22 Xi Ruoyao <ryxi@stu.xidian.edu.cn>
>
> * c-family/c-common.h (maybe_warn_string_plus_int): new prototype
> * c-family/c-warn.h (maybe_warn_string_plus_int): new function
> * c-family/c-opt: new option -Wstring-plus-int
> * cp/call.c (build_new_op_1): Checking for -Wstring-plus-int
>
> ---
> gcc/c-family/c-common.h | 2 ++
> gcc/c-family/c-warn.c | 34 ++++++++++++++++++++++++++++++++++
> gcc/c-family/c.opt | 5 +++++
> gcc/cp/call.c | 35 +++++++++++++++++++++++++++++++++++
> 4 files changed, 76 insertions(+)
>
> diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
> index 72fba56..8c748e9 100644
> --- a/gcc/c-family/c-common.h
> +++ b/gcc/c-family/c-common.h
> @@ -1503,6 +1503,8 @@ 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, vec<tree, va_gc> *);
> +extern void maybe_warn_string_plus_int (location_t, tree, tree,
> + enum tree_code, enum tree_code);
>
> /* 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 09c5760..0b038a7 100644
> --- a/gcc/c-family/c-warn.c
> +++ b/gcc/c-family/c-warn.c
> @@ -2290,3 +2290,37 @@ do_warn_duplicated_branches_r (tree *tp, int *, void *)
> do_warn_duplicated_branches (*tp);
> return NULL_TREE;
> }
> +
> +static inline bool
> +char_type_cv_p (tree type)
Needs a comment describing what the function does.
> +{
> + return char_type_p (build_qualified_type (type, TYPE_UNQUALIFIED));
> +}
Can you use TYPE_MAIN_VARIANT (type) here rather than building a new type?
> +
> +/* Warn about adding string literals/pointers and chars.
> + Produce a warning when:
> + 1) arg1 is a string literal, and arg2 is a non-literal integer;
> + 2) arg1 is a string pointer, and arg2 is a character. */
> +
> +void
> +maybe_warn_string_plus_int (location_t loc, tree type1, tree type2,
> + enum tree_code code1, enum tree_code code2)
> +{
> + bool arg2_is_char = char_type_cv_p (type2);
> + /* In C11, char16_t and char32_t are typedefs. Prevent false positives
> + about uint_least16_t and uint_least32_t. */
s/about/for/ I think
> + if (type2 == uint_least16_type_node ||
> + type2 == uint_least32_type_node)
> + arg2_is_char = false;
Formatting. Operator goes on the next line.
> +
> + if (code1 == STRING_CST && TREE_CODE (type2) == INTEGER_TYPE &&
> + (code2 != INTEGER_CST || arg2_is_char))
Formatting.
> + warning_at (loc, OPT_Wstring_plus_int,
> + "adding %qT to string literal does not append to "
> + "the string", type2);
> + else if (TREE_CODE (type1) == POINTER_TYPE &&
> + char_type_cv_p (TREE_TYPE (type1)) && arg2_is_char)
Formatting.
> diff --git a/gcc/cp/call.c b/gcc/cp/call.c
> index 93fae0d..95bda1b 100644
> --- a/gcc/cp/call.c
> +++ b/gcc/cp/call.c
> @@ -5555,6 +5555,8 @@ build_new_op_1 (location_t loc, enum tree_code code, int flags, tree arg1,
> void *p;
> bool strict_p;
> bool any_viable_p;
> + tree orig_arg1 = error_mark_node;
> + tree orig_arg2 = error_mark_node;
>
> if (error_operand_p (arg1)
> || error_operand_p (arg2)
> @@ -5605,8 +5607,20 @@ build_new_op_1 (location_t loc, enum tree_code code, int flags, tree arg1,
> code_orig_arg2 = TREE_CODE (TREE_TYPE (arg2));
> break;
>
> + case PLUS_EXPR:
> + /* These are saved for the sake of warn_string_plus_int. */
> + orig_arg1 = arg1;
> + orig_arg2 = arg2;
> + break;
> +
> /* =, ->, [], () must be non-static member functions. */
> case MODIFY_EXPR:
> + if (code2 == PLUS_EXPR)
> + {
> + /* Like PLUS_EXPR. */
> + orig_arg1 = arg1;
> + orig_arg2 = arg2;
> + }
> if (code2 != NOP_EXPR)
> break;
> /* FALLTHRU */
Formatting problems should be obvious :-)
I think Jason probably needs to chime in on this patch. I'm not keen on
the way we stash away the arguments into orig_argX. Perhaps he's got a
better suggestion.
Jeff
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 2/5] c++: New warning option -Wstring-plus-int
2017-04-28 20:45 ` Jeff Law
@ 2017-04-29 7:17 ` Xi Ruoyao
0 siblings, 0 replies; 3+ messages in thread
From: Xi Ruoyao @ 2017-04-29 7:17 UTC (permalink / raw)
To: Jeff Law, gcc-patches; +Cc: ryxi
On 2017-04-28 14:34 -0600, Jeff Law wrote:
> Can you use TYPE_MAIN_VARIANT (type) here rather than building a new type?
I'll check the macros.
> Formatting problems should be obvious :-)
I'll follow the GCC formatting rules.
> I think Jason probably needs to chime in on this patch.  I'm not keen onÂ
> the way we stash away the arguments into orig_argX.  Perhaps he's got aÂ
> better suggestion.
In the recent version of Clang, this warning options has been seperated
into two options -Wstring-plus-int and -Wstring-plus-char. Â Maybe I should
do the same.
--
Xi Ruoyao <ryxi@stu.xidian.edu.cn>
School of Aerospace Science and Technology, Xidian University
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-04-29 2:57 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-22 11:10 [PATCH 2/5] c++: New warning option -Wstring-plus-int Xi Ruoyao
2017-04-28 20:45 ` Jeff Law
2017-04-29 7:17 ` Xi Ruoyao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).