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 [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id E37EE3858D35 for ; Mon, 22 Nov 2021 13:48:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E37EE3858D35 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-67-7dzu8ZZJMGC5gbXzHZQHCw-1; Mon, 22 Nov 2021 08:48:06 -0500 X-MC-Unique: 7dzu8ZZJMGC5gbXzHZQHCw-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 693DF87D549 for ; Mon, 22 Nov 2021 13:48:05 +0000 (UTC) Received: from [10.97.116.84] (ovpn-116-84.gru2.redhat.com [10.97.116.84]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 2BC0E60843 for ; Mon, 22 Nov 2021 13:48:03 +0000 (UTC) Message-ID: <8462b175-ad01-0519-2f81-73666557b2ab@redhat.com> Date: Mon, 22 Nov 2021 10:47:59 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.0 Subject: [PING] [PATCH v2] PR gdb/28480: Improve ambiguous member detection To: "gdb-patches@sourceware.org" References: <20211108182722.29510-1-blarsen@redhat.com> From: Bruno Larsen In-Reply-To: <20211108182722.29510-1-blarsen@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-13.5 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP 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: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 22 Nov 2021 13:48:09 -0000 Ping On 11/8/21 15:27, Bruno Larsen wrote: > Basic ambiguity detection assumes that when 2 fields with the same name > have the same boffset, it must be an unambiguous request. This is not > always correct. Consider the following code: > > class empty { }; > > class A { > public: > [[no_unique_address]] empty e; > }; > > class B { > public: > int e; > }; > > class C: public A, public B { }; > > if we tried to use c.e in code, the compiler would warn of an ambiguity, > however, since A::e does not demand an unique address, it gets the same > address (and thus boffset) of the members, making A::e and B::e have the > same address. however, "print c.e" would fail to report the ambiguity, > and would instead print it as an empty class (first path found). > > The new code solves this by checking for other found_fields that have > different m_struct_path.back() (final class that the member was found > in), despite having the same boffset. > > The testcase gdb.cp/ambiguous.exp was also changed to test for this > behavior. > --- > gdb/testsuite/gdb.cp/ambiguous.cc | 19 +++++++++++++++++++ > gdb/testsuite/gdb.cp/ambiguous.exp | 10 ++++++++++ > gdb/valops.c | 27 +++++++++++++++++++++++++++ > 3 files changed, 56 insertions(+) > > diff --git a/gdb/testsuite/gdb.cp/ambiguous.cc b/gdb/testsuite/gdb.cp/ambiguous.cc > index a55686547f2..af2198dcfbc 100644 > --- a/gdb/testsuite/gdb.cp/ambiguous.cc > +++ b/gdb/testsuite/gdb.cp/ambiguous.cc > @@ -1,3 +1,4 @@ > +class empty { }; > > class A1 { > public: > @@ -17,6 +18,17 @@ public: > int y; > }; > > +#if !defined (__GNUC__) || __GNUC__ > 7 > +# define NO_UNIQUE_ADDRESS [[no_unique_address]] > +#else > +# define NO_UNIQUE_ADDRESS > +#endif > + > +class A4 { > +public: > + NO_UNIQUE_ADDRESS empty x; > +}; > + > class X : public A1, public A2 { > public: > int z; > @@ -77,6 +89,10 @@ public: > int jva1v; > }; > > +class JE : public A1, public A4 { > +public: > +}; > + > int main() > { > A1 a1; > @@ -92,6 +108,7 @@ int main() > JVA1 jva1; > JVA2 jva2; > JVA1V jva1v; > + JE je; > > int i; > > @@ -173,5 +190,7 @@ int main() > jva1v.i = 4; > jva1v.jva1v = 5; > > + je.A1::x = 1; > + > return 0; /* set breakpoint here */ > } > diff --git a/gdb/testsuite/gdb.cp/ambiguous.exp b/gdb/testsuite/gdb.cp/ambiguous.exp > index 008898c5818..a2a7b02b113 100644 > --- a/gdb/testsuite/gdb.cp/ambiguous.exp > +++ b/gdb/testsuite/gdb.cp/ambiguous.exp > @@ -264,3 +264,13 @@ gdb_test "print (A1)(KV)jva1" " = \{x = 3, y = 4\}" > # JVA1V is derived from A1; A1 is a virtual base indirectly > # and also directly; must not report ambiguity when a JVA1V is cast to an A1. > gdb_test "print (A1)jva1v" " = {x = 1, y = 2}" > + > +# C++20 introduced a way to have ambiguous fields with the same boffset. > +# This class explicitly tests for that. > +# if this is tested with a compiler that can't handle [[no_unique_address]] > +# the code should still correctly identify the ambiguity because of > +# different boffsets. > +test_ambiguous "je.x" "x" "JE" { > + "'int A1::x' (JE -> A1)" > + "'empty A4::x' (JE -> A4)" > +} > diff --git a/gdb/valops.c b/gdb/valops.c > index 9787cdbb513..2989a93df1a 100644 > --- a/gdb/valops.c > +++ b/gdb/valops.c > @@ -1962,6 +1962,33 @@ struct_field_searcher::update_result (struct value *v, LONGEST boffset) > space. */ > if (m_fields.empty () || m_last_boffset != boffset) > m_fields.push_back ({m_struct_path, v}); > + else > + /* Some fields may occupy the same space and still be ambiguous. > + This happens when [[no_unique_address]] is used by a member > + of the class. We assume that this only happens when the types are > + different. This is not necessarily complete, but a situation where > + this assumption is incorrect is currently (2021) impossible. */ > + { > + bool ambiguous = false, insert = true; > + for (const found_field& field: m_fields) { > + if(field.path.back () != m_struct_path.back ()) > + { > + /* Same boffset points to members of different classes. > + We have found an ambiguity and should record it. */ > + ambiguous = true; > + } > + else > + { > + /* We don't need to insert this value again, because a > + non-ambiguous path already leads to it. */ > + insert = false; > + break; > + } > + } > + if (ambiguous && insert) { > + m_fields.push_back ({m_struct_path, v}); > + } > + } > } > } > } > -- Cheers! Bruno Larsen