From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 83229 invoked by alias); 9 Oct 2018 16:14:49 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 83220 invoked by uid 89); 9 Oct 2018 16:14:49 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 09 Oct 2018 16:14:47 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2E6A290906; Tue, 9 Oct 2018 16:14:46 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5BE5A7A5D6; Tue, 9 Oct 2018 16:14:45 +0000 (UTC) Subject: Re: [PATCH v2 1/2] Add lang_struct_return to _push_dummy_call To: Alan Hayward , gdb-patches@sourceware.org References: <20181001155255.14859-1-alan.hayward@arm.com> <20181001155255.14859-2-alan.hayward@arm.com> Cc: nd@arm.com From: Pedro Alves Message-ID: <5ce2656e-1ba6-bbab-48a6-7a12dea92a61@redhat.com> Date: Tue, 09 Oct 2018 16:14:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20181001155255.14859-2-alan.hayward@arm.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-10/txt/msg00215.txt.bz2 On 10/01/2018 04:52 PM, Alan Hayward wrote: > Make call_function_by_hand_dummy pass this down. > > 2018-10-01 Alan Hayward > > diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c > index 023e8eb453..504b040c2e 100644 > --- a/gdb/aarch64-tdep.c > +++ b/gdb/aarch64-tdep.c > @@ -1512,8 +1512,8 @@ pass_in_v_vfp_candidate (struct gdbarch *gdbarch, struct regcache *regcache, > static CORE_ADDR > aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function, > struct regcache *regcache, CORE_ADDR bp_addr, > - int nargs, > - struct value **args, CORE_ADDR sp, int struct_return, > + int nargs, struct value **args, CORE_ADDR sp, > + int struct_return, int lang_struct_return_unused, Here this is called "lang_struct_return_unused", but all the other cases were just "lang_struct_return". I suppose it'd be clearer if "struct_return" were renamed to "abi_struct_return", but I empathize with not having changed it in this patch... > CORE_ADDR struct_addr) > { > int argnum; > --- a/gdb/gdbarch.sh > +++ b/gdb/gdbarch.sh > @@ -485,7 +485,7 @@ M;struct frame_id;dummy_id;struct frame_info *this_frame;this_frame > # deprecated_fp_regnum. > v;int;deprecated_fp_regnum;;;-1;-1;;0 > > -M;CORE_ADDR;push_dummy_call;struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, int struct_return, CORE_ADDR struct_addr;function, regcache, bp_addr, nargs, args, sp, struct_return, struct_addr > +M;CORE_ADDR;push_dummy_call;struct value *function, struct regcache *regcache, CORE_ADDR bp_addr, int nargs, struct value **args, CORE_ADDR sp, int struct_return, int lang_struct_return, CORE_ADDR struct_addr;function, regcache, bp_addr, nargs, args, sp, struct_return, lang_struct_return, struct_addr > v;int;call_dummy_location;;;;AT_ENTRY_POINT;;0 > M;CORE_ADDR;push_dummy_code;CORE_ADDR sp, CORE_ADDR funaddr, struct value **args, int nargs, struct type *value_type, CORE_ADDR *real_pc, CORE_ADDR *bp_addr, struct regcache *regcache;sp, funaddr, args, nargs, value_type, real_pc, bp_addr, regcache > No documentation, no goddie. What does the new parameter mean? I'd think that a bool instead of an int would be better. Or clearer still, an enum, like: enum class return_how { STRUCT, NORMAL }; If you want to do a preparatory patch adding the enum and renaming "struct_return", it'd be awesome, I think. But I suppose that given the existence of the "int struct_return" parameter, I can't really complain. We could always add such an enum later on and change both parameters at the same time throughout. So feel free to leave it be as is. Thanks, Pedro Alves