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.133.124]) by sourceware.org (Postfix) with ESMTPS id 126673858D28 for ; Mon, 22 Nov 2021 18:35:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 126673858D28 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-294-z_vw6ej2MMS7cgSMjWXGgw-1; Mon, 22 Nov 2021 13:35:39 -0500 X-MC-Unique: z_vw6ej2MMS7cgSMjWXGgw-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id AEC5D19253C0 for ; Mon, 22 Nov 2021 18:35:38 +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 6D05579448; Mon, 22 Nov 2021 18:35:32 +0000 (UTC) Message-ID: Date: Mon, 22 Nov 2021 15:35:29 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.0 Subject: Re: [PATCH v2] PR gdb/28480: Improve ambiguous member detection To: Andrew Burgess Cc: gdb-patches@sourceware.org References: <20211108182722.29510-1-blarsen@redhat.com> <20211122180011.GG2514@redhat.com> From: Bruno Larsen In-Reply-To: <20211122180011.GG2514@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 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.4 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_LOW, RCVD_IN_MSPIKE_H2, 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 18:35:42 -0000 Hi Andrew, Thanks for the review. All of the easy addressable comments will be changed. On 11/22/21 15:00, Andrew Burgess wrote: > * Bruno Larsen via Gdb-patches [2021-11-08 15:27:22 -0300]: > >> 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. */ > > This comment should be moved inside the "{ ... }" block. > > I found this comment difficult to understand. When you say "...when > the types are different", I guess this is referring to the path check > below maybe? In which case I wonder if we can find a different way to > phrase this, rather than "different types" ... "paths to the two > fields are different" maybe? > > Additional the whole final sentence just leaves me confused, it seems > to hint that there is a situation not covered by this code "This is > not necessarily complete...", but also that there is no such situation > "... is currently impossible". > > I wonder if you are saying that should we ever have two fields of the > same name, in the same class, that occur at the same address, then > this code wouldn't cover that case? But that seems a pretty weird > thing to worry about, so I assume I'm not understand you correctly. > > Could you rephrase the last part please? How does the following sound: Some fields may occupy the same space and still be ambiguous. This happens when [[no_unique_address]] is used in the inferior's code. The current solution assumes that the compiler will only place 2 struct members in the same location if they are of different types. As of 2021, this is mandatory, but this may change in the future Or I can remove the last sentence, if that is still confusing or unnecessary -- Cheers! Bruno Larsen