This is the mail archive of the gcc-patches@gcc.gnu.org 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]

[PATCH] [C] Warn when calculating abs(unsigned_value)


Hi,

when you try compiling a call to function abs and provide an unsigned
int in the argument in C++, you will get an error about ambiguous
overload.  In C however, it will pass without silently.  The following
patch adds a warning for these cases, because I think it is likely that
such code does not do what the author intended.

I thought it a bit of an overkill to add a new warning group because of
this, so I added it to -Wtype-limits, which seemed the best fit.  As a
consequence, the warning is on with -Wextra.  The warning can be
suppressed with an explicit type-cast (which at the moment is implicit),
if someone really used it for some bit-tricks.

Bootstrapped and tested on x86_64-linux, also with make info.  What do
you think, is this a good idea?  Is it perhaps OK for trunk?

Thanks,

Martin



2018-08-14  Martin Jambor  <mjambor@suse.cz>

	* doc/invoke.texi (Warning Options): Document warning on calculating
	absolute value of an unsigned value.

c/ChangeLog:
	* c-parser.c (c_parser_postfix_expression_after_primary): Warn if
	calculating an absolute value of an unsigned value.

testsuite/ChangeLog
	* gcc.dg/warn-abs-unsigned-1.c: New test.
---
 gcc/c/c-parser.c                           | 20 ++++++++++++++------
 gcc/doc/invoke.texi                        |  5 +++--
 gcc/testsuite/gcc.dg/warn-abs-unsigned-1.c | 21 +++++++++++++++++++++
 3 files changed, 38 insertions(+), 8 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/warn-abs-unsigned-1.c

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 7a926285f3a..bf3b8cc75f9 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -9160,13 +9160,21 @@ c_parser_postfix_expression_after_primary (c_parser *parser,
 					      sizeof_arg,
 					      sizeof_ptr_memacc_comptypes);
 	  if (TREE_CODE (expr.value) == FUNCTION_DECL
-	      && DECL_BUILT_IN_CLASS (expr.value) == BUILT_IN_NORMAL
-	      && DECL_FUNCTION_CODE (expr.value) == BUILT_IN_MEMSET
-	      && vec_safe_length (exprlist) == 3)
+	      && DECL_BUILT_IN_CLASS (expr.value) == BUILT_IN_NORMAL)
 	    {
-	      tree arg0 = (*exprlist)[0];
-	      tree arg2 = (*exprlist)[2];
-	      warn_for_memset (expr_loc, arg0, arg2, literal_zero_mask);
+	      if (DECL_FUNCTION_CODE (expr.value) == BUILT_IN_MEMSET
+		  && vec_safe_length (exprlist) == 3)
+		{
+		  tree arg0 = (*exprlist)[0];
+		  tree arg2 = (*exprlist)[2];
+		  warn_for_memset (expr_loc, arg0, arg2, literal_zero_mask);
+		}
+	      else if (DECL_FUNCTION_CODE (expr.value) == BUILT_IN_ABS
+		       && vec_safe_length (exprlist) == 1
+		       && TYPE_UNSIGNED (TREE_TYPE ((*exprlist)[0])))
+		warning_at (expr_loc, OPT_Wtype_limits,
+			    "calculation of an absolute value of "
+			    "a value of unsigned type");
 	    }
 
 	  start = expr.get_start ();
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index d7fd0e17555..b757ac0d0c8 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -6179,8 +6179,9 @@ This warning is enabled by default.
 Warn if a comparison is always true or always false due to the limited
 range of the data type, but do not warn for constant expressions.  For
 example, warn if an unsigned variable is compared against zero with
-@code{<} or @code{>=}.  This warning is also enabled by
-@option{-Wextra}.
+@code{<} or @code{>=}.  When compiling C, also warn when calculating
+an absolute value from an unsigned type.  This warning is also enabled
+by @option{-Wextra}.
 
 @include cppwarnopts.texi
 
diff --git a/gcc/testsuite/gcc.dg/warn-abs-unsigned-1.c b/gcc/testsuite/gcc.dg/warn-abs-unsigned-1.c
new file mode 100644
index 00000000000..8d2a6d87e6b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/warn-abs-unsigned-1.c
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-options "-Wtype-limits" } */
+
+#include <stdlib.h>
+
+unsigned __attribute__((noipa))
+get_input (void)
+{
+  return 22;
+}
+
+volatile unsigned g;
+
+int
+main(int argc __attribute__((unused)), char **argv __attribute__((unused)))
+{
+  unsigned u = get_input ();
+  u = abs (u);  /* { dg-warning "calculation of an absolute value of a value of unsigned type" } */
+  g = u;
+  return 0;
+}
-- 
2.18.0


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