From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io1-xd32.google.com (mail-io1-xd32.google.com [IPv6:2607:f8b0:4864:20::d32]) by sourceware.org (Postfix) with ESMTPS id 7A2333858D20 for ; Tue, 1 Mar 2022 19:07:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7A2333858D20 Received: by mail-io1-xd32.google.com with SMTP id h16so19553202iol.11 for ; Tue, 01 Mar 2022 11:07:51 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=NiM/5Ij+ppwrR8TURVualEcJo95Qq9LyccFmW3IiaOU=; b=XGLUgb6qd7zb2h3O1dhrx8lo4AQYzJHsdFm1a4psKzpHJfx+t98kTcaIUKtEIOrNEW C7ijSkta6PKGMK1H0ouAIsgHFGtEBtZgY1p1fYDTl0s5Np3KLU28CxB/63rRccC0ETNL h8Et2go6djVTKxRGVHt4s1ZpywZ8Bmt1gg5x7iyBISYjSgNB9r1I25aPra7aTzOYgeIx 7Ln/pBSt9NWuhBZ77DZk7mUI+3igt0+jD8kVceGhFfKkle9WHRRaiFP6XtEynn0wyASQ MvZBoJlnCxPl5L0cxQdMYYdgyZxuR4/8iQxzkJU3cXclk1Ns2zAy42pkzvURaHe7ZxWX rJPA== X-Gm-Message-State: AOAM530a7/giiqu0m8iFmc5LKPr06sLRiHbzs99WsiUdg+fE0OmVfUbO L+5clzv6ZwhYT5NfgXKBMAc= X-Google-Smtp-Source: ABdhPJx36qtDoZnxU1qPbBHuYFrM3rrI4oYod6kWcKlZbpwqJo7bxrYINk2pkK7cc8RyDCekCrgaSw== X-Received: by 2002:a05:6638:6a7:b0:317:494c:35a9 with SMTP id d7-20020a05663806a700b00317494c35a9mr5262920jad.140.1646161670662; Tue, 01 Mar 2022 11:07:50 -0800 (PST) Received: from [192.168.0.41] (97-118-101-70.hlrn.qwest.net. [97.118.101.70]) by smtp.gmail.com with ESMTPSA id q7-20020a5d87c7000000b0064132d5bd73sm7720079ios.4.2022.03.01.11.07.50 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 01 Mar 2022 11:07:50 -0800 (PST) Message-ID: <2f312e75-772b-e8b6-ecbe-01daff873526@gmail.com> Date: Tue, 1 Mar 2022 12:07:49 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.4.0 Subject: Re: [PATCH] warn-access: Fix up check_pointer_uses [PR104715] Content-Language: en-US To: Jakub Jelinek , Richard Biener , Jeff Law Cc: gcc-patches@gcc.gnu.org References: From: Martin Sebor In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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, 01 Mar 2022 19:07:53 -0000 On 3/1/22 11:41, Jakub Jelinek wrote: > Hi! > > The following testcase emits bogus -Wdangling-pointer warnings. > The bug is that when it sees that ptr immediate use is a call that > returns one of its arguments, it will assume that the return value > is based on ptr, but that is the case only if ptr is passed to the > argument that is actually returned (so e.g. for memcpy the first argument, > etc.). When the builtins guarantee e.g. that the result is based on the > first argument (either ERF_RETURNS_ARG 0 in which case it will always > just returns the first argument as is, or when it is something like > strstr or strpbrk or mempcpy that it returns some pointer based on the > first argument), it means the result is not based on second or following > argument if any. The second hunk fixes this. > > The first hunk just removes an unnecessary TREE_CODE check, the code only > pushes SSA_NAMEs into the pointers vector and if it didn't, it uses > FOR_EACH_IMM_USE_FAST (use_p, iter, ptr) > a few lines below this, which of course requires that ptr is a SSA_NAME. > Tree checking on SSA_NAME_VERSION will already ensure that if it wasn't > a SSA_NAME, we'd ICE. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Thanks for the fix. It makes sense to me. Besides the test for the false positives I would suggest to add one to verify that using the first argument to a strstr() call is diagnosed if it's dangling (both as is, as well as with an offset from the first element). There are tests for memchr and strchr in the -Wdangling-pointer test suite but none for strstr. Martin > > 2022-03-01 Jakub Jelinek > > PR tree-optimization/104715 > * gimple-ssa-warn-access.cc (pass_waccess::check_pointer_uses): Don't > unnecessarily test if ptr is a SSA_NAME, it has to be. Only push lhs > of a call if gimple_call_return_arg is equal to ptr, not just when it > is non-NULL. > > * c-c++-common/Wdangling-pointer-7.c: New test. > > --- gcc/gimple-ssa-warn-access.cc.jj 2022-02-28 16:22:40.860520930 +0100 > +++ gcc/gimple-ssa-warn-access.cc 2022-02-28 16:55:01.242272499 +0100 > @@ -4169,8 +4169,7 @@ pass_waccess::check_pointer_uses (gimple > for (unsigned i = 0; i != pointers.length (); ++i) > { > tree ptr = pointers[i]; > - if (TREE_CODE (ptr) == SSA_NAME > - && !bitmap_set_bit (visited, SSA_NAME_VERSION (ptr))) > + if (!bitmap_set_bit (visited, SSA_NAME_VERSION (ptr))) > /* Avoid revisiting the same pointer. */ > continue; > > @@ -4267,7 +4266,7 @@ pass_waccess::check_pointer_uses (gimple > > if (gcall *call = dyn_cast (use_stmt)) > { > - if (gimple_call_return_arg (call)) > + if (gimple_call_return_arg (call) == ptr) > if (tree lhs = gimple_call_lhs (call)) > if (TREE_CODE (lhs) == SSA_NAME) > pointers.safe_push (lhs); > --- gcc/testsuite/c-c++-common/Wdangling-pointer-7.c.jj 2022-02-28 17:09:09.906355082 +0100 > +++ gcc/testsuite/c-c++-common/Wdangling-pointer-7.c 2022-02-28 17:03:50.533839892 +0100 > @@ -0,0 +1,36 @@ > +/* PR tree-optimization/104715 */ > +/* { dg-do compile } */ > +/* { dg-options "-Wdangling-pointer" } */ > + > +char * > +foo (char *p) > +{ > + { > + char q[61] = "012345678901234567890123456789012345678901234567890123456789"; > + char *r = q; > + p = __builtin_strcat (p, r); > + } > + return p; /* { dg-bogus "using dangling pointer" } */ > +} > + > +char * > +bar (char *p) > +{ > + { > + char q[] = "0123456789"; > + char *r = q; > + p = __builtin_strstr (p, r); > + } > + return p; /* { dg-bogus "using dangling pointer" } */ > +} > + > +char * > +baz (char *p) > +{ > + { > + char q[] = "0123456789"; > + char *r = q; > + p = __builtin_strpbrk (p, r); > + } > + return p; /* { dg-bogus "using dangling pointer" } */ > +} > > Jakub >