From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2606 invoked by alias); 11 Jun 2014 12:26:44 -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 2595 invoked by uid 89); 11 Jun 2014 12:26:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.0 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mga03.intel.com Received: from mga03.intel.com (HELO mga03.intel.com) (143.182.124.21) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 11 Jun 2014 12:26:43 +0000 Received: from azsmga001.ch.intel.com ([10.2.17.19]) by azsmga101.ch.intel.com with ESMTP; 11 Jun 2014 05:26:19 -0700 X-ExtLoop1: 1 Received: from kboell-mobl2.ger.corp.intel.com (HELO [172.28.205.55]) ([172.28.205.55]) by azsmga001.ch.intel.com with ESMTP; 11 Jun 2014 05:26:17 -0700 Message-ID: <53984AE9.7020200@linux.intel.com> Date: Wed, 11 Jun 2014 12:26:00 -0000 From: Keven Boell User-Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 MIME-Version: 1.0 To: Joel Brobecker , Keven Boell CC: gdb-patches@sourceware.org, sanimir.agovic@intel.com Subject: Re: [PATCH 01/23] dwarf: add dwarf3 DW_OP_push_object_address opcode References: <1401861266-6240-1-git-send-email-keven.boell@intel.com> <1401861266-6240-2-git-send-email-keven.boell@intel.com> <20140610095445.GA5259@adacore.com> In-Reply-To: <20140610095445.GA5259@adacore.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2014-06/txt/msg00444.txt.bz2 Hi Joel, The address fields were introduced as DW_OP_push_object_address needs an object address to be pushed and this address needs to be stored somewhere. However, I will extend the log message why this address fields were introduced. I've addressed all your other comments and will send out a version 2 of the patch series soon, because some rebasing had to be done anyway. Keven > Hello Keven, > > On Wed, Jun 04, 2014 at 07:54:04AM +0200, Keven Boell wrote: >> The opcode pushes the address of the object being evaluated. The semantic is >> equivalent to the implicit push of the base address of a data member location. >> >> 2014-05-28 Sanimir Agovic >> Keven Boell >> >> * dwarf2expr.c (execute_stack_op) : New case. >> * dwarf2expr.h (struct dwarf_expr_context_funcs) >> : New function pointer get_object_addr. >> * dwarf2loc.c (struct dwarf_expr_baton): Add obj_address. >> (dwarf_expr_get_obj_addr): New function. >> (struct dwarf_expr_context_funcs): Add >> dwarf_expr_get_obj_addr to dwarf_expr_ctx_funcs. >> (dwarf2_evaluate_loc_desc_full): Initialize baton.obj_address. >> (dwarf2_locexpr_baton_eval): Set baton.obj_address to addr. >> (needs_get_obj_addr): New function. >> (struct dwarf_expr_context_funcs): Add needs_get_obj_addr to >> needs_frame_ctx_funcs. > > Some comments and questions below... > > My main question is relative to the new "addr" field. In many cases, > inferior data is not referenced by address, but via "struct value" > objects. And such objects are not necessarily addressable (Eg: they > live inside a register, or are broken down into pieces, etc). > I am wondering how things are going to be working in this case, > and in particular, for a code that have a struct value object, > and tries to resolve its type into a static type, how does it > provide an address, knowning that it does not always exist? Perhaps > we can make the assumption for such complex objects that when > a DW_OP_push_object_address is used, the object is always assumed > to be addressable? Whatever the answer is, I think it's worth > documentation clearly what the expectations are. Also, it's interesting > to note that several callers currently pass 0 as the (unused) address, > but will eventually need to be update to pass the correct info. > >> +/* DW_OP_push_object_address has a frame already passed thru. */ > ^^^^ > Sorry to nit-pick on this one, but I'd rather we spelled words > correctly. > >> CORE_ADDR dwarf2_read_addr_index (struct dwarf2_per_cu_data *per_cu, >> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c >> index d58193e..2a0cfe4 100644 >> --- a/gdb/gdbtypes.c >> +++ b/gdb/gdbtypes.c >> @@ -1646,7 +1646,7 @@ is_dynamic_type (struct type *type) >> } >> >> static struct type * >> -resolve_dynamic_range (struct type *dyn_range_type) >> +resolve_dynamic_range (struct type *dyn_range_type, CORE_ADDR addr) > > I just noticed that this function was missing a short description > (my bad), so I added it. Would you mind amending this patch to > update the description and add the meaning of ADDR? >