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 [63.128.21.124]) by sourceware.org (Postfix) with ESMTP id E922F38438A4 for ; Fri, 8 Jan 2021 23:18:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org E922F38438A4 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-450-i2901usZOWaLPfq4a5_aQg-1; Fri, 08 Jan 2021 18:18:18 -0500 X-MC-Unique: i2901usZOWaLPfq4a5_aQg-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 9A4C510054FF; Fri, 8 Jan 2021 23:18:17 +0000 (UTC) Received: from localhost.localdomain (ovpn-114-217.phx2.redhat.com [10.3.114.217]) by smtp.corp.redhat.com (Postfix) with ESMTP id 693E012D7E; Fri, 8 Jan 2021 23:18:17 +0000 (UTC) Subject: Re: [PATCH] issue -Wstring-compare for member arrays (PR 98097) To: Martin Sebor , gcc-patches References: From: Jeff Law Message-ID: Date: Fri, 8 Jan 2021 16:18:16 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Content-Language: en-US X-Spam-Status: No, score=-5.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, 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: Fri, 08 Jan 2021 23:18:22 -0000 On 1/7/21 5:53 PM, Martin Sebor via Gcc-patches wrote: > In PR 98097 Richard expects -Wstring-compare for a call to strcmp() > with a member array and a string literal of larger size, used in > an equality test. > > In virtually all cases the test will indicate the two are unequal > because the string stored in the member must be shorter (to fit > the terminating nul), but GCC doesn't fold the result because > there's wicked code out there that treats whole aggregates as if > they were strings, up their full size.  Because the warning is > based on the same conservative assumptions as the optimization, > it doesn't trigger, letting the almost certain bug go unnoticed. > > The attached patch allows -Wstring-compare to trigger for these > bugs by partly decoupling the warning from the underlying strcmp > optimization.  Making this possible requires adding a new member > to the c_strlen_data struct, which in turn called for changing > the meaning of the existing decl member to nonstr.  That led to > changes elsewhere, simply to adjust to the name change.  For > the purposes of review, the meat of the warning changes is in > tree-ssa-strlen.c.  All the rest of changes simply adjust code > to the new name. > > Tested on x86_64-linux (None of Binutils, GDB, Glibc, or Valgrind > triggers any instances of the warning with this change.) > > Martin > > gcc-98097.diff > > PR middle-end/98097 - missing -Wstring-compare with a member array > > gcc/ChangeLog: > > PR middle-end/98097 > * builtins.c (unterminated_array): Adjust to a name change. Adjust > indentation. > (c_strlen): Use a member instead of a local variable. > (expand_builtin_stpcpy_1): Adjust to a name change. > (fold_builtin_strlen): Same. > * builtins.h (struct c_strlen_data::nonstr): New data member to use > instead of decl. > (struct c_strlen_data::decl): Adjust comment. > * gimple-fold.c (get_range_strlen_tree): Set c_strlen_data::nonstr > in addition to c_strlen_data::decl. > (get_maxval_strlen): Adjust to a name change. > (gimple_fold_builtin_stpcpy): Same. > (gimple_fold_builtin_strlen): Same. > * gimple-ssa-sprintf.c (get_string_length): Same. > * tree-ssa-strlen.c (get_range_strlen_dynamic): Same. Also set > struct c_strlen_data::decl. > (get_len_or_size): Use c_strlen_data::decl. Succeed even for > nonconstant member arrays. > (strxcmp_eqz_result): Handle member arrays. > (handle_builtin_string_cmp): Issue warnings for member arrays. > > gcc/testsuite/ChangeLog: > > PR middle-end/98097 > * gcc.dg/Wstring-compare.c: > * gcc.dg/strcmpopt_10.c: > * gcc.dg/Wstring-compare-4.c: New test. > * gcc.dg/Wstring-compare-5.c: New test. I think you need to update the function comment for gen_len_or_size to describe the special case where we make the range invalidate and inverted. OK with that change. jeff