From patchwork Mon Nov 28 17:20:16 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yao Qi X-Patchwork-Id: 18004 Received: (qmail 88223 invoked by alias); 28 Nov 2016 17:20:39 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 88209 invoked by uid 89); 28 Nov 2016 17:20:38 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=no version=3.3.2 spammy=cares, H*r:AES128-SHA, lazy, value's X-HELO: mail-wj0-f194.google.com Received: from mail-wj0-f194.google.com (HELO mail-wj0-f194.google.com) (209.85.210.194) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 28 Nov 2016 17:20:28 +0000 Received: by mail-wj0-f194.google.com with SMTP id jb2so14946090wjb.3 for ; Mon, 28 Nov 2016 09:20:28 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=QbzGuIn3sS7hHh1oks/a53hz1opeF3Ur3jcI87jhgFc=; b=hB8S3avbkyR5MZAGIynWl2CmL0dqXRQqluRKx2GPBEldoBzfU06PmqTLu8EJ9oK31I gk8X9H25Rq18ga2DYX8lZsnx3KNzdgSnihM5Ofkq+0Mm/OOC/9qMDGnFjqsLDMlVaq1z IuQWxzXEyUDmHUS0NDMKxY/1LmHn/7+MLXLKpQz9dGm6JaRLKxrF1WtG3BFN2w1DUCyr odgqru9rZ9w+ZQTHOg/wEgFTPfw9ekLVsD7cxcqcCgNZSQ6VishL004tGJZ41FuKrkqp cgFU7FLA0uJVjRyy4OFmxSNCsQYSe+LX4z8MtM6y6srxoX8spdnvWbU81Dr5/hI3WZv9 YXzw== X-Gm-Message-State: AKaTC03mo9iW9WjfnuMUu0nkr2QqS2dieejqSCVDKhNP4JifQs+TfGIpzVuhczMCaAbPIg== X-Received: by 10.194.20.68 with SMTP id l4mr19995097wje.49.1480353626436; Mon, 28 Nov 2016 09:20:26 -0800 (PST) Received: from E107787-LIN (power8-aix.osuosl.org. [140.211.9.96]) by smtp.gmail.com with ESMTPSA id d17sm63321659wjr.14.2016.11.28.09.20.23 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Mon, 28 Nov 2016 09:20:25 -0800 (PST) Date: Mon, 28 Nov 2016 17:20:16 +0000 From: Yao Qi To: Ulrich Weigand Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 1/3] Move computed value's frame id to piece_closure Message-ID: <20161128172016.GB22209@E107787-LIN> References: <1480068407-22616-2-git-send-email-yao.qi@linaro.org> <20161125114836.530D010BCB8@oc8523832656.ibm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20161125114836.530D010BCB8@oc8523832656.ibm.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes On Fri, Nov 25, 2016 at 12:48:36PM +0100, Ulrich Weigand wrote: > Yao Qi wrote: > > > + if (frame == NULL) > > + c->frame_id = null_frame_id; > > + else > > + c->frame_id = get_frame_id (get_next_frame_sentinel_okay (frame)); > [...] > > + struct frame_info *frame > > + = frame_find_by_id (get_prev_frame_id_by_id (c->frame_id)); > > I guess now that this is a separate field, there's no longer a reason > to go via next-frame / prev-frame here. It looks like this was introduced > when VALUE_FRAME_ID was changed to VALUE_NEXT_FRAME_ID, but this isn't > really necessary for the piece logic here. I think this can simply be: > > c->frame_id = get_frame_id (frame); > [...] > struct frame_info *frame = frame_find_by_id (c->frame_id); > > instead. > > Otherwise, this looks good to me. > Patch below is pushed in. diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c index 128f654..aaf88e4 100644 --- a/gdb/dwarf2loc.c +++ b/gdb/dwarf2loc.c @@ -1465,6 +1465,10 @@ struct piece_closure /* The pieces themselves. */ struct dwarf_expr_piece *pieces; + + /* Frame ID of frame to which a register value is relative, used + only by DWARF_VALUE_REGISTER. */ + struct frame_id frame_id; }; /* Allocate a closure for a value formed from separately-described @@ -1473,7 +1477,7 @@ struct piece_closure static struct piece_closure * allocate_piece_closure (struct dwarf2_per_cu_data *per_cu, int n_pieces, struct dwarf_expr_piece *pieces, - int addr_size) + int addr_size, struct frame_info *frame) { struct piece_closure *c = XCNEW (struct piece_closure); int i; @@ -1483,6 +1487,10 @@ allocate_piece_closure (struct dwarf2_per_cu_data *per_cu, c->n_pieces = n_pieces; c->addr_size = addr_size; c->pieces = XCNEWVEC (struct dwarf_expr_piece, n_pieces); + if (frame == NULL) + c->frame_id = null_frame_id; + else + c->frame_id = get_frame_id (frame); memcpy (c->pieces, pieces, n_pieces * sizeof (struct dwarf_expr_piece)); for (i = 0; i < n_pieces; ++i) @@ -1731,18 +1739,12 @@ read_pieced_value (struct value *v) gdb_byte *contents; struct piece_closure *c = (struct piece_closure *) value_computed_closure (v); - struct frame_info *frame; size_t type_len; size_t buffer_size = 0; std::vector buffer; int bits_big_endian = gdbarch_bits_big_endian (get_type_arch (value_type (v))); - /* VALUE_FRAME_ID is used instead of VALUE_NEXT_FRAME_ID here - because FRAME is passed to get_frame_register_bytes(), which - does its own "->next" operation. */ - frame = frame_find_by_id (VALUE_FRAME_ID (v)); - if (value_type (v) != value_enclosing_type (v)) internal_error (__FILE__, __LINE__, _("Should not be able to create a lazy value with " @@ -1802,6 +1804,7 @@ read_pieced_value (struct value *v) { case DWARF_VALUE_REGISTER: { + struct frame_info *frame = frame_find_by_id (c->frame_id); struct gdbarch *arch = get_frame_arch (frame); int gdb_regnum = dwarf_reg_to_regnum_or_error (arch, p->v.regno); int optim, unavail; @@ -2370,10 +2373,6 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame, if (ctx.num_pieces > 0) { struct piece_closure *c; - struct frame_id frame_id - = frame == NULL - ? null_frame_id - : get_frame_id (get_next_frame_sentinel_okay (frame)); ULONGEST bit_size = 0; int i; @@ -2383,12 +2382,11 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame, invalid_synthetic_pointer (); c = allocate_piece_closure (per_cu, ctx.num_pieces, ctx.pieces, - ctx.addr_size); + ctx.addr_size, frame); /* We must clean up the value chain after creating the piece closure but before allocating the result. */ do_cleanups (value_chain); retval = allocate_computed_value (type, &pieced_value_funcs, c); - VALUE_NEXT_FRAME_ID (retval) = frame_id; set_value_offset (retval, byte_offset); } else