From patchwork Tue Oct 25 15:21:15 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 10866 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp1066056wru; Tue, 25 Oct 2022 08:22:16 -0700 (PDT) X-Google-Smtp-Source: AMsMyM5s1jfP/3sJmD0JiX03ilunZHeIPBDMtqLphgT+UT9730W3HEquHkT1EobUCY9pg8nLaT9/ X-Received: by 2002:a05:6402:1c88:b0:461:94a0:33df with SMTP id cy8-20020a0564021c8800b0046194a033dfmr14429724edb.0.1666711336518; Tue, 25 Oct 2022 08:22:16 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1666711336; cv=none; d=google.com; s=arc-20160816; b=Itp5I2+upU3NehKt3x31N5KEcmN0czbeEfIqs+UyrZ9UL5v5Uc0+7zVucQA9DZg7x/ vgjt2NGSZcsPsklS0KzqfpmAefIdhcpQQLfVglB58VGjcGxv+Xe+Cq8mCt0vlaUzbEQs +er8pmVP0oAyRRehgk6fV6CyAqQB08FWr6PnW9UCjYEx7OHUlD9XDOjxDygsYZ6TxuUP T1ZZxCDXMXbTrhGYLddtZ4W8hZekGfrNoICcHwZ1sKzWLTZdFkHqxU2MXAtxgLMdxRVm N0SidzDUs4N+qH5iiBIDrf+HbZ7APw+DklSNOsMwbrZorvD5P4IflEhCgiNtY9dJffmS drrA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:cc:reply-to:from:list-subscribe:list-help :list-post:list-archive:list-unsubscribe:list-id:precedence :content-disposition:user-agent:in-reply-to:mime-version:references :message-id:subject:to:date:dmarc-filter:delivered-to:dkim-signature :dkim-filter; bh=NCvqa/sYdcgHwiS/Yfu+Ys83cP0jmr+2Uew1F4B+1S8=; b=V/sf8o+RladJcEtRpOw2O4aa6JZ5cNbQOAOycdg+hf/9IO496Vxv95fB2OtrxKMUbw EfjFl1ahCUYv86zhEGDnML+7Q7DOBJYH1WT46eeVQfoIkzjPVB5BIJPiypZxC8n1WDJs H/yk1B6qIgvhqFirv9z6kJgtEsMkBMeL+qeE46/pYz0nh29Vrsa9UmsjJq1QVY3JWVEI a6bqqv5uxugjyZtwyme87+6QjXHDkDuVTbeZc6+GJGcpq+8ONWpnw5w32Dn9FIl5/kbu Aa9yyYX7eCXfCJ4Eh6xPXnSQeflesS3sn2lMzznhcOVe23TBIsQOKAxt9w5dhXdxUoTj mWIw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=S8Mr4kdl; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from sourceware.org (server2.sourceware.org. [2620:52:3:1:0:246e:9693:128c]) by mx.google.com with ESMTPS id l6-20020a056402254600b00459c73bd1c6si3550388edb.550.2022.10.25.08.22.16 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 25 Oct 2022 08:22:16 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) client-ip=2620:52:3:1:0:246e:9693:128c; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=S8Mr4kdl; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 7F3FA385AC3E for ; Tue, 25 Oct 2022 15:22:15 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7F3FA385AC3E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1666711335; bh=NCvqa/sYdcgHwiS/Yfu+Ys83cP0jmr+2Uew1F4B+1S8=; h=Date:To:Subject:References:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=S8Mr4kdl2CSyW3b+BqQRCxe72gMxNf72IYTm3aqfV+IXU3zQRQL7xoM7U5A7nDf8c OlRtk6TsPTwC83sIpd7w1hTrhuivv3AIZwBnEUxolbv1d9ADM+/rtDhf6oJzJgxGo0 0yOFpTnuuWO4Y30I33jl84W93TWqn0gl4YXioets= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 97293385AC32 for ; Tue, 25 Oct 2022 15:21:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 97293385AC32 Received: from mail-qv1-f69.google.com (mail-qv1-f69.google.com [209.85.219.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-31-rL3qERlpMv6pPawJFOSNag-1; Tue, 25 Oct 2022 11:21:21 -0400 X-MC-Unique: rL3qERlpMv6pPawJFOSNag-1 Received: by mail-qv1-f69.google.com with SMTP id c2-20020a05621401c200b004bb71b13dfcso2976372qvt.6 for ; Tue, 25 Oct 2022 08:21:20 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=user-agent:in-reply-to:content-disposition:mime-version:references :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=NCvqa/sYdcgHwiS/Yfu+Ys83cP0jmr+2Uew1F4B+1S8=; b=Du1VX5QBPmCkrHAxNHYIa6KV1b5VUtFTlkKAS3VXvGj2Eut6ZQbIc2oZGnrXjz9ZfF pfPkJ1S9dbObuhZq+BQGuOpxiZHuzWNNhBmn4Qjc+08Gs9n9XHRsJD1TVKYoFXizdH8x hnTWZCZ/+/pU09BoiK9eGi4jnc5En8mHXNeeW3BBhEaVV3QkSHhoHCKGkNSiV+dk62aw YvOvrcCFlp4yIzxkX7Zj6I9e5nBULbG8vORo1ZNUJmB5GL+EYHfxWFMMSm2dknRc6T8z 3OJPzjZ3x06u0X/dcsQosq7nWdtsmVSfiUWqmOaluHsDw8lF6hsvJSw6nb9jrRjLanAx Pkuw== X-Gm-Message-State: ACrzQf2xtWRNlhyPmkSGOmq1+BWw4KGmDMppDFK5UDyX/IbvdbmQl9zB 7+wn1Wwsfg6hf6rqatWz3ep4Cpr0AHD5RDSIrW4aNhvPOMB91ky3LdKMn7sK97bJq636aajlNzJ i33gJqLszt+ySQq7Rqw== X-Received: by 2002:a05:622a:1806:b0:39a:68be:5c with SMTP id t6-20020a05622a180600b0039a68be005cmr31986634qtc.327.1666711279414; Tue, 25 Oct 2022 08:21:19 -0700 (PDT) X-Received: by 2002:a05:622a:1806:b0:39a:68be:5c with SMTP id t6-20020a05622a180600b0039a68be005cmr31986583qtc.327.1666711278848; Tue, 25 Oct 2022 08:21:18 -0700 (PDT) Received: from redhat.com (2603-7000-9500-2e39-0000-0000-0000-1db4.res6.spectrum.com. [2603:7000:9500:2e39::1db4]) by smtp.gmail.com with ESMTPSA id o19-20020a05620a2a1300b006ee9d734479sm2317257qkp.33.2022.10.25.08.21.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 25 Oct 2022 08:21:17 -0700 (PDT) Date: Tue, 25 Oct 2022 11:21:15 -0400 To: Jason Merrill Subject: [PATCH v2] c++: Implement -Wdangling-reference [PR106393] Message-ID: References: <20221021232824.1093138-1-polacek@redhat.com> MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/2.2.7 (2022-08-07) X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-Spam-Status: No, score=-12.5 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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-Patchwork-Original-From: Marek Polacek via Gcc-patches From: Marek Polacek Reply-To: Marek Polacek Cc: Jonathan Wakely , GCC Patches Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1747341830420507974?= X-GMAIL-MSGID: =?utf-8?q?1747673506465073463?= On Mon, Oct 24, 2022 at 01:30:42PM -0400, Jason Merrill wrote: > On 10/21/22 19:28, Marek Polacek wrote: > > This patch implements a new experimental warning (enabled by -Wextra) to > > detect references bound to temporaries whose lifetime has ended. The > > Great! > > > primary motivation is the Note in > > : > > > > Capturing the result of std::max by reference produces a dangling reference > > if one of the parameters is a temporary and that parameter is returned: > > > > int n = 1; > > const int& r = std::max(n-1, n+1); // r is dangling > > > > That's because both temporaries for n-1 and n+1 are destroyed at the end > > of the full expression. With this warning enabled, you'll get: > > > > g.C:3:12: warning: possibly dangling reference to a temporary [-Wdangling-reference] > > 3 | const int& r = std::max(n-1, n+1); > > | ^ > > g.C:3:24: note: the temporary was destroyed at the end of the full expression 'std::max((n - 1), (n + 1))' > > 3 | const int& r = std::max(n-1, n+1); > > | ~~~~~~~~^~~~~~~~~~ > > > > The warning works by checking if a reference is initialized with a function > > that returns a reference, and at least one parameter of the function is > > a reference that is bound to a temporary. It assumes that such a function > > actually returns one of its arguments! (I added code to check_return_expr > > to suppress the warning when we've seen the definition of the function > > and we can say that it can return something other than its parameter.) > > Hmm, that misses returning a reference to a subobject or container element > that will also go away when the object is destroyed. Yes :-(. Fixed, tests added. I'm just checking TREE_STATIC now. > Does it also avoid a lot of false positives? Not at all, I just thought it may be worth it. > > It doesn't warn when the function in question is a member function, otherwise > > it'd emit loads of warnings for valid code like obj.emplace({0}, 0). > > We had discussed warning if the object argument is a temporary (and for the > above check, the function returns *this)? Presumably you mean detecting something like this: struct S { const S& self () { return *this; } }; const S& s = S().self(); I don't currently have a way to detect it, can I steal a METHOD_TYPE flag that says "this member function returns *this"? Alternatively, walk its DECL_SAVED_TREE and look for RETURN_EXPR? > > It warns in member initializer lists as well: > > > > const int& f(const int& i) { return i; } > > struct S { > > const int &r; // { dg-warning "dangling reference" } > > S() : r(f(10)) { } // { dg-message "destroyed" } > > }; > > > > I've run the testsuite/bootstrap with the warning enabled by default. > > There were just a few FAILs: > > * g++.dg/warn/Wdangling-pointer-2.C > > * 20_util/any/misc/any_cast.cc > > * 20_util/forward/c_neg.cc > > * 20_util/forward/f_neg.cc > > * experimental/any/misc/any_cast.cc > > all of these look like genuine bugs. A bootstrap with the warning > > enabled by default passed. > > > > When testing a previous version of the patch, there were many FAILs in > > libstdc++'s 22_locale/; all of them because the warning triggered on > > > > const test_type& obj = std::use_facet(std::locale()); > > > > but this code looks valid -- std::use_facet doesn't return a reference > > to its parameter. Therefore I added code to suppress the warning when > > the call is std::use_facet. Now 22_locale/* pass even with the warning > > on. We could exclude more std:: functions like this if desirable. > > Instead of adding special cases in the compiler, let's disable the warning > around the definition of use_facet (and adjust the compiler as needed so > that avoids the warning). As I said in I don't think it's possible without inventing an attribute (?). > I was remembering range adaptors being a stated motivation for Nico's P2012, > but looking back at the paper I now see that this problem was avoided for > them by disallowing rvalue arguments to range composition. Aha, and if you can't pass a temporary, you will not have this problem. > > gcc/testsuite/ChangeLog: > > > > * g++.dg/cpp23/elision4.C: Use -Wdangling-reference, add dg-warning. > > * g++.dg/cpp23/elision7.C: Likewise. > > * g++.dg/warn/Wdangling-reference1.C: New test. > > * g++.dg/warn/Wdangling-reference2.C: New test. > > Could use a test with a virtual base. Added (fns yum/lox in Wdangling-reference1.C). > > +static tree > > +find_initializing_call_expr (tree expr) > > +{ > > + STRIP_NOPS (expr); > > + switch (TREE_CODE (expr)) > > + { > > + case CALL_EXPR: > > + return expr; > > + case COMPOUND_EXPR: > > + return find_initializing_call_expr (TREE_OPERAND (expr, 1)); > > + case COND_EXPR: > > + if (tree t = find_initializing_call_expr (TREE_OPERAND (expr, 1))) > > + return t; > > + return find_initializing_call_expr (TREE_OPERAND (expr, 2)); > > For COND_EXPR I think we want to check both sides, in case there are calls > on both sides but only the second one has a problematic temporary. Wow, good catch. That was a bug. I've fixed it by moving the expr_represents_temporary_p checking into find_initializing_call_expr. > > + case PAREN_EXPR: > > + return find_initializing_call_expr (TREE_OPERAND (expr, 0)); > > + default: > > + return NULL_TREE; > > + } > > +} > > + > > +/* Implement -Wdangling-reference, to detect cases like > > + > > + int n = 1; > > + const int& r = std::max(n - 1, n + 1); // r is dangling > > + > > + This creates temporaries from the arguments, returns a reference to > > + one of the temporaries, but both temporaries are destroyed at the end > > + of the full expression. > > + > > + This works by checking if a reference is initialized with a function > > + that returns a reference, and at least one parameter of the function > > + is a reference that is bound to a temporary. It assumes that such a > > + function actually returns one of its arguments. > > + > > + This warning doesn't warn when the function in question is a member > > + function. > > + > > + DECL is the reference being initialized, CALL is the initializer. */ > > + > > +static void > > +do_warn_dangling_reference (const_tree decl, tree call) > > +{ > > + if (!warn_dangling_reference) > > + return; > > + if (!TYPE_REF_P (TREE_TYPE (decl))) > > + return; > > + call = find_initializing_call_expr (call); > > + if (call == NULL_TREE) > > + return; > > + > > + tree fndecl = cp_get_callee_fndecl_nofold (call); > > + if (!fndecl > > + || warning_suppressed_p (fndecl, OPT_Wdangling_reference) > > + /* Don't warn about member functions; the warning would trigger in > > + valid code like > > + std::any a(...); > > + S& s = a.emplace({0}, 0); > > + which constructs a new object and returns a reference to it. */ > > + || DECL_NONSTATIC_MEMBER_FUNCTION_P (fndecl) > > + /* It seems unreasonable to warn about operator functions. */ > > + || DECL_OVERLOADED_OPERATOR_P (fndecl) > > I guess I'd expect false positives on << and >> because of iostreams, do you > see false positives with other operators? This was just a guess. The warning triggered in g++.dg/overload/operator6.C. I suppose this could be limited to << and >>? I'm not sure. > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference1.C > > @@ -0,0 +1,103 @@ > > +// PR c++/106393 > > +// { dg-do compile { target c++11 } } > > +// { dg-options "-Wdangling-reference" } > > + > > +const int& f(const int& i) { return i; } > > +const int& h(int); > > +int g; > > +const int& globref(const int&) { return g; } > > +struct X { > > + int* i; > > + operator const int&() const { return *i; } > > +}; > > +X x{&g}; > > + > > +const int& r1 = f(10); // { dg-warning "dangling reference" } > > +// r2 = _ZGR2r2_ = (int) *f ((const int &) &TARGET_EXPR ) + 1; (const int &) &_ZGR2r2_ > > +const int& r2 = f(10) + 1; > > +// Don't warn here, we have > > +// r3 = f (X::operator const int& (&x)) > > +const int& r3 = f(x); > > +// Don't warn here, because we've seen the definition of globref > > +// and could figure out that it may not return one of its parms. > > +// Questionable -- it can also hide bugs --, but it helps here. > > We could suppress specifically for the case of returning a variable with > static storage duration? Done. > > +const int& r4 = globref(1); > > +const int& r5 = (42, f(10)); // { dg-warning "dangling reference" } > > +const int& r6 = (f(10), 42); > > +const int& r7 = (f(10)); // { dg-warning "dangling reference" } > > +const int& r8 = g ? f(10) : f(9); // { dg-warning "dangling reference" } > > +const int& r9 = (42, g ? f(10) : f(9)); // { dg-warning "dangling reference" } > > +const int& r10 = (g ? f(10) : f(9), 42); > > +// Binds to a reference temporary for r11. > > +const int& r11 = g ? f(10) : 9; > > Why no warning? I don't think there's a dangling reference here, we get: r11 = _ZGR3r11_ = g != 0 ? (int) *f ((const int &) &TARGET_EXPR ) : 9;, (const int &) &_ZGR3r11_; > > +// Invalid, but we don't warn here yet. > > +// r12 = f (f ((const int &) &TARGET_EXPR )) > > +const int& r12 = f(f(1)); > > This should be a simple recursion? Hmm, the inner call is just a sub-expression of the full-expression so there you can still use the returned temporary. But in this case the temporary is used beyond the full-expression so it's invalid. I've added if (tree r = find_initializing_call_expr (arg)) if (cp_tree_equal (CALL_EXPR_FN (r), CALL_EXPR_FN (expr))) return expr; so that we detect f(f(1)) but I'm dubious that this is actually useful. I've added const int& r15 = rp(&f(1)); to the test where I think we can't warn. Hence the cp_tree_equal. Here's a v2 which fixes bugs in v1, but still doesn't handle member functions in any way. FWIW, bootstrapped/regtested on x86_64-pc-linux-gnu. -- >8 -- This patch implements a new experimental warning (enabled by -Wextra) to detect references bound to temporaries whose lifetime has ended. The primary motivation is the Note in : Capturing the result of std::max by reference produces a dangling reference if one of the parameters is a temporary and that parameter is returned: int n = 1; const int& r = std::max(n-1, n+1); // r is dangling That's because both temporaries for n-1 and n+1 are destroyed at the end of the full expression. With this warning enabled, you'll get: g.C:3:12: warning: possibly dangling reference to a temporary [-Wdangling-reference] 3 | const int& r = std::max(n-1, n+1); | ^ g.C:3:24: note: the temporary was destroyed at the end of the full expression 'std::max((n - 1), (n + 1))' 3 | const int& r = std::max(n-1, n+1); | ~~~~~~~~^~~~~~~~~~ The warning works by checking if a reference is initialized with a function that returns a reference, and at least one parameter of the function is a reference that is bound to a temporary. It assumes that such a function actually returns one of its arguments! (I added code to check_return_expr to suppress the warning when we've seen the definition of the function and we can say that it can return a variable with static storage duration.) It doesn't warn when the function in question is a member function, otherwise it'd emit loads of warnings for valid code like obj.emplace({0}, 0). It warns in member initializer lists as well: const int& f(const int& i) { return i; } struct S { const int &r; // { dg-warning "dangling reference" } S() : r(f(10)) { } // { dg-message "destroyed" } }; I've run the testsuite/bootstrap with the warning enabled by default. There were just a few FAILs: * g++.dg/warn/Wdangling-pointer-2.C * 20_util/any/misc/any_cast.cc * 20_util/forward/c_neg.cc * 20_util/forward/f_neg.cc * experimental/any/misc/any_cast.cc all of these look like genuine bugs. A bootstrap with the warning enabled by default passed. When testing a previous version of the patch, there were many FAILs in libstdc++'s 22_locale/; all of them because the warning triggered on const test_type& obj = std::use_facet(std::locale()); but this code looks valid -- std::use_facet doesn't return a reference to its parameter. Therefore I added code to suppress the warning when the call is std::use_facet. Now 22_locale/* pass even with the warning on. We could exclude more std:: functions like this if desirable. PR c++/106393 gcc/c-family/ChangeLog: * c.opt (Wdangling-reference): New. gcc/cp/ChangeLog: * call.cc (expr_represents_temporary_p): New, factored out of conv_binds_ref_to_temporary. (conv_binds_ref_to_temporary): Don't return false just because a ck_base is missing. Use expr_represents_temporary_p. (find_initializing_call_expr): New. (do_warn_dangling_reference): New. (extend_ref_init_temps): Call do_warn_dangling_reference. * typeck.cc (check_return_expr): Suppress -Wdangling-reference warnings. gcc/ChangeLog: * doc/invoke.texi: Document -Wdangling-reference. gcc/testsuite/ChangeLog: * g++.dg/cpp23/elision4.C: Use -Wdangling-reference, add dg-warning. * g++.dg/cpp23/elision7.C: Likewise. * g++.dg/warn/Wdangling-reference1.C: New test. * g++.dg/warn/Wdangling-reference2.C: New test. --- gcc/c-family/c.opt | 4 + gcc/cp/call.cc | 144 ++++++++++++++++-- gcc/cp/cp-tree.h | 4 +- gcc/cp/typeck.cc | 10 ++ gcc/doc/invoke.texi | 34 ++++- gcc/testsuite/g++.dg/cpp23/elision4.C | 5 +- gcc/testsuite/g++.dg/cpp23/elision7.C | 3 +- .../g++.dg/warn/Wdangling-reference1.C | 128 ++++++++++++++++ .../g++.dg/warn/Wdangling-reference2.C | 28 ++++ 9 files changed, 343 insertions(+), 17 deletions(-) create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference1.C create mode 100644 gcc/testsuite/g++.dg/warn/Wdangling-reference2.C base-commit: 4c5b1160776382772fc0a33130dfaf621699fdbf diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 01d480759ae..02d79991aeb 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -555,6 +555,10 @@ Wdangling-pointer= C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_dangling_pointer) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall, 2, 0) IntegerRange(0, 2) Warn for uses of pointers to auto variables whose lifetime has ended. +Wdangling-reference +C++ ObjC++ Var(warn_dangling_reference) Warning LangEnabledBy(C++ ObjC++, Wextra) +Warn when a reference is bound to a temporary whose lifetime has ended. + Wdate-time C ObjC C++ ObjC++ CPP(warn_date_time) CppReason(CPP_W_DATE_TIME) Var(cpp_warn_date_time) Init(0) Warning Warn about __TIME__, __DATE__ and __TIMESTAMP__ usage. diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc index 6a34e9c2ae1..cf17ee04cf7 100644 --- a/gcc/cp/call.cc +++ b/gcc/cp/call.cc @@ -9313,6 +9313,16 @@ conv_binds_ref_to_prvalue (conversion *c) return conv_is_prvalue (next_conversion (c)); } +/* True iff EXPR represents a (subobject of a) temporary. */ + +static bool +expr_represents_temporary_p (tree expr) +{ + while (handled_component_p (expr)) + expr = TREE_OPERAND (expr, 0); + return TREE_CODE (expr) == TARGET_EXPR; +} + /* True iff C is a conversion that binds a reference to a temporary. This is a superset of conv_binds_ref_to_prvalue: here we're also interested in xvalues. */ @@ -9330,18 +9340,14 @@ conv_binds_ref_to_temporary (conversion *c) struct Derived : Base {}; const Base& b(Derived{}); where we bind 'b' to the Base subobject of a temporary object of type - Derived. The subobject is an xvalue; the whole object is a prvalue. */ - if (c->kind != ck_base) - return false; - c = next_conversion (c); - if (c->kind == ck_identity && c->u.expr) - { - tree expr = c->u.expr; - while (handled_component_p (expr)) - expr = TREE_OPERAND (expr, 0); - if (TREE_CODE (expr) == TARGET_EXPR) - return true; - } + Derived. The subobject is an xvalue; the whole object is a prvalue. + + The ck_base doesn't have to be present for cases like X{}.m. */ + if (c->kind == ck_base) + c = next_conversion (c); + if (c->kind == ck_identity && c->u.expr + && expr_represents_temporary_p (c->u.expr)) + return true; return false; } @@ -13428,6 +13434,117 @@ initialize_reference (tree type, tree expr, return expr; } +/* Helper for do_warn_dangling_reference to find a non-nested CALL_EXPR + that initializes the LHS (and at least one of its arguments represents + a temporary), or NULL_TREE if none found. For instance: + + const int& r = (42, f(1)); // f(1) + const int& t = b ? f(1) : f(2); // f(1) + const int& u = b ? f(1) : f(g); // f(1) + const int& v = b ? f(g) : f(2); // f(2) + const int& w = b ? f(g) : f(g); // NULL_TREE + const int& y = (f(1), 42); // NULL_TREE + const int& z = f(f(1)); // f(f(1)) + + EXPR is the initializer. */ + +static tree +find_initializing_call_expr (tree expr) +{ + STRIP_NOPS (expr); + switch (TREE_CODE (expr)) + { + case CALL_EXPR: + for (int i = 0; i < call_expr_nargs (expr); ++i) + { + /* We're looking to see if ARG is something like + (const int &) &TARGET_EXPR <...>. */ + tree arg = CALL_EXPR_ARG (expr, i); + /* It could also be the same call taking a temporary. */ + if (tree r = find_initializing_call_expr (arg)) + if (cp_tree_equal (CALL_EXPR_FN (r), CALL_EXPR_FN (expr))) + return expr; + STRIP_NOPS (arg); + if (TREE_CODE (arg) == ADDR_EXPR) + arg = TREE_OPERAND (arg, 0); + if (expr_represents_temporary_p (arg)) + return expr; + } + return NULL_TREE; + case COMPOUND_EXPR: + return find_initializing_call_expr (TREE_OPERAND (expr, 1)); + case COND_EXPR: + if (tree t = find_initializing_call_expr (TREE_OPERAND (expr, 1))) + return t; + return find_initializing_call_expr (TREE_OPERAND (expr, 2)); + case PAREN_EXPR: + return find_initializing_call_expr (TREE_OPERAND (expr, 0)); + default: + return NULL_TREE; + } +} + +/* Implement -Wdangling-reference, to detect cases like + + int n = 1; + const int& r = std::max(n - 1, n + 1); // r is dangling + + This creates temporaries from the arguments, returns a reference to + one of the temporaries, but both temporaries are destroyed at the end + of the full expression. + + This works by checking if a reference is initialized with a function + that returns a reference, and at least one parameter of the function + is a reference that is bound to a temporary. It assumes that such a + function actually returns one of its arguments. + + This warning doesn't warn when the function in question is a member + function. + + DECL is the reference being initialized, CALL is the initializer. */ + +static void +do_warn_dangling_reference (const_tree decl, tree call) +{ + if (!warn_dangling_reference) + return; + if (!TYPE_REF_P (TREE_TYPE (decl))) + return; + call = find_initializing_call_expr (call); + if (call == NULL_TREE) + return; + + tree fndecl = cp_get_callee_fndecl_nofold (call); + if (!fndecl + || warning_suppressed_p (fndecl, OPT_Wdangling_reference) + /* Don't warn about member functions; the warning would trigger in + valid code like + std::any a(...); + S& s = a.emplace({0}, 0); + which constructs a new object and returns a reference to it. */ + || DECL_NONSTATIC_MEMBER_FUNCTION_P (fndecl) + /* It seems unreasonable to warn about operator functions. */ + || DECL_OVERLOADED_OPERATOR_P (fndecl) + /* If the function doesn't return a reference, don't warn. This can + be e.g. + const int& z = std::min({1, 2, 3, 4, 5, 6, 7}); + which doesn't dangle: std::min here returns an int. */ + || !TYPE_REF_P (TREE_TYPE (TREE_TYPE (fndecl)))) + return; + + /* Mitigate some known cases. */ + if (decl_in_std_namespace_p (fndecl)) + if (tree name = DECL_NAME (fndecl)) + if (id_equal (name, "use_facet")) + return; + + auto_diagnostic_group d; + if (warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wdangling_reference, + "possibly dangling reference to a temporary")) + inform (EXPR_LOCATION (call), "the temporary was destroyed at " + "the end of the full expression %qE", call); +} + /* If *P is an xvalue expression, prevent temporary lifetime extension if it gets used to initialize a reference. */ @@ -13525,6 +13642,9 @@ extend_ref_init_temps (tree decl, tree init, vec **cleanups, tree type = TREE_TYPE (init); if (processing_template_decl) return init; + + do_warn_dangling_reference (decl, init); + if (TYPE_REF_P (type)) init = extend_ref_init_temps_1 (decl, init, cleanups, cond_guard); else diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 2cca20be6c1..d22c5e1b731 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -459,7 +459,6 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX]; TI_PENDING_TEMPLATE_FLAG. TEMPLATE_PARMS_FOR_INLINE. DELETE_EXPR_USE_VEC (in DELETE_EXPR). - (TREE_CALLS_NEW) (in _EXPR or _REF) (commented-out). ICS_ELLIPSIS_FLAG (in _CONV) DECL_INITIALIZED_P (in VAR_DECL) TYPENAME_IS_CLASS_P (in TYPENAME_TYPE) @@ -4567,6 +4566,9 @@ get_vec_init_expr (tree t) When appearing in a CONSTRUCTOR, the expression is an unconverted compound literal. + When appearing in a CALL_EXPR, it means that it is a call to + a constructor. + When appearing in a FIELD_DECL, it means that this field has been duly initialized in its constructor. */ #define TREE_HAS_CONSTRUCTOR(NODE) (TREE_LANG_FLAG_4 (NODE)) diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc index ab6979bcc50..54fac880d8c 100644 --- a/gcc/cp/typeck.cc +++ b/gcc/cp/typeck.cc @@ -11246,6 +11246,16 @@ check_return_expr (tree retval, bool *no_warning) if (processing_template_decl) return saved_retval; + /* A naive attempt to reduce the number of -Wdangling-reference false + positives: if we know that this function can return a variable with + static storage duration rather than one of its parameters, suppress + the warning. */ + if (warn_dangling_reference + && TYPE_REF_P (functype) + && bare_retval + && TREE_STATIC (bare_retval)) + suppress_warning (current_function_decl, OPT_Wdangling_reference); + /* Actually copy the value returned into the appropriate location. */ if (retval && retval != result) retval = cp_build_init_expr (result, retval); diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 64f77e8367a..e94fe20779a 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -249,7 +249,8 @@ in the following sections. -Wno-class-conversion -Wclass-memaccess @gol -Wcomma-subscript -Wconditionally-supported @gol -Wno-conversion-null -Wctad-maybe-unsupported @gol --Wctor-dtor-privacy -Wno-delete-incomplete @gol +-Wctor-dtor-privacy -Wdangling-reference @gol +-Wno-delete-incomplete @gol -Wdelete-non-virtual-dtor -Wno-deprecated-array-compare @gol -Wdeprecated-copy -Wdeprecated-copy-dtor @gol -Wno-deprecated-enum-enum-conversion -Wno-deprecated-enum-float-conversion @gol @@ -3627,6 +3628,36 @@ public static member functions. Also warn if there are no non-private methods, and there's at least one private member function that isn't a constructor or destructor. +@item -Wdangling-reference @r{(C++ and Objective-C++ only)} +@opindex Wdangling-reference +@opindex Wno-dangling-reference +Warn when a reference is bound to a temporary whose lifetime has ended. +For example: + +@smallexample +int n = 1; +const int& r = std::max(n - 1, n + 1); // r is dangling +@end smallexample + +In the example above, two temporaries are created, one for each +argument, and a reference to one of the temporaries is returned. +However, both temporaries are destroyed at the end of the full +expression, so the reference @code{r} is dangling. This warning +also detects dangling references in member initializer lists: + +@smallexample +const int& f(const int& i) @{ return i; @} +struct S @{ + const int &r; // r is dangling + S() : r(f(10)) @{ @} +@}; +@end smallexample + +Member functions are not checked. Certain standard functions, such +as @code{std::use_facet}, are also excluded from checking. + +This warning is enabled by @option{-Wextra}. + @item -Wdelete-non-virtual-dtor @r{(C++ and Objective-C++ only)} @opindex Wdelete-non-virtual-dtor @opindex Wno-delete-non-virtual-dtor @@ -5936,6 +5967,7 @@ name is still supported, but the newer name is more descriptive.) @gccoptlist{-Wclobbered @gol -Wcast-function-type @gol +-Wdangling-reference @r{(C++ only)} @gol -Wdeprecated-copy @r{(C++ only)} @gol -Wempty-body @gol -Wenum-conversion @r{(C only)} @gol diff --git a/gcc/testsuite/g++.dg/cpp23/elision4.C b/gcc/testsuite/g++.dg/cpp23/elision4.C index c19b86b8b5f..d39053ad741 100644 --- a/gcc/testsuite/g++.dg/cpp23/elision4.C +++ b/gcc/testsuite/g++.dg/cpp23/elision4.C @@ -1,5 +1,6 @@ // PR c++/101165 - P2266R1 - Simpler implicit move // { dg-do compile { target c++23 } } +// { dg-options "-Wdangling-reference" } // Test from P2266R1, $ 5.2. LibreOffice OString constructor. struct X { @@ -33,6 +34,6 @@ T& temporary2(T&& x) { return static_cast(x); } void test () { - int& r1 = temporary1 (42); - int& r2 = temporary2 (42); + int& r1 = temporary1 (42); // { dg-warning "dangling reference" } + int& r2 = temporary2 (42); // { dg-warning "dangling reference" } } diff --git a/gcc/testsuite/g++.dg/cpp23/elision7.C b/gcc/testsuite/g++.dg/cpp23/elision7.C index 19fa89ae133..0045842b34f 100644 --- a/gcc/testsuite/g++.dg/cpp23/elision7.C +++ b/gcc/testsuite/g++.dg/cpp23/elision7.C @@ -1,5 +1,6 @@ // PR c++/101165 - P2266R1 - Simpler implicit move // { dg-do compile { target c++23 } } +// { dg-options "-Wdangling-reference" } struct X { X (); @@ -68,5 +69,5 @@ f7 (T &&t) void do_f7 () { - const int &x = f7 (0); + const int &x = f7 (0); // { dg-warning "dangling reference" } } diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference1.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference1.C new file mode 100644 index 00000000000..00bee232d9f --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference1.C @@ -0,0 +1,128 @@ +// PR c++/106393 +// { dg-do compile { target c++11 } } +// { dg-options "-Wdangling-reference" } + +const int& f(const int& i) { return i; } +const int& h(int); +const int& rp(const int *); +int g; +const int& globref(const int&) { return g; } +struct X { + int* i; + operator const int&() const { return *i; } +}; +X x{&g}; + +const int& r1 = f(10); // { dg-warning "dangling reference" } +// r2 = _ZGR2r2_ = (int) *f ((const int &) &TARGET_EXPR ) + 1; (const int &) &_ZGR2r2_ +const int& r2 = f(10) + 1; +// Don't warn here, we have +// r3 = f (X::operator const int& (&x)) +const int& r3 = f(x); +// Don't warn here, because we've seen the definition of globref +// and could figure out that it may not return one of its parms. +// Questionable -- it can also hide bugs --, but it helps here. +const int& r4 = globref(1); +const int& r5 = (42, f(10)); // { dg-warning "dangling reference" } +const int& r6 = (f(10), 42); +const int& r7 = (f(10)); // { dg-warning "dangling reference" } +const int& r8 = g ? f(10) : f(9); // { dg-warning "dangling reference" } +const int& r9 = (42, g ? f(10) : f(9)); // { dg-warning "dangling reference" } +const int& r10 = (g ? f(10) : f(9), 42); +// Binds to a reference temporary for r11. +const int& r11 = g ? f(10) : 9; +const int& r11_ = g ? 9 : f(10); +// r12 = f (f ((const int &) &TARGET_EXPR )) +const int& r12 = f(f(1)); // { dg-warning "dangling reference" } +const int& r13 = f(g ? f(1) : f(2)); // { dg-warning "dangling reference" } +const int& r14 = f(*&f(1)); // { dg-warning "dangling reference" } +const int& r15 = rp(&f(1)); +const int& r16 = rp(&f(g)); +const int& r17 = h(f(1)); +// Other forms of initializers. +const int& r18(f(10)); // { dg-warning "dangling reference" } +const int& r19(f(10)); // { dg-warning "dangling reference" } +// Returns a ref, but doesn't have a parameter of reference type. +const int& r20 = h(10); +const int& r21 = g ? h(10) : f(10); // { dg-warning "dangling reference" } +const int& r22 = g ? f(10) : h(10); // { dg-warning "dangling reference" } +const int& r23 = g ? h(10) : (1, f(10)); // { dg-warning "dangling reference" } +const int& r24 = g ? (1, f(10)) : h(10); // { dg-warning "dangling reference" } + +// OK: the reference is bound to the 10 so still valid at the point +// where it's copied into i1. +int i1 = f(10); + +int +test1 () +{ + const int &lr = f(10); // { dg-warning "dangling reference" } + int i2 = f(10); + return lr; +} + +struct B { }; +struct D : B { }; +struct C { + D d; +}; + +C c; +D d; + +using U = D[3]; + +const B& frotz(const D&); +const B& b1 = frotz(C{}.d); // { dg-warning "dangling reference" } +const B& b2 = frotz(D{}); // { dg-warning "dangling reference" } +const B& b3 = frotz(c.d); +const B& b4 = frotz(d); +const B& b5 = frotz(U{}[0]); // { dg-warning "dangling reference" } + +// Try returning a subobject. +const B& bar (const D& d) { return d; } +const B& b6 = bar (D{}); // { dg-warning "dangling reference" } +const B& baz (const C& c) { return c.d; } +const B& b7 = baz (C{}); // { dg-warning "dangling reference" } +const D& qux (const C& c) { return c.d; } +const D& d1 = qux (C{}); // { dg-warning "dangling reference" } + +struct E { + E(int); +}; +const E& operator*(const E&); +const E& b8 = *E(1); + +struct F : virtual B { }; +struct G : virtual B { }; +struct H : F, G { }; +const B& yum (const F& f) { return f; } +const B& b9 = yum (F{}); // { dg-warning "dangling reference" } +const B& lox (const H& h) { return h; } +const B& b10 = lox (H{}); // { dg-warning "dangling reference" } + +struct S { + const int &r; // { dg-warning "dangling reference" } + S() : r(f(10)) { } // { dg-message "destroyed" } +}; + +// From cppreference. +template +const T& max(const T& a, const T& b) +{ + return (a < b) ? b : a; +} + +int n = 1; +const int& refmax = max(n - 1, n + 1); // { dg-warning "dangling reference" } + +// Don't warn about member functions. +struct Y { + operator int&&(); + const int& foo(const int&); +}; + +// x1 = Y::operator int&& (&TARGET_EXPR ) +int&& x1 = Y(); +int&& x2 = Y{}; +const int& t1 = Y().foo(10); diff --git a/gcc/testsuite/g++.dg/warn/Wdangling-reference2.C b/gcc/testsuite/g++.dg/warn/Wdangling-reference2.C new file mode 100644 index 00000000000..dafdb43f1b9 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wdangling-reference2.C @@ -0,0 +1,28 @@ +// PR c++/106393 +// { dg-do compile { target c++11 } } +// { dg-options "-Wdangling-reference" } + +namespace std { +struct any {}; +template _ValueType any_cast(any &&); +template struct remove_reference { using type = _Tp; }; +template _Tp forward(typename remove_reference<_Tp>::type); +template typename remove_reference<_Tp>::type move(_Tp); +} // namespace std + +const int &r = std::any_cast(std::any()); // { dg-warning "dangling reference" } + +template struct C { + T t_; // { dg-warning "dangling reference" } + C(T); + template C(U c) : t_(std::forward(c.t_)) {} +}; +struct A {}; +struct B { + B(A); +}; +int main() { + A a; + C ca(a); + C(std::move(ca)); +}