From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20477 invoked by alias); 30 Apr 2012 21:43:13 -0000 Received: (qmail 20465 invoked by uid 22791); 30 Apr 2012 21:43:11 -0000 X-SWARE-Spam-Status: No, hits=-3.3 required=5.0 tests=AWL,BAYES_00,KAM_STOCKGEN,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,TW_BP,TW_EG,TW_HP X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 30 Apr 2012 21:42:57 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1SOyMy-0003Kv-1X from Maciej_Rozycki@mentor.com ; Mon, 30 Apr 2012 14:42:56 -0700 Received: from SVR-IES-FEM-01.mgc.mentorg.com ([137.202.0.104]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Mon, 30 Apr 2012 14:42:51 -0700 Received: from [172.30.13.173] (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.1.289.1; Mon, 30 Apr 2012 22:42:54 +0100 Date: Mon, 30 Apr 2012 23:45:00 -0000 From: "Maciej W. Rozycki" To: Tom Tromey , Jan Kratochvil CC: Subject: Re: [RFA] MIPS16 FP manual call/return fixes In-Reply-To: <87mx5y70mv.fsf@fleche.redhat.com> Message-ID: References: <87mx5y70mv.fsf@fleche.redhat.com> User-Agent: Alpine 1.10 (DEB 962 2008-03-14) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" 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 X-SW-Source: 2012-04/txt/msg01091.txt.bz2 On Thu, 26 Apr 2012, Tom Tromey wrote: > Maciej> Therefore I had to change the return handlers' internal API to > Maciej> take the value of the function being handled rather than its > Maciej> lone type, if available. This has led to adjusting the whole > Maciej> infrastructure. > > It seems reasonable to me. > > I didn't try to read the MIPS part of the patch. > > Maybe Jan could read the ifunc change in elfread.c. Jan, would you please find a spare minute to look? The changes are meant to be straightforward, we don't need resolved_pc before we have tracked down the ifunc resolver breakpoint, at which point we can use its location to track down the function actually called. Then we can use its address with gdbarch_return_value for the backend to determine the actual ABI used. That said (and as noted before, I am not terribly familiar with the IFUNC feature), I suspect that any such function has to be treated as a regular indirect function call and therefore be reached via one of the MIPS16 call/return (as applicable) stubs concerned recently, so the standard ABI can probably be assumed. Then again, maybe not, in a pure-MIPS16 executable (or maybe really just a mixed-mode one that happens to call the function in question from MIPS16 code only -- there's really no sense to use hard-FP with pure-MIPS16 code as there's no access to the FPU in the MIPS16 mode) the resolver could optimise IFUNC lookup to the proper MIPS16 entry point. Anyway, any thunks used only copy registers and do not clobber the originals, so fetching the return value from the original registers is going to work as expected. Besides, the design is meant to be generic and work for any other platforms/ABIs that may suffer from such peculiarities (i.e. a different calling convention on an externally determined case-by-case basis). The extra assertion verifies that we only have a single resolver breakpoint assigned with this call; my understanding of the concept and code is this must always be the case. > Maciej> # stored into the appropriate register. This can be used when we want > Maciej> # to force the value returned by a function (see the "return" command > Maciej> # for instance). > Maciej> -M:enum return_value_convention:return_value:struct type *functype, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf:functype, valtype, regcache, readbuf, writebuf > Maciej> +M:enum return_value_convention:return_value:struct value *function, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf:functype, valtype, regcache, readbuf, writebuf > > A couple of nits here: > > Update the introductory comment to refer to FUNCTION, not FUNCTYPE. > > Also, please s/functype/function/ in the list of argument names (at the > end of the line). Ugh, I must have obviously messed up something here at one point as I got gdbarch.c right but not gdbarch.h or gdbarch.sh. > Maciej> + CORE_ADDR faddr = BLOCK_START (SYMBOL_BLOCK_VALUE (a->function)); > Maciej> + struct value *func = allocate_value (SYMBOL_TYPE (a->function)); > > I think read_var_value would be better here. Thanks for the pointer, I tend to agree and regression testing looks good after these changes. From the context I think it's safe and appropriate to call get_current_frame from finish_command_continuation (not sure if the frame chosen should ever matter for the calculation of any function's address -- perhaps for nested functions?), the other places already have a frame available. > The rest looked good to me. Thanks for the review, here's the resulting update. Any other comments, anyone? Maciej gdb-mips16-calls-ret-val-func-fix.diff Index: gdb-fsf-trunk-quilt/gdb/gdbarch.h =================================================================== --- gdb-fsf-trunk-quilt.orig/gdb/gdbarch.h 2012-04-27 21:31:50.085560611 +0100 +++ gdb-fsf-trunk-quilt/gdb/gdbarch.h 2012-04-27 19:50:02.965563474 +0100 @@ -433,8 +433,8 @@ typedef CORE_ADDR (gdbarch_integer_to_ad extern CORE_ADDR gdbarch_integer_to_address (struct gdbarch *gdbarch, struct type *type, const gdb_byte *buf); extern void set_gdbarch_integer_to_address (struct gdbarch *gdbarch, gdbarch_integer_to_address_ftype *integer_to_address); -/* Return the return-value convention that will be used by FUNCTYPE - to return a value of type VALTYPE. FUNCTYPE may be NULL in which +/* Return the return-value convention that will be used by FUNCTION + to return a value of type VALTYPE. FUNCTION may be NULL in which case the return convention is computed based only on VALTYPE. If READBUF is not NULL, extract the return value and save it in this buffer. Index: gdb-fsf-trunk-quilt/gdb/gdbarch.sh =================================================================== --- gdb-fsf-trunk-quilt.orig/gdb/gdbarch.sh 2012-04-27 21:31:50.085560611 +0100 +++ gdb-fsf-trunk-quilt/gdb/gdbarch.sh 2012-04-27 19:42:02.335573553 +0100 @@ -503,8 +503,8 @@ m:CORE_ADDR:pointer_to_address:struct ty m:void:address_to_pointer:struct type *type, gdb_byte *buf, CORE_ADDR addr:type, buf, addr::unsigned_address_to_pointer::0 M:CORE_ADDR:integer_to_address:struct type *type, const gdb_byte *buf:type, buf -# Return the return-value convention that will be used by FUNCTYPE -# to return a value of type VALTYPE. FUNCTYPE may be NULL in which +# Return the return-value convention that will be used by FUNCTION +# to return a value of type VALTYPE. FUNCTION may be NULL in which # case the return convention is computed based only on VALTYPE. # # If READBUF is not NULL, extract the return value and save it in this buffer. @@ -513,7 +513,7 @@ M:CORE_ADDR:integer_to_address:struct ty # stored into the appropriate register. This can be used when we want # to force the value returned by a function (see the "return" command # for instance). -M:enum return_value_convention:return_value:struct value *function, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf:functype, valtype, regcache, readbuf, writebuf +M:enum return_value_convention:return_value:struct value *function, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf:function, valtype, regcache, readbuf, writebuf m:CORE_ADDR:skip_prologue:CORE_ADDR ip:ip:0:0 M:CORE_ADDR:skip_main_prologue:CORE_ADDR ip:ip Index: gdb-fsf-trunk-quilt/gdb/infcmd.c =================================================================== --- gdb-fsf-trunk-quilt.orig/gdb/infcmd.c 2012-04-27 21:31:50.085560611 +0100 +++ gdb-fsf-trunk-quilt/gdb/infcmd.c 2012-04-27 20:08:31.935596617 +0100 @@ -1547,11 +1547,10 @@ finish_command_continuation (void *arg, if (TYPE_CODE (value_type) != TYPE_CODE_VOID) { - CORE_ADDR faddr = BLOCK_START (SYMBOL_BLOCK_VALUE (a->function)); - struct value *func = allocate_value (SYMBOL_TYPE (a->function)); volatile struct gdb_exception ex; + struct value *func; - set_value_address (func, faddr); + func = read_var_value (a->function, get_current_frame ()); TRY_CATCH (ex, RETURN_MASK_ALL) { /* print_return_value can throw an exception in some Index: gdb-fsf-trunk-quilt/gdb/stack.c =================================================================== --- gdb-fsf-trunk-quilt.orig/gdb/stack.c 2012-04-27 21:31:50.155559621 +0100 +++ gdb-fsf-trunk-quilt/gdb/stack.c 2012-04-27 20:11:06.425607247 +0100 @@ -2284,11 +2284,7 @@ return_command (char *retval_exp, int fr value_fetch_lazy (return_value); if (thisfun != NULL) - { - function = allocate_value (SYMBOL_TYPE (thisfun)); - set_value_address (function, - BLOCK_START (SYMBOL_BLOCK_VALUE (thisfun))); - } + function = read_var_value (thisfun, thisframe); if (TYPE_CODE (return_type) == TYPE_CODE_VOID) /* If the return-type is "void", don't try to find the Index: gdb-fsf-trunk-quilt/gdb/python/py-finishbreakpoint.c =================================================================== --- gdb-fsf-trunk-quilt.orig/gdb/python/py-finishbreakpoint.c 2012-04-27 21:31:50.195565239 +0100 +++ gdb-fsf-trunk-quilt/gdb/python/py-finishbreakpoint.c 2012-04-27 20:18:47.945561048 +0100 @@ -252,14 +252,11 @@ bpfinishpy_init (PyObject *self, PyObjec if (TYPE_CODE (ret_type) != TYPE_CODE_VOID) { struct value *func_value; - CORE_ADDR func_addr; /* Ignore Python errors at this stage. */ self_bpfinish->return_type = type_to_type_object (ret_type); PyErr_Clear (); - func_value = allocate_value (SYMBOL_TYPE (function)); - func_addr = BLOCK_START (SYMBOL_BLOCK_VALUE (function)); - set_value_address (func_value, func_addr); + func_value = read_var_value (function, frame); self_bpfinish->function_value = value_to_value_object (func_value); PyErr_Clear ();