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

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

> @@ -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