Re: [PATCH 15/43] Make DWARF evaluator return a single struct value

Simon Marchi via Gdb-patches
2021-04-28T02:21:33Z

> @@ -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
> + <optimized out> instead of <not saved>. */
> + 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
> - <optimized out> instead of <not saved>. */
> - 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