PR35503 - warn for restrict pointer

Marek Polacek polacek@redhat.com
Mon Aug 29 14:12:00 GMT 2016


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



More information about the Gcc-patches mailing list