diff mbox

PR35503 - warn for restrict pointer

Message ID CAAgBjM=McC6sgZiBtSBxQN7MiHuAQn+mr7Eh5fvXampE++viMg@mail.gmail.com
State Superseded
Headers show

Commit Message

Prathamesh Kulkarni Aug. 30, 2016, 11:38 a.m. UTC
On 30 August 2016 at 05:34, David Malcolm <dmalcolm@redhat.com> wrote:
> On Mon, 2016-08-29 at 20:01 -0400, David Malcolm wrote:

>> On Mon, 2016-08-29 at 19:55 -0400, David Malcolm wrote:

>> [...]

>> > Assuming you have the location_t values available, you can create a

>> > rich_location for the primary range, and then add secondary ranges

>> > like

>> > this:

>> >

>> >   rich_location richloc (loc_of_arg1);

>>

>> Oops, the above should be:

>>

>>     rich_location richloc (line_table, loc_of_arg1);

>>

>> or:

>>

>>     gcc_rich_location (loc_of_arg1);

> and this should be:

>

>      gcc_rich_location richloc (loc_of_arg1);

>> which does the same thing (#include "gcc-rich-location.h").

>

> Clearly I need to sleep :)

Hi David,
Thanks for the suggestions. I can now see multiple source ranges for
pr35503-2.c (included in patch).
Output shows: http://pastebin.com/FNAVDU8A
(Posted pastebin link to avoid mangling by the mailer)

However the test for underline fails:
FAIL: c-c++-common/pr35503-2.c  -Wc++-compat   expected multiline
pattern lines 12-13 not found: "\s*f \(&alpha, &beta, &alpha,
&alpha\);.*\n      \^~~~~~         ~~~~~~  ~~~~~~ .*\n"
I have attached gcc.log for the test-case. Presumably I have written
the test-case incorrectly.
Could you please have a look at it ?

Thanks,
Prathamesh
>

>> >   richloc.add_range (loc_of_arg3, false);  /* false here = don't

>> > draw

>> > a

>> > caret, just the underline */

>> >   richloc.add_range (loc_of_arg4, false);

>> >   warning_at_rich_loc (&richloc, OPT_Wrestrict, etc...

>> >

>> > See line-map.h for more information on rich_location.

>>

>> [...]

Comments

Prathamesh Kulkarni Aug. 31, 2016, 4 p.m. UTC | #1
On 30 August 2016 at 18:49, David Malcolm <dmalcolm@redhat.com> wrote:
> On Tue, 2016-08-30 at 17:08 +0530, Prathamesh Kulkarni wrote:

>> On 30 August 2016 at 05:34, David Malcolm <dmalcolm@redhat.com>

>> wrote:

>> > On Mon, 2016-08-29 at 20:01 -0400, David Malcolm wrote:

>> > > On Mon, 2016-08-29 at 19:55 -0400, David Malcolm wrote:

>> > > [...]

>> > > > Assuming you have the location_t values available, you can

>> > > > create a

>> > > > rich_location for the primary range, and then add secondary

>> > > > ranges

>> > > > like

>> > > > this:

>> > > >

>> > > >   rich_location richloc (loc_of_arg1);

>> > >

>> > > Oops, the above should be:

>> > >

>> > >     rich_location richloc (line_table, loc_of_arg1);

>> > >

>> > > or:

>> > >

>> > >     gcc_rich_location (loc_of_arg1);

>> > and this should be:

>> >

>> >      gcc_rich_location richloc (loc_of_arg1);

>> > > which does the same thing (#include "gcc-rich-location.h").

>> >

>> > Clearly I need to sleep :)

>> Hi David,

>> Thanks for the suggestions. I can now see multiple source ranges for

>> pr35503-2.c (included in patch).

>> Output shows: http://pastebin.com/FNAVDU8A

>> (Posted pastebin link to avoid mangling by the mailer)

>

> The underlines look great, thanks for working on this.

Thanks -;)
>

>> However the test for underline fails:

>> FAIL: c-c++-common/pr35503-2.c  -Wc++-compat   expected multiline

>> pattern lines 12-13 not found: "\s*f \(&alpha, &beta, &alpha,

>> &alpha\);.*\n      \^~~~~~         ~~~~~~  ~~~~~~ .*\n"

>> I have attached gcc.log for the test-case. Presumably I have written

>> the test-case incorrectly.

>> Could you please have a look at it ?

>

> (I hope this doesn't get too badly mangled by Evolution...)

>

> I think you have an extra trailing space on the line containing the

> expected underlines within the multiline directive:

>

> +/* { dg-begin-multiline-output "" }

> +   f (&alpha, &beta, &alpha, &alpha);

> +      ^~~~~~         ~~~~~~  ~~~~~~

>                                     ^ EXTRA SPACE HERE

> +   { dg-end-multiline-output "" } */

> +}

>

> as the actual output is:

>

>    f (&alpha, &beta, &alpha, &alpha);

>       ^~~~~~         ~~~~~~  ~~~~~~

>                                   ^ LINE ENDS HERE, with no trailing

> space present

>

> This space shows up in the error here:

>

> FAIL: c-c++-common/pr35503-2.c  -Wc++-compat   expected multiline

> pattern lines 12-13 not found: "\s*f \(&alpha, &beta, &alpha,

> &alpha\);.*\n      \^~~~~~         ~~~~~~  ~~~~~~ .*\n"

>                                                  ^ EXTRA SPACE

>

> BTW, the .* at the end of the pattern means it's ok to have additional

> material in the actual output that isn't matched (e.g. for comments

> containing dg- directives [1] ) but it doesn't work the other way

> around: all of the text within the dg-begin/end-multiline directives

> has to be in the actual output.

>

>

> [1] so you can have e.g.:

>

>   f (&alpha, &beta, &alpha, &alpha); /* { dg-warning "passing argument 1 to restrict-qualified parameter aliases with arguments 3, 4" } */

>

> and:

>

> /* { dg-begin-multiline-output "" }

>    f (&alpha, &beta, &alpha, &alpha);

>       ^~~~~~         ~~~~~~  ~~~~~~

>    { dg-end-multiline-output "" } */

>

> where the actual output will look like:

>

> pr35503-2.c:8:6: warning: passing argument 1 to restrict-qualified parameter aliases with arguments 3, 4 [-Wrestrict]

>    f (&alpha, &beta, &alpha, &alpha); /* { dg-warning "passing argument 1 to restrict-qualified parameter aliases with arguments 3, 4" } */

>      ^~~~~~         ~~~~~~  ~~~~~~

>

> and you can omit the copy of the dg-warning directive in the expected

> multiline output (which would otherwise totally confuse DejaGnu).

> Doing so avoids having to specify the line number.

Thanks for the detailed explanation! The test-case is now fixed.

Regards,
Prathamesh
>

>> Thanks,

>> Prathamesh

>> >

>> > > >   richloc.add_range (loc_of_arg3, false);  /* false here =

>> > > > don't

>> > > > draw

>> > > > a

>> > > > caret, just the underline */

>> > > >   richloc.add_range (loc_of_arg4, false);

>> > > >   warning_at_rich_loc (&richloc, OPT_Wrestrict, etc...

>> > > >

>> > > > See line-map.h for more information on rich_location.

>> > >

>> > > [...]
diff mbox

Patch

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 3feb910..4239cef 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -47,6 +47,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "gimplify.h"
 #include "substring-locations.h"
 #include "spellcheck.h"
+#include "gcc-rich-location.h"
 
 cpp_reader *parse_in;		/* Declared in c-pragma.h.  */
 
@@ -13057,4 +13058,86 @@  diagnose_mismatched_attributes (tree olddecl, tree newdecl)
   return warned;
 }
 
+/* Warn if an argument at position param_pos is passed to a
+   restrict-qualified param, and it aliases with another argument.  */
+
+void
+warn_for_restrict (unsigned param_pos, vec<tree, va_gc> *args)
+{
+  tree arg = (*args)[param_pos];
+  if (TREE_VISITED (arg) || operand_equal_p (arg, null_pointer_node, 0))
+    return;
+
+  location_t loc = EXPR_LOC_OR_LOC (arg, input_location);
+  gcc_rich_location richloc (loc);
+
+  unsigned i;
+  tree current_arg;
+  auto_vec<unsigned> arg_positions;
+
+  FOR_EACH_VEC_ELT (*args, i, current_arg) 
+    {
+      if (i == param_pos)
+	continue;
+
+      tree current_arg = (*args)[i];
+      if (operand_equal_p (arg, current_arg, 0))
+	{
+	  TREE_VISITED (current_arg) = 1; 
+	  arg_positions.safe_push (i);
+	}
+    }
+
+  if (arg_positions.is_empty ())
+    return;
+
+  struct obstack fmt_obstack;
+  gcc_obstack_init (&fmt_obstack);
+  char *fmt = (char *) obstack_alloc (&fmt_obstack, 0);
+
+  char num[32];
+  sprintf (num, "%u", param_pos + 1);
+
+  obstack_grow (&fmt_obstack, "passing argument ",
+		strlen ("passing argument "));
+  obstack_grow (&fmt_obstack, num, strlen (num));
+  obstack_grow (&fmt_obstack,
+		" to restrict-qualified parameter aliases with argument",
+		strlen (" to restrict-qualified parameter "
+			"aliases with argument"));
+
+  /* make argument plural and append space.  */
+  if (arg_positions.length () > 1)
+    obstack_1grow (&fmt_obstack, 's');
+  obstack_1grow (&fmt_obstack, ' ');
+
+  unsigned pos;
+  unsigned ranges_added = 1;
+
+  FOR_EACH_VEC_ELT (arg_positions, i, pos)
+    {
+      /* FIXME: Fow now, only 3 ranges can be added. Remove this once
+	 the restriction is lifted.  */
+      if (ranges_added < 3)
+	{
+	  tree arg = (*args)[pos];
+	  if (EXPR_HAS_LOCATION (arg))
+	    {
+	      richloc.add_range (EXPR_LOCATION (arg), false);
+	      ranges_added++;
+	    }
+	}
+
+      sprintf (num, "%u", pos + 1);
+      obstack_grow (&fmt_obstack, num, strlen (num));
+
+      if (i < arg_positions.length () - 1)
+	obstack_grow (&fmt_obstack, ", ",  strlen (", "));
+    }
+
+  obstack_1grow (&fmt_obstack, 0);
+  warning_at_rich_loc (&richloc, OPT_Wrestrict, fmt);
+  obstack_free (&fmt_obstack, fmt);
+}
+
 #include "gt-c-family-c-common.h"
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index bc22baa..cdb762e 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -920,6 +920,7 @@  extern void c_parse_final_cleanups (void);
 
 extern void warn_for_omitted_condop (location_t, tree);
 extern void warn_for_memset (location_t, tree, tree, int);
+extern void warn_for_restrict (unsigned, vec<tree, va_gc> *);
 
 /* These macros provide convenient access to the various _STMT nodes.  */
 
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index a5358ed..a029a86 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1012,6 +1012,11 @@  Wduplicate-decl-specifier
 C ObjC Var(warn_duplicate_decl_specifier) Warning LangEnabledBy(C ObjC,Wall)
 Warn when a declaration has duplicate const, volatile, restrict or _Atomic specifier.
 
+Wrestrict
+C C++ Var(warn_restrict) Warning LangEnabledBy(C C++,Wall)
+Warn when an argument passed to a restrict-qualified parameter aliases with
+another argument.
+
 ansi
 C ObjC C++ ObjC++
 A synonym for -std=c89 (for C) or -std=c++98 (for C++).
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index fe0c95f..d27a4be 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -8369,6 +8369,24 @@  c_parser_postfix_expression_after_primary (c_parser *parser,
 	      warn_for_memset (expr_loc, arg0, arg2, literal_zero_mask);
 	    }
 
+	  if (TREE_CODE (expr.value) == FUNCTION_DECL && warn_restrict)
+	    {
+	      unsigned i;
+	      tree arg;
+	      FOR_EACH_VEC_SAFE_ELT (exprlist, i, arg)
+		TREE_VISITED (arg) = 0;
+
+	      unsigned param_pos = 0;
+	      function_args_iterator iter;
+	      tree t;
+	      FOREACH_FUNCTION_ARGS (TREE_TYPE (expr.value), t, iter)
+		{
+		  if (POINTER_TYPE_P (t) && TYPE_RESTRICT (t))
+		    warn_for_restrict (param_pos, exprlist);
+		  param_pos++;
+		}
+	    }
+
 	  start = expr.get_start ();
 	  finish = parser->tokens_buf[0].get_finish ();
 	  expr.value
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 690e928..dce117d 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -6878,6 +6878,25 @@  cp_parser_postfix_expression (cp_parser *parser, bool address_p, bool cast_p,
 		warn_for_memset (input_location, arg0, arg2, literal_mask);
 	      }
 
+	    if (TREE_CODE (postfix_expression) == FUNCTION_DECL
+		&& warn_restrict)
+	      {
+		unsigned i;
+		tree arg;
+		FOR_EACH_VEC_SAFE_ELT (args, i, arg)
+		  TREE_VISITED (arg) = 0;
+
+		unsigned param_pos = 0;
+		for (tree decl = DECL_ARGUMENTS (postfix_expression);
+		     decl != NULL_TREE;
+		     decl = DECL_CHAIN (decl), param_pos++)
+		  {
+		    tree type = TREE_TYPE (decl);
+		    if (POINTER_TYPE_P (type) && TYPE_RESTRICT (type))
+		      warn_for_restrict (param_pos, args);
+		  }
+	      }
+
 	    if (TREE_CODE (postfix_expression) == COMPONENT_REF)
 	      {
 		tree instance = TREE_OPERAND (postfix_expression, 0);
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 1f04501..3bd9612 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -288,7 +288,7 @@  Objective-C and Objective-C++ Dialects}.
 -Wparentheses -Wno-pedantic-ms-format @gol
 -Wplacement-new -Wplacement-new=@var{n} @gol
 -Wpointer-arith  -Wno-pointer-to-int-cast @gol
--Wno-pragmas -Wredundant-decls  -Wno-return-local-addr @gol
+-Wno-pragmas -Wredundant-decls -Wrestrict  -Wno-return-local-addr @gol
 -Wreturn-type  -Wsequence-point  -Wshadow  -Wno-shadow-ivar @gol
 -Wshift-overflow -Wshift-overflow=@var{n} @gol
 -Wshift-count-negative -Wshift-count-overflow -Wshift-negative-value @gol
@@ -5274,6 +5274,12 @@  compilations.
 Warn when deleting a pointer to incomplete type, which may cause
 undefined behavior at runtime.  This warning is enabled by default.
 
+@item -Wrestrict @r{(C and C++ only)}
+@opindex Wrestrict
+@opindex Wno-restrict
+Warn when an argument passed to a restrict-qualified parameter
+aliases with another argument
+
 @item -Wuseless-cast @r{(C++ and Objective-C++ only)}
 @opindex Wuseless-cast
 @opindex Wno-useless-cast
diff --git a/gcc/testsuite/c-c++-common/pr35503-1.c b/gcc/testsuite/c-c++-common/pr35503-1.c
new file mode 100644
index 0000000..b47fdda
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr35503-1.c
@@ -0,0 +1,10 @@ 
+/* { dg-do compile } */
+/* { dg-options "-Wrestrict" } */
+
+int foo (const char *__restrict buf, const char *__restrict fmt, ...);
+
+void f(void)
+{
+  char buf[100] = "hello";
+  foo (buf, "%s-%s", buf, "world"); /*  { dg-warning "passing argument 1 to restrict-qualified parameter aliases with argument 3" } */
+}
diff --git a/gcc/testsuite/c-c++-common/pr35503-2.c b/gcc/testsuite/c-c++-common/pr35503-2.c
new file mode 100644
index 0000000..b09872a
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr35503-2.c
@@ -0,0 +1,15 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fdiagnostics-show-caret -Wrestrict" } */
+
+void f(int *__restrict x, int *y, int *__restrict z, int *w);
+
+void foo(int alpha, int beta)
+{
+  f (&alpha, &beta, &alpha, &alpha);
+  /* { dg-warning "passing argument 1 to restrict-qualified parameter aliases with arguments 3, 4" "" { target *-*-* } 8 } */
+
+/* { dg-begin-multiline-output "" }
+   f (&alpha, &beta, &alpha, &alpha);
+      ^~~~~~         ~~~~~~  ~~~~~~ 
+   { dg-end-multiline-output "" } */
+}
diff --git a/gcc/testsuite/c-c++-common/pr35503-3.c b/gcc/testsuite/c-c++-common/pr35503-3.c
new file mode 100644
index 0000000..8cbacab
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr35503-3.c
@@ -0,0 +1,9 @@ 
+/* { dg-do compile } */
+/* { dg-options "-Wrestrict" } */
+
+void f(int *x, int *__restrict y);
+
+void foo(int a)
+{
+  f (&a, &a); /* { dg-warning "passing argument 2 to restrict-qualified parameter aliases with argument 1" } */
+}