PR35503 - warn for restrict pointer

PR35503 - warn for restrict pointer

[Prathamesh Kulkarni]

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

Hi,
The following patch adds option -Wrestrict that warns when an argument
is passed to a restrict qualified parameter and it aliases with
another argument.

eg:
int foo (const char *__restrict buf, const char *__restrict fmt, ...);

void f(void)
{
  char buf[100] = "hello";
  foo (buf, "%s-%s", buf, "world");
}

With the patch, C FE warns:
test-3.c: In function ‘f’:
test-3.c:6:3: warning: passing argument 1 to restrict qualified
parameter aliases with argument 3

   foo (buf, "%s-%s", buf, "world");
   ^~~

However with C++FE it appears TYPE_RESTRICT is not set for the
parameters (buf and fmt)
and hence the warning doesn't get emitted for C++.
C FE sets TYPE_RESTRICT for them. I am not sure how to workaround this
issue, and would be grateful for suggestions.

Thanks,
Prathamesh

[-- Attachment #2: pr35503-1.diff --]
[-- Type: text/plain, Size: 4967 bytes --]

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 3feb910..e1f2823 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -13057,4 +13057,35 @@ diagnose_mismatched_attributes (tree olddecl, tree newdecl)
   return warned;
 }
 
+/* Warn if an argument is passed to a restrict-qualified param,
+   and it aliases with another argument.  */
+
+void
+warn_for_restrict (location_t loc, tree fn_decl, vec<tree, va_gc> *args)
+{
+  unsigned param_pos = 0;
+
+  for (tree t = TYPE_ARG_TYPES (TREE_TYPE (fn_decl)); t; t = TREE_CHAIN (t), param_pos++)
+    {
+      tree type = TREE_VALUE (t);
+      if (POINTER_TYPE_P (type) && TYPE_RESTRICT (type))
+	{
+	  tree arg = (*args)[param_pos];
+	  gcc_assert (POINTER_TYPE_P (TREE_TYPE (arg)));
+
+	  for (unsigned i = 0; i < args->length (); ++i)
+	    {
+	      if (i == param_pos)
+		continue;
+
+	      tree current_arg = (*args)[i];
+	      if (operand_equal_p (arg, current_arg, 0))
+		warning_at (loc, 0,
+			    "passing argument %u to restrict qualified parameter aliases with "
+			    "argument %u\n", param_pos + 1, i + 1);
+	    }
+	}
+    }
+}
+
 #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..006cb13 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 (location_t, tree, 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..0016183 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -8369,6 +8369,9 @@ 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)
+	    warn_for_restrict (expr_loc, expr.value, exprlist);
+
 	  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..0fd7cd1 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -6878,6 +6878,10 @@ 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)
+	      warn_for_restrict (input_location, postfix_expression, 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..ac8aab5 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -261,7 +261,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wc++-compat -Wc++11-compat -Wc++14-compat -Wcast-align  -Wcast-qual  @gol
 -Wchar-subscripts -Wclobbered  -Wcomment -Wconditionally-supported  @gol
 -Wconversion -Wcoverage-mismatch -Wno-cpp -Wdangling-else -Wdate-time @gol
--Wdelete-incomplete @gol
+-Wdelete-incomplete -Wrestrict @gol
 -Wno-deprecated -Wno-deprecated-declarations -Wno-designated-init @gol
 -Wdisabled-optimization @gol
 -Wno-discarded-qualifiers -Wno-discarded-array-qualifiers @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.c b/gcc/testsuite/c-c++-common/pr35503.c
new file mode 100644
index 0000000..d689a8e
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr35503.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" */
+}

Re: PR35503 - warn for restrict pointer

[Joseph Myers]

Arguments passed to diagnostic functions should not end with '\n'.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: PR35503 - warn for restrict pointer

[Jason Merrill]

On Fri, Aug 26, 2016 at 7:39 AM, Prathamesh Kulkarni
<prathamesh.kulkarni@linaro.org> wrote:
> However with C++FE it appears TYPE_RESTRICT is not set for the
> parameters (buf and fmt)
> and hence the warning doesn't get emitted for C++.
> C FE sets TYPE_RESTRICT for them. I am not sure how to workaround this
> issue, and would be grateful for suggestions.

In the C++ FE you can see TYPE_RESTRICT on the types of the
DECL_ARGUMENTS of the function.

Jason

Re: PR35503 - warn for restrict pointer

[Prathamesh Kulkarni]

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

On 26 August 2016 at 21:25, Jason Merrill <jason@redhat.com> wrote:
> On Fri, Aug 26, 2016 at 7:39 AM, Prathamesh Kulkarni
> <prathamesh.kulkarni@linaro.org> wrote:
>> However with C++FE it appears TYPE_RESTRICT is not set for the
>> parameters (buf and fmt)
>> and hence the warning doesn't get emitted for C++.
>> C FE sets TYPE_RESTRICT for them. I am not sure how to workaround this
>> issue, and would be grateful for suggestions.
>
> In the C++ FE you can see TYPE_RESTRICT on the types of the
> DECL_ARGUMENTS of the function.
Thanks for the suggestions, I could indeed see TYPE_RESTRICT set on
DECL_ARGUMENTS.
The attached patch warns for both C and C++ with -Wrestrict, and I
have put it under -Wall.
I suppose we don't want to warn for null pointer args ?
for eg:
void f(int *restrict x, int *y);

void foo(void)
{
  f(NULL, NULL) ?
}

However I suppose warning for pointer constants other than zero is desirable ?
I am not sure whether restrict is valid for obj-c/obj-c++, so I have
just kept it to C and C++
in the patch. Should the warning also be included for obj-c and obj-c++ FE's ?
Boostrapped and tested on x86_64-unknown-linux-gnu.
OK to commit ?

Thanks,
Prathamesh
>
> Jason

[-- Attachment #2: pr35503-3.txt --]
[-- Type: text/plain, Size: 5761 bytes --]

2016-08-28  Prathamesh Kulkarni  <prathamesh.kulkarni@linaro.org>

	PR c/35503
	* doc/invoke.texi: Document -Wrestrict.
c-family/
	* c-common.c (warn_for_restrict): New function.
	* c-common.h (warn_for_restrict): Declare it.
	* c.opt: New option -Wrestrit.
c/
	* c-parser.c (c_parser_postfix_expression_after_primary): Call
	warn_for_restrict.
cp/
	* parser.c (cp_parser_postfix_expression): Likewise.
testsuite/
	* c-c++-common/pr35503.c: New test-case.

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 3feb910..ec8778f 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -13057,4 +13057,27 @@ 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 (location_t loc, unsigned param_pos, vec<tree, va_gc> *args)
+{
+  tree arg = (*args)[param_pos];
+  if (operand_equal_p (arg, null_pointer_node, 0))
+    return;
+
+  for (unsigned i = 0; i < args->length (); ++i)
+    {
+      if (i == param_pos)
+	continue;
+
+      tree current_arg = (*args)[i];
+      if (operand_equal_p (arg, current_arg, 0))
+	warning_at (loc, 0,
+		    "passing argument %u to restrict qualified parameter aliases with "
+		    "argument %u", param_pos + 1, i + 1);
+    }
+}
+
 #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..0526ad5 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 (location_t, 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..54e1e87 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -8369,6 +8369,17 @@ 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 param_pos = 0;
+	      for (tree t = TYPE_ARG_TYPES (TREE_TYPE (expr.value)); t; t = TREE_CHAIN (t), param_pos++) 
+		{
+		  tree type = TREE_VALUE (t);
+		  if (POINTER_TYPE_P (type) && TYPE_RESTRICT (type))
+		    warn_for_restrict (expr_loc, param_pos, exprlist);	
+		}
+	    }
+
 	  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..4710a08 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -6878,6 +6878,20 @@ 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 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 (input_location, 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..ac8aab5 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -261,7 +261,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wc++-compat -Wc++11-compat -Wc++14-compat -Wcast-align  -Wcast-qual  @gol
 -Wchar-subscripts -Wclobbered  -Wcomment -Wconditionally-supported  @gol
 -Wconversion -Wcoverage-mismatch -Wno-cpp -Wdangling-else -Wdate-time @gol
--Wdelete-incomplete @gol
+-Wdelete-incomplete -Wrestrict @gol
 -Wno-deprecated -Wno-deprecated-declarations -Wno-designated-init @gol
 -Wdisabled-optimization @gol
 -Wno-discarded-qualifiers -Wno-discarded-array-qualifiers @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.c b/gcc/testsuite/c-c++-common/pr35503.c
new file mode 100644
index 0000000..fb12a71
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr35503.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" } */
+}

Re: PR35503 - warn for restrict pointer

[Marek Polacek]

On Sun, Aug 28, 2016 at 06:32:59PM +0530, Prathamesh Kulkarni wrote:
> On 26 August 2016 at 21:25, Jason Merrill <jason@redhat.com> wrote:
> > On Fri, Aug 26, 2016 at 7:39 AM, Prathamesh Kulkarni
> > <prathamesh.kulkarni@linaro.org> wrote:
> >> However with C++FE it appears TYPE_RESTRICT is not set for the
> >> parameters (buf and fmt)
> >> and hence the warning doesn't get emitted for C++.
> >> C FE sets TYPE_RESTRICT for them. I am not sure how to workaround this
> >> issue, and would be grateful for suggestions.
> >
> > In the C++ FE you can see TYPE_RESTRICT on the types of the
> > DECL_ARGUMENTS of the function.
> Thanks for the suggestions, I could indeed see TYPE_RESTRICT set on
> DECL_ARGUMENTS.
> The attached patch warns for both C and C++ with -Wrestrict, and I
> have put it under -Wall.
> I suppose we don't want to warn for null pointer args ?
> for eg:
> void f(int *restrict x, int *y);
> 
> void foo(void)
> {
>   f(NULL, NULL) ?
> }
> 
> However I suppose warning for pointer constants other than zero is desirable ?
> I am not sure whether restrict is valid for obj-c/obj-c++, so I have
> just kept it to C and C++
> in the patch. Should the warning also be included for obj-c and obj-c++ FE's ?
> Boostrapped and tested on x86_64-unknown-linux-gnu.
> OK to commit ?
> 
> Thanks,
> Prathamesh
> >
> > Jason

> 2016-08-28  Prathamesh Kulkarni  <prathamesh.kulkarni@linaro.org>
> 
> 	PR c/35503
> 	* doc/invoke.texi: Document -Wrestrict.
> c-family/
> 	* c-common.c (warn_for_restrict): New function.
> 	* c-common.h (warn_for_restrict): Declare it.
> 	* c.opt: New option -Wrestrit.

Typo.

> c/
> 	* c-parser.c (c_parser_postfix_expression_after_primary): Call
> 	warn_for_restrict.
> cp/
> 	* parser.c (cp_parser_postfix_expression): Likewise.

This is a ChangeLog entry in another directory, so Likewise wouldn't work here.

> testsuite/
> 	* c-c++-common/pr35503.c: New test-case.
> 
> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
> index 3feb910..ec8778f 100644
> --- a/gcc/c-family/c-common.c
> +++ b/gcc/c-family/c-common.c
> @@ -13057,4 +13057,27 @@ 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 (location_t loc, unsigned param_pos, vec<tree, va_gc> *args)
> +{
> +  tree arg = (*args)[param_pos];
> +  if (operand_equal_p (arg, null_pointer_node, 0))
> +    return;
> +
> +  for (unsigned i = 0; i < args->length (); ++i)

Use FOR_EACH_VEC_ELT?

> +    {
> +      if (i == param_pos)
> +	continue;
> +
> +      tree current_arg = (*args)[i];
> +      if (operand_equal_p (arg, current_arg, 0))
> +	warning_at (loc, 0,
> +		    "passing argument %u to restrict qualified parameter aliases with "

I think this should be "restrict-qualified".

> +		    "argument %u", param_pos + 1, i + 1);
> +    }
> +}
> +
>  #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..0526ad5 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 (location_t, 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..54e1e87 100644
> --- a/gcc/c/c-parser.c
> +++ b/gcc/c/c-parser.c
> @@ -8369,6 +8369,17 @@ 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 param_pos = 0;
> +	      for (tree t = TYPE_ARG_TYPES (TREE_TYPE (expr.value)); t; t = TREE_CHAIN (t), param_pos++) 
> +		{
> +		  tree type = TREE_VALUE (t);
> +		  if (POINTER_TYPE_P (type) && TYPE_RESTRICT (type))
> +		    warn_for_restrict (expr_loc, param_pos, exprlist);	
> +		}
> +	    }
> +

Line too long, but I think I'd prefer something like

          if (TREE_CODE (expr.value) == FUNCTION_DECL && warn_restrict)
            {
              unsigned int 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 (expr_loc, param_pos, exprlist);
                  param_pos++;
                }     
            }

instead.

> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 1f04501..ac8aab5 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -261,7 +261,7 @@ Objective-C and Objective-C++ Dialects}.
>  -Wc++-compat -Wc++11-compat -Wc++14-compat -Wcast-align  -Wcast-qual  @gol
>  -Wchar-subscripts -Wclobbered  -Wcomment -Wconditionally-supported  @gol
>  -Wconversion -Wcoverage-mismatch -Wno-cpp -Wdangling-else -Wdate-time @gol
> --Wdelete-incomplete @gol
> +-Wdelete-incomplete -Wrestrict @gol

I thought these were supposed to be in alphabetical order.

>  -Wno-deprecated -Wno-deprecated-declarations -Wno-designated-init @gol
>  -Wdisabled-optimization @gol
>  -Wno-discarded-qualifiers -Wno-discarded-array-qualifiers @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

Missing period.

> +
>  @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.c b/gcc/testsuite/c-c++-common/pr35503.c
> new file mode 100644
> index 0000000..fb12a71
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/pr35503.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" } */
> +}

I'm not sure if this test coverage is enough...

	Marek

Re: PR35503 - warn for restrict pointer

[Tobias Burnus]

Prathamesh Kulkarni wrote:
> Attachment: pr35503-3.txt

I tried the patch - and it found a bug in our code; nice!


(a) Regarding the [-Werror] output:

   error: passing argument 24 to restrict qualified parameter aliases with argument 29 [-Werror]

Shouldn't that output "[-Werror=restrict]" instead of a bare "[-Werror]? Namely, instead of

+	warning_at (loc, 0,
+		    "passing argument %u to restrict qualified parameter aliases with "
+		    "argument %u", param_pos + 1, i + 1);

I think one gets this with
        warning_at (loc, OPT_Wrestrict, ...


 * * *

(b) I get:


file.cc:5582:41: error: passing argument 24 to restrict qualified parameter aliases with argument 29 [-Werror]
                                out, out2);
                                         ^
file.cc:5582:41: error: passing argument 29 to restrict qualified parameter aliases with argument 24 [-Werror]


Thus, the message is kind of shown twice, one for 24 and once for 29;
but only one has a "^" possition, pointing after the 31st argument.


(i) I had expected that the message is shown only once. The issue here that
both the 24th and the 29th argument have the restrict qualifier. A simple

+      if (i < param_pos)
+	continue;

in warn_for_restrict would have silenced the second call, but would have
missed the output if only the 29th argument had the qualifier and not
also the 24th argument. Thus, one needs some more logic.


(ii) It would be great if the "^" wouldn't point to the end of the
argument list but to the one of the aliasing arguments - or even better
to "^~~~~~~" to the first argument and to "~~~~~~~~~" to the second.

I have to admit that I don't know how to use rich_location, but I am sure
digging in the source would help; possibly, also David or Marek can give
you a hint.

Cheers,

Tobias

Re: PR35503 - warn for restrict pointer

[Marek Polacek]

On Mon, Aug 29, 2016 at 04:25:25PM +0200, Tobias Burnus wrote:
> Prathamesh Kulkarni wrote:
> > Attachment: pr35503-3.txt
> 
> I tried the patch - and it found a bug in our code; nice!
> 
> 
> (a) Regarding the [-Werror] output:
> 
>    error: passing argument 24 to restrict qualified parameter aliases with argument 29 [-Werror]
> 
> Shouldn't that output "[-Werror=restrict]" instead of a bare "[-Werror]? Namely, instead of
> 
> +	warning_at (loc, 0,
> +		    "passing argument %u to restrict qualified parameter aliases with "
> +		    "argument %u", param_pos + 1, i + 1);
> 
> I think one gets this with
>         warning_at (loc, OPT_Wrestrict, ...
 
Yes, this needs to be fixed in the patch.

	Marek

Re: PR35503 - warn for restrict pointer

[Prathamesh Kulkarni]

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

On 29 August 2016 at 19:59, Marek Polacek <polacek@redhat.com> wrote:
> On Mon, Aug 29, 2016 at 04:25:25PM +0200, Tobias Burnus wrote:
>> Prathamesh Kulkarni wrote:
>> > Attachment: pr35503-3.txt
>>
>> I tried the patch - and it found a bug in our code; nice!
>>
>>
>> (a) Regarding the [-Werror] output:
>>
>>    error: passing argument 24 to restrict qualified parameter aliases with argument 29 [-Werror]
>>
>> Shouldn't that output "[-Werror=restrict]" instead of a bare "[-Werror]? Namely, instead of
>>
>> +     warning_at (loc, 0,
>> +                 "passing argument %u to restrict qualified parameter aliases with "
>> +                 "argument %u", param_pos + 1, i + 1);
>>
>> I think one gets this with
>>         warning_at (loc, OPT_Wrestrict, ...
>
> Yes, this needs to be fixed in the patch.
Hi,
Thanks for all the suggestions, I have tried to incorporate them in
the attached version.
To avoid duplicating the warnings for multiple aliased arguments,
I am using TREE_VISITED to mark all the aliased arguments of the current
arg, which makes diagnostic emitted only once per same set of aliases.
Is using TREE_VISITED ok as in the patch ?

eg:
void f(int *__restrict x, int *y, int *__restrict z, int *w);

void foo(int a, int b)
{
  f (&a, &b, &a, &a);
}

Output:
test-2.c: In function ‘foo’:
test-2.c:5:6: warning: passing argument 1 to restrict-qualified
parameter aliases with arguments 3, 4 [-Wrestrict]
   f (&a, &b, &a, &a);
       ^~

Using EXPR_LOCATION (arg), helps underline the argument for both C and C++
as in the example above. However in case of variadic functions, it
appears C++FE doesn't set EXPR_LOCATION,
so I am falling back to using input_location if EXPR_LOCATION is unknown.
However it gives the location correctly for non-variadic functions.
For setting locations correctly I will try the approach suggested by Jason in
https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01987.html

I was wondering how to underline all the aliased arguments ?
I would like to see the output for above eg as:

test-2.c: In function ‘foo’:
test-2.c:5:6: warning: passing argument 1 to restrict-qualified
parameter aliases with arguments 3, 4 [-Wrestrict]
   f (&a, &b, &a, &a);
       ^~        ~~   ~~

I would be grateful for suggestions for the same.
Bootstrap+test in progress on x86_64-unknown-linux-gnu.

Thanks,
Prathamesh
>
>         Marek

[-- Attachment #2: pr35503-4.diff --]
[-- Type: text/plain, Size: 7619 bytes --]

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 3feb910..abb92b3 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -13057,4 +13057,76 @@ 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))
+    return;
+
+  location_t loc = (EXPR_LOCATION (arg) != UNKNOWN_LOCATION)
+		   ? EXPR_LOCATION (arg)
+		   : input_location;
+
+  if (operand_equal_p (arg, null_pointer_node, 0))
+    return;
+
+  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;
+  FOR_EACH_VEC_ELT (arg_positions, i, pos)
+    {
+      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 (loc, 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..7eaca04 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_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..5d0182f 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_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..c7828b8
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr35503-2.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-Wrestrict" } */
+
+void f(int *__restrict x, int *__restrict y);
+
+void foo(int a)
+{
+  f (&a, &a); /* { dg-warning "passing argument 1 to restrict-qualified parameter aliases with argument 2" } */
+}
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" } */
+}

Re: PR35503 - warn for restrict pointer

[David Malcolm]

On Tue, 2016-08-30 at 03:23 +0530, Prathamesh Kulkarni wrote:
> On 29 August 2016 at 19:59, Marek Polacek <polacek@redhat.com> wrote:
> > On Mon, Aug 29, 2016 at 04:25:25PM +0200, Tobias Burnus wrote:
> > > Prathamesh Kulkarni wrote:
> > > > Attachment: pr35503-3.txt
> > > 
> > > I tried the patch - and it found a bug in our code; nice!
> > > 
> > > 
> > > (a) Regarding the [-Werror] output:
> > > 
> > >    error: passing argument 24 to restrict qualified parameter
> > > aliases with argument 29 [-Werror]
> > > 
> > > Shouldn't that output "[-Werror=restrict]" instead of a bare "[
> > > -Werror]? Namely, instead of
> > > 
> > > +     warning_at (loc, 0,
> > > +                 "passing argument %u to restrict qualified
> > > parameter aliases with "
> > > +                 "argument %u", param_pos + 1, i + 1);
> > > 
> > > I think one gets this with
> > >         warning_at (loc, OPT_Wrestrict, ...
> > 
> > Yes, this needs to be fixed in the patch.
> Hi,
> Thanks for all the suggestions, I have tried to incorporate them in
> the attached version.
> To avoid duplicating the warnings for multiple aliased arguments,
> I am using TREE_VISITED to mark all the aliased arguments of the
> current
> arg, which makes diagnostic emitted only once per same set of
> aliases.
> Is using TREE_VISITED ok as in the patch ?
> 
> eg:
> void f(int *__restrict x, int *y, int *__restrict z, int *w);
> 
> void foo(int a, int b)
> {
>   f (&a, &b, &a, &a);
> }
> 
> Output:
> test-2.c: In function ‘foo’:
> test-2.c:5:6: warning: passing argument 1 to restrict-qualified
> parameter aliases with arguments 3, 4 [-Wrestrict]
>    f (&a, &b, &a, &a);
>        ^~
> 
> Using EXPR_LOCATION (arg), helps underline the argument for both C
> and C++
> as in the example above. However in case of variadic functions, it
> appears C++FE doesn't set EXPR_LOCATION,
> so I am falling back to using input_location if EXPR_LOCATION is
> unknown.

You may be able to use EXPR_LOC_OR_LOC for this.

> However it gives the location correctly for non-variadic functions.
> For setting locations correctly I will try the approach suggested by
> Jason in
> https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01987.html
> 
> I was wondering how to underline all the aliased arguments ?
> I would like to see the output for above eg as:
> 
> test-2.c: In function ‘foo’:
> test-2.c:5:6: warning: passing argument 1 to restrict-qualified
> parameter aliases with arguments 3, 4 [-Wrestrict]
>    f (&a, &b, &a, &a);
>        ^~        ~~   ~~
> 
> I would be grateful for suggestions for the same.
> Bootstrap+test in progress on x86_64-unknown-linux-gnu.


The above came through email a bit mangled; presumably you mean:

   f (&a, &b, &a, &a);
      ^~      ~~  ~~ 

(hopefully Evolution won't mangle the above).

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);
  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.

Currently we can only have up to 3 ranges in a rich_location; I have a
patch in one of my working copies that fixes that so we can have an
arbitrary number; I'll try to dig that out as it's clearly relevant
here.

It's a good idea to have a testcase that verifies the underlines.

Use:

  /* { dg-options "-fdiagnostics-show-caret" } */

and then use:

  /* { dg-begin-multiline-output "" }
quote the expected output here, omitting trailing dg-directives
     { dg-end-multiline-output "" } */

e.g.
  /* { dg-begin-multiline-output "" }
   f (&a, &b, &a, &a);
      ^~      ~~  ~~ 
     { dg-end-multiline-output "" } */

Please use identifiers that are longer than one char in such a test, since otherwise there's a chance that we're not properly underlining all of the pertinent range (i.e. testing the start != finish case).

Hope the above is helpful
Dave

Re: PR35503 - warn for restrict pointer

[David Malcolm]

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);

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

>   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.

[...]

Re: PR35503 - warn for restrict pointer

[David Malcolm]

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 :)

> >   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.
> 
> [...]

Re: PR35503 - warn for restrict pointer

[Prathamesh Kulkarni]

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

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.
>>
>> [...]

[-- Attachment #2: pr35503-5.diff --]
[-- Type: text/plain, Size: 8440 bytes --]

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" } */
+}

[-- Attachment #3: gcc.log --]
[-- Type: text/x-log, Size: 5027 bytes --]

Test Run By bilbo on Tue Aug 30 16:52:28 2016
Native configuration is x86_64-pc-linux-gnu

		=== gcc tests ===

Schedule of variations:
    unix

Running target unix
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /home/bilbo/gnu-toolchain/gcc/pr35503/gcc/gcc/testsuite/config/default.exp as tool-and-target-specific interface file.
Running /home/bilbo/gnu-toolchain/gcc/pr35503/gcc/gcc/testsuite/gcc.dg/dg.exp ...
LD_LIBRARY_PATH=:/home/bilbo/gnu-toolchain/gcc/pr35503/stage1-build/gcc:/home/bilbo/gnu-toolchain/gcc/pr35503/stage1-build/gcc/32:
LD_RUN_PATH=:/home/bilbo/gnu-toolchain/gcc/pr35503/stage1-build/gcc:/home/bilbo/gnu-toolchain/gcc/pr35503/stage1-build/gcc/32
SHLIB_PATH=:/home/bilbo/gnu-toolchain/gcc/pr35503/stage1-build/gcc:/home/bilbo/gnu-toolchain/gcc/pr35503/stage1-build/gcc/32
LD_LIBRARY_PATH_32=:/home/bilbo/gnu-toolchain/gcc/pr35503/stage1-build/gcc:/home/bilbo/gnu-toolchain/gcc/pr35503/stage1-build/gcc/32:
LD_LIBRARY_PATH_64=:/home/bilbo/gnu-toolchain/gcc/pr35503/stage1-build/gcc:/home/bilbo/gnu-toolchain/gcc/pr35503/stage1-build/gcc/32:
DYLD_LIBRARY_PATH=:/home/bilbo/gnu-toolchain/gcc/pr35503/stage1-build/gcc:/home/bilbo/gnu-toolchain/gcc/pr35503/stage1-build/gcc/32
Executing on host: /home/bilbo/gnu-toolchain/gcc/pr35503/stage1-build/gcc/xgcc -B/home/bilbo/gnu-toolchain/gcc/pr35503/stage1-build/gcc/    -fno-diagnostics-show-caret -fdiagnostics-color=never  -flto -c -o lto13374.o lto13374.c    (timeout = 300)
spawn -ignore SIGHUP /home/bilbo/gnu-toolchain/gcc/pr35503/stage1-build/gcc/xgcc -B/home/bilbo/gnu-toolchain/gcc/pr35503/stage1-build/gcc/ -fno-diagnostics-show-caret -fdiagnostics-color=never -flto -c -o lto13374.o lto13374.c
Executing on host: /home/bilbo/gnu-toolchain/gcc/pr35503/stage1-build/gcc/xgcc -B/home/bilbo/gnu-toolchain/gcc/pr35503/stage1-build/gcc/ linker_plugin13374.c    -fno-diagnostics-show-caret -fdiagnostics-color=never  -flto -fuse-linker-plugin  -lm  -o linker_plugin13374.exe    (timeout = 300)
spawn -ignore SIGHUP /home/bilbo/gnu-toolchain/gcc/pr35503/stage1-build/gcc/xgcc -B/home/bilbo/gnu-toolchain/gcc/pr35503/stage1-build/gcc/ linker_plugin13374.c -fno-diagnostics-show-caret -fdiagnostics-color=never -flto -fuse-linker-plugin -lm -o linker_plugin13374.exe
Executing on host: /home/bilbo/gnu-toolchain/gcc/pr35503/stage1-build/gcc/xgcc -B/home/bilbo/gnu-toolchain/gcc/pr35503/stage1-build/gcc/ /home/bilbo/gnu-toolchain/gcc/pr35503/gcc/gcc/testsuite/c-c++-common/pr35503-2.c    -fno-diagnostics-show-caret -fdiagnostics-color=never   -Wc++-compat  -fdiagnostics-show-caret -Wrestrict -S -o pr35503-2.s    (timeout = 300)
spawn -ignore SIGHUP /home/bilbo/gnu-toolchain/gcc/pr35503/stage1-build/gcc/xgcc -B/home/bilbo/gnu-toolchain/gcc/pr35503/stage1-build/gcc/ /home/bilbo/gnu-toolchain/gcc/pr35503/gcc/gcc/testsuite/c-c++-common/pr35503-2.c -fno-diagnostics-show-caret -fdiagnostics-color=never -Wc++-compat -fdiagnostics-show-caret -Wrestrict -S -o pr35503-2.s
/home/bilbo/gnu-toolchain/gcc/pr35503/gcc/gcc/testsuite/c-c++-common/pr35503-2.c: In function 'foo':
/home/bilbo/gnu-toolchain/gcc/pr35503/gcc/gcc/testsuite/c-c++-common/pr35503-2.c:8:6: warning: passing argument 1 to restrict-qualified parameter aliases with arguments 3, 4 [-Wrestrict]
   f (&alpha, &beta, &alpha, &alpha);
      ^~~~~~         ~~~~~~  ~~~~~~
output is:
/home/bilbo/gnu-toolchain/gcc/pr35503/gcc/gcc/testsuite/c-c++-common/pr35503-2.c: In function 'foo':
/home/bilbo/gnu-toolchain/gcc/pr35503/gcc/gcc/testsuite/c-c++-common/pr35503-2.c:8:6: warning: passing argument 1 to restrict-qualified parameter aliases with arguments 3, 4 [-Wrestrict]
   f (&alpha, &beta, &alpha, &alpha);
      ^~~~~~         ~~~~~~  ~~~~~~

PASS: c-c++-common/pr35503-2.c  -Wc++-compat   (test for warnings, line 8)
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"
FAIL: c-c++-common/pr35503-2.c  -Wc++-compat  (test for excess errors)
Excess errors:
   f (&alpha, &beta, &alpha, &alpha);
      ^~~~~~         ~~~~~~  ~~~~~~

testcase /home/bilbo/gnu-toolchain/gcc/pr35503/gcc/gcc/testsuite/gcc.dg/dg.exp completed in 0 seconds

		=== gcc Summary ===

# of expected passes		1
# of unexpected failures	2
Executing on host: /home/bilbo/gnu-toolchain/gcc/pr35503/stage1-build/gcc/xgcc -v    (timeout = 300)
spawn -ignore SIGHUP /home/bilbo/gnu-toolchain/gcc/pr35503/stage1-build/gcc/xgcc -v
Using built-in specs.
COLLECT_GCC=/home/bilbo/gnu-toolchain/gcc/pr35503/stage1-build/gcc/xgcc
Target: x86_64-pc-linux-gnu
Configured with: ../gcc/configure --enable-languages=c,c++,fortran --disable-bootstrap
Thread model: posix
gcc version 7.0.0 20160824 (experimental) (GCC) 
/home/bilbo/gnu-toolchain/gcc/pr35503/stage1-build/gcc/xgcc  version 7.0.0 20160824 (experimental) (GCC) 

runtest completed at Tue Aug 30 16:52:28 2016

Re: PR35503 - warn for restrict pointer

[David Malcolm]

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.

> 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,
> 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.
> > > 
> > > [...]

Re: PR35503 - warn for restrict pointer

[Tom de Vries]

On 26/08/16 13:39, Prathamesh Kulkarni wrote:
> Hi,
> The following patch adds option -Wrestrict that warns when an argument
> is passed to a restrict qualified parameter and it aliases with
> another argument.
>
> eg:
> int foo (const char *__restrict buf, const char *__restrict fmt, ...);
>
> void f(void)
> {
>   char buf[100] = "hello";
>   foo (buf, "%s-%s", buf, "world");
> }

Does -Wrestrict generate a warning for this example?

...
void h(int n, int * restrict p, int * restrict q, int * restrict r)
{
   int i;
   for (i = 0; i < n; i++)
     p[i] = q[i] + r[i];
}

h (100, a, b, b)
...

[ Note that this is valid C, and does not violate the restrict definition. ]

If -Wrestrict indeed generates a warning, then we should be explicit in 
the documentation that while the warning triggers on this type of 
example, the code is correct.

Thanks,
- Tom

Re: PR35503 - warn for restrict pointer

[Prathamesh Kulkarni]

On 30 August 2016 at 20:24, Tom de Vries <Tom_deVries@mentor.com> wrote:
> On 26/08/16 13:39, Prathamesh Kulkarni wrote:
>>
>> Hi,
>> The following patch adds option -Wrestrict that warns when an argument
>> is passed to a restrict qualified parameter and it aliases with
>> another argument.
>>
>> eg:
>> int foo (const char *__restrict buf, const char *__restrict fmt, ...);
>>
>> void f(void)
>> {
>>   char buf[100] = "hello";
>>   foo (buf, "%s-%s", buf, "world");
>> }
>
>
> Does -Wrestrict generate a warning for this example?
>
> ...
> void h(int n, int * restrict p, int * restrict q, int * restrict r)
> {
>   int i;
>   for (i = 0; i < n; i++)
>     p[i] = q[i] + r[i];
> }
>
> h (100, a, b, b)
> ...
>
> [ Note that this is valid C, and does not violate the restrict definition. ]
>
> If -Wrestrict indeed generates a warning, then we should be explicit in the
> documentation that while the warning triggers on this type of example, the
> code is correct.
I am afraid it would warn for the above case. The patch just checks if
the parameter is qualified
with restrict, and if the corresponding argument has aliases in the
call (by calling operand_equal_p).
Is such code common in practice ? If it is, I wonder if we should keep
the warning in -Wall ?

To avoid such false positives, we would need to track which arguments
are modified by the call.
I suppose that could be done with ipa mod/ref analysis (and moving the
warning to middle-end) ?
I tried looking into gcc sources but couldn't find where it's implemented.

Thanks,
Prathamesh
>
> Thanks,
> - Tom

Re: PR35503 - warn for restrict pointer

[Tom de Vries]

On 30/08/16 17:34, Prathamesh Kulkarni wrote:
> On 30 August 2016 at 20:24, Tom de Vries <Tom_deVries@mentor.com> wrote:
>> On 26/08/16 13:39, Prathamesh Kulkarni wrote:
>>>
>>> Hi,
>>> The following patch adds option -Wrestrict that warns when an argument
>>> is passed to a restrict qualified parameter and it aliases with
>>> another argument.
>>>
>>> eg:
>>> int foo (const char *__restrict buf, const char *__restrict fmt, ...);
>>>
>>> void f(void)
>>> {
>>>   char buf[100] = "hello";
>>>   foo (buf, "%s-%s", buf, "world");
>>> }
>>
>>
>> Does -Wrestrict generate a warning for this example?
>>
>> ...
>> void h(int n, int * restrict p, int * restrict q, int * restrict r)
>> {
>>   int i;
>>   for (i = 0; i < n; i++)
>>     p[i] = q[i] + r[i];
>> }
>>
>> h (100, a, b, b)
>> ...
>>
>> [ Note that this is valid C, and does not violate the restrict definition. ]
>>
>> If -Wrestrict indeed generates a warning, then we should be explicit in the
>> documentation that while the warning triggers on this type of example, the
>> code is correct.
> I am afraid it would warn for the above case. The patch just checks if
> the parameter is qualified
> with restrict, and if the corresponding argument has aliases in the
> call (by calling operand_equal_p).

> Is such code common in practice ?

[ The example is from the definition of restrict in the c99 standard. ]

According to the definition of restrict, only the restrict on p is 
required to know that p doesn't alias with q and that p doesn't alias 
with r.

AFAIK the current implementation of gcc already generates optimal code 
if restrict is only on p. But earlier versions (and possibly other 
compilers as well?) need the restrict on q and r as well.

So I expect this code to occur.

> If it is, I wonder if we should keep
> the warning in -Wall ?
>

Hmm, I wonder if we can use the const keyword to silence the warning.

So if we generate a warning for:
...
void h(int n, int * restrict p, int * restrict q, int * restrict r)
{
   int i;
   for (i = 0; i < n; i++)
     p[i] = q[i] + r[i];
}
h (100, a, b, b)
...

but not for:
...
void h(int n, int * restrict p, const int * restrict q, const int * 
restrict r)
{
   int i;
   for (i = 0; i < n; i++)
     p[i] = q[i] + r[i];
}
h (100, a, b, b)
...

Then there's an easy way to rewrite the example such that the warning 
doesn't trigger, without having to remove the restrict.

Thanks,
- Tom

Re: PR35503 - warn for restrict pointer

[Prathamesh Kulkarni]

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

On 30 August 2016 at 22:58, Tom de Vries <Tom_deVries@mentor.com> wrote:
> On 30/08/16 17:34, Prathamesh Kulkarni wrote:
>>
>> On 30 August 2016 at 20:24, Tom de Vries <Tom_deVries@mentor.com> wrote:
>>>
>>> On 26/08/16 13:39, Prathamesh Kulkarni wrote:
>>>>
>>>>
>>>> Hi,
>>>> The following patch adds option -Wrestrict that warns when an argument
>>>> is passed to a restrict qualified parameter and it aliases with
>>>> another argument.
>>>>
>>>> eg:
>>>> int foo (const char *__restrict buf, const char *__restrict fmt, ...);
>>>>
>>>> void f(void)
>>>> {
>>>>   char buf[100] = "hello";
>>>>   foo (buf, "%s-%s", buf, "world");
>>>> }
>>>
>>>
>>>
>>> Does -Wrestrict generate a warning for this example?
>>>
>>> ...
>>> void h(int n, int * restrict p, int * restrict q, int * restrict r)
>>> {
>>>   int i;
>>>   for (i = 0; i < n; i++)
>>>     p[i] = q[i] + r[i];
>>> }
>>>
>>> h (100, a, b, b)
>>> ...
>>>
>>> [ Note that this is valid C, and does not violate the restrict
>>> definition. ]
>>>
>>> If -Wrestrict indeed generates a warning, then we should be explicit in
>>> the
>>> documentation that while the warning triggers on this type of example,
>>> the
>>> code is correct.
>>
>> I am afraid it would warn for the above case. The patch just checks if
>> the parameter is qualified
>> with restrict, and if the corresponding argument has aliases in the
>> call (by calling operand_equal_p).
>
>
>> Is such code common in practice ?
>
>
> [ The example is from the definition of restrict in the c99 standard. ]
>
> According to the definition of restrict, only the restrict on p is required
> to know that p doesn't alias with q and that p doesn't alias with r.
>
> AFAIK the current implementation of gcc already generates optimal code if
> restrict is only on p. But earlier versions (and possibly other compilers as
> well?) need the restrict on q and r as well.
>
> So I expect this code to occur.
>
>> If it is, I wonder if we should keep
>> the warning in -Wall ?
>>
>
> Hmm, I wonder if we can use the const keyword to silence the warning.
>
> So if we generate a warning for:
> ...
> void h(int n, int * restrict p, int * restrict q, int * restrict r)
> {
>   int i;
>   for (i = 0; i < n; i++)
>     p[i] = q[i] + r[i];
> }
> h (100, a, b, b)
> ...
>
> but not for:
> ...
> void h(int n, int * restrict p, const int * restrict q, const int * restrict
> r)
> {
>   int i;
>   for (i = 0; i < n; i++)
>     p[i] = q[i] + r[i];
> }
> h (100, a, b, b)
> ...
>
> Then there's an easy way to rewrite the example such that the warning
> doesn't trigger, without having to remove the restrict.
The attached (untested) patch silences the warning if parameter is
const qualified.
I will give it some testing after resolving a different issue.

Thanks,
Prathamesh
>
> Thanks,
> - Tom

[-- Attachment #2: pr35503-6.diff --]
[-- Type: text/plain, Size: 8524 bytes --]

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..05510f6 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -8369,6 +8369,25 @@ 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)
+		      && !TYPE_READONLY (TREE_TYPE (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..ab73655 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -6878,6 +6878,26 @@ 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)
+			&& !TYPE_READONLY (TREE_TYPE (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" } */
+}

Re: PR35503 - warn for restrict pointer

[David Malcolm]

On Tue, 2016-08-30 at 23:26 +0530, Prathamesh Kulkarni wrote:
[...]

> The attached (untested) patch silences the warning if parameter is
> const qualified.
> I will give it some testing after resolving a different issue.

I've now removed the hard-coded limit of 3 ranges per rich_location, so
you should now be able to add an arbitrary number of underlined ranges
to the diagnostic (r239879 removes the limit).

Dave

Re: PR35503 - warn for restrict pointer

[Tom de Vries]

On 30/08/16 16:54, Tom de Vries wrote:
> On 26/08/16 13:39, Prathamesh Kulkarni wrote:
>> Hi,
>> The following patch adds option -Wrestrict that warns when an argument
>> is passed to a restrict qualified parameter and it aliases with
>> another argument.
>>
>> eg:
>> int foo (const char *__restrict buf, const char *__restrict fmt, ...);
>>
>> void f(void)
>> {
>>   char buf[100] = "hello";
>>   foo (buf, "%s-%s", buf, "world");
>> }
>
> Does -Wrestrict generate a warning for this example?
>
> ...
> void h(int n, int * restrict p, int * restrict q, int * restrict r)
> {
>   int i;
>   for (i = 0; i < n; i++)
>     p[i] = q[i] + r[i];
> }
>
> h (100, a, b, b)
> ...
>
> [ Note that this is valid C, and does not violate the restrict
> definition. ]
>
> If -Wrestrict indeed generates a warning, then we should be explicit in
> the documentation that while the warning triggers on this type of
> example, the code is correct.

So, what I had in mind is something like this:
...
Warn when an argument passed to a restrict-qualified parameter pointing 
to a non-const type aliases with another argument.

The warning will trigger for the following illegal use of restrict:

   void foo (int *restrict a, int *b) { *a = 1; *b = 2; }
   void foo2 (void) { int o; foo (&o, &o); }

The use of restrict is illegal because in the lifetime of the scope of 
the restrict pointer 'a' both:
- the object 'o' that the restrict pointer 'a' points to is modified,
   and
- the object 'o' is accessed through pointer 'b', which is not based on
   restrict pointer 'a'.

However, the warning will also trigger for the following legal use of 
restrict:

   int foo (int *restrict a, int *b) { return *a + *b; }
   int foo2 (void) { int o = 1; return foo (&o, &o); }

The use of restrict is legal, because the object 'o' that the restrict 
pointer 'a' points to is not modified  in the lifetime of the scope of 
the restrict pointer 'a'. The warning can be silenced by changing the 
type of 'a' to 'const int *restrict'.
...

Thanks,
- Tom

Re: PR35503 - warn for restrict pointer

[Prathamesh Kulkarni]

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.
>> > >
>> > > [...]

Re: PR35503 - warn for restrict pointer

[Fabien Chêne]

2016-08-30 17:34 GMT+02:00 Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org>:
> On 30 August 2016 at 20:24, Tom de Vries <Tom_deVries@mentor.com> wrote:
>> On 26/08/16 13:39, Prathamesh Kulkarni wrote:
>>>
>>> Hi,
>>> The following patch adds option -Wrestrict that warns when an argument
>>> is passed to a restrict qualified parameter and it aliases with
>>> another argument.
>>>
>>> eg:
>>> int foo (const char *__restrict buf, const char *__restrict fmt, ...);
>>>
>>> void f(void)
>>> {
>>>   char buf[100] = "hello";
>>>   foo (buf, "%s-%s", buf, "world");
>>> }
>>
>>
>> Does -Wrestrict generate a warning for this example?
>>
>> ...
>> void h(int n, int * restrict p, int * restrict q, int * restrict r)
>> {
>>   int i;
>>   for (i = 0; i < n; i++)
>>     p[i] = q[i] + r[i];
>> }
>>
>> h (100, a, b, b)
>> ...
>>
>> [ Note that this is valid C, and does not violate the restrict definition. ]
>>
>> If -Wrestrict indeed generates a warning, then we should be explicit in the
>> documentation that while the warning triggers on this type of example, the
>> code is correct.
> I am afraid it would warn for the above case. The patch just checks if
> the parameter is qualified
> with restrict, and if the corresponding argument has aliases in the
> call (by calling operand_equal_p).
> Is such code common in practice ? If it is, I wonder if we should keep
> the warning in -Wall ?
>
> To avoid such false positives, we would need to track which arguments
> are modified by the call.
> I suppose that could be done with ipa mod/ref analysis (and moving the
> warning to middle-end) ?
> I tried looking into gcc sources but couldn't find where it's implemented.

I don't know either which pass can be used, but I guess it should be doable.
If so, a first step would be to determine useless restrict-qualified
args (and possibly warn for them), and then perform the current
-Wrestrict analysis only on non-useless restrict-qualified args ?

--
Fabien

Re: PR35503 - warn for restrict pointer

[Richard Biener]

On Tue, 30 Aug 2016, Tom de Vries wrote:

> On 30/08/16 17:34, Prathamesh Kulkarni wrote:
> > On 30 August 2016 at 20:24, Tom de Vries <Tom_deVries@mentor.com> wrote:
> > > On 26/08/16 13:39, Prathamesh Kulkarni wrote:
> > > > 
> > > > Hi,
> > > > The following patch adds option -Wrestrict that warns when an argument
> > > > is passed to a restrict qualified parameter and it aliases with
> > > > another argument.
> > > > 
> > > > eg:
> > > > int foo (const char *__restrict buf, const char *__restrict fmt, ...);
> > > > 
> > > > void f(void)
> > > > {
> > > >   char buf[100] = "hello";
> > > >   foo (buf, "%s-%s", buf, "world");
> > > > }
> > > 
> > > 
> > > Does -Wrestrict generate a warning for this example?
> > > 
> > > ...
> > > void h(int n, int * restrict p, int * restrict q, int * restrict r)
> > > {
> > >   int i;
> > >   for (i = 0; i < n; i++)
> > >     p[i] = q[i] + r[i];
> > > }
> > > 
> > > h (100, a, b, b)
> > > ...
> > > 
> > > [ Note that this is valid C, and does not violate the restrict definition.
> > > ]
> > > 
> > > If -Wrestrict indeed generates a warning, then we should be explicit in
> > > the
> > > documentation that while the warning triggers on this type of example, the
> > > code is correct.
> > I am afraid it would warn for the above case. The patch just checks if
> > the parameter is qualified
> > with restrict, and if the corresponding argument has aliases in the
> > call (by calling operand_equal_p).
> 
> > Is such code common in practice ?
> 
> [ The example is from the definition of restrict in the c99 standard. ]
> 
> According to the definition of restrict, only the restrict on p is required to
> know that p doesn't alias with q and that p doesn't alias with r.
> 
> AFAIK the current implementation of gcc already generates optimal code if
> restrict is only on p. But earlier versions (and possibly other compilers as
> well?) need the restrict on q and r as well.
> 
> So I expect this code to occur.
> 
> > If it is, I wonder if we should keep
> > the warning in -Wall ?
> > 
> 
> Hmm, I wonder if we can use the const keyword to silence the warning.
> 
> So if we generate a warning for:
> ...
> void h(int n, int * restrict p, int * restrict q, int * restrict r)
> {
>   int i;
>   for (i = 0; i < n; i++)
>     p[i] = q[i] + r[i];
> }
> h (100, a, b, b)
> ...
> 
> but not for:
> ...
> void h(int n, int * restrict p, const int * restrict q, const int * restrict
> r)
> {
>   int i;
>   for (i = 0; i < n; i++)
>     p[i] = q[i] + r[i];
> }
> h (100, a, b, b)
> ...
> 
> Then there's an easy way to rewrite the example such that the warning doesn't
> trigger, without having to remove the restrict.

Note that I've seen restrict being used even for

void h(int n, int * restrict p, int * restrict q)
{
  int i;
  for (i = 0; i < n; i++)
    p[2*i] = p[2*i] + q[2*i + 1];
}

thus where the actual accesses do not alias (the pointers are used
to access every other element).  I think the definition of "object"
(based on accessed lvalues) makes this example valid.  So we
shouldn't warn about

h (n, p, p)

but we could warn about

h (n, p, p + 1)

and yes, only one of the pointers need to be restrict qualified.

Note that as with all other alias warnings if you want to avoid
false positives and rely on conservative analysis then you can
as well simply avoid taking advantate of the bug in the code
(as we do for TBAA and trivial must-alias cases).  If you allow
false positives then you ultimatively end up with a mess like
our existing -Wstrict-aliasing warnings (which are IMHO quite
useless).

Note that nowhere in GCC we implement anything closely matching
the formal definition of restrict as writte in the C standard --
only in fronted code could we properly derive the 'based-on'
property and note lvalues affected by restrict.  Currently we
are restricted to looking at restrict qualified parameters
because of this.

Richard.

Re: PR35503 - warn for restrict pointer

[Prathamesh Kulkarni]

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

On 1 September 2016 at 12:25, Richard Biener <rguenther@suse.de> wrote:
> On Tue, 30 Aug 2016, Tom de Vries wrote:
>
>> On 30/08/16 17:34, Prathamesh Kulkarni wrote:
>> > On 30 August 2016 at 20:24, Tom de Vries <Tom_deVries@mentor.com> wrote:
>> > > On 26/08/16 13:39, Prathamesh Kulkarni wrote:
>> > > >
>> > > > Hi,
>> > > > The following patch adds option -Wrestrict that warns when an argument
>> > > > is passed to a restrict qualified parameter and it aliases with
>> > > > another argument.
>> > > >
>> > > > eg:
>> > > > int foo (const char *__restrict buf, const char *__restrict fmt, ...);
>> > > >
>> > > > void f(void)
>> > > > {
>> > > >   char buf[100] = "hello";
>> > > >   foo (buf, "%s-%s", buf, "world");
>> > > > }
>> > >
>> > >
>> > > Does -Wrestrict generate a warning for this example?
>> > >
>> > > ...
>> > > void h(int n, int * restrict p, int * restrict q, int * restrict r)
>> > > {
>> > >   int i;
>> > >   for (i = 0; i < n; i++)
>> > >     p[i] = q[i] + r[i];
>> > > }
>> > >
>> > > h (100, a, b, b)
>> > > ...
>> > >
>> > > [ Note that this is valid C, and does not violate the restrict definition.
>> > > ]
>> > >
>> > > If -Wrestrict indeed generates a warning, then we should be explicit in
>> > > the
>> > > documentation that while the warning triggers on this type of example, the
>> > > code is correct.
>> > I am afraid it would warn for the above case. The patch just checks if
>> > the parameter is qualified
>> > with restrict, and if the corresponding argument has aliases in the
>> > call (by calling operand_equal_p).
>>
>> > Is such code common in practice ?
>>
>> [ The example is from the definition of restrict in the c99 standard. ]
>>
>> According to the definition of restrict, only the restrict on p is required to
>> know that p doesn't alias with q and that p doesn't alias with r.
>>
>> AFAIK the current implementation of gcc already generates optimal code if
>> restrict is only on p. But earlier versions (and possibly other compilers as
>> well?) need the restrict on q and r as well.
>>
>> So I expect this code to occur.
>>
>> > If it is, I wonder if we should keep
>> > the warning in -Wall ?
>> >
>>
>> Hmm, I wonder if we can use the const keyword to silence the warning.
>>
>> So if we generate a warning for:
>> ...
>> void h(int n, int * restrict p, int * restrict q, int * restrict r)
>> {
>>   int i;
>>   for (i = 0; i < n; i++)
>>     p[i] = q[i] + r[i];
>> }
>> h (100, a, b, b)
>> ...
>>
>> but not for:
>> ...
>> void h(int n, int * restrict p, const int * restrict q, const int * restrict
>> r)
>> {
>>   int i;
>>   for (i = 0; i < n; i++)
>>     p[i] = q[i] + r[i];
>> }
>> h (100, a, b, b)
>> ...
>>
>> Then there's an easy way to rewrite the example such that the warning doesn't
>> trigger, without having to remove the restrict.
>
> Note that I've seen restrict being used even for
>
> void h(int n, int * restrict p, int * restrict q)
> {
>   int i;
>   for (i = 0; i < n; i++)
>     p[2*i] = p[2*i] + q[2*i + 1];
> }
>
> thus where the actual accesses do not alias (the pointers are used
> to access every other element).  I think the definition of "object"
> (based on accessed lvalues) makes this example valid.  So we
> shouldn't warn about
>
> h (n, p, p)
>
> but we could warn about
>
> h (n, p, p + 1)
>
> and yes, only one of the pointers need to be restrict qualified.
>
> Note that as with all other alias warnings if you want to avoid
> false positives and rely on conservative analysis then you can
> as well simply avoid taking advantate of the bug in the code
> (as we do for TBAA and trivial must-alias cases).  If you allow
> false positives then you ultimatively end up with a mess like
> our existing -Wstrict-aliasing warnings (which are IMHO quite
> useless).
>
> Note that nowhere in GCC we implement anything closely matching
> the formal definition of restrict as writte in the C standard --
> only in fronted code could we properly derive the 'based-on'
> property and note lvalues affected by restrict.  Currently we
> are restricted to looking at restrict qualified parameters
> because of this.
Hi,
The attached version passes bootstrap+test on ppc64le-linux-gnu.
Given that it only looks if parameters are restrict qualified and not
how they're used inside the callee,
this can have false positives as in above test-cases.
Should the warning be put in Wextra rather than Wall (I have left it
in Wall in the patch)  or only enabled with -Wrestrict ?

Thanks,
Prathamesh
>
> Richard.

[-- Attachment #2: pr35503-7.diff --]
[-- Type: text/plain, Size: 8281 bytes --]

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 3feb910..a3dae34 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,76 @@ 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;
+  FOR_EACH_VEC_ELT (arg_positions, i, pos)
+    {
+      tree arg = (*args)[pos];
+      if (EXPR_HAS_LOCATION (arg))
+	richloc.add_range (EXPR_LOCATION (arg), false);
+
+      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..5ec3a25 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 ObjC C++ ObjC++ Var(warn_restrict) Warning LangEnabledBy(C ObjC C++ ObjC++,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..05510f6 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -8369,6 +8369,25 @@ 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)
+		      && !TYPE_READONLY (TREE_TYPE (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..ab73655 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -6878,6 +6878,26 @@ 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)
+			&& !TYPE_READONLY (TREE_TYPE (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..869bf07 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
+@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..25e3721
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr35503-1.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-Wrestrict" } */
+
+int foo (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..bfcd944
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr35503-2.c
@@ -0,0 +1,14 @@
+/* { 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" } */
+
+/* { 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" } */
+}

Re: PR35503 - warn for restrict pointer

[Martin Sebor]

> The attached version passes bootstrap+test on ppc64le-linux-gnu.
> Given that it only looks if parameters are restrict qualified and not
> how they're used inside the callee,
> this can have false positives as in above test-cases.
> Should the warning be put in Wextra rather than Wall (I have left it
> in Wall in the patch)  or only enabled with -Wrestrict ?

Awesome!  I've wished for years for a warning like this!

I'm curious if you've tested other examples from 6.7.3.1 of C11
besides Example 3.  Example 4 seems like something GCC should be
able to detect but I didn't get a warning with the patch.

I would expect the warning to be especially valuable with string
manipulation functions like memcpy that have undefined behavior
for overlapping regions, such as in:

   char a[4];

   void g (void)
   {
     __builtin_memcpy (a, a + 1, 3);
   }

But here, too, I didn't get a warning.  I understand that this
case cannot be handled for arbitrary functions whose semantics
aren't known but with standard functions for which GCC provides
intrinsics the effects are known and aliasing violations can in
common cases be detected.

Martin

Re: PR35503 - warn for restrict pointer

[David Malcolm]

On Thu, 2016-09-01 at 14:55 +0530, Prathamesh Kulkarni wrote:

[...]

> The attached version passes bootstrap+test on ppc64le-linux-gnu.
> Given that it only looks if parameters are restrict qualified and not
> how they're used inside the callee,
> this can have false positives as in above test-cases.
> Should the warning be put in Wextra rather than Wall (I have left it
> in Wall in the patch)  or only enabled with -Wrestrict ?
> 
> Thanks,
> Prathamesh
> > 
> > Richard.


diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 3feb910..a3dae34 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,76 @@ 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;
+  FOR_EACH_VEC_ELT (arg_positions, i, pos)
+    {
+      tree arg = (*args)[pos];
+      if (EXPR_HAS_LOCATION (arg))
+	richloc.add_range (EXPR_LOCATION (arg), false);
+
+      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);
+}

Thanks for working on this.

I'm not a fan of how the patch builds "fmt" here.  If nothing else,
this perhaps should be:

  warning_at_rich_loc (&richloc, OPT_Wrestrict, "%s", fmt);

but building up strings like the patch does makes localization
difficult.

Much better would be to have the formatting be done inside the
diagnostics subsystem's call into pp_format, with something like this:

  warning_at_rich_loc_n (&richloc, OPT_Wrestrict,
			 arg_positions
.length (),
                         "passing argument %i to restrict
-qualified"
	                 " parameter aliases with argument
%FIXME",
                         "passing argument %i to restrict
-qualified"
	                 " parameter aliases with arguments
%FIXME",
                         param_pos + 1,
                        
 &arg_positions);


and have %FIXME (or somesuch) consume &arg_positions in the va_arg,
printing the argument numbers there.  Doing it this way also avoids
building the string for the case where Wrestrict is disabled, since the
pp_format only happens after we know we're going to print the warning.

Assuming that there isn't yet a pre-canned way to print a set of
argument numbers that I've missed, the place to add the %FIXME-handling
would presumably be in default_tree_printer in tree-diagnostic.c -
though it's obviously nothing to do with trees. (Or if this is too
single-purpose, perhaps there's a need to temporarily inject one-time
callbacks for consuming custom args??).

We can then have a fun discussion about the usage of the Oxford comma :) [1]

IIRC we don't yet have warning_at_rich_loc_n, but it would be fairly easy to add.

[...]

[1] which seems to be locale-dependent itself:
https://en.wikipedia.org/wiki/Serial_comma#Other_languages

Re: PR35503 - warn for restrict pointer

[Manuel López-Ibáñez]

On 02/09/16 18:44, David Malcolm wrote:
> Much better would be to have the formatting be done inside the
> diagnostics subsystem's call into pp_format, with something like this:
>
>   warning_at_rich_loc_n (&richloc, OPT_Wrestrict,
> 			 arg_positions
> .length (),
>                          "passing argument %i to restrict
> -qualified"
> 	                 " parameter aliases with argument
> %FIXME",
>                          "passing argument %i to restrict
> -qualified"
> 	                 " parameter aliases with arguments
> %FIXME",
>                          param_pos + 1,
>
>  &arg_positions);

Yes, building up diagnostic messages from pieces is discouraged: 
https://gcc.gnu.org/codingconventions.html#Diagnostics

> and have %FIXME (or somesuch) consume &arg_positions in the va_arg,
> printing the argument numbers there.  Doing it this way also avoids
> building the string for the case where Wrestrict is disabled, since the
> pp_format only happens after we know we're going to print the warning.

Is it possible to pass template arguments through ... ? And how does va_arg 
know the type of the particular template passed?

> Assuming that there isn't yet a pre-canned way to print a set of
> argument numbers that I've missed, the place to add the %FIXME-handling
> would presumably be in default_tree_printer in tree-diagnostic.c -
> though it's obviously nothing to do with trees. (Or if this is too
> single-purpose, perhaps there's a need to temporarily inject one-time
> callbacks for consuming custom args??).

I'm surprised we don't have a function pp_vec to print/debug a vec<>, but 
perhaps it is simpler to convert arg_pos to a 'char *' and use %s instead of 
%FIXME or call-backs.

Cheers,

	Manuel.

Re: PR35503 - warn for restrict pointer

[Prathamesh Kulkarni]

On 1 September 2016 at 21:28, Martin Sebor <msebor@gmail.com> wrote:
>> The attached version passes bootstrap+test on ppc64le-linux-gnu.
>> Given that it only looks if parameters are restrict qualified and not
>> how they're used inside the callee,
>> this can have false positives as in above test-cases.
>> Should the warning be put in Wextra rather than Wall (I have left it
>> in Wall in the patch)  or only enabled with -Wrestrict ?
>
>
> Awesome!  I've wished for years for a warning like this!
Thanks for experimenting with the patch!
>
> I'm curious if you've tested other examples from 6.7.3.1 of C11
> besides Example 3.  Example 4 seems like something GCC should be
> able to detect but I didn't get a warning with the patch.
Oops, I wasn't aware about example 4, and only implemented the warning
for function call. IIUC, the assignment p = q will result in undefined behavior
if q is qualified with restrict and p is at an outer-scope relative to q ?
>
> I would expect the warning to be especially valuable with string
> manipulation functions like memcpy that have undefined behavior
> for overlapping regions, such as in:
>
>   char a[4];
>
>   void g (void)
>   {
>     __builtin_memcpy (a, a + 1, 3);
>   }
>
> But here, too, I didn't get a warning.  I understand that this
> case cannot be handled for arbitrary functions whose semantics
> aren't known but with standard functions for which GCC provides
> intrinsics the effects are known and aliasing violations can in
> common cases be detected.
Indeed, thanks for the suggestions. I will add support for some
standard functions in a follow-up patch.

Thanks,
Prathamesh
>
> Martin

Re: PR35503 - warn for restrict pointer

[Prathamesh Kulkarni]

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

On 2 September 2016 at 23:14, David Malcolm <dmalcolm@redhat.com> wrote:
> On Thu, 2016-09-01 at 14:55 +0530, Prathamesh Kulkarni wrote:
>
> [...]
>
>> The attached version passes bootstrap+test on ppc64le-linux-gnu.
>> Given that it only looks if parameters are restrict qualified and not
>> how they're used inside the callee,
>> this can have false positives as in above test-cases.
>> Should the warning be put in Wextra rather than Wall (I have left it
>> in Wall in the patch)  or only enabled with -Wrestrict ?
>>
>> Thanks,
>> Prathamesh
>> >
>> > Richard.
>
>
> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
> index 3feb910..a3dae34 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,76 @@ 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;
> +  FOR_EACH_VEC_ELT (arg_positions, i, pos)
> +    {
> +      tree arg = (*args)[pos];
> +      if (EXPR_HAS_LOCATION (arg))
> +       richloc.add_range (EXPR_LOCATION (arg), false);
> +
> +      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);
> +}
>
> Thanks for working on this.
>
> I'm not a fan of how the patch builds "fmt" here.  If nothing else,
> this perhaps should be:
>
>   warning_at_rich_loc (&richloc, OPT_Wrestrict, "%s", fmt);
>
> but building up strings like the patch does makes localization
> difficult.
>
> Much better would be to have the formatting be done inside the
> diagnostics subsystem's call into pp_format, with something like this:
>
>   warning_at_rich_loc_n (&richloc, OPT_Wrestrict,
>                          arg_positions
> .length (),
>                          "passing argument %i to restrict
> -qualified"
>                          " parameter aliases with argument
> %FIXME",
>                          "passing argument %i to restrict
> -qualified"
>                          " parameter aliases with arguments
> %FIXME",
>                          param_pos + 1,
>
>  &arg_positions);
>
>
> and have %FIXME (or somesuch) consume &arg_positions in the va_arg,
> printing the argument numbers there.  Doing it this way also avoids
> building the string for the case where Wrestrict is disabled, since the
> pp_format only happens after we know we're going to print the warning.
>
> Assuming that there isn't yet a pre-canned way to print a set of
> argument numbers that I've missed, the place to add the %FIXME-handling
> would presumably be in default_tree_printer in tree-diagnostic.c -
> though it's obviously nothing to do with trees. (Or if this is too
> single-purpose, perhaps there's a need to temporarily inject one-time
> callbacks for consuming custom args??).
>
> We can then have a fun discussion about the usage of the Oxford comma :) [1]
>
> IIRC we don't yet have warning_at_rich_loc_n, but it would be fairly easy to add.
Hi David,
Sorry for late response. In the attached patch, I removed obstack
building on fmt, and used
pp_format for formatting arg_positions by adding specifier %I (name
chosen arbitrarily).
Does that look OK ?

However it results in following warning during gcc build and am not
sure how to fix this:
../../gcc/gcc/c-family/c-common.c: In function ‘void
warn_for_restrict(unsigned int, vec<tree_node*, va_gc>*)’:
../../gcc/gcc/c-family/c-common.c:13132:24: warning: unknown
conversion type character ‘I’ in format [-Wformat=]

Thanks,
Prathamesh
>
> [...]
>
> [1] which seems to be locale-dependent itself:
> https://en.wikipedia.org/wiki/Serial_comma#Other_languages

[-- Attachment #2: pr35503-8.diff --]
[-- Type: text/plain, Size: 8210 bytes --]

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index fc25686..5081637 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.  */
 
@@ -13084,4 +13085,51 @@ 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<int> 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 + 1); 
+	}
+    }
+
+  if (arg_positions.is_empty ())
+    return;
+
+  int pos;
+  FOR_EACH_VEC_ELT (arg_positions, i, pos)
+    {
+      tree arg = (*args)[pos - 1];
+      if (EXPR_HAS_LOCATION (arg))
+	richloc.add_range (EXPR_LOCATION (arg), false);
+    }
+
+  warning_at_rich_loc (&richloc, OPT_Wrestrict, "passing argument %i to"
+		       " restrict-qualified parameter aliases with argument%s %I",
+		       param_pos + 1, (arg_positions.length() > 1) ? "s" : "",
+		       &arg_positions); 
+}
+
 #include "gt-c-family-c-common.h"
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 5bbf951..f29ea1f 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -922,6 +922,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 c55c7c3..08edea0 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1032,6 +1032,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 ObjC C++ ObjC++ Var(warn_restrict) Warning LangEnabledBy(C ObjC C++ ObjC++,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 58424a9..f9043ba 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -8368,6 +8368,25 @@ 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)
+		      && !TYPE_READONLY (TREE_TYPE (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 fb88021..87d9ef6 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -6878,6 +6878,26 @@ 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)
+			&& !TYPE_READONLY (TREE_TYPE (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 8eb5eff..d24eb6f 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
@@ -5348,6 +5348,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
+@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/pretty-print.c b/gcc/pretty-print.c
index a39815e..e8bd1ef 100644
--- a/gcc/pretty-print.c
+++ b/gcc/pretty-print.c
@@ -610,6 +610,23 @@ pp_format (pretty_printer *pp, text_info *text)
 	      (pp, *text->args_ptr, precision, unsigned, "u");
 	  break;
 
+	case 'I':
+	  {
+	    vec<int> *v = va_arg (*text->args_ptr, vec<int> *);
+	    unsigned i;
+	    int pos;
+
+	    FOR_EACH_VEC_ELT ((*v), i, pos)
+	      {
+		pp_scalar (pp, "%i", pos); 
+		if (v->length () > 1 && i < (v->length () - 1))
+		  {
+		    pp_comma (pp);
+		    pp_space (pp);
+		  }
+	      }
+	    break;
+	 }
 	case 'x':
 	  if (wide)
 	    pp_scalar (pp, HOST_WIDE_INT_PRINT_HEX,
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..25e3721
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr35503-1.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-Wrestrict" } */
+
+int foo (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..bfcd944
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr35503-2.c
@@ -0,0 +1,14 @@
+/* { 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" } */
+
+/* { 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" } */
+}

Re: PR35503 - warn for restrict pointer

[Jason Merrill]

On Sun, Sep 18, 2016 at 1:16 PM, Prathamesh Kulkarni
<prathamesh.kulkarni@linaro.org> wrote:
> Sorry for late response. In the attached patch, I removed obstack
> building on fmt, and used pp_format for formatting arg_positions by adding specifier %I (name
> chosen arbitrarily). Does that look OK ?
>
> However it results in following warning during gcc build and am not
> sure how to fix this:
> ../../gcc/gcc/c-family/c-common.c: In function ‘void
> warn_for_restrict(unsigned int, vec<tree_node*, va_gc>*)’:
> ../../gcc/gcc/c-family/c-common.c:13132:24: warning: unknown
> conversion type character ‘I’ in format [-Wformat=]

This warning (in stage 1) happens whenever we add a new specifier,
don't worry about it.

+ FOR_EACH_VEC_SAFE_ELT (args, i, arg)
+  TREE_VISITED (arg) = 0;

Hmm, so you're clearing TREE_VISITED before you check the arguments
and leaving it set afterward?  That seems like a problem, a lot of
code assumes that TREE_VISITED has been cleared already and clears it
after.

Jason

Re: PR35503 - warn for restrict pointer

[David Malcolm]

On Mon, 2016-09-19 at 14:21 -0400, Jason Merrill wrote:
> On Sun, Sep 18, 2016 at 1:16 PM, Prathamesh Kulkarni
> <prathamesh.kulkarni@linaro.org> wrote:
> > Sorry for late response. In the attached patch, I removed obstack
> > building on fmt, and used pp_format for formatting arg_positions by
> > adding specifier %I (name
> > chosen arbitrarily). Does that look OK ?
> > 
> > However it results in following warning during gcc build and am not
> > sure how to fix this:
> > ../../gcc/gcc/c-family/c-common.c: In function ‘void
> > warn_for_restrict(unsigned int, vec<tree_node*, va_gc>*)’:
> > ../../gcc/gcc/c-family/c-common.c:13132:24: warning: unknown
> > conversion type character ‘I’ in format [-Wformat=]
> 
> This warning (in stage 1) happens whenever we add a new specifier,
> don't worry about it.

Right: there's no way the stage 1 compiler could know about a new
specifier.

But does the new specifier need to be added to c-family/c-format.c, so
that stage 2 and 3 compilers do know about it?

Re: PR35503 - warn for restrict pointer

[David Malcolm]

On Sun, 2016-09-18 at 22:46 +0530, Prathamesh Kulkarni wrote:
> On 2 September 2016 at 23:14, David Malcolm <dmalcolm@redhat.com>
> wrote:
> > On Thu, 2016-09-01 at 14:55 +0530, Prathamesh Kulkarni wrote:
> > 
> > [...]
> > 
> > > The attached version passes bootstrap+test on ppc64le-linux-gnu.
> > > Given that it only looks if parameters are restrict qualified and
> > > not
> > > how they're used inside the callee,
> > > this can have false positives as in above test-cases.
> > > Should the warning be put in Wextra rather than Wall (I have left
> > > it
> > > in Wall in the patch)  or only enabled with -Wrestrict ?
> > > 
> > > Thanks,
> > > Prathamesh
> > > > 
> > > > Richard.
> > 
> > 
> > diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
> > index 3feb910..a3dae34 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,76 @@ 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;
> > +  FOR_EACH_VEC_ELT (arg_positions, i, pos)
> > +    {
> > +      tree arg = (*args)[pos];
> > +      if (EXPR_HAS_LOCATION (arg))
> > +       richloc.add_range (EXPR_LOCATION (arg), false);
> > +
> > +      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);
> > +}
> > 
> > Thanks for working on this.
> > 
> > I'm not a fan of how the patch builds "fmt" here.  If nothing else,
> > this perhaps should be:
> > 
> >   warning_at_rich_loc (&richloc, OPT_Wrestrict, "%s", fmt);
> > 
> > but building up strings like the patch does makes localization
> > difficult.
> > 
> > Much better would be to have the formatting be done inside the
> > diagnostics subsystem's call into pp_format, with something like
> > this:
> > 
> >   warning_at_rich_loc_n (&richloc, OPT_Wrestrict,
> >                          arg_positions
> > .length (),
> >                          "passing argument %i to restrict
> > -qualified"
> >                          " parameter aliases with argument
> > %FIXME",
> >                          "passing argument %i to restrict
> > -qualified"
> >                          " parameter aliases with arguments
> > %FIXME",
> >                          param_pos + 1,
> > 
> >  &arg_positions);
> > 
> > 
> > and have %FIXME (or somesuch) consume &arg_positions in the va_arg,
> > printing the argument numbers there.  Doing it this way also avoids
> > building the string for the case where Wrestrict is disabled, since
> > the
> > pp_format only happens after we know we're going to print the
> > warning.
> > 
> > Assuming that there isn't yet a pre-canned way to print a set of
> > argument numbers that I've missed, the place to add the %FIXME
> > -handling
> > would presumably be in default_tree_printer in tree-diagnostic.c -
> > though it's obviously nothing to do with trees. (Or if this is too
> > single-purpose, perhaps there's a need to temporarily inject one
> > -time
> > callbacks for consuming custom args??).
> > 
> > We can then have a fun discussion about the usage of the Oxford
> > comma :) [1]
> > 
> > IIRC we don't yet have warning_at_rich_loc_n, but it would be
> > fairly easy to add.
> Hi David,
> Sorry for late response. In the attached patch, I removed obstack
> building on fmt, and used
> pp_format for formatting arg_positions by adding specifier %I (name
> chosen arbitrarily).
> Does that look OK ?

Thanks - this is a big improvement.

In pretty-print.c, please can you add a description of "%I" to the
comment at the top of pp_format.  Also, please add some selftests for
%I to test_pp_format, covering, say the case of 0, 1 and 2 elements in
the vec (to ensure that the separator commas are working properly).

> However it results in following warning during gcc build and am not
> sure how to fix this:
> ../../gcc/gcc/c-family/c-common.c: In function ‘void
> warn_for_restrict(unsigned int, vec<tree_node*, va_gc>*)’:
> ../../gcc/gcc/c-family/c-common.c:13132:24: warning: unknown
> conversion type character ‘I’ in format [-Wformat=]



> Thanks,
> Prathamesh
> > 
> > [...]
> > 
> > [1] which seems to be locale-dependent itself:
> > https://en.wikipedia.org/wiki/Serial_comma#Other_languages

Re: PR35503 - warn for restrict pointer

[Joseph Myers]

On Mon, 19 Sep 2016, David Malcolm wrote:

> But does the new specifier need to be added to c-family/c-format.c, so
> that stage 2 and 3 compilers do know about it?

Yes.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: PR35503 - warn for restrict pointer

[Martin Sebor]

> and used
> pp_format for formatting arg_positions by adding specifier %I (name
> chosen arbitrarily).
> Does that look OK ?

diff --git a/gcc/pretty-print.c b/gcc/pretty-print.c
index a39815e..e8bd1ef 100644
--- a/gcc/pretty-print.c
+++ b/gcc/pretty-print.c
@@ -610,6 +610,23 @@ pp_format (pretty_printer *pp, text_info *text)
  	      (pp, *text->args_ptr, precision, unsigned, "u");
  	  break;

+	case 'I':

The comment just above pp_format that lists the format specifiers
understood by the function should be updated.  There probably (I
hope) is more formal documentation of the format specifiers
somewhere else that should be updated as well.

That said, since this specifier formats a vec<int>, it seems that
it might be useful to be able to format vectors of other elements,
such as short and long.  With that in mind, would adding a new V
length modifier instead be a more regular way to extend the pretty
printer to these types?  The V length modifier would have to be
accepted in conjunction with other length modifiers (h, l, etc
and type specifiers (d, i, o, etc.).  E.g, "%hVu" would accept
a vec<unsigned short>*  as an argument and format it as a sequence
of decimal numbers, and "%lVx" would do the same for vec<long>*
formatting them in hex.

Martin

Re: PR35503 - warn for restrict pointer

[Pedro Alves]

On 09/18/2016 06:16 PM, Prathamesh Kulkarni wrote:
> +  warning_at_rich_loc (&richloc, OPT_Wrestrict, "passing argument %i to"
> +		       " restrict-qualified parameter aliases with argument%s %I",
> +		       param_pos + 1, (arg_positions.length() > 1) ? "s" : "",
> +		       &arg_positions); 
> +}

This way of building a plural string is not i18n friendly:

 https://www.gnu.org/software/gettext/manual/html_node/Plural-forms.html

I see that there are _n suffixed versions of a few diagnostic
functions, like inform_n, warning_n, etc. that handle plurals
by calling ngettext in diagnostic_n_impl, but it looks like a
warning_at_rich_loc_n variant of warning_at_rich_loc is missing.

Thanks,
Pedro Alves

Re: PR35503 - warn for restrict pointer

[Prathamesh Kulkarni]

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

On 20 September 2016 at 08:57, Martin Sebor <msebor@gmail.com> wrote:
>> and used
>> pp_format for formatting arg_positions by adding specifier %I (name
>> chosen arbitrarily).
>> Does that look OK ?
>
>
> diff --git a/gcc/pretty-print.c b/gcc/pretty-print.c
> index a39815e..e8bd1ef 100644
> --- a/gcc/pretty-print.c
> +++ b/gcc/pretty-print.c
> @@ -610,6 +610,23 @@ pp_format (pretty_printer *pp, text_info *text)
>               (pp, *text->args_ptr, precision, unsigned, "u");
>           break;
>
> +       case 'I':
>
> The comment just above pp_format that lists the format specifiers
> understood by the function should be updated.  There probably (I
> hope) is more formal documentation of the format specifiers
> somewhere else that should be updated as well.
>
> That said, since this specifier formats a vec<int>, it seems that
> it might be useful to be able to format vectors of other elements,
> such as short and long.  With that in mind, would adding a new V
> length modifier instead be a more regular way to extend the pretty
> printer to these types?  The V length modifier would have to be
> accepted in conjunction with other length modifiers (h, l, etc
> and type specifiers (d, i, o, etc.).  E.g, "%hVu" would accept
> a vec<unsigned short>*  as an argument and format it as a sequence
> of decimal numbers, and "%lVx" would do the same for vec<long>*
> formatting them in hex.
Hi,
Thanks everyone for the suggestions.

This patch does the following:
a) adds warning_at_rich_loc_n(). Somehow I missed David's clear explanation of
warning_at_rich_loc_n earlier :(

b) clears TREE_VISITED flag in C/C++ FE after we are done with
warn_for_restrict().
I assumed that TREE_VISITED would be treated as a "scratch" flag and any pass
wishing to use it must set it first, so didn't bother clearing at first.

c) It appears '%I' is already used for some format specifier in
c-family/c-format.c ?
I changed name to %Z instead. Martin suggested a better idea above to
implement %Z as a length modifier so we can extend it to print vec of other
types, like %hZx would print vec<short> formatted in hex.
I am not sure though if it would be a simple fix, and left it as a FIXME
in the patch adding %Z just to print vec<int>, maybe we could revisit it later ?
Could someone please take a look at the change to c-format.c, I am not sure
if I have added that correctly.

Thanks,
Prathamesh
>
> Martin
>

[-- Attachment #2: pr35503-9.diff --]
[-- Type: text/x-patch, Size: 13921 bytes --]

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index fc25686..742f06d 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.  */
 
@@ -13084,4 +13085,53 @@ 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<int> 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 + 1); 
+	}
+    }
+
+  if (arg_positions.is_empty ())
+    return;
+
+  int pos;
+  FOR_EACH_VEC_ELT (arg_positions, i, pos)
+    {
+      tree arg = (*args)[pos - 1];
+      if (EXPR_HAS_LOCATION (arg))
+	richloc.add_range (EXPR_LOCATION (arg), false);
+    }
+
+  warning_at_rich_loc_n (&richloc, OPT_Wrestrict, arg_positions.length (),
+			 "passing argument %i to restrict-qualified parameter"
+			 " aliases with argument %Z",
+			 "passing argument %i to restrict-qualified parameter"
+			 " aliases with arguments %Z",
+			 param_pos + 1, &arg_positions); 
+}
+
 #include "gt-c-family-c-common.h"
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 5bbf951..f29ea1f 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -922,6 +922,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-format.c b/gcc/c-family/c-format.c
index bf39ee0..b27d34f 100644
--- a/gcc/c-family/c-format.c
+++ b/gcc/c-family/c-format.c
@@ -713,6 +713,7 @@ static const format_char_info gcc_tdiag_char_table[] =
   { "r",   1, STD_C89, { T89_C,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "",    "cR",   NULL },
   { "<>'R",0, STD_C89, NOARGUMENTS, "",      "",   NULL },
   { "m",   0, STD_C89, NOARGUMENTS, "q",     "",   NULL },
+  { "Z",   1, STD_C89, { T89_C,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "",    "",   NULL },
   { NULL,  0, STD_C89, NOLENGTHS, NULL, NULL, NULL }
 };
 
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index c55c7c3..08edea0 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1032,6 +1032,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 ObjC C++ ObjC++ Var(warn_restrict) Warning LangEnabledBy(C ObjC C++ ObjC++,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 58424a9..3ad1f67 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -8368,6 +8368,28 @@ 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)
+		      && !TYPE_READONLY (TREE_TYPE (t)))
+		    warn_for_restrict (param_pos, exprlist);
+		  param_pos++;
+		}
+
+	      FOR_EACH_VEC_SAFE_ELT (exprlist, i, arg)
+		TREE_VISITED (arg) = 0;
+	    }
+
 	  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 fb88021..58fc615 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -6878,6 +6878,29 @@ 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)
+			&& !TYPE_READONLY (TREE_TYPE (type)))
+		      warn_for_restrict (param_pos, args);
+		  }
+
+		FOR_EACH_VEC_SAFE_ELT (args, i, arg)
+		  TREE_VISITED (arg) = 0;
+	      }
+
 	    if (TREE_CODE (postfix_expression) == COMPONENT_REF)
 	      {
 		tree instance = TREE_OPERAND (postfix_expression, 0);
diff --git a/gcc/diagnostic-core.h b/gcc/diagnostic-core.h
index 51df150..4a1766c 100644
--- a/gcc/diagnostic-core.h
+++ b/gcc/diagnostic-core.h
@@ -65,6 +65,9 @@ extern bool warning_at (location_t, int, const char *, ...)
     ATTRIBUTE_GCC_DIAG(3,4);
 extern bool warning_at_rich_loc (rich_location *, int, const char *, ...)
     ATTRIBUTE_GCC_DIAG(3,4);
+extern bool warning_at_rich_loc_n (rich_location *, int, int, const char *,
+				  const char *, ...)
+    ATTRIBUTE_GCC_DIAG(4, 6) ATTRIBUTE_GCC_DIAG(5, 6);
 extern void error (const char *, ...) ATTRIBUTE_GCC_DIAG(1,2);
 extern void error_n (location_t, int, const char *, const char *, ...)
     ATTRIBUTE_GCC_DIAG(3,5) ATTRIBUTE_GCC_DIAG(4,5);
diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
index 585028e..167a1b5 100644
--- a/gcc/diagnostic.c
+++ b/gcc/diagnostic.c
@@ -53,6 +53,10 @@ static bool diagnostic_impl (rich_location *, int, const char *,
 static bool diagnostic_n_impl (location_t, int, int, const char *,
 			       const char *, va_list *,
 			       diagnostic_t) ATTRIBUTE_GCC_DIAG(5,0);
+static bool diagnostic_n_impl_richloc (rich_location *, int, int, const char *,
+				       const char *, va_list *,
+				       diagnostic_t) ATTRIBUTE_GCC_DIAG(5,0);
+
 static void error_recursion (diagnostic_context *) ATTRIBUTE_NORETURN;
 static void real_abort (void) ATTRIBUTE_NORETURN;
 
@@ -1064,6 +1068,22 @@ diagnostic_impl (rich_location *richloc, int opt,
   return report_diagnostic (&diagnostic);
 }
 
+/* Same as diagonostic_n_impl taking rich_location instead of location_t.  */
+static bool
+diagnostic_n_impl_richloc (rich_location *richloc, int opt, int n,
+			   const char *singular_gmsgid,
+			   const char *plural_gmsgid,
+			   va_list *ap, diagnostic_t kind)
+{
+  diagnostic_info diagnostic;
+  diagnostic_set_info_translated (&diagnostic,
+                                  ngettext (singular_gmsgid, plural_gmsgid, n),
+                                  ap, richloc, kind);
+  if (kind == DK_WARNING)
+    diagnostic.option_index = opt;
+  return report_diagnostic (&diagnostic);
+} 
+
 /* Implement inform_n, warning_n, and error_n, as documented and
    defined below.  */
 static bool
@@ -1072,14 +1092,9 @@ diagnostic_n_impl (location_t location, int opt, int n,
 		   const char *plural_gmsgid,
 		   va_list *ap, diagnostic_t kind)
 {
-  diagnostic_info diagnostic;
   rich_location richloc (line_table, location);
-  diagnostic_set_info_translated (&diagnostic,
-                                  ngettext (singular_gmsgid, plural_gmsgid, n),
-                                  ap, &richloc, kind);
-  if (kind == DK_WARNING)
-    diagnostic.option_index = opt;
-  return report_diagnostic (&diagnostic);
+  return diagnostic_n_impl_richloc (&richloc, opt, n,
+				    singular_gmsgid, plural_gmsgid, ap, kind);
 }
 
 bool
@@ -1170,6 +1185,21 @@ warning_at_rich_loc (rich_location *richloc, int opt, const char *gmsgid, ...)
   return ret;
 }
 
+/* Same as warning_at_rich_loc but for plural variant.  */
+
+bool
+warning_at_rich_loc_n (rich_location *richloc, int opt, int n,
+		       const char *singular_gmsgid, const char *plural_gmsgid, ...)
+{
+  va_list ap;
+  va_start (ap, plural_gmsgid);
+  bool ret = diagnostic_n_impl_richloc (richloc, opt, n,
+					singular_gmsgid, plural_gmsgid,
+					&ap, DK_WARNING);
+  va_end (ap);
+  return ret;
+}
+
 /* A warning at LOCATION.  Use this for code which is correct according to the
    relevant language specification but is likely to be buggy anyway.
    Returns true if the warning was printed, false if it was inhibited.  */
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 8eb5eff..d24eb6f 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
@@ -5348,6 +5348,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
+@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/pretty-print.c b/gcc/pretty-print.c
index a39815e..20e697d 100644
--- a/gcc/pretty-print.c
+++ b/gcc/pretty-print.c
@@ -295,6 +295,11 @@ pp_indent (pretty_printer *pp)
    %Ns: likewise, but length specified as constant in the format string.
    Flag 'q': quote formatted text (must come immediately after '%').
 
+   FIXME: It would be a better idea to add %Z as a length modifier to print
+   vec of other types. For instance %hZx would print vec<short> in hex format.
+   Currently %Z just prints vec<int> formatted in decimal.
+   %Z: vec<int>.
+
    Arguments can be used sequentially, or through %N$ resp. *N$
    notation Nth argument after the format string.  If %N$ / *N$
    notation is used, it must be used for all arguments, except %m, %%,
@@ -610,6 +615,23 @@ pp_format (pretty_printer *pp, text_info *text)
 	      (pp, *text->args_ptr, precision, unsigned, "u");
 	  break;
 
+	case 'Z':
+	  {
+	    vec<int> *v = va_arg (*text->args_ptr, vec<int> *);
+	    unsigned i;
+	    int pos;
+
+	    FOR_EACH_VEC_ELT ((*v), i, pos)
+	      {
+		pp_scalar (pp, "%i", pos);
+		if (i < v->length () - 1)
+		  {
+		    pp_comma (pp);
+		    pp_space (pp);
+		  }
+	      }
+	    break;
+	 }
 	case 'x':
 	  if (wide)
 	    pp_scalar (pp, HOST_WIDE_INT_PRINT_HEX,
@@ -1424,6 +1446,17 @@ test_pp_format ()
 			    "`\33[01m\33[Kfoo\33[m\33[K' 12345678", "%qs %x",
 			    "foo", 0x12345678);
 
+  /* Verify %Z.  */
+  auto_vec<int> v;
+  v.safe_push (1);
+  v.safe_push (2);
+  v.safe_push (3);
+  ASSERT_PP_FORMAT_2 ("1, 2, 3 12345678", "%Z %x", &v, 0x12345678);
+
+  auto_vec<int> v2;
+  v2.safe_push (0);
+  ASSERT_PP_FORMAT_2 ("0 12345678", "%Z %x", &v2, 0x12345678);
+
   /* Verify that combinations work, along with unformatted text.  */
   assert_pp_format (SELFTEST_LOCATION,
 		    "the quick brown fox jumps over the lazy dog",
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..25e3721
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr35503-1.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-Wrestrict" } */
+
+int foo (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..bfcd944
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr35503-2.c
@@ -0,0 +1,14 @@
+/* { 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" } */
+
+/* { 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" } */
+}

Re: PR35503 - warn for restrict pointer

[Joseph Myers]

On Tue, 20 Sep 2016, Martin Sebor wrote:

> That said, since this specifier formats a vec<int>, it seems that
> it might be useful to be able to format vectors of other elements,
> such as short and long.  With that in mind, would adding a new V
> length modifier instead be a more regular way to extend the pretty
> printer to these types?  The V length modifier would have to be
> accepted in conjunction with other length modifiers (h, l, etc
> and type specifiers (d, i, o, etc.).  E.g, "%hVu" would accept

That's much harder to support in format checking (which expects one length 
modifier, not a combination like that).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: PR35503 - warn for restrict pointer

[Joseph Myers]

On Tue, 20 Sep 2016, Prathamesh Kulkarni wrote:

> Could someone please take a look at the change to c-format.c, I am not sure
> if I have added that correctly.

Any changes to these GCC formats also require tests to be updated 
(gcc.dg/format/gcc_diag*).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: PR35503 - warn for restrict pointer

[Martin Sebor]

On 09/20/2016 07:00 AM, Joseph Myers wrote:
> On Tue, 20 Sep 2016, Martin Sebor wrote:
>
>> That said, since this specifier formats a vec<int>, it seems that
>> it might be useful to be able to format vectors of other elements,
>> such as short and long.  With that in mind, would adding a new V
>> length modifier instead be a more regular way to extend the pretty
>> printer to these types?  The V length modifier would have to be
>> accepted in conjunction with other length modifiers (h, l, etc
>> and type specifiers (d, i, o, etc.).  E.g, "%hVu" would accept
>
> That's much harder to support in format checking (which expects one length
> modifier, not a combination like that).

I haven't looked into it detail but since the format checker supports
one-to-two character sequences of length modifiers (h or hh, etc) it
should be possible to extend it to handle one-to-three character
sequences (h, hV, or hhV, etc.)  The V character would not be a new
length modifier on its own but instead be recognized as part of
a longer length modifier sequence.  Do you see a problem with that?

Martin

Re: PR35503 - warn for restrict pointer

[Joseph Myers]

On Tue, 20 Sep 2016, Martin Sebor wrote:

> > That's much harder to support in format checking (which expects one length
> > modifier, not a combination like that).
> 
> I haven't looked into it detail but since the format checker supports
> one-to-two character sequences of length modifiers (h or hh, etc) it
> should be possible to extend it to handle one-to-three character
> sequences (h, hV, or hhV, etc.)  The V character would not be a new
> length modifier on its own but instead be recognized as part of
> a longer length modifier sequence.  Do you see a problem with that?

You could arrange for length modifiers to have extra variants like that 
(at present they have single and double variants listed), but it's clearly 
more complicated than a modifier that's used on its own.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: PR35503 - warn for restrict pointer

[Prathamesh Kulkarni]

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

On 20 September 2016 at 18:31, Joseph Myers <joseph@codesourcery.com> wrote:
> On Tue, 20 Sep 2016, Prathamesh Kulkarni wrote:
>
>> Could someone please take a look at the change to c-format.c, I am not sure
>> if I have added that correctly.
>
> Any changes to these GCC formats also require tests to be updated
> (gcc.dg/format/gcc_diag*).
Hi,
I previously used T_C for 'Z' specifier which was incorrect because
that expected
the argument type to be char *, while the actual type expected is vec<int> *.
I have currently added the following, similar to %p, which I suppose
would at least
silence the "unknown conversion specifier" warning:

 { "Z",   1, STD_C89, { T89_V,   BADLEN,  BADLEN,  BADLEN,  BADLEN,
BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "",
 "",   NULL },

Would that be acceptable ? I am not sure how to make %Z check if the
argument has type vec<int> *
since vec<int> is not really a builtin C type.
Could you suggest me a better solution so that the format checker will check
if arg has type vec<int> * instead of checking if it's just a pointer ?
Also for testing, should I create a testcase in g++.dg since
gcc.dg/format/ tests are C-only ?

Thanks,
Prathamesh
>
> --
> Joseph S. Myers
> joseph@codesourcery.com

[-- Attachment #2: pr35503-10.diff --]
[-- Type: text/x-patch, Size: 13921 bytes --]

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index fc25686..742f06d 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.  */
 
@@ -13084,4 +13085,53 @@ 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<int> 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 + 1); 
+	}
+    }
+
+  if (arg_positions.is_empty ())
+    return;
+
+  int pos;
+  FOR_EACH_VEC_ELT (arg_positions, i, pos)
+    {
+      tree arg = (*args)[pos - 1];
+      if (EXPR_HAS_LOCATION (arg))
+	richloc.add_range (EXPR_LOCATION (arg), false);
+    }
+
+  warning_at_rich_loc_n (&richloc, OPT_Wrestrict, arg_positions.length (),
+			 "passing argument %i to restrict-qualified parameter"
+			 " aliases with argument %Z",
+			 "passing argument %i to restrict-qualified parameter"
+			 " aliases with arguments %Z",
+			 param_pos + 1, &arg_positions); 
+}
+
 #include "gt-c-family-c-common.h"
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 5bbf951..f29ea1f 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -922,6 +922,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-format.c b/gcc/c-family/c-format.c
index bf39ee0..6db2fee 100644
--- a/gcc/c-family/c-format.c
+++ b/gcc/c-family/c-format.c
@@ -713,6 +713,7 @@ static const format_char_info gcc_tdiag_char_table[] =
   { "r",   1, STD_C89, { T89_C,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "",    "cR",   NULL },
   { "<>'R",0, STD_C89, NOARGUMENTS, "",      "",   NULL },
   { "m",   0, STD_C89, NOARGUMENTS, "q",     "",   NULL },
+  { "Z",   1, STD_C89, { T89_V,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "",    "",   NULL },
   { NULL,  0, STD_C89, NOLENGTHS, NULL, NULL, NULL }
 };
 
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index c55c7c3..08edea0 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1032,6 +1032,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 ObjC C++ ObjC++ Var(warn_restrict) Warning LangEnabledBy(C ObjC C++ ObjC++,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 58424a9..3ad1f67 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -8368,6 +8368,28 @@ 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)
+		      && !TYPE_READONLY (TREE_TYPE (t)))
+		    warn_for_restrict (param_pos, exprlist);
+		  param_pos++;
+		}
+
+	      FOR_EACH_VEC_SAFE_ELT (exprlist, i, arg)
+		TREE_VISITED (arg) = 0;
+	    }
+
 	  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 fb88021..58fc615 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -6878,6 +6878,29 @@ 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)
+			&& !TYPE_READONLY (TREE_TYPE (type)))
+		      warn_for_restrict (param_pos, args);
+		  }
+
+		FOR_EACH_VEC_SAFE_ELT (args, i, arg)
+		  TREE_VISITED (arg) = 0;
+	      }
+
 	    if (TREE_CODE (postfix_expression) == COMPONENT_REF)
 	      {
 		tree instance = TREE_OPERAND (postfix_expression, 0);
diff --git a/gcc/diagnostic-core.h b/gcc/diagnostic-core.h
index 51df150..4a1766c 100644
--- a/gcc/diagnostic-core.h
+++ b/gcc/diagnostic-core.h
@@ -65,6 +65,9 @@ extern bool warning_at (location_t, int, const char *, ...)
     ATTRIBUTE_GCC_DIAG(3,4);
 extern bool warning_at_rich_loc (rich_location *, int, const char *, ...)
     ATTRIBUTE_GCC_DIAG(3,4);
+extern bool warning_at_rich_loc_n (rich_location *, int, int, const char *,
+				  const char *, ...)
+    ATTRIBUTE_GCC_DIAG(4, 6) ATTRIBUTE_GCC_DIAG(5, 6);
 extern void error (const char *, ...) ATTRIBUTE_GCC_DIAG(1,2);
 extern void error_n (location_t, int, const char *, const char *, ...)
     ATTRIBUTE_GCC_DIAG(3,5) ATTRIBUTE_GCC_DIAG(4,5);
diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
index 585028e..167a1b5 100644
--- a/gcc/diagnostic.c
+++ b/gcc/diagnostic.c
@@ -53,6 +53,10 @@ static bool diagnostic_impl (rich_location *, int, const char *,
 static bool diagnostic_n_impl (location_t, int, int, const char *,
 			       const char *, va_list *,
 			       diagnostic_t) ATTRIBUTE_GCC_DIAG(5,0);
+static bool diagnostic_n_impl_richloc (rich_location *, int, int, const char *,
+				       const char *, va_list *,
+				       diagnostic_t) ATTRIBUTE_GCC_DIAG(5,0);
+
 static void error_recursion (diagnostic_context *) ATTRIBUTE_NORETURN;
 static void real_abort (void) ATTRIBUTE_NORETURN;
 
@@ -1064,6 +1068,22 @@ diagnostic_impl (rich_location *richloc, int opt,
   return report_diagnostic (&diagnostic);
 }
 
+/* Same as diagonostic_n_impl taking rich_location instead of location_t.  */
+static bool
+diagnostic_n_impl_richloc (rich_location *richloc, int opt, int n,
+			   const char *singular_gmsgid,
+			   const char *plural_gmsgid,
+			   va_list *ap, diagnostic_t kind)
+{
+  diagnostic_info diagnostic;
+  diagnostic_set_info_translated (&diagnostic,
+                                  ngettext (singular_gmsgid, plural_gmsgid, n),
+                                  ap, richloc, kind);
+  if (kind == DK_WARNING)
+    diagnostic.option_index = opt;
+  return report_diagnostic (&diagnostic);
+} 
+
 /* Implement inform_n, warning_n, and error_n, as documented and
    defined below.  */
 static bool
@@ -1072,14 +1092,9 @@ diagnostic_n_impl (location_t location, int opt, int n,
 		   const char *plural_gmsgid,
 		   va_list *ap, diagnostic_t kind)
 {
-  diagnostic_info diagnostic;
   rich_location richloc (line_table, location);
-  diagnostic_set_info_translated (&diagnostic,
-                                  ngettext (singular_gmsgid, plural_gmsgid, n),
-                                  ap, &richloc, kind);
-  if (kind == DK_WARNING)
-    diagnostic.option_index = opt;
-  return report_diagnostic (&diagnostic);
+  return diagnostic_n_impl_richloc (&richloc, opt, n,
+				    singular_gmsgid, plural_gmsgid, ap, kind);
 }
 
 bool
@@ -1170,6 +1185,21 @@ warning_at_rich_loc (rich_location *richloc, int opt, const char *gmsgid, ...)
   return ret;
 }
 
+/* Same as warning_at_rich_loc but for plural variant.  */
+
+bool
+warning_at_rich_loc_n (rich_location *richloc, int opt, int n,
+		       const char *singular_gmsgid, const char *plural_gmsgid, ...)
+{
+  va_list ap;
+  va_start (ap, plural_gmsgid);
+  bool ret = diagnostic_n_impl_richloc (richloc, opt, n,
+					singular_gmsgid, plural_gmsgid,
+					&ap, DK_WARNING);
+  va_end (ap);
+  return ret;
+}
+
 /* A warning at LOCATION.  Use this for code which is correct according to the
    relevant language specification but is likely to be buggy anyway.
    Returns true if the warning was printed, false if it was inhibited.  */
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 8eb5eff..d24eb6f 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
@@ -5348,6 +5348,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
+@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/pretty-print.c b/gcc/pretty-print.c
index a39815e..20e697d 100644
--- a/gcc/pretty-print.c
+++ b/gcc/pretty-print.c
@@ -295,6 +295,11 @@ pp_indent (pretty_printer *pp)
    %Ns: likewise, but length specified as constant in the format string.
    Flag 'q': quote formatted text (must come immediately after '%').
 
+   FIXME: It would be a better idea to add %Z as a length modifier to print
+   vec of other types. For instance %hZx would print vec<short> in hex format.
+   Currently %Z just prints vec<int> formatted in decimal.
+   %Z: vec<int>.
+
    Arguments can be used sequentially, or through %N$ resp. *N$
    notation Nth argument after the format string.  If %N$ / *N$
    notation is used, it must be used for all arguments, except %m, %%,
@@ -610,6 +615,23 @@ pp_format (pretty_printer *pp, text_info *text)
 	      (pp, *text->args_ptr, precision, unsigned, "u");
 	  break;
 
+	case 'Z':
+	  {
+	    vec<int> *v = va_arg (*text->args_ptr, vec<int> *);
+	    unsigned i;
+	    int pos;
+
+	    FOR_EACH_VEC_ELT ((*v), i, pos)
+	      {
+		pp_scalar (pp, "%i", pos);
+		if (i < v->length () - 1)
+		  {
+		    pp_comma (pp);
+		    pp_space (pp);
+		  }
+	      }
+	    break;
+	 }
 	case 'x':
 	  if (wide)
 	    pp_scalar (pp, HOST_WIDE_INT_PRINT_HEX,
@@ -1424,6 +1446,17 @@ test_pp_format ()
 			    "`\33[01m\33[Kfoo\33[m\33[K' 12345678", "%qs %x",
 			    "foo", 0x12345678);
 
+  /* Verify %Z.  */
+  auto_vec<int> v;
+  v.safe_push (1);
+  v.safe_push (2);
+  v.safe_push (3);
+  ASSERT_PP_FORMAT_2 ("1, 2, 3 12345678", "%Z %x", &v, 0x12345678);
+
+  auto_vec<int> v2;
+  v2.safe_push (0);
+  ASSERT_PP_FORMAT_2 ("0 12345678", "%Z %x", &v2, 0x12345678);
+
   /* Verify that combinations work, along with unformatted text.  */
   assert_pp_format (SELFTEST_LOCATION,
 		    "the quick brown fox jumps over the lazy dog",
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..25e3721
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr35503-1.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-Wrestrict" } */
+
+int foo (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..bfcd944
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr35503-2.c
@@ -0,0 +1,14 @@
+/* { 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" } */
+
+/* { 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" } */
+}

Re: PR35503 - warn for restrict pointer

[Joseph Myers]

On Thu, 22 Sep 2016, Prathamesh Kulkarni wrote:

> Would that be acceptable ? I am not sure how to make %Z check if the
> argument has type vec<int> *
> since vec<int> is not really a builtin C type.
> Could you suggest me a better solution so that the format checker will check
> if arg has type vec<int> * instead of checking if it's just a pointer ?
> Also for testing, should I create a testcase in g++.dg since
> gcc.dg/format/ tests are C-only ?

If it's C++-only then it would need to be in g++.dg.

The way we handle GCC-specific types in checking these formats is that the 
code using these formats has to define typedefs which the format-checking 
code then looks up.  In most cases it can just look up names like 
location_t or tree, but for HOST_WIDE_INT it looks up 
__gcc_host_wide_int__ which the user must have defined as a typedef.  
Probably that's the way to go in this case: the user must do "typedef 
vec<int> __gcc_vec_int__;" or similar, and the code looks up 
__gcc_vec_int__.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: PR35503 - warn for restrict pointer

[Prathamesh Kulkarni]

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

On 22 September 2016 at 23:15, Joseph Myers <joseph@codesourcery.com> wrote:
> On Thu, 22 Sep 2016, Prathamesh Kulkarni wrote:
>
>> Would that be acceptable ? I am not sure how to make %Z check if the
>> argument has type vec<int> *
>> since vec<int> is not really a builtin C type.
>> Could you suggest me a better solution so that the format checker will check
>> if arg has type vec<int> * instead of checking if it's just a pointer ?
>> Also for testing, should I create a testcase in g++.dg since
>> gcc.dg/format/ tests are C-only ?
>
> If it's C++-only then it would need to be in g++.dg.
>
> The way we handle GCC-specific types in checking these formats is that the
> code using these formats has to define typedefs which the format-checking
> code then looks up.  In most cases it can just look up names like
> location_t or tree, but for HOST_WIDE_INT it looks up
> __gcc_host_wide_int__ which the user must have defined as a typedef.
> Probably that's the way to go in this case: the user must do "typedef
> vec<int> __gcc_vec_int__;" or similar, and the code looks up
> __gcc_vec_int__.
Thanks for the suggestions. To keep it simple, instead of vec<int>,
I made %Z take two args: int *v, unsigned len, and prints elements in
v having length == len.
Is that OK ?

Bootstrapped+tested on x86_64-unknown-linux-gnu.
As pointed out earlier in the thread, the patch can give false positives because
it only checks whether parameters are qualified with restrict, not how
parameters
are used inside the function. For instance it warned for example 10
mentioned in n1570
under section 6.7.3.1 - "Formal definition of restrict".
Should we keep the warning in Wall or keep it in Wextra ?
The attached patch enables it with Wall.

Thanks,
Prathamesh
>
> --
> Joseph S. Myers
> joseph@codesourcery.com

[-- Attachment #2: pr35503-12.diff --]
[-- Type: text/plain, Size: 16652 bytes --]

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 491c637..751af90 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -48,6 +48,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.  */
 
@@ -13156,4 +13157,59 @@ 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;
+  int *arg_positions = XNEWVEC (int, args->length ());
+  unsigned arg_positions_len = 0;
+
+  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[arg_positions_len++] = (i + 1);
+	}
+    }
+
+  if (arg_positions_len == 0)
+    {
+      free (arg_positions);
+      return;
+    }
+
+  for (unsigned i = 0; i < arg_positions_len; i++)
+    {
+      unsigned pos = arg_positions[i];
+      tree arg = (*args)[pos - 1];
+      if (EXPR_HAS_LOCATION (arg))
+	richloc.add_range (EXPR_LOCATION (arg), false);
+    }
+
+  warning_at_rich_loc_n (&richloc, OPT_Wrestrict, arg_positions_len,
+			 "passing argument %i to restrict-qualified parameter"
+			 " aliases with argument %Z",
+			 "passing argument %i to restrict-qualified parameter"
+			 " aliases with arguments %Z",
+			 param_pos + 1, arg_positions, arg_positions_len);
+
+  free (arg_positions);
+}
+
 #include "gt-c-family-c-common.h"
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index c88619b..45b7439 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -923,6 +923,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-format.c b/gcc/c-family/c-format.c
index bf39ee0..8a4bf6f 100644
--- a/gcc/c-family/c-format.c
+++ b/gcc/c-family/c-format.c
@@ -713,6 +713,7 @@ static const format_char_info gcc_tdiag_char_table[] =
   { "r",   1, STD_C89, { T89_C,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "",    "cR",   NULL },
   { "<>'R",0, STD_C89, NOARGUMENTS, "",      "",   NULL },
   { "m",   0, STD_C89, NOARGUMENTS, "q",     "",   NULL },
+  { "Z",   1, STD_C89, { T89_I,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "",    "", &gcc_tdiag_char_table[0] },
   { NULL,  0, STD_C89, NOLENGTHS, NULL, NULL, NULL }
 };
 
@@ -736,6 +737,7 @@ static const format_char_info gcc_cdiag_char_table[] =
   { "r",   1, STD_C89, { T89_C,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "",    "cR",   NULL },
   { "<>'R",0, STD_C89, NOARGUMENTS, "",      "",   NULL },
   { "m",   0, STD_C89, NOARGUMENTS, "q",     "",   NULL },
+  { "Z",   1, STD_C89, { T89_I,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "",    "", &gcc_tdiag_char_table[0] },
   { NULL,  0, STD_C89, NOLENGTHS, NULL, NULL, NULL }
 };
 
@@ -762,6 +764,7 @@ static const format_char_info gcc_cxxdiag_char_table[] =
   { "r",   1, STD_C89, { T89_C,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "",    "cR",   NULL },
   { "<>'R",0, STD_C89, NOARGUMENTS, "",      "",   NULL },
   { "m",   0, STD_C89, NOARGUMENTS, "q",     "",   NULL },
+  { "Z",   1, STD_C89, { T89_I,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "",    "", &gcc_tdiag_char_table[0] },
   { NULL,  0, STD_C89, NOLENGTHS, NULL, NULL, NULL }
 };
 
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index e146781..da89e9f 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1054,6 +1054,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 ObjC C++ ObjC++ Var(warn_restrict) Warning LangEnabledBy(C ObjC C++ ObjC++,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 6bc42da..52a5edd 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -8451,6 +8451,28 @@ 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)
+		      && !TYPE_READONLY (TREE_TYPE (t)))
+		    warn_for_restrict (param_pos, exprlist);
+		  param_pos++;
+		}
+
+	      FOR_EACH_VEC_SAFE_ELT (exprlist, i, arg)
+		TREE_VISITED (arg) = 0;
+	    }
+
 	  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 16b895c..444132f3 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -6878,6 +6878,29 @@ 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)
+			&& !TYPE_READONLY (TREE_TYPE (type)))
+		      warn_for_restrict (param_pos, args);
+		  }
+
+		FOR_EACH_VEC_SAFE_ELT (args, i, arg)
+		  TREE_VISITED (arg) = 0;
+	      }
+
 	    if (TREE_CODE (postfix_expression) == COMPONENT_REF)
 	      {
 		tree instance = TREE_OPERAND (postfix_expression, 0);
diff --git a/gcc/diagnostic-core.h b/gcc/diagnostic-core.h
index 51df150..4a1766c 100644
--- a/gcc/diagnostic-core.h
+++ b/gcc/diagnostic-core.h
@@ -65,6 +65,9 @@ extern bool warning_at (location_t, int, const char *, ...)
     ATTRIBUTE_GCC_DIAG(3,4);
 extern bool warning_at_rich_loc (rich_location *, int, const char *, ...)
     ATTRIBUTE_GCC_DIAG(3,4);
+extern bool warning_at_rich_loc_n (rich_location *, int, int, const char *,
+				  const char *, ...)
+    ATTRIBUTE_GCC_DIAG(4, 6) ATTRIBUTE_GCC_DIAG(5, 6);
 extern void error (const char *, ...) ATTRIBUTE_GCC_DIAG(1,2);
 extern void error_n (location_t, int, const char *, const char *, ...)
     ATTRIBUTE_GCC_DIAG(3,5) ATTRIBUTE_GCC_DIAG(4,5);
diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
index 585028e..167a1b5 100644
--- a/gcc/diagnostic.c
+++ b/gcc/diagnostic.c
@@ -53,6 +53,10 @@ static bool diagnostic_impl (rich_location *, int, const char *,
 static bool diagnostic_n_impl (location_t, int, int, const char *,
 			       const char *, va_list *,
 			       diagnostic_t) ATTRIBUTE_GCC_DIAG(5,0);
+static bool diagnostic_n_impl_richloc (rich_location *, int, int, const char *,
+				       const char *, va_list *,
+				       diagnostic_t) ATTRIBUTE_GCC_DIAG(5,0);
+
 static void error_recursion (diagnostic_context *) ATTRIBUTE_NORETURN;
 static void real_abort (void) ATTRIBUTE_NORETURN;
 
@@ -1064,6 +1068,22 @@ diagnostic_impl (rich_location *richloc, int opt,
   return report_diagnostic (&diagnostic);
 }
 
+/* Same as diagonostic_n_impl taking rich_location instead of location_t.  */
+static bool
+diagnostic_n_impl_richloc (rich_location *richloc, int opt, int n,
+			   const char *singular_gmsgid,
+			   const char *plural_gmsgid,
+			   va_list *ap, diagnostic_t kind)
+{
+  diagnostic_info diagnostic;
+  diagnostic_set_info_translated (&diagnostic,
+                                  ngettext (singular_gmsgid, plural_gmsgid, n),
+                                  ap, richloc, kind);
+  if (kind == DK_WARNING)
+    diagnostic.option_index = opt;
+  return report_diagnostic (&diagnostic);
+} 
+
 /* Implement inform_n, warning_n, and error_n, as documented and
    defined below.  */
 static bool
@@ -1072,14 +1092,9 @@ diagnostic_n_impl (location_t location, int opt, int n,
 		   const char *plural_gmsgid,
 		   va_list *ap, diagnostic_t kind)
 {
-  diagnostic_info diagnostic;
   rich_location richloc (line_table, location);
-  diagnostic_set_info_translated (&diagnostic,
-                                  ngettext (singular_gmsgid, plural_gmsgid, n),
-                                  ap, &richloc, kind);
-  if (kind == DK_WARNING)
-    diagnostic.option_index = opt;
-  return report_diagnostic (&diagnostic);
+  return diagnostic_n_impl_richloc (&richloc, opt, n,
+				    singular_gmsgid, plural_gmsgid, ap, kind);
 }
 
 bool
@@ -1170,6 +1185,21 @@ warning_at_rich_loc (rich_location *richloc, int opt, const char *gmsgid, ...)
   return ret;
 }
 
+/* Same as warning_at_rich_loc but for plural variant.  */
+
+bool
+warning_at_rich_loc_n (rich_location *richloc, int opt, int n,
+		       const char *singular_gmsgid, const char *plural_gmsgid, ...)
+{
+  va_list ap;
+  va_start (ap, plural_gmsgid);
+  bool ret = diagnostic_n_impl_richloc (richloc, opt, n,
+					singular_gmsgid, plural_gmsgid,
+					&ap, DK_WARNING);
+  va_end (ap);
+  return ret;
+}
+
 /* A warning at LOCATION.  Use this for code which is correct according to the
    relevant language specification but is likely to be buggy anyway.
    Returns true if the warning was printed, false if it was inhibited.  */
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index d9667e7..9fcc742 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -290,7 +290,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
@@ -5544,6 +5544,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
+@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/pretty-print.c b/gcc/pretty-print.c
index a39815e..e58619d 100644
--- a/gcc/pretty-print.c
+++ b/gcc/pretty-print.c
@@ -294,6 +294,8 @@ pp_indent (pretty_printer *pp)
 	 integer.
    %Ns: likewise, but length specified as constant in the format string.
    Flag 'q': quote formatted text (must come immediately after '%').
+   %Z: Requires two arguments - array of int, and len. Prints elements
+   of the array.
 
    Arguments can be used sequentially, or through %N$ resp. *N$
    notation Nth argument after the format string.  If %N$ / *N$
@@ -610,6 +612,23 @@ pp_format (pretty_printer *pp, text_info *text)
 	      (pp, *text->args_ptr, precision, unsigned, "u");
 	  break;
 
+	case 'Z':
+	  {
+	    int *v = va_arg (*text->args_ptr, int *);
+	    unsigned len = va_arg (*text->args_ptr, unsigned); 
+
+	    for (unsigned i = 0; i < len; ++i)
+	      {
+		pp_scalar (pp, "%i", v[i]);
+		if (i < len - 1)
+		  {
+		    pp_comma (pp);
+		    pp_space (pp);
+		  }
+	      }
+	    break;
+	 }
+
 	case 'x':
 	  if (wide)
 	    pp_scalar (pp, HOST_WIDE_INT_PRINT_HEX,
@@ -1424,6 +1443,13 @@ test_pp_format ()
 			    "`\33[01m\33[Kfoo\33[m\33[K' 12345678", "%qs %x",
 			    "foo", 0x12345678);
 
+  /* Verify %Z.  */
+  int v[] = { 1, 2, 3 }; 
+  ASSERT_PP_FORMAT_3 ("1, 2, 3 12345678", "%Z %x", v, 3, 0x12345678);
+
+  int v2[] = { 0 }; 
+  ASSERT_PP_FORMAT_3 ("0 12345678", "%Z %x", v2, 1, 0x12345678);
+
   /* Verify that combinations work, along with unformatted text.  */
   assert_pp_format (SELFTEST_LOCATION,
 		    "the quick brown fox jumps over the lazy dog",
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..25e3721
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr35503-1.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-Wrestrict" } */
+
+int foo (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..bfcd944
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr35503-2.c
@@ -0,0 +1,14 @@
+/* { 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" } */
+
+/* { 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" } */
+}
diff --git a/gcc/testsuite/gcc.dg/format/gcc_diag-1.c b/gcc/testsuite/gcc.dg/format/gcc_diag-1.c
index 953c944..f4922cd 100644
--- a/gcc/testsuite/gcc.dg/format/gcc_diag-1.c
+++ b/gcc/testsuite/gcc.dg/format/gcc_diag-1.c
@@ -32,7 +32,7 @@ foo (int i, int i1, int i2, unsigned int u, double d, char *s, void *p,
      ullong ull, unsigned int *un, const int *cn, signed char *ss,
      unsigned char *us, const signed char *css, unsigned int u1,
      unsigned int u2, location_t *loc, tree t1, union tree_node *t2,
-     tree *t3, tree t4[])
+     tree *t3, tree t4[], int *v, unsigned v_len)
 {
   /* Acceptable C90 specifiers, flags and modifiers.  */
   diag ("%%");
@@ -90,6 +90,10 @@ foo (int i, int i1, int i2, unsigned int u, double d, char *s, void *p,
   cdiag ("%v%qv%#v", i, i, i);
   cxxdiag ("%v%qv%#v", i, i, i);
 
+  tdiag ("%Z", v, v_len);
+  cdiag ("%Z", v, v_len);
+  cxxdiag ("%Z", v, v_len);
+
   /* Bad stuff with extensions.  */
   diag ("%m", i); /* { dg-warning "format" "extra arg" } */
   tdiag ("%m", i); /* { dg-warning "format" "extra arg" } */
@@ -133,6 +137,9 @@ foo (int i, int i1, int i2, unsigned int u, double d, char *s, void *p,
   cdiag ("%v", t1); /* { dg-warning "format" "wrong arg" } */
   cxxdiag ("%v", t1); /* { dg-warning "format" "wrong arg" } */
 
+  tdiag ("%Z"); /* { dg-warning "format" "missing arg" } */
+  tdiag ("%Z", t1); /* { dg-warning "format" "wrong arg" } */
+
   /* Standard specifiers not accepted in the diagnostic framework.  */
   diag ("%X\n", u); /* { dg-warning "format" "HEX" } */
   diag ("%f\n", d); /* { dg-warning "format" "float" } */

Re: PR35503 - warn for restrict pointer

[David Malcolm]

On Fri, 2016-10-07 at 10:33 +0530, Prathamesh Kulkarni wrote:
> On 22 September 2016 at 23:15, Joseph Myers <joseph@codesourcery.com>
> wrote:
> > On Thu, 22 Sep 2016, Prathamesh Kulkarni wrote:
> > 
> > > Would that be acceptable ? I am not sure how to make %Z check if
> > > the
> > > argument has type vec<int> *
> > > since vec<int> is not really a builtin C type.
> > > Could you suggest me a better solution so that the format checker
> > > will check
> > > if arg has type vec<int> * instead of checking if it's just a
> > > pointer ?
> > > Also for testing, should I create a testcase in g++.dg since
> > > gcc.dg/format/ tests are C-only ?
> > 
> > If it's C++-only then it would need to be in g++.dg.
> > 
> > The way we handle GCC-specific types in checking these formats is
> > that the
> > code using these formats has to define typedefs which the format
> > -checking
> > code then looks up.  In most cases it can just look up names like
> > location_t or tree, but for HOST_WIDE_INT it looks up
> > __gcc_host_wide_int__ which the user must have defined as a
> > typedef.
> > Probably that's the way to go in this case: the user must do
> > "typedef
> > vec<int> __gcc_vec_int__;" or similar, and the code looks up
> > __gcc_vec_int__.
> Thanks for the suggestions. To keep it simple, instead of vec<int>,
> I made %Z take two args: int *v, unsigned len, and prints elements in
> v having length == len.
> Is that OK ?
> 
> Bootstrapped+tested on x86_64-unknown-linux-gnu.
> As pointed out earlier in the thread, the patch can give false
> positives because
> it only checks whether parameters are qualified with restrict, not
> how
> parameters
> are used inside the function. For instance it warned for example 10
> mentioned in n1570
> under section 6.7.3.1 - "Formal definition of restrict".
> Should we keep the warning in Wall or keep it in Wextra ?
> The attached patch enables it with Wall.
> 
> Thanks,
> Prathamesh

This needs a ChangeLog.

The changes to diagnostic-core.h and diagnostic.c are OK for trunk,
given a suitable ChangeLog (and could be split into a separate patch if
you like).

Re: PR35503 - warn for restrict pointer

[Prathamesh Kulkarni]

On 7 October 2016 at 17:49, David Malcolm <dmalcolm@redhat.com> wrote:
> On Fri, 2016-10-07 at 10:33 +0530, Prathamesh Kulkarni wrote:
>> On 22 September 2016 at 23:15, Joseph Myers <joseph@codesourcery.com>
>> wrote:
>> > On Thu, 22 Sep 2016, Prathamesh Kulkarni wrote:
>> >
>> > > Would that be acceptable ? I am not sure how to make %Z check if
>> > > the
>> > > argument has type vec<int> *
>> > > since vec<int> is not really a builtin C type.
>> > > Could you suggest me a better solution so that the format checker
>> > > will check
>> > > if arg has type vec<int> * instead of checking if it's just a
>> > > pointer ?
>> > > Also for testing, should I create a testcase in g++.dg since
>> > > gcc.dg/format/ tests are C-only ?
>> >
>> > If it's C++-only then it would need to be in g++.dg.
>> >
>> > The way we handle GCC-specific types in checking these formats is
>> > that the
>> > code using these formats has to define typedefs which the format
>> > -checking
>> > code then looks up.  In most cases it can just look up names like
>> > location_t or tree, but for HOST_WIDE_INT it looks up
>> > __gcc_host_wide_int__ which the user must have defined as a
>> > typedef.
>> > Probably that's the way to go in this case: the user must do
>> > "typedef
>> > vec<int> __gcc_vec_int__;" or similar, and the code looks up
>> > __gcc_vec_int__.
>> Thanks for the suggestions. To keep it simple, instead of vec<int>,
>> I made %Z take two args: int *v, unsigned len, and prints elements in
>> v having length == len.
>> Is that OK ?
>>
>> Bootstrapped+tested on x86_64-unknown-linux-gnu.
>> As pointed out earlier in the thread, the patch can give false
>> positives because
>> it only checks whether parameters are qualified with restrict, not
>> how
>> parameters
>> are used inside the function. For instance it warned for example 10
>> mentioned in n1570
>> under section 6.7.3.1 - "Formal definition of restrict".
>> Should we keep the warning in Wall or keep it in Wextra ?
>> The attached patch enables it with Wall.
>>
>> Thanks,
>> Prathamesh
>
> This needs a ChangeLog.
>
> The changes to diagnostic-core.h and diagnostic.c are OK for trunk,
> given a suitable ChangeLog (and could be split into a separate patch if
> you like).
Thanks, I committed diagnostic.c and diagnostic-core.h changes (with ChangeLog)
in r240891.

Thanks,
Prathamesh

Re: PR35503 - warn for restrict pointer

[Jason Merrill]

On Sat, Oct 8, 2016 at 1:07 PM, Prathamesh Kulkarni
<prathamesh.kulkarni@linaro.org> wrote:
> On 7 October 2016 at 17:49, David Malcolm <dmalcolm@redhat.com> wrote:
>> On Fri, 2016-10-07 at 10:33 +0530, Prathamesh Kulkarni wrote:
>>> On 22 September 2016 at 23:15, Joseph Myers <joseph@codesourcery.com>
>>> wrote:
>>> > On Thu, 22 Sep 2016, Prathamesh Kulkarni wrote:
>>> >
>>> > > Would that be acceptable ? I am not sure how to make %Z check if
>>> > > the
>>> > > argument has type vec<int> *
>>> > > since vec<int> is not really a builtin C type.
>>> > > Could you suggest me a better solution so that the format checker
>>> > > will check
>>> > > if arg has type vec<int> * instead of checking if it's just a
>>> > > pointer ?
>>> > > Also for testing, should I create a testcase in g++.dg since
>>> > > gcc.dg/format/ tests are C-only ?
>>> >
>>> > If it's C++-only then it would need to be in g++.dg.
>>> >
>>> > The way we handle GCC-specific types in checking these formats is
>>> > that the
>>> > code using these formats has to define typedefs which the format
>>> > -checking
>>> > code then looks up.  In most cases it can just look up names like
>>> > location_t or tree, but for HOST_WIDE_INT it looks up
>>> > __gcc_host_wide_int__ which the user must have defined as a
>>> > typedef.
>>> > Probably that's the way to go in this case: the user must do
>>> > "typedef
>>> > vec<int> __gcc_vec_int__;" or similar, and the code looks up
>>> > __gcc_vec_int__.
>>> Thanks for the suggestions. To keep it simple, instead of vec<int>,
>>> I made %Z take two args: int *v, unsigned len, and prints elements in
>>> v having length == len.
>>> Is that OK ?
>>>
>>> Bootstrapped+tested on x86_64-unknown-linux-gnu.
>>> As pointed out earlier in the thread, the patch can give false
>>> positives because
>>> it only checks whether parameters are qualified with restrict, not
>>> how
>>> parameters
>>> are used inside the function. For instance it warned for example 10
>>> mentioned in n1570
>>> under section 6.7.3.1 - "Formal definition of restrict".
>>> Should we keep the warning in Wall or keep it in Wextra ?
>>> The attached patch enables it with Wall.
>>>
>>
>> This needs a ChangeLog.
>>
>> The changes to diagnostic-core.h and diagnostic.c are OK for trunk,
>> given a suitable ChangeLog (and could be split into a separate patch if
>> you like).
> Thanks, I committed diagnostic.c and diagnostic-core.h changes (with ChangeLog)
> in r240891.

The C++ changes are also OK.

Jason

Re: PR35503 - warn for restrict pointer

[Prathamesh Kulkarni]

On 7 October 2016 at 10:33, Prathamesh Kulkarni
<prathamesh.kulkarni@linaro.org> wrote:
> On 22 September 2016 at 23:15, Joseph Myers <joseph@codesourcery.com> wrote:
>> On Thu, 22 Sep 2016, Prathamesh Kulkarni wrote:
>>
>>> Would that be acceptable ? I am not sure how to make %Z check if the
>>> argument has type vec<int> *
>>> since vec<int> is not really a builtin C type.
>>> Could you suggest me a better solution so that the format checker will check
>>> if arg has type vec<int> * instead of checking if it's just a pointer ?
>>> Also for testing, should I create a testcase in g++.dg since
>>> gcc.dg/format/ tests are C-only ?
>>
>> If it's C++-only then it would need to be in g++.dg.
>>
>> The way we handle GCC-specific types in checking these formats is that the
>> code using these formats has to define typedefs which the format-checking
>> code then looks up.  In most cases it can just look up names like
>> location_t or tree, but for HOST_WIDE_INT it looks up
>> __gcc_host_wide_int__ which the user must have defined as a typedef.
>> Probably that's the way to go in this case: the user must do "typedef
>> vec<int> __gcc_vec_int__;" or similar, and the code looks up
>> __gcc_vec_int__.
> Thanks for the suggestions. To keep it simple, instead of vec<int>,
> I made %Z take two args: int *v, unsigned len, and prints elements in
> v having length == len.
> Is that OK ?
>
> Bootstrapped+tested on x86_64-unknown-linux-gnu.
> As pointed out earlier in the thread, the patch can give false positives because
> it only checks whether parameters are qualified with restrict, not how
> parameters
> are used inside the function. For instance it warned for example 10
> mentioned in n1570
> under section 6.7.3.1 - "Formal definition of restrict".
> Should we keep the warning in Wall or keep it in Wextra ?
> The attached patch enables it with Wall.
Ping for c, c-family changes:
https://gcc.gnu.org/ml/gcc-patches/2016-10/msg00446.html

Thanks,
Prathamesh
>
> Thanks,
> Prathamesh
>>
>> --
>> Joseph S. Myers
>> joseph@codesourcery.com