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 [216.205.24.124]) by sourceware.org (Postfix) with ESMTPS id 72EED3858D35 for ; Thu, 4 Nov 2021 15:13:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 72EED3858D35 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-421-htXOmEEkOMy4Fzwpaf7owQ-1; Thu, 04 Nov 2021 11:13:44 -0400 X-MC-Unique: htXOmEEkOMy4Fzwpaf7owQ-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id DB26310A8E02 for ; Thu, 4 Nov 2021 15:13:43 +0000 (UTC) Received: from blarsen.com (ovpn-116-55.gru2.redhat.com [10.97.116.55]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 7DB8E101E814; Thu, 4 Nov 2021 15:13:39 +0000 (UTC) From: Bruno Larsen To: gdb-patches@sourceware.org Subject: [RFC PATCH] [gdb] Fix std::unique_ptr printing regression (PR 28480) Date: Thu, 4 Nov 2021 12:13:17 -0300 Message-Id: <20211104151317.19615-1-blarsen@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII" X-Spam-Status: No, score=-13.3 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_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: Thu, 04 Nov 2021 15:13:47 -0000 The commit 87a37e5e078f506fa9905b74e9238593c537fcd5, introduced to fix another regression, failed to correctly identify the desired field when printing a unique_ptr. Before this commit, printing a unique_ptr would result in the following output: $1 = std::unique_ptr = {get() = {}} while the expected output would be in the form of: $1 = std::unique_ptr = {get() = 0x0} Or with a valid memory address. This happened because the searching algorithm, when looking for internal members of __uniq_ptr, would stop at the first _M_head_impl, which is of type class Deleter, instead of returning the second finding, which is the managed pointer. This commit fixes that by adding all members with the searched name and letting search_struct_field decide which is the best candidate, instead of adding only one at struct_field_searcher::update_result. An unfortunate side effect of this change is that the output for part of the test case has changed to returning the last baseclass that virtually inherits the ambiguous members, instead of retuning the first, so the test case had to be changed a little. This is an RFC for two reasons: The first is that I'm not sure changing the search function is the best way to fix a printing issue, but I couldn't find another way to fix this, so I'm asking for extra input; The second is that I'm having trouble with the testing. running with -data-directory gdb/data-directory gives me a very verbose printing that is always correct, but also counter-productive for debugging. How can I make the testsuite have the minimal printing? --- gdb/testsuite/gdb.cp/ambiguous.cc | 2 ++ gdb/testsuite/gdb.cp/ambiguous.exp | 10 ++++++++-- gdb/valops.c | 5 +++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/gdb/testsuite/gdb.cp/ambiguous.cc b/gdb/testsuite/gdb.cp/ambiguous.cc index a55686547f2..c32f13554f1 100644 --- a/gdb/testsuite/gdb.cp/ambiguous.cc +++ b/gdb/testsuite/gdb.cp/ambiguous.cc @@ -1,3 +1,4 @@ +#include class A1 { public: @@ -92,6 +93,7 @@ int main() JVA1 jva1; JVA2 jva2; JVA1V jva1v; + std::unique_ptr uptr(nullptr); int i; diff --git a/gdb/testsuite/gdb.cp/ambiguous.exp b/gdb/testsuite/gdb.cp/ambiguous.exp index 008898c5818..dff5f2605c8 100644 --- a/gdb/testsuite/gdb.cp/ambiguous.exp +++ b/gdb/testsuite/gdb.cp/ambiguous.exp @@ -224,7 +224,7 @@ gdb_test "print jv.x" " = 1" # ancestors, and as a non-virtual immediate base. Ambiguity must # be reported. test_ambiguous "jva1.x" "x" "JVA1" { - "'int A1::x' (JVA1 -> KV -> A1)" + "'int A1::x' (JVA1 -> LV -> A1)" "'int A1::x' (JVA1 -> A1)" } @@ -232,7 +232,7 @@ test_ambiguous "jva1.x" "x" "JVA1" { # ancestors, and A2 is a non-virtual immediate base. Ambiguity must # be reported as A1 and A2 both have a member 'x'. test_ambiguous "jva2.x" "x" "JVA2" { - "'int A1::x' (JVA2 -> KV -> A1)" + "'int A1::x' (JVA2 -> LV -> A1)" "'int A2::x' (JVA2 -> A2)" } @@ -264,3 +264,9 @@ 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}" + +#unique_ptr is a weird edge-case that interacts differently with the +#ambiguity detection, so we should test it directly +set print pretty +print uptr +gdb_test "print uptr" " = {get() = 0x0}" diff --git a/gdb/valops.c b/gdb/valops.c index 9787cdbb513..44ebaf84202 100644 --- a/gdb/valops.c +++ b/gdb/valops.c @@ -1962,6 +1962,11 @@ 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 + { + m_fields.pop_back (); + m_fields.push_back ({m_struct_path, v}); + } } } } -- 2.27.0