From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id xvCtF+4rMmKrNwAAWB0awg (envelope-from ) for ; Wed, 16 Mar 2022 14:26:54 -0400 Received: by simark.ca (Postfix, from userid 112) id 4D3EA1F3CC; Wed, 16 Mar 2022 14:26:54 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.9 required=5.0 tests=BAYES_00,MAILING_LIST_MULTI, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 637A41EA69 for ; Wed, 16 Mar 2022 14:26:53 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id DF7A639484AE for ; Wed, 16 Mar 2022 18:26:52 +0000 (GMT) Received: from angie.orcam.me.uk (angie.orcam.me.uk [IPv6:2001:4190:8020::34]) by sourceware.org (Postfix) with ESMTP id A74FA394843F for ; Wed, 16 Mar 2022 18:26:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A74FA394843F Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=orcam.me.uk Authentication-Results: sourceware.org; spf=none smtp.mailfrom=orcam.me.uk Received: by angie.orcam.me.uk (Postfix, from userid 500) id 57E3B92009C; Wed, 16 Mar 2022 19:26:25 +0100 (CET) Received: from localhost (localhost [127.0.0.1]) by angie.orcam.me.uk (Postfix) with ESMTP id 511B192009B; Wed, 16 Mar 2022 18:26:25 +0000 (GMT) Date: Wed, 16 Mar 2022 18:26:25 +0000 (GMT) From: "Maciej W. Rozycki" To: Andrew Burgess Subject: Re: [PATCH] gdb: mips: Fix the handling of complex type of function return value In-Reply-To: <877d8uuop8.fsf@redhat.com> Message-ID: References: <1647406106-25723-1-git-send-email-tangyouling@loongson.cn> <20220316084248.m5m2et3njtngeoge@Plymouth> <877d8uuop8.fsf@redhat.com> User-Agent: Alpine 2.21 (DEB 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII 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: , Cc: Lancelot SIX , Youling Tang , gdb-patches@sourceware.org Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" On Wed, 16 Mar 2022, Andrew Burgess wrote: > >>> $ objdump -d outputs/gdb.base/varargs/varargs > >>> 00000001200012e8 : > >>> ... > >>> 1200013b8: c7c10000 lwc1 $f1,0(s8) > >>> 1200013bc: c7c00004 lwc1 $f0,4(s8) > >>> 1200013c0: 46000886 mov.s $f2,$f1 > >>> 1200013c4: 46000046 mov.s $f1,$f0 > >>> 1200013c8: 46001006 mov.s $f0,$f2 > >>> 1200013cc: 46000886 mov.s $f2,$f1 > >>> 1200013d0: 03c0e825 move sp,s8 > >>> 1200013d4: dfbe0038 ld s8,56(sp) > >>> 1200013d8: 67bd0080 daddiu sp,sp,128 > >>> 1200013dc: 03e00008 jr ra > >>> 1200013e0: 00000000 nop > >>> > >>> From the above disassembly, we can see that when the return value of the > >>> function is a complex type and len <= 2 * MIPS64_REGSIZE, the return value > >>> will be passed through $f0 and $f2, so fix the corresponding processing > >>> in mips_n32n64_return_value(). > >>> > >>> $ make check RUNTESTFLAGS='GDB=../gdb gdb.base/varargs.exp --outdir=test' > >>> > >>> Before applying the patch: > >>> FAIL: gdb.base/varargs.exp: print find_max_float_real(4, fc1, fc2, fc3, fc4) > >>> FAIL: gdb.base/varargs.exp: print find_max_double_real(4, dc1, dc2, dc3, dc4) > >>> > >>> # of expected passes 9 > >>> # of unexpected failures 2 > >>> > >>> After applying the patch: > >>> # of expected passes 11 > >>> > >>> Signed-off-by: Youling Tang > >>> --- > >>> gdb/mips-tdep.c | 3 ++- > >>> 1 file changed, 2 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c > >>> index 7e37578..cddb8f8 100644 > >>> --- a/gdb/mips-tdep.c > >>> +++ b/gdb/mips-tdep.c > >>> @@ -5224,9 +5224,10 @@ mips_n32n64_return_value (struct gdbarch *gdbarch, struct value *function, > >>> > >>> if (TYPE_LENGTH (type) > 2 * MIPS64_REGSIZE) > >>> return RETURN_VALUE_STRUCT_CONVENTION; > >>> - else if (type->code () == TYPE_CODE_FLT > >>> + else if ((type->code () == TYPE_CODE_FLT > >>> && TYPE_LENGTH (type) == 16 > >>> && tdep->mips_fpu_type != MIPS_FPU_NONE) > >> Hi, > >> > >> Just minor note, those 2 lines above should be indented 2 more space I > >> think (so the && operator continues to vertically align with "type->code > >> ()"). > >> > >>> + || (type->code () == TYPE_CODE_COMPLEX)) > >> I do not think the extra set of parens are requires (but they do no harm > >> either). > > Ok, I will modify this code style in the next version. > > This change looks good to me with the style adjusments that Lancelot > pointed out. This has to be double-checked, because as I recall we have an ABI bug in GCC in this area. Which is also the reason why the relevant test cases have not been fixed in 15+ years now (I've been aware of this issue). OTOH, if things have been like this for so long, then I suppose they need to stay as they are. In any case I think this does have to be thoroughly understood and documented. Maciej