From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id Z5jIHK3GiGAIFwAAWB0awg (envelope-from ) for ; Tue, 27 Apr 2021 22:21:33 -0400 Received: by simark.ca (Postfix, from userid 112) id 672451F11C; Tue, 27 Apr 2021 22:21:33 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 30E541E783 for ; Tue, 27 Apr 2021 22:21:32 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 8540C3894C18; Wed, 28 Apr 2021 02:21:31 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 8540C3894C18 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1619576491; bh=lEB8jzvqfkp0fnMApELVneV9BJuvTS6Uzq/xJERKaEc=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=Z5xzK2cnWDhY5372sA5faNDfSK7MEBb75gMV6fK8M62Ei0s7tfAENooOXb1rpnYl4 V8H5+H1WktPhkU73MLWpsedXe4g+rfplOqVf20V4OWpg5B1rpp2AvdYqF6P+u6gEXJ I055v/J4Uhs1n6T6Z0CRw5dEz0QrEvUdzo8xO/uI= Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 982DD386EC3E for ; Wed, 28 Apr 2021 02:21:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 982DD386EC3E 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 13S2LLwx027856 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 27 Apr 2021 22:21:26 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 13S2LLwx027856 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)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id D26A11E783; Tue, 27 Apr 2021 22:21:21 -0400 (EDT) Subject: Re: [PATCH 15/43] Make DWARF evaluator return a single struct value To: Zoran Zaric , gdb-patches@sourceware.org References: <20210301144620.103016-1-Zoran.Zaric@amd.com> <20210301144620.103016-16-Zoran.Zaric@amd.com> Message-ID: Date: Tue, 27 Apr 2021 22:21:21 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: <20210301144620.103016-16-Zoran.Zaric@amd.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Wed, 28 Apr 2021 02:21:22 +0000 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: , From: Simon Marchi via Gdb-patches Reply-To: Simon Marchi Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" > @@ -886,6 +887,162 @@ dwarf_expr_context::push_dwarf_reg_entry_value > this->eval (data_src, size); > } > > +/* See expr.h. */ > + > +struct value * > +dwarf_expr_context::fetch_result (struct type *type, > + struct type *subobj_type, > + LONGEST subobj_offset) > +{ > + struct value *retval = nullptr; > + > + if (type == nullptr) > + type = address_type (); > + > + if (subobj_type == nullptr) > + subobj_type = type; > + > + if (this->pieces.size () > 0) > + { > + struct piece_closure *c; You can move this to where it's first initialized (and drop the struct keyword). > + ULONGEST bit_size = 0; > + > + for (dwarf_expr_piece &piece : this->pieces) > + bit_size += piece.size; > + /* Complain if the expression is larger than the size of the > + outer type. */ > + if (bit_size > 8 * TYPE_LENGTH (type)) > + invalid_synthetic_pointer (); > + > + c = allocate_piece_closure (this->per_cu, this->per_objfile, > + std::move (this->pieces), this->frame); > + retval = allocate_computed_value (subobj_type, > + &pieced_value_funcs, c); > + set_value_offset (retval, subobj_offset); > + } > + else > + { > + switch (this->location) > + { > + case DWARF_VALUE_REGISTER: > + { > + int dwarf_regnum > + = longest_to_int (value_as_long (this->fetch (0))); > + int gdb_regnum = dwarf_reg_to_regnum_or_error (this->gdbarch, > + dwarf_regnum); > + > + if (subobj_offset != 0) > + error (_("cannot use offset on synthetic pointer to register")); > + > + gdb_assert (this->frame != NULL); > + > + retval = value_from_register (subobj_type, gdb_regnum, > + this->frame); > + if (value_optimized_out (retval)) > + { > + struct value *tmp; > + > + /* This means the register has undefined value / was > + not saved. As we're computing the location of some > + variable etc. in the program, not a value for > + inspecting a register ($pc, $sp, etc.), return a > + generic optimized out value instead, so that we show > + instead of . */ > + tmp = allocate_value (subobj_type); > + value_contents_copy (tmp, 0, retval, 0, > + TYPE_LENGTH (subobj_type)); > + retval = tmp; > + } > + } > + break; > + > + case DWARF_VALUE_MEMORY: > + { > + struct type *ptr_type; > + CORE_ADDR address = this->fetch_address (0); > + bool in_stack_memory = this->fetch_in_stack_memory (0); > + > + /* DW_OP_deref_size (and possibly other operations too) may > + create a pointer instead of an address. Ideally, the > + pointer to address conversion would be performed as part > + of those operations, but the type of the object to > + which the address refers is not known at the time of > + the operation. Therefore, we do the conversion here > + since the type is readily available. */ > + > + switch (subobj_type->code ()) > + { > + case TYPE_CODE_FUNC: > + case TYPE_CODE_METHOD: > + ptr_type = builtin_type (this->gdbarch)->builtin_func_ptr; > + break; > + default: > + ptr_type = builtin_type (this->gdbarch)->builtin_data_ptr; > + break; > + } > + address = value_as_address (value_from_pointer (ptr_type, address)); > + > + retval = value_at_lazy (subobj_type, > + address + subobj_offset); > + if (in_stack_memory) > + set_value_stack (retval, 1); > + } > + break; > + > + case DWARF_VALUE_STACK: > + { > + struct value *value = this->fetch (0); > + size_t n = TYPE_LENGTH (value_type (value)); > + size_t len = TYPE_LENGTH (subobj_type); > + size_t max = TYPE_LENGTH (type); > + > + if (subobj_offset + len > max) > + invalid_synthetic_pointer (); > + > + retval = allocate_value (subobj_type); > + > + /* The given offset is relative to the actual object. */ > + if (gdbarch_byte_order (this->gdbarch) == BFD_ENDIAN_BIG) > + subobj_offset += n - max; > + > + memcpy (value_contents_raw (retval), > + value_contents_all (value) + subobj_offset, len); > + } > + break; > + > + case DWARF_VALUE_LITERAL: > + { > + bfd_byte *contents; Same here, declare where initialized. > + size_t n = TYPE_LENGTH (subobj_type); > + > + if (subobj_offset + n > this->len) > + invalid_synthetic_pointer (); > + > + retval = allocate_value (subobj_type); > + contents = value_contents_raw (retval); > + memcpy (contents, this->data + subobj_offset, n); > + } > + break; > + > + case DWARF_VALUE_OPTIMIZED_OUT: > + retval = allocate_optimized_out_value (subobj_type); > + break; > + > + /* DWARF_VALUE_IMPLICIT_POINTER was converted to a pieced > + operation by execute_stack_op. */ > + case DWARF_VALUE_IMPLICIT_POINTER: > + /* DWARF_VALUE_OPTIMIZED_OUT can't occur in this context -- > + it can only be encountered when making a piece. */ > + default: > + internal_error (__FILE__, __LINE__, _("invalid location type")); You could change this to use gdb_assert_not_reached. > @@ -1479,155 +1481,15 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame, > throw; > } > > - if (ctx.pieces.size () > 0) > - { > - struct piece_closure *c; > - ULONGEST bit_size = 0; > - > - for (dwarf_expr_piece &piece : ctx.pieces) > - bit_size += piece.size; > - /* Complain if the expression is larger than the size of the > - outer type. */ > - if (bit_size > 8 * TYPE_LENGTH (type)) > - invalid_synthetic_pointer (); > - > - c = allocate_piece_closure (per_cu, per_objfile, std::move (ctx.pieces), > - frame); > - /* We must clean up the value chain after creating the piece > - closure but before allocating the result. */ > - free_values.free_to_mark (); > - retval = allocate_computed_value (subobj_type, > - &pieced_value_funcs, c); > - set_value_offset (retval, subobj_byte_offset); > - } > - else > - { > - switch (ctx.location) > - { > - case DWARF_VALUE_REGISTER: > - { > - struct gdbarch *arch = get_frame_arch (frame); > - int dwarf_regnum > - = longest_to_int (value_as_long (ctx.fetch (0))); > - int gdb_regnum = dwarf_reg_to_regnum_or_error (arch, dwarf_regnum); > - > - if (subobj_byte_offset != 0) > - error (_("cannot use offset on synthetic pointer to register")); > - free_values.free_to_mark (); > - retval = value_from_register (subobj_type, gdb_regnum, frame); > - if (value_optimized_out (retval)) > - { > - struct value *tmp; > - > - /* This means the register has undefined value / was > - not saved. As we're computing the location of some > - variable etc. in the program, not a value for > - inspecting a register ($pc, $sp, etc.), return a > - generic optimized out value instead, so that we show > - instead of . */ > - tmp = allocate_value (subobj_type); > - value_contents_copy (tmp, 0, retval, 0, > - TYPE_LENGTH (subobj_type)); > - retval = tmp; > - } > - } > - break; > - > - case DWARF_VALUE_MEMORY: > - { > - struct type *ptr_type; > - CORE_ADDR address = ctx.fetch_address (0); > - bool in_stack_memory = ctx.fetch_in_stack_memory (0); > - > - /* DW_OP_deref_size (and possibly other operations too) may > - create a pointer instead of an address. Ideally, the > - pointer to address conversion would be performed as part > - of those operations, but the type of the object to > - which the address refers is not known at the time of > - the operation. Therefore, we do the conversion here > - since the type is readily available. */ > - > - switch (subobj_type->code ()) > - { > - case TYPE_CODE_FUNC: > - case TYPE_CODE_METHOD: > - ptr_type = builtin_type (ctx.gdbarch)->builtin_func_ptr; > - break; > - default: > - ptr_type = builtin_type (ctx.gdbarch)->builtin_data_ptr; > - break; > - } > - address = value_as_address (value_from_pointer (ptr_type, address)); > - > - free_values.free_to_mark (); > - retval = value_at_lazy (subobj_type, > - address + subobj_byte_offset); > - if (in_stack_memory) > - set_value_stack (retval, 1); > - } > - break; > - > - case DWARF_VALUE_STACK: > - { > - struct value *value = ctx.fetch (0); > - size_t n = TYPE_LENGTH (value_type (value)); > - size_t len = TYPE_LENGTH (subobj_type); > - size_t max = TYPE_LENGTH (type); > - gdbarch *objfile_gdbarch = per_objfile->objfile->arch (); > - > - if (subobj_byte_offset + len > max) > - invalid_synthetic_pointer (); > - > - /* Preserve VALUE because we are going to free values back > - to the mark, but we still need the value contents > - below. */ > - value_ref_ptr value_holder = value_ref_ptr::new_reference (value); > - free_values.free_to_mark (); > - > - retval = allocate_value (subobj_type); > - > - /* The given offset is relative to the actual object. */ > - if (gdbarch_byte_order (objfile_gdbarch) == BFD_ENDIAN_BIG) > - subobj_byte_offset += n - max; > - > - memcpy (value_contents_raw (retval), > - value_contents_all (value) + subobj_byte_offset, len); > - } > - break; > - > - case DWARF_VALUE_LITERAL: > - { > - bfd_byte *contents; > - size_t n = TYPE_LENGTH (subobj_type); > - > - if (subobj_byte_offset + n > ctx.len) > - invalid_synthetic_pointer (); > - > - free_values.free_to_mark (); > - retval = allocate_value (subobj_type); > - contents = value_contents_raw (retval); > - memcpy (contents, ctx.data + subobj_byte_offset, n); > - } > - break; > - > - case DWARF_VALUE_OPTIMIZED_OUT: > - free_values.free_to_mark (); > - retval = allocate_optimized_out_value (subobj_type); > - break; > - > - /* DWARF_VALUE_IMPLICIT_POINTER was converted to a pieced > - operation by execute_stack_op. */ > - case DWARF_VALUE_IMPLICIT_POINTER: > - /* DWARF_VALUE_OPTIMIZED_OUT can't occur in this context -- > - it can only be encountered when making a piece. */ > - default: > - internal_error (__FILE__, __LINE__, _("invalid location type")); > - } > - } > - > - set_value_initialized (retval, ctx.initialized); > + /* We need to clean up all the values that are not needed any more. > + The problem with a value_ref_ptr class is that it disconnects the > + RETVAL from the value garbage collection, so we need to make > + a copy of that value on the stack to keep everything consistent. > + The value_ref_ptr will clean up after itself at the end of this block. */ > + value_ref_ptr value_holder = value_ref_ptr::new_reference (retval); > + free_values.free_to_mark (); > > - return retval; > + return value_copy(retval); Missing space. Simon