From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 cc: Youling Tang , Lancelot SIX , gdb-patches@sourceware.org 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-Spam-Status: No, score=-3495.1 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_INFOUSMEBIZ, KAM_LAZY_DOMAIN_SECURITY, SPF_HELO_NONE, SPF_NONE, 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: Wed, 16 Mar 2022 18:26:28 -0000 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