From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 80D1E3858C2C for ; Thu, 3 Feb 2022 01:04:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 80D1E3858C2C Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 21313jMv025071 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 2 Feb 2022 20:03:50 -0500 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 21313jMv025071 Received: from [10.0.0.11] (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 051931EDEE; Wed, 2 Feb 2022 20:03:44 -0500 (EST) Message-ID: Date: Wed, 2 Feb 2022 20:03:44 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Subject: Re: [PATCH v4] gdb: add a symbol* argument to get_return_value Content-Language: en-US To: Lancelot SIX , simark@simark.ca Cc: lsix@lancelotsix.com, gdb-patches@sourceware.org References: <8ebbde37-0acc-c430-ea36-d21c14163ef8@simark.ca> <20220202215914.23534-1-lancelot.six@amd.com> From: Simon Marchi In-Reply-To: <20220202215914.23534-1-lancelot.six@amd.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Thu, 3 Feb 2022 01:03:45 +0000 X-Spam-Status: No, score=-3038.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_LOTSOFHASH, KAM_STOCKGEN, NICE_REPLY_A, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, 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: Thu, 03 Feb 2022 01:04:57 -0000 On 2022-02-02 16:59, Lancelot SIX via Gdb-patches wrote: > Hi, > > Here is a revised version of the patch. > > Since V3: > > - Update documentation of get_return_value and move in inferior.h. > > Best, > Lancelot. > > --- > > Add an argument to the get_return_value function to indicate the symbol > of the function the debuggee is returning from. This will be used by > the following patch. > > No user visible change after this patch. Hi Lancelot, Every time I look I keep finding nits, sorry :( > > Tested on x86_64-linux. > > Change-Id: Idf1279f1f7199f5022738a6679e0fa63fbd22edc > --- > gdb/infcmd.c | 9 ++++----- > gdb/inferior.h | 10 +++++++++- > gdb/python/py-finishbreakpoint.c | 29 ++++++++++++++++++++--------- > 3 files changed, 33 insertions(+), 15 deletions(-) > > diff --git a/gdb/infcmd.c b/gdb/infcmd.c > index 994dd5b32a3..6be126aedab 100644 > --- a/gdb/infcmd.c > +++ b/gdb/infcmd.c > @@ -1405,12 +1405,11 @@ advance_command (const char *arg, int from_tty) > until_break_command (arg, from_tty, 1); > } > > -/* Return the value of the result of a function at the end of a 'finish' > - command/BP. DTOR_DATA (if not NULL) can represent inferior registers > - right after an inferior call has finished. */ > +/* See inferior.h. */ > > struct value * > -get_return_value (struct value *function, struct type *value_type) > +get_return_value (struct symbol *func_symbol, struct value *function, > + struct type *value_type) I have the feeling that value_type is redundant with func_symbol, since the return value type initially comes from the symbol. So I think we could remove value_type. In fact, it might have been redundant already, since from `struct value *function`, you should also be able to get the function type. But there may be edge cases I don't know about. Do you think this change below (that builds on top of your patch) would work? Tests gdb.*/*finish*.exp pass here. >From 7062c0afb88b5a845f8d44e4ad3bae20acbd0dd3 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Wed, 2 Feb 2022 20:00:04 -0500 Subject: [PATCH] fixup Change-Id: I3644b669ab6a4e32cf59129f5c4b8e01fe90196e --- gdb/infcmd.c | 8 +++---- gdb/inferior.h | 8 +++---- gdb/python/py-finishbreakpoint.c | 37 ++++++++++++-------------------- 3 files changed, 21 insertions(+), 32 deletions(-) diff --git a/gdb/infcmd.c b/gdb/infcmd.c index 6be126aedabb..9a3214374e3a 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -1408,14 +1408,14 @@ advance_command (const char *arg, int from_tty) /* See inferior.h. */ struct value * -get_return_value (struct symbol *func_symbol, struct value *function, - struct type *value_type) +get_return_value (struct symbol *func_symbol, struct value *function) { regcache *stop_regs = get_current_regcache (); struct gdbarch *gdbarch = stop_regs->arch (); struct value *value; - value_type = check_typedef (value_type); + struct type *value_type + = check_typedef (TYPE_TARGET_TYPE (SYMBOL_TYPE (func_symbol))); gdb_assert (value_type->code () != TYPE_CODE_VOID); /* FIXME: 2003-09-27: When returning from a nested inferior function @@ -1576,7 +1576,7 @@ finish_command_fsm::should_stop (struct thread_info *tp) struct value *func; func = read_var_value (function, NULL, get_current_frame ()); - rv->value = get_return_value (function, func, rv->type); + rv->value = get_return_value (function, func); if (rv->value != NULL) rv->value_history_index = record_latest_value (rv->value); } diff --git a/gdb/inferior.h b/gdb/inferior.h index ef18fcb2482b..45de3c2d9c8b 100644 --- a/gdb/inferior.h +++ b/gdb/inferior.h @@ -222,13 +222,11 @@ extern void notice_new_inferior (struct thread_info *, bool, int); /* Return the value of the result of a function at the end of a 'finish' command/BP. If the result's value cannot be retrieved, return NULL. - FUNC_SYMBOL is the symbol of the function being returned from, FUNCTION is - a value containing the address of the function, and VALUE_TYPE the type of - the value the function returns. */ + FUNC_SYMBOL is the symbol of the function being returned from. FUNCTION is + a value containing the address of the function. */ extern struct value *get_return_value (struct symbol *func_symbol, - struct value *function, - struct type *value_type); + struct value *function); /* Prepare for execution command. TARGET is the target that will run the command. BACKGROUND determines whether this is a foreground diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c index a322918dfca6..034f8e69c0a2 100644 --- a/gdb/python/py-finishbreakpoint.c +++ b/gdb/python/py-finishbreakpoint.c @@ -40,15 +40,15 @@ struct finish_breakpoint_object { /* gdb.Breakpoint base class. */ gdbpy_breakpoint_object py_bp; + /* gdb.Symbol object of the function finished by this breakpoint. */ PyObject *func_symbol; - /* gdb.Type object of the value return by the breakpointed function. - May be NULL if no debug information was available or return type - was VOID. */ - PyObject *return_type; - /* gdb.Value object of the function finished by this breakpoint. Will be - NULL if return_type is NULL. */ + + /* gdb.Value object of the function finished by this breakpoint. + + nullptr if no debug information was available or return type was VOID. */ PyObject *function_value; + /* When stopped at this FinishBreakpoint, gdb.Value object returned by the function; Py_None if the value is not computable; NULL if GDB is not stopped at a FinishBreakpoint. */ @@ -84,7 +84,6 @@ bpfinishpy_dealloc (PyObject *self) Py_XDECREF (self_bpfinish->func_symbol); Py_XDECREF (self_bpfinish->function_value); - Py_XDECREF (self_bpfinish->return_type); Py_XDECREF (self_bpfinish->return_value); Py_TYPE (self)->tp_free (self); } @@ -102,7 +101,7 @@ bpfinishpy_pre_stop_hook (struct gdbpy_breakpoint_object *bp_obj) /* Can compute return_value only once. */ gdb_assert (!self_finishbp->return_value); - if (!self_finishbp->return_type) + if (self_finishbp->func_symbol == nullptr) return; try @@ -111,10 +110,8 @@ bpfinishpy_pre_stop_hook (struct gdbpy_breakpoint_object *bp_obj) symbol_object_to_symbol (self_finishbp->func_symbol); struct value *function = value_object_to_value (self_finishbp->function_value); - struct type *value_type = - type_object_to_type (self_finishbp->return_type); struct value *ret = - get_return_value (func_symbol, function, value_type); + get_return_value (func_symbol, function); if (ret) { @@ -245,7 +242,6 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs) /* Find the function we will return from. */ self_bpfinish->func_symbol = nullptr; - self_bpfinish->return_type = nullptr; self_bpfinish->function_value = nullptr; try @@ -255,22 +251,20 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs) struct symbol *function = find_pc_function (pc); if (function != nullptr) { - self_bpfinish->func_symbol = symbol_to_symbol_object (function); struct type *ret_type = check_typedef (TYPE_TARGET_TYPE (SYMBOL_TYPE (function))); /* Remember only non-void return types. */ if (ret_type->code () != TYPE_CODE_VOID) { - struct value *func_value; - /* Ignore Python errors at this stage. */ - self_bpfinish->return_type = type_to_type_object (ret_type); - PyErr_Clear (); - func_value = read_var_value (function, NULL, frame); - self_bpfinish->function_value = - value_to_value_object (func_value); + value *func_value = read_var_value (function, NULL, frame); + self_bpfinish->function_value + = value_to_value_object (func_value); PyErr_Clear (); + + self_bpfinish->func_symbol + = symbol_to_symbol_object (function); } } } @@ -282,16 +276,13 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs) } if (self_bpfinish->func_symbol == nullptr - || self_bpfinish->return_type == nullptr || self_bpfinish->function_value == nullptr) { /* Won't be able to compute return value. */ Py_XDECREF (self_bpfinish->func_symbol); - Py_XDECREF (self_bpfinish->return_type); Py_XDECREF (self_bpfinish->function_value); self_bpfinish->func_symbol = nullptr; - self_bpfinish->return_type = nullptr; self_bpfinish->function_value = nullptr; } base-commit: 36a13a0e62bc672f59c6d27bc2b963edee32b488 prerequisite-patch-id: 2ac1882ff2c8573a1f4abfe6ec081c23bad1f2fe -- 2.34.1