From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 79DC8385780B for ; Tue, 22 Sep 2020 20:07:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 79DC8385780B Received: from mail-qk1-f197.google.com (mail-qk1-f197.google.com [209.85.222.197]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-379-HTZGZ5jTM3m5xEnVUT8MJw-1; Tue, 22 Sep 2020 16:07:45 -0400 X-MC-Unique: HTZGZ5jTM3m5xEnVUT8MJw-1 Received: by mail-qk1-f197.google.com with SMTP id r128so14749402qkc.9 for ; Tue, 22 Sep 2020 13:07:45 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=Or3oQTSYcE1EoqWdEw3hHBQkkHLCV4RcREs9EDZqpV0=; b=WdHpf98R4Mf/IEcd8I4OdgEclzro2UkxSFJgnnJA+XZQQNvhI0ejE9A/TJhoMV5tgR U///Mc8OXvO2bZKqiD3RnwFMYAL/oXjT6ieV9OXrREIdzlN2gNsebk2Myao48jK/hB6J 6XRtmvirBRMs/W5vECEOxBrd44+4sJVHowBMHefc0FT1WmS0cgHQ8/niuaKvjO4fvWZ3 2AQcmvCW482yk/RR7njCHYcDfrp1Y+FMSXeib++jPtJN9GUvm+bqMb60mKQ3FUTW63Xq VrRXbuir12M+AVI13JsvqmwG/5UN9l36CTtrZvKcMhOQ+NZloKzI90CVr40dKBGp/KZk Ndrw== X-Gm-Message-State: AOAM532LfQAZQvoBSqdcHlu/m4NKsVkhYoOg9UzppN5yEy4MaQseqOx2 n4Qnt36yZEhg/ioeGF3refSLljMFEnzEU5Ih4D+6tJ4jVo1AJQiRqtTAmqEtnk6knb3cE6vrbLI mUXqXaM6nmjWyUznufQ== X-Received: by 2002:ae9:e70b:: with SMTP id m11mr6490748qka.210.1600805263961; Tue, 22 Sep 2020 13:07:43 -0700 (PDT) X-Google-Smtp-Source: ABdhPJy+pEQGNB4niCJLWzTQ4YJJJk6nnIPRKg5FPz2q0Q0MBDMjy30Z7dZxBHCiifDv0/DNntmRGw== X-Received: by 2002:ae9:e70b:: with SMTP id m11mr6490700qka.210.1600805263328; Tue, 22 Sep 2020 13:07:43 -0700 (PDT) Received: from [192.168.1.148] (209-6-216-142.s141.c3-0.smr-cbr1.sbo-smr.ma.cable.rcncustomer.com. [209.6.216.142]) by smtp.gmail.com with ESMTPSA id e26sm12041980qka.24.2020.09.22.13.07.42 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 22 Sep 2020 13:07:42 -0700 (PDT) Subject: Re: [PATCH v3] c, c++: Implement -Wsizeof-array-div [PR91741] To: Marek Polacek , Jakub Jelinek , GCC Patches , Joseph Myers References: <20200914183415.1184637-1-polacek@redhat.com> <20200915013044.GP5926@redhat.com> <20200915070441.GK21814@tucnak> <20200915203305.GU5926@redhat.com> <20200922172930.GO5926@redhat.com> From: Jason Merrill Message-ID: <020a21c3-c8f3-f3b2-a3a9-a8dfcd780e04@redhat.com> Date: Tue, 22 Sep 2020 16:07:41 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: <20200922172930.GO5926@redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-Spam-Status: No, score=-16.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 22 Sep 2020 20:07:52 -0000 On 9/22/20 1:29 PM, Marek Polacek wrote: > Ping. The C++ change is OK. > On Tue, Sep 15, 2020 at 04:33:05PM -0400, Marek Polacek via Gcc-patches wrote: >> On Tue, Sep 15, 2020 at 09:04:41AM +0200, Jakub Jelinek via Gcc-patches wrote: >>> On Mon, Sep 14, 2020 at 09:30:44PM -0400, Marek Polacek via Gcc-patches wrote: >>>> --- a/gcc/c/c-tree.h >>>> +++ b/gcc/c/c-tree.h >>>> @@ -147,6 +147,11 @@ struct c_expr >>>> etc), so we stash a copy here. */ >>>> source_range src_range; >>>> >>>> + /* True iff the sizeof expression was enclosed in parentheses. >>>> + NB: This member is currently only initialized when .original_code >>>> + is a SIZEOF_EXPR. ??? Add a default constructor to this class. */ >>>> + bool parenthesized_p; >>>> + >>>> /* Access to the first and last locations within the source spelling >>>> of this expression. */ >>>> location_t get_start () const { return src_range.m_start; } >>> >>> I think a magic tree code would be better, c_expr is used in too many places >>> and returned by many functions, so it is copied over and over. >>> Even if you must add it, it would be better to change the struct layout, >>> because right now there are fields: tree, location_t, tree, 2xlocation_t, >>> which means 32-bit gap on 64-bit hosts before the second tree, so the new >>> field would fit in there. But, if it is mostly uninitialized, it is kind of >>> unclean. >> >> Ok, here's a version with PAREN_SIZEOF_EXPR. It doesn't require changes to >> c_expr, but adding a new tree code is always a pain... >> >> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? >> >> -- >8 -- >> This patch implements a new warning, -Wsizeof-array-div. It warns about >> code like >> >> int arr[10]; >> sizeof (arr) / sizeof (short); >> >> where we have a division of two sizeof expressions, where the first >> argument is an array, and the second sizeof does not equal the size >> of the array element. See e.g. . >> >> Clang makes it possible to suppress the warning by parenthesizing the >> second sizeof like this: >> >> sizeof (arr) / (sizeof (short)); >> >> so I followed suit. In the C++ FE this was rather easy, because >> finish_parenthesized_expr already set TREE_NO_WARNING. In the C FE >> I've added a new tree code, PAREN_SIZEOF_EXPR, to discern between the >> non-() and () versions. >> >> This warning is enabled by -Wall. An example of the output: >> >> x.c:5:23: warning: expression does not compute the number of elements in this array; element type is ‘int’, not ‘short int’ [-Wsizeof-array-div] >> 5 | return sizeof (arr) / sizeof (short); >> | ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~ >> x.c:5:25: note: add parentheses around ‘sizeof (short int)’ to silence this warning >> 5 | return sizeof (arr) / sizeof (short); >> | ^~~~~~~~~~~~~~ >> | ( ) >> x.c:4:7: note: array ‘arr’ declared here >> 4 | int arr[10]; >> | ^~~ >> >> gcc/c-family/ChangeLog: >> >> PR c++/91741 >> * c-common.c (verify_tree): Handle PAREN_SIZEOF_EXPR. >> (c_common_init_ts): Likewise. >> * c-common.def (PAREN_SIZEOF_EXPR): New tree code. >> * c-common.h (maybe_warn_sizeof_array_div): Declare. >> * c-warn.c (sizeof_pointer_memaccess_warning): Unwrap NOP_EXPRs. >> (maybe_warn_sizeof_array_div): New function. >> * c.opt (Wsizeof-array-div): New option. >> >> gcc/c/ChangeLog: >> >> PR c++/91741 >> * c-parser.c (c_parser_binary_expression): Implement -Wsizeof-array-div. >> (c_parser_postfix_expression): Set PAREN_SIZEOF_EXPR. >> (c_parser_expr_list): Handle PAREN_SIZEOF_EXPR like SIZEOF_EXPR. >> * c-tree.h (char_type_p): Declare. >> * c-typeck.c (char_type_p): No longer static. >> >> gcc/cp/ChangeLog: >> >> PR c++/91741 >> * typeck.c (cp_build_binary_op): Implement -Wsizeof-array-div. >> >> gcc/ChangeLog: >> >> PR c++/91741 >> * doc/invoke.texi: Document -Wsizeof-array-div. >> >> gcc/testsuite/ChangeLog: >> >> PR c++/91741 >> * c-c++-common/Wsizeof-pointer-div.c: Add dg-warning. >> * c-c++-common/Wsizeof-array-div1.c: New test. >> * g++.dg/warn/Wsizeof-array-div1.C: New test. >> * g++.dg/warn/Wsizeof-array-div2.C: New test. >> --- >> gcc/c-family/c-common.c | 2 + >> gcc/c-family/c-common.def | 3 + >> gcc/c-family/c-common.h | 1 + >> gcc/c-family/c-warn.c | 47 ++++++++++++++++ >> gcc/c-family/c.opt | 5 ++ >> gcc/c/c-parser.c | 48 ++++++++++------ >> gcc/c/c-tree.h | 1 + >> gcc/c/c-typeck.c | 2 +- >> gcc/cp/typeck.c | 10 +++- >> gcc/doc/invoke.texi | 19 +++++++ >> .../c-c++-common/Wsizeof-array-div1.c | 56 +++++++++++++++++++ >> .../c-c++-common/Wsizeof-pointer-div.c | 2 +- >> .../g++.dg/warn/Wsizeof-array-div1.C | 37 ++++++++++++ >> .../g++.dg/warn/Wsizeof-array-div2.C | 15 +++++ >> 14 files changed, 226 insertions(+), 22 deletions(-) >> create mode 100644 gcc/testsuite/c-c++-common/Wsizeof-array-div1.c >> create mode 100644 gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C >> create mode 100644 gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C >> >> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c >> index 873bea9e2b2..b0a71d10e19 100644 >> --- a/gcc/c-family/c-common.c >> +++ b/gcc/c-family/c-common.c >> @@ -1854,6 +1854,7 @@ verify_tree (tree x, struct tlist **pbefore_sp, struct tlist **pno_sp, >> { >> case CONSTRUCTOR: >> case SIZEOF_EXPR: >> + case PAREN_SIZEOF_EXPR: >> return; >> >> case COMPOUND_EXPR: >> @@ -8124,6 +8125,7 @@ void >> c_common_init_ts (void) >> { >> MARK_TS_EXP (SIZEOF_EXPR); >> + MARK_TS_EXP (PAREN_SIZEOF_EXPR); >> MARK_TS_EXP (C_MAYBE_CONST_EXPR); >> MARK_TS_EXP (EXCESS_PRECISION_EXPR); >> } >> diff --git a/gcc/c-family/c-common.def b/gcc/c-family/c-common.def >> index 7ceb9a22bd4..06f112e5683 100644 >> --- a/gcc/c-family/c-common.def >> +++ b/gcc/c-family/c-common.def >> @@ -55,6 +55,9 @@ DEFTREECODE (USERDEF_LITERAL, "userdef_literal", tcc_exceptional, 3) >> or for the purpose of -Wsizeof-pointer-memaccess warning. */ >> DEFTREECODE (SIZEOF_EXPR, "sizeof_expr", tcc_expression, 1) >> >> +/* Like above, but enclosed in parentheses. Used to suppress warnings. */ >> +DEFTREECODE (PAREN_SIZEOF_EXPR, "paren_sizeof_expr", tcc_expression, 1) >> + >> /* >> Local variables: >> mode:c >> diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h >> index 4fc64bc4aa6..443aaa2ca9c 100644 >> --- a/gcc/c-family/c-common.h >> +++ b/gcc/c-family/c-common.h >> @@ -1321,6 +1321,7 @@ extern void c_do_switch_warnings (splay_tree, location_t, tree, tree, bool); >> extern void warn_for_omitted_condop (location_t, tree); >> extern bool warn_for_restrict (unsigned, tree *, unsigned); >> extern void warn_for_address_or_pointer_of_packed_member (tree, tree); >> +extern void maybe_warn_sizeof_array_div (location_t, tree, tree, tree, tree); >> >> /* 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-warn.c b/gcc/c-family/c-warn.c >> index c32d8228b5c..6fdada5f9b7 100644 >> --- a/gcc/c-family/c-warn.c >> +++ b/gcc/c-family/c-warn.c >> @@ -3099,3 +3099,50 @@ warn_for_address_or_pointer_of_packed_member (tree type, tree rhs) >> >> check_and_warn_address_or_pointer_of_packed_member (type, rhs); >> } >> + >> +/* Warn about divisions of two sizeof operators when the first one is applied >> + to an array and the divisor does not equal the size of the array element. >> + For instance: >> + >> + sizeof (ARR) / sizeof (OP) >> + >> + ARR is the array argument of the first sizeof, ARR_TYPE is its ARRAY_TYPE. >> + OP1 is the whole second SIZEOF_EXPR, or its argument; TYPE1 is the type >> + of the second argument. */ >> + >> +void >> +maybe_warn_sizeof_array_div (location_t loc, tree arr, tree arr_type, >> + tree op1, tree type1) >> +{ >> + tree elt_type = TREE_TYPE (arr_type); >> + >> + if (!warn_sizeof_array_div >> + /* Don't warn on multidimensional arrays. */ >> + || TREE_CODE (elt_type) == ARRAY_TYPE) >> + return; >> + >> + if (!tree_int_cst_equal (TYPE_SIZE (elt_type), TYPE_SIZE (type1))) >> + { >> + auto_diagnostic_group d; >> + if (warning_at (loc, OPT_Wsizeof_array_div, >> + "expression does not compute the number of " >> + "elements in this array; element type is " >> + "%qT, not %qT", elt_type, type1)) >> + { >> + if (EXPR_HAS_LOCATION (op1)) >> + { >> + location_t op1_loc = EXPR_LOCATION (op1); >> + gcc_rich_location richloc (op1_loc); >> + richloc.add_fixit_insert_before (op1_loc, "("); >> + richloc.add_fixit_insert_after (op1_loc, ")"); >> + inform (&richloc, "add parentheses around %qE to " >> + "silence this warning", op1); >> + } >> + else >> + inform (loc, "add parentheses around the second % " >> + "to silence this warning"); >> + if (DECL_P (arr)) >> + inform (DECL_SOURCE_LOCATION (arr), "array %qD declared here", arr); >> + } >> + } >> +} >> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt >> index c1d8fd336e8..9ab44550140 100644 >> --- a/gcc/c-family/c.opt >> +++ b/gcc/c-family/c.opt >> @@ -799,6 +799,11 @@ Wsizeof-pointer-div >> C ObjC C++ ObjC++ Var(warn_sizeof_pointer_div) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall) >> Warn about suspicious divisions of two sizeof expressions that don't work correctly with pointers. >> >> +Wsizeof-array-div >> +C ObjC C++ ObjC++ Var(warn_sizeof_array_div) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall) >> +Warn about divisions of two sizeof operators when the first one is applied >> +to an array and the divisor does not equal the size of the array element. >> + >> Wsizeof-pointer-memaccess >> C ObjC C++ ObjC++ Var(warn_sizeof_pointer_memaccess) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall) >> Warn about suspicious length parameters to certain string functions if the argument uses sizeof. >> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c >> index a8bc301ffad..148832d388b 100644 >> --- a/gcc/c/c-parser.c >> +++ b/gcc/c/c-parser.c >> @@ -7870,7 +7870,7 @@ c_parser_binary_expression (c_parser *parser, struct c_expr *after, >> enum tree_code op; >> /* The source location of this operation. */ >> location_t loc; >> - /* The sizeof argument if expr.original_code == SIZEOF_EXPR. */ >> + /* The sizeof argument if expr.original_code == {PAREN_,}SIZEOF_EXPR. */ >> tree sizeof_arg; >> } stack[NUM_PRECS]; >> int sp; >> @@ -7888,9 +7888,11 @@ c_parser_binary_expression (c_parser *parser, struct c_expr *after, >> c_inhibit_evaluation_warnings -= (stack[sp - 1].expr.value \ >> == truthvalue_true_node); \ >> break; \ >> - case TRUNC_DIV_EXPR: \ >> - if (stack[sp - 1].expr.original_code == SIZEOF_EXPR \ >> - && stack[sp].expr.original_code == SIZEOF_EXPR) \ >> + case TRUNC_DIV_EXPR: \ >> + if ((stack[sp - 1].expr.original_code == SIZEOF_EXPR \ >> + || stack[sp - 1].expr.original_code == PAREN_SIZEOF_EXPR) \ >> + && (stack[sp].expr.original_code == SIZEOF_EXPR \ >> + || stack[sp].expr.original_code == PAREN_SIZEOF_EXPR)) \ >> { \ >> tree type0 = stack[sp - 1].sizeof_arg; \ >> tree type1 = stack[sp].sizeof_arg; \ >> @@ -7904,18 +7906,23 @@ c_parser_binary_expression (c_parser *parser, struct c_expr *after, >> && !(TREE_CODE (first_arg) == PARM_DECL \ >> && C_ARRAY_PARAMETER (first_arg) \ >> && warn_sizeof_array_argument)) \ >> - { \ >> - auto_diagnostic_group d; \ >> - if (warning_at (stack[sp].loc, OPT_Wsizeof_pointer_div, \ >> - "division % " \ >> - "does not compute the number of array " \ >> - "elements", \ >> - type0, type1)) \ >> - if (DECL_P (first_arg)) \ >> - inform (DECL_SOURCE_LOCATION (first_arg), \ >> - "first % operand was declared here"); \ >> - } \ >> - } \ >> + { \ >> + auto_diagnostic_group d; \ >> + if (warning_at (stack[sp].loc, OPT_Wsizeof_pointer_div, \ >> + "division % " \ >> + "does not compute the number of array " \ >> + "elements", \ >> + type0, type1)) \ >> + if (DECL_P (first_arg)) \ >> + inform (DECL_SOURCE_LOCATION (first_arg), \ >> + "first % operand was declared here"); \ >> + } \ >> + else if (TREE_CODE (type0) == ARRAY_TYPE \ >> + && !char_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (type0))) \ >> + && stack[sp].expr.original_code != PAREN_SIZEOF_EXPR) \ >> + maybe_warn_sizeof_array_div (stack[sp].loc, first_arg, type0, \ >> + stack[sp].sizeof_arg, type1); \ >> + } \ >> break; \ >> default: \ >> break; \ >> @@ -9171,6 +9178,9 @@ c_parser_postfix_expression (c_parser *parser) >> if (expr.original_code != C_MAYBE_CONST_EXPR >> && expr.original_code != SIZEOF_EXPR) >> expr.original_code = ERROR_MARK; >> + /* Remember that we saw ( ) around the sizeof. */ >> + if (expr.original_code == SIZEOF_EXPR) >> + expr.original_code = PAREN_SIZEOF_EXPR; >> /* Don't change EXPR.ORIGINAL_TYPE. */ >> location_t loc_close_paren = c_parser_peek_token (parser)->location; >> set_c_expr_source_range (&expr, loc_open_paren, loc_close_paren); >> @@ -10786,7 +10796,8 @@ c_parser_expr_list (c_parser *parser, bool convert_p, bool fold_p, >> if (locations) >> locations->safe_push (expr.get_location ()); >> if (sizeof_arg != NULL >> - && expr.original_code == SIZEOF_EXPR) >> + && (expr.original_code == SIZEOF_EXPR >> + || expr.original_code == PAREN_SIZEOF_EXPR)) >> { >> sizeof_arg[0] = c_last_sizeof_arg; >> sizeof_arg_loc[0] = c_last_sizeof_loc; >> @@ -10809,7 +10820,8 @@ c_parser_expr_list (c_parser *parser, bool convert_p, bool fold_p, >> locations->safe_push (expr.get_location ()); >> if (++idx < 3 >> && sizeof_arg != NULL >> - && expr.original_code == SIZEOF_EXPR) >> + && (expr.original_code == SIZEOF_EXPR >> + || expr.original_code == PAREN_SIZEOF_EXPR)) >> { >> sizeof_arg[idx] = c_last_sizeof_arg; >> sizeof_arg_loc[idx] = c_last_sizeof_loc; >> diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h >> index 10938cf0857..5b3751efb3a 100644 >> --- a/gcc/c/c-tree.h >> +++ b/gcc/c/c-tree.h >> @@ -664,6 +664,7 @@ extern location_t c_last_sizeof_loc; >> >> extern struct c_switch *c_switch_stack; >> >> +extern bool char_type_p (tree); >> extern tree c_objc_common_truthvalue_conversion (location_t, tree); >> extern tree require_complete_type (location_t, tree); >> extern bool same_translation_unit_p (const_tree, const_tree); >> diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c >> index bb27099bfe1..8caf3dbf6db 100644 >> --- a/gcc/c/c-typeck.c >> +++ b/gcc/c/c-typeck.c >> @@ -3719,7 +3719,7 @@ parser_build_unary_op (location_t loc, enum tree_code code, struct c_expr arg) >> >> /* Returns true if TYPE is a character type, *not* including wchar_t. */ >> >> -static bool >> +bool >> char_type_p (tree type) >> { >> return (type == char_type_node >> diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c >> index 9166156a5d5..7705f15c25f 100644 >> --- a/gcc/cp/typeck.c >> +++ b/gcc/cp/typeck.c >> @@ -4706,14 +4706,13 @@ cp_build_binary_op (const op_location_t &location, >> { >> tree type0 = TREE_OPERAND (op0, 0); >> tree type1 = TREE_OPERAND (op1, 0); >> - tree first_arg = type0; >> + tree first_arg = tree_strip_any_location_wrapper (type0); >> if (!TYPE_P (type0)) >> type0 = TREE_TYPE (type0); >> if (!TYPE_P (type1)) >> type1 = TREE_TYPE (type1); >> if (INDIRECT_TYPE_P (type0) && same_type_p (TREE_TYPE (type0), type1)) >> { >> - STRIP_ANY_LOCATION_WRAPPER (first_arg); >> if (!(TREE_CODE (first_arg) == PARM_DECL >> && DECL_ARRAY_PARAMETER_P (first_arg) >> && warn_sizeof_array_argument) >> @@ -4729,6 +4728,13 @@ cp_build_binary_op (const op_location_t &location, >> "first % operand was declared here"); >> } >> } >> + else if (TREE_CODE (type0) == ARRAY_TYPE >> + && !char_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (type0))) >> + /* Set by finish_parenthesized_expr. */ >> + && !TREE_NO_WARNING (op1) >> + && (complain & tf_warning)) >> + maybe_warn_sizeof_array_div (location, first_arg, type0, >> + op1, non_reference (type1)); >> } >> >> if ((code0 == INTEGER_TYPE || code0 == REAL_TYPE >> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi >> index 6d9ff2c3362..97fef872a54 100644 >> --- a/gcc/doc/invoke.texi >> +++ b/gcc/doc/invoke.texi >> @@ -362,6 +362,7 @@ Objective-C and Objective-C++ Dialects}. >> -Wno-shift-overflow -Wshift-overflow=@var{n} @gol >> -Wsign-compare -Wsign-conversion @gol >> -Wno-sizeof-array-argument @gol >> +-Wsizeof-array-div @gol >> -Wsizeof-pointer-div -Wsizeof-pointer-memaccess @gol >> -Wstack-protector -Wstack-usage=@var{byte-size} -Wstrict-aliasing @gol >> -Wstrict-aliasing=n -Wstrict-overflow -Wstrict-overflow=@var{n} @gol >> @@ -5255,6 +5256,7 @@ Options} and @ref{Objective-C and Objective-C++ Dialect Options}. >> -Wreturn-type @gol >> -Wsequence-point @gol >> -Wsign-compare @r{(only in C++)} @gol >> +-Wsizeof-array-div @gol >> -Wsizeof-pointer-div @gol >> -Wsizeof-pointer-memaccess @gol >> -Wstrict-aliasing @gol >> @@ -7947,6 +7949,23 @@ real to lower precision real values. This option is also enabled by >> @opindex Wscalar-storage-order >> Do not warn on suspicious constructs involving reverse scalar storage order. >> >> +@item -Wsizeof-array-div >> +@opindex Wsizeof-array-div >> +@opindex Wno-sizeof-array-div >> +Warn about divisions of two sizeof operators when the first one is applied >> +to an array and the divisor does not equal the size of the array element. >> +In such a case, the computation will not yield the number of elements in the >> +array, which is likely what the user intended. This warning warns e.g. about >> +@smallexample >> +int fn () >> +@{ >> + int arr[10]; >> + return sizeof (arr) / sizeof (short); >> +@} >> +@end smallexample >> + >> +This warning is enabled by @option{-Wall}. >> + >> @item -Wsizeof-pointer-div >> @opindex Wsizeof-pointer-div >> @opindex Wno-sizeof-pointer-div >> diff --git a/gcc/testsuite/c-c++-common/Wsizeof-array-div1.c b/gcc/testsuite/c-c++-common/Wsizeof-array-div1.c >> new file mode 100644 >> index 00000000000..84d9a730cba >> --- /dev/null >> +++ b/gcc/testsuite/c-c++-common/Wsizeof-array-div1.c >> @@ -0,0 +1,56 @@ >> +/* PR c++/91741 */ >> +/* { dg-do compile } */ >> +/* { dg-options "-Wall" } */ >> + >> +typedef int T; >> + >> +int >> +fn (int ap[]) >> +{ >> + int arr[10]; >> + int *arr2[10]; >> + int *p = &arr[0]; >> + int r = 0; >> + >> + r += sizeof (arr) / sizeof (*arr); >> + r += sizeof (arr) / sizeof (p); /* { dg-warning "expression does not compute" } */ >> + r += sizeof (arr) / sizeof p; /* { dg-warning "expression does not compute" } */ >> + r += sizeof (arr) / (sizeof p); >> + r += sizeof (arr) / (sizeof (p)); >> + r += sizeof (arr2) / sizeof p; >> + r += sizeof (arr2) / sizeof (int); /* { dg-warning "expression does not compute" } */ >> + r += sizeof (arr2) / sizeof (int *); >> + r += sizeof (arr2) / sizeof (short *); >> + r += sizeof (arr) / sizeof (int); >> + r += sizeof (arr) / sizeof (unsigned int); >> + r += sizeof (arr) / sizeof (T); >> + r += sizeof (arr) / sizeof (short); /* { dg-warning "expression does not compute" } */ >> + r += sizeof (arr) / (sizeof (short)); >> + >> + r += sizeof (ap) / sizeof (char); /* { dg-warning ".sizeof. on array function parameter" } */ >> + >> + const char arr3[] = "foo"; >> + r += sizeof (arr3) / sizeof(char); >> + r += sizeof (arr3) / sizeof(int); >> + r += sizeof (arr3) / sizeof (*arr3); >> + >> + int arr4[5][5]; >> + r += sizeof (arr4) / sizeof (arr4[0]); >> + r += sizeof (arr4) / sizeof (*arr4); >> + r += sizeof (arr4) / sizeof (**arr4); >> + r += sizeof (arr4) / sizeof (int *); >> + r += sizeof (arr4) / sizeof (int); >> + r += sizeof (arr4) / sizeof (short int); >> + >> + T arr5[10]; >> + r += sizeof (arr5) / sizeof (T); >> + r += sizeof (arr5) / sizeof (int); >> + r += sizeof (arr5) / sizeof (short); /* { dg-warning "expression does not compute" } */ >> + >> + double arr6[10]; >> + r += sizeof (arr6) / sizeof (double); >> + r += sizeof (arr6) / sizeof (float); /* { dg-warning "expression does not compute" } */ >> + r += sizeof (arr6) / sizeof (*arr6); >> + >> + return r; >> +} >> diff --git a/gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c b/gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c >> index 83116183902..e9bad1fa420 100644 >> --- a/gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c >> +++ b/gcc/testsuite/c-c++-common/Wsizeof-pointer-div.c >> @@ -29,7 +29,7 @@ f2 (void) >> i += sizeof(array) / sizeof(array[0]); >> i += (sizeof(array)) / (sizeof(array[0])); >> i += sizeof(array) / sizeof(int); >> - i += sizeof(array) / sizeof(char); >> + i += sizeof(array) / sizeof(char); /* { dg-warning "expression does not compute" } */ >> i += sizeof(*array) / sizeof(char); >> i += sizeof(array[0]) / sizeof(char); >> return i; >> diff --git a/gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C b/gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C >> new file mode 100644 >> index 00000000000..da220cd57ba >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/warn/Wsizeof-array-div1.C >> @@ -0,0 +1,37 @@ >> +// PR c++/91741 >> +// { dg-do compile { target c++11 } } >> +// { dg-options "-Wall" } >> + >> +int >> +fn1 () >> +{ >> + int arr[10]; >> + return sizeof (arr) / sizeof (decltype(arr[0])); >> +} >> + >> +template >> +int fn2 (T (&arr)[N]) >> +{ >> + return sizeof (arr) / sizeof (T); >> +} >> + >> +template >> +int fn3 (T (&arr)[N]) >> +{ >> + return sizeof (arr) / sizeof (bool); // { dg-warning "expression does not compute" } >> +} >> + >> +template >> +int fn4 (T (&arr)[N]) >> +{ >> + return sizeof (arr) / sizeof (U); // { dg-warning "expression does not compute" } >> +} >> + >> +void >> +fn () >> +{ >> + int arr[10]; >> + fn2 (arr); >> + fn3 (arr); >> + fn4 (arr); >> +} >> diff --git a/gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C b/gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C >> new file mode 100644 >> index 00000000000..7962c23522c >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/warn/Wsizeof-array-div2.C >> @@ -0,0 +1,15 @@ >> +// PR c++/91741 >> +// { dg-do compile } >> +// { dg-options "-Wall" } >> +// From . >> + >> +const int kBaudrates[] = { 50, 75, 110 }; >> + >> +void >> +foo () >> +{ >> + for(int i = sizeof(kBaudrates) / sizeof(char*); // { dg-warning "expression does not compute" } >> + --i >= 0;) >> + { >> + } >> +} >> >> base-commit: d1a31689a736cdfb5e7cfa01f1168e338510e63b >> -- >> 2.26.2 >> > > Marek >