diff mbox

PR35503 - warn for restrict pointer

Message ID CAAgBjMkRNk3f47mXwZ6J_MyMHQym4vo3+zXCZ1R6jjrqDkKRYA@mail.gmail.com
State New
Headers show

Commit Message

Prathamesh Kulkarni Aug. 26, 2016, 11:39 a.m. UTC 
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

Comments

Prathamesh Kulkarni Aug. 30, 2016, 3:34 p.m. UTC | #1 
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
diff mbox

Patch

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