From patchwork Wed Oct 19 14:16:19 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Prathamesh Kulkarni X-Patchwork-Id: 78253 Delivered-To: patch@linaro.org Received: by 10.140.97.247 with SMTP id m110csp272148qge; Wed, 19 Oct 2016 07:16:46 -0700 (PDT) X-Received: by 10.98.64.140 with SMTP id f12mr11835077pfd.45.1476886606650; Wed, 19 Oct 2016 07:16:46 -0700 (PDT) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id n19si3773069pgk.202.2016.10.19.07.16.46 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 19 Oct 2016 07:16:46 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-return-439026-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org; spf=pass (google.com: domain of gcc-patches-return-439026-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-439026-patch=linaro.org@gcc.gnu.org; dmarc=fail (p=NONE dis=NONE) header.from=linaro.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:from:date:message-id:subject:to:content-type; q= dns; s=default; b=n/qSPpTnaY/sZlit4k2QAjEngvjnKi6CC6RDezJgfkvwFK u6cyJLyHyslIAKxYcrcux8bn42YIifpKlYyDRRpstOz0M4/gFmVgHvmT2J979Gey lbEME6e46m4FPFbBErAsCXbJE/TTNjghYmSomvyQAX8ooEI5D4UjEZcH/ax+g= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:from:date:message-id:subject:to:content-type; s= default; bh=qs8XXubpcpNLSBien1eKj3Fgxko=; b=PKHp09zEyNbJiOKK5oIN xuHXAZLzxN39wMkf4gIDX8lKB3tO2IwmZoCRBaeTYZAjVWuRDJ9gSnW/6SrQDLYW aUjbs1pYJjK4QE+PpPpJGc69AEEP7IvlpU8i7VbGK+/d8zFQ4tQKQJ38ID69ocVt u28fvcI4GlnEZtZI2Mw70Tk= Received: (qmail 45154 invoked by alias); 19 Oct 2016 14:16:32 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 45137 invoked by uid 89); 19 Oct 2016 14:16:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=no version=3.3.2 spammy=fox, 906, *n, *N X-HELO: mail-io0-f176.google.com Received: from mail-io0-f176.google.com (HELO mail-io0-f176.google.com) (209.85.223.176) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 19 Oct 2016 14:16:22 +0000 Received: by mail-io0-f176.google.com with SMTP id j37so32755864ioo.3 for ; Wed, 19 Oct 2016 07:16:21 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:from:date:message-id:subject:to; bh=eQR2wIbCr7U+5tWqW8X70t6PKWTb0yUR4navjQCZdQA=; b=HzbAsr0arzoelSXI02HfoAxVvaI8EMbG6y5fQ7cjRwEGDgoqWJ2p/JSqu+xNy5OcIB WMDNjpu5W4gAfSyLhYnCBkSOWv/J9xAA/DmwaNh1m+S9GTvL7+yJk98W7sdh37q0clsn GERx7g0FOJ47EtNMkM8+b7f6abwGhCk2wBcmDGnZEW1JQse2f8/tjm8MIdWZigy3CR0n HFjeAowW0uR3elEFWlVfect/7Y98kW8aM5OWd6yVpE4YwVtoYAHozR4/TtoVm6ge9htz Ah0RdwozxUs149b8qISee8NlWhEzpl2jpQHhSVxIsex7dYm4yfjqq6YC2GLbjlj6Td67 TG6w== X-Gm-Message-State: AA6/9RnF2rwbG6oGXmEUiSEYB9B6suLZ9iL63vthcDMPb0EPQ1MKqNsC2QhddYasmlatGPIWBewE4Ys0ySU/gmpD X-Received: by 10.107.20.85 with SMTP id 82mr6817300iou.187.1476886580181; Wed, 19 Oct 2016 07:16:20 -0700 (PDT) MIME-Version: 1.0 Received: by 10.36.112.78 with HTTP; Wed, 19 Oct 2016 07:16:19 -0700 (PDT) From: Prathamesh Kulkarni Date: Wed, 19 Oct 2016 19:46:19 +0530 Message-ID: Subject: [ping * 2] PR35503 - warn for restrict To: gcc Patches , Marek Polacek , "Joseph S. Myers" X-IsSubscribed: yes Hi, The attached patch is a rebased version of the patch posted at: https://gcc.gnu.org/ml/gcc-patches/2016-10/msg00446.html The C++ changes are approved by Jason, other parts still require approval. Bootstrapped and tested on x86_64-unknown-linux-gnu. OK to commit ? I was wondering if the warning should go in Wall or Wextra ? 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". Currently I have kept the warning in Wall. Thanks, Prathamesh 2016-10-19 Prathamesh Kulkarni PR c/35503 * doc/invoke.texi: Document Wrestrict. * pretty-print.c (pp_format): Add case for "Z" specifier. (test_pp_format): Test "Z" specifier. c-family/ * c-common.h (warn_for_restrict): Declare. * c-warn.c: Include gcc-rich-location.h. (warn_for_restrict): New function. * c-format.c (gcc_tdiag_char_table): Add entry for "Z" specifier. (gcc_cdiag_char_table): Likewise. (gcc_cxxdiag_char_table): Likewise. * c.opt (Wrestrict): New option. c/ * c-parser.c (c_parser_postfix_expression_after_primary): Call warn_for_restrict. cp/ * parser.c (cp_parser_postfix_pexpression): Call warn_for_restrict. testsuite/ * c-c++-common/pr35503-1.c: New test. * c-c++-common/pr35503-2.c: Likewise. * c-c++-common/pr35503-3.c: Likewise. * gcc.dg/format/gcc_diag-1.c: Add tests for "Z" specifier. diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h index bfdbda0..4d8866d 100644 --- a/gcc/c-family/c-common.h +++ b/gcc/c-family/c-common.h @@ -1492,6 +1492,7 @@ extern void warnings_for_convert_and_check (location_t, tree, tree, tree); extern void c_do_switch_warnings (splay_tree, location_t, tree, tree, bool, bool); extern void warn_for_omitted_condop (location_t, tree); +extern void warn_for_restrict (unsigned, vec *); /* Places where an lvalue, or modifiable lvalue, may be required. Used to select diagnostic messages in lvalue_error and 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-warn.c b/gcc/c-family/c-warn.c index 904f6d3..36dce74 100644 --- a/gcc/c-family/c-warn.c +++ b/gcc/c-family/c-warn.c @@ -28,7 +28,7 @@ along with GCC; see the file COPYING3. If not see #include "tm_p.h" #include "diagnostic.h" #include "intl.h" - +#include "gcc-rich-location.h" /* Print a warning if a constant expression had overflow in folding. Invoke this function on every expression that the language @@ -2154,3 +2154,58 @@ maybe_warn_bool_compare (location_t loc, enum tree_code code, tree op0, "with boolean expression is always false", cst); } } + +/* 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 *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); +} diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 458d453..a72946c 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -1078,6 +1078,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 643c1e7..1b751d8 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -6894,6 +6894,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/doc/invoke.texi b/gcc/doc/invoke.texi index 556ad36..451089c 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -291,7 +291,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 @@ -5675,6 +5675,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" } */