From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x42f.google.com (mail-wr1-x42f.google.com [IPv6:2a00:1450:4864:20::42f]) by sourceware.org (Postfix) with ESMTPS id 343AB3858D3C for ; Sun, 13 Mar 2022 05:28:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 343AB3858D3C Received: by mail-wr1-x42f.google.com with SMTP id q14so19011353wrc.4 for ; Sat, 12 Mar 2022 21:28:26 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=uKsyKnx2IzBFu5T5kU0UDfuketEK8JC+NbAsGIwOG3Y=; b=h18aNbUvhzMX8VtUuKQ8ip9bxjcWkz5MK5D70bDegd22NuHjwJ767uTMl/TuaGsy6p 6j5G/CAkkNcv6wyzzdwVk80+zkDJIHimtJ29LF4vbZmDzaIaxOjVe5xa2Qg5xWADYcw+ VhEvrN0CaRil+TFy/sLK0unUv/fKCK3XreFqCdyNz9E2hQ4CS5qjRJRXNmnOT/L9Rb++ gB0VP0noJCJqhKMxgHLQQx/ptzZ7rFUtlMGvpNLA5JDXp21DFDRq+05ZFYUYlpJk4H91 nThpix93anmohVmdwbkHtCsS9kJBbeWbRk0jAl1e32d6W9y07OKVM+F3Ivts6YUozrYg a/ig== X-Gm-Message-State: AOAM530UYg7SRF0Lf0+EpxqwtQzea4DnZRgkLz1lDWIWSni25UML3XuS 67F6WpDmoAxGqVPsHA6EG2HXndcTD6UJ X-Google-Smtp-Source: ABdhPJyiOlTEpC+gkkmg6Hw6FpyvbrMLfbtF2Sj7jHHBGVcTbiEFz97cQSBHrClXOdc5MFtKC5yPJw== X-Received: by 2002:a5d:47c5:0:b0:1ef:f2e8:11fc with SMTP id o5-20020a5d47c5000000b001eff2e811fcmr12125922wrc.109.1647149304950; Sat, 12 Mar 2022 21:28:24 -0800 (PST) Received: from takamaka.home ([2a01:cb22:1d5:1100:649a:6ea9:3e07:fe1f]) by smtp.gmail.com with ESMTPSA id p22-20020a1c5456000000b00389e7e62800sm7319826wmi.8.2022.03.12.21.28.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 12 Mar 2022 21:28:24 -0800 (PST) Received: by takamaka.home (Postfix, from userid 1000) id A957EA4AD3; Sun, 13 Mar 2022 09:28:21 +0400 (+04) Date: Sun, 13 Mar 2022 09:28:21 +0400 From: Joel Brobecker To: Carl Love via Gdb-patches Cc: Ulrich Weigand , Rogerio Alves , Joel Brobecker Subject: Re: [PATCH] PowerPC, add support for printing non-trivial C++ object for the finish command. Message-ID: References: <28ce795ca489f69829207b2a7a535cf7f77f6dd8.camel@us.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <28ce795ca489f69829207b2a7a535cf7f77f6dd8.camel@us.ibm.com> X-Spam-Status: No, score=-10.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, 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: 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: Sun, 13 Mar 2022 05:28:28 -0000 Hello, > The following patch fixes an issue on Powerpc where the function return > value for a C++ program prints garbage when the function return type is > a class object. The fix is based on an x86-64 patch for the same > issue. > > The patch has been tested on Power 10 with no regressions. Please let > me know if this patch is acceptable for mainline. Thanks. Thanks for the patch. I don't know the precise specifications regarding returning C++ objects on PowerPC targets, but I think we can trust you on that. For the rest, the patch looks OK to me. Maybe one small teeny weeny wittle nitpick, erm, I mean, suggestion. Which lead me to realize that there was a small GNU Coding Style issue. See below: > > Carl Love > --------------------------------------------------- > PowerPC, add support for printing non-trivial C++ object for the finish command. > > This patch fixes five testcase failures in gdb/cpp/non-trival-retval.exp. > The testcases that fail were added by commit: > > commit b1718fcdd1d2a5c514f8ee504ba07fb3f42b8608 > Author: Andrew Burgess > Date: Mon Dec 13 16:56:16 2021 +0000 > > gdb: on x86-64 non-trivial C++ objects are returned in memory > > Fixes PR gdb/28681. It was observed that after using the `finish` > command an incorrect value was displayed in some cases. Specifically, > this behaviour was observed on an x86-64 target. > > The commit adds new tests for printing the return value of a C++ function. > The return value of the function is a C++ Class. The current PowerPC code > is looking for the return value in register r3. The return value is actually > in memory as described in the commit message for x86-64 above. > > The fix for Powerpc is based on the fix in the above commit plus examining > how x86-64 handles the value in function amd64_return_value in file > gdb/amd64-tdep.c. > --- > gdb/ppc-sysv-tdep.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c > index 9a3b02f028d..50277e7bdd1 100644 > --- a/gdb/ppc-sysv-tdep.c > +++ b/gdb/ppc-sysv-tdep.c > @@ -2035,6 +2035,20 @@ ppc64_sysv_abi_return_value (struct gdbarch *gdbarch, struct value *function, > return RETURN_VALUE_REGISTER_CONVENTION; > } > > + /* If the object is a non-trivial C++ object the object is in memory. */ nitpick: I would add a coma after "non-trivial C++ object". Also, can you add a second space after the period, before the "*/"? (it's a GNU Coding Style requirement) Thank you! > + if (!language_pass_by_reference (valtype).trivially_copyable) > + { > + if (readbuf) > + { > + ULONGEST addr; > + int regnum = tdep->ppc_gp0_regnum + 3; > + > + regcache_raw_read_unsigned (regcache, regnum, &addr); > + read_memory (addr, readbuf, TYPE_LENGTH (valtype)); > + } > + return RETURN_VALUE_ABI_RETURNS_ADDRESS; > + } > + > /* In the ELFv2 ABI, aggregate types of up to 16 bytes are > returned in registers r3:r4. */ > if (tdep->elf_abi == POWERPC_ELF_V2 > -- > 2.32.0 > > -- Joel