[PATCH] Big-endian targets: Don't ignore offset into DW_OP_stack_value

[PATCH] Big-endian targets: Don't ignore offset into DW_OP_stack_value

[Andreas Arnez]

Recently I fixed a bug that caused a DW_OP_implicit_pointer with non-zero
offset into a DW_OP_implicit_value to be handled incorrectly on big-endian
targets.  GDB ignored the offset and copied the wrong bytes:

  https://sourceware.org/ml/gdb-patches/2017-01/msg00251.html

But there is still a similar issue when a DW_OP_implicit_pointer points
into a DW_OP_stack_value instead; and again, the offset is ignored.  There
is an important difference, though: While implicit values are treated like
blocks of data and anchored at the lowest-addressed byte, stack values
traditionally contain integer numbers and are anchored at the *least
significant* byte.  Also, stack values do not come in varying sizes, but
are cut down appropriately when used.  Thus, on big-endian targets the
scenario looks like this (higher addresses shown right):

  |<- - - - - Stack value - - - - - - ->|
                  |                     |
                  |<- original object ->|
                  |
                  | offset ->|####|
			      ^^^^
                              de-referenced
			      implicit pointer

(Note how the original object's size influences the position of the
de-referenced implicit pointer within the stack value.  This is not the
case for little-endian targets, where the original object starts at offset
zero within the stack value.)

This patch implements the logic indicated in the above diagram and adds an
appropriate test case.  A new function dwarf2_fetch_die_type_sect_off is
added; it is used for retrieving the original object's type, so its size
can be determined.  That type is passed to dwarf2_evaluate_loc_desc_full
via a new parameter.

gdb/ChangeLog:

	* dwarf2loc.c (indirect_synthetic_pointer): Get data type of
	pointed-to DIE and pass it to dwarf2_evaluate_loc_desc_full.
	(dwarf2_evaluate_loc_desc_full): Add parameter 'orig_type'.  Fix
	the handling of DWARF_VALUE_STACK on big-endian targets when
	coming via an implicit pointer.
	(dwarf2_evaluate_loc_desc): Adjust call to
	dwarf2_evaluate_loc_desc_full.
	* dwarf2loc.h (dwarf2_fetch_die_type_sect_off): New declaration.
	* dwarf2read.c (dwarf2_fetch_die_type_sect_off): New function.

gdb/testsuite/ChangeLog:

  Andreas Arnez  <arnez@linux.vnet.ibm.com>

	* gdb.dwarf2/nonvar-access.exp: Add test for stack value location
	and implicit pointer into such a location.
---
 gdb/dwarf2loc.c                            | 39 +++++++++++++++---------------
 gdb/dwarf2loc.h                            |  3 +++
 gdb/dwarf2read.c                           | 25 +++++++++++++++++++
 gdb/testsuite/gdb.dwarf2/nonvar-access.exp | 26 +++++++++++++++++++-
 4 files changed, 72 insertions(+), 21 deletions(-)

diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index c1e02eb..65479d8 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -50,7 +50,8 @@ static struct value *dwarf2_evaluate_loc_desc_full (struct type *type,
 						    const gdb_byte *data,
 						    size_t size,
 						    struct dwarf2_per_cu_data *per_cu,
-						    LONGEST byte_offset);
+						    LONGEST byte_offset,
+						    struct type *orig_type);
 
 static struct call_site_parameter *dwarf_expr_reg_to_entry_parameter
     (struct frame_info *frame,
@@ -2098,13 +2099,16 @@ indirect_synthetic_pointer (sect_offset die, LONGEST byte_offset,
     = dwarf2_fetch_die_loc_sect_off (die, per_cu,
 				     get_frame_address_in_block_wrapper, frame);
 
+  /* Get type of pointed-to DIE.  */
+  struct type *orig_type = dwarf2_fetch_die_type_sect_off (die, per_cu);
+
   /* If pointed-to DIE has a DW_AT_location, evaluate it and return the
      resulting value.  Otherwise, it may have a DW_AT_const_value instead,
      or it may've been optimized out.  */
   if (baton.data != NULL)
     return dwarf2_evaluate_loc_desc_full (TYPE_TARGET_TYPE (type), frame,
 					  baton.data, baton.size, baton.per_cu,
-					  byte_offset);
+					  byte_offset, orig_type);
   else
     return fetch_const_value_from_synthetic_pointer (die, byte_offset, per_cu,
 						     type);
@@ -2269,7 +2273,7 @@ static struct value *
 dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
 			       const gdb_byte *data, size_t size,
 			       struct dwarf2_per_cu_data *per_cu,
-			       LONGEST byte_offset)
+			       LONGEST byte_offset, struct type *orig_type)
 {
   struct value *retval;
   struct objfile *objfile = dwarf2_per_cu_objfile (per_cu);
@@ -2404,18 +2408,15 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
 	case DWARF_VALUE_STACK:
 	  {
 	    struct value *value = ctx.fetch (0);
-	    gdb_byte *contents;
-	    const gdb_byte *val_bytes;
 	    size_t n = TYPE_LENGTH (value_type (value));
+	    size_t len = TYPE_LENGTH (type);
+	    size_t max = orig_type ? TYPE_LENGTH (orig_type) : len;
+	    struct gdbarch *objfile_gdbarch = get_objfile_arch (objfile);
 	    struct cleanup *cleanup;
 
-	    if (byte_offset + TYPE_LENGTH (type) > n)
+	    if (byte_offset + len > max)
 	      invalid_synthetic_pointer ();
 
-	    val_bytes = value_contents_all (value);
-	    val_bytes += byte_offset;
-	    n -= byte_offset;
-
 	    /* Preserve VALUE because we are going to free values back
 	       to the mark, but we still need the value contents
 	       below.  */
@@ -2424,16 +2425,13 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
 	    cleanup = make_cleanup_value_free (value);
 
 	    retval = allocate_value (type);
-	    contents = value_contents_raw (retval);
-	    if (n > TYPE_LENGTH (type))
-	      {
-		struct gdbarch *objfile_gdbarch = get_objfile_arch (objfile);
 
-		if (gdbarch_byte_order (objfile_gdbarch) == BFD_ENDIAN_BIG)
-		  val_bytes += n - TYPE_LENGTH (type);
-		n = TYPE_LENGTH (type);
-	      }
-	    memcpy (contents, val_bytes, n);
+	    /* The given offset is relative to the actual object.  */
+	    if (gdbarch_byte_order (objfile_gdbarch) == BFD_ENDIAN_BIG)
+	      byte_offset += n - max;
+
+	    memcpy (value_contents_raw (retval),
+		    value_contents_all (value) + byte_offset, len);
 
 	    do_cleanups (cleanup);
 	  }
@@ -2482,7 +2480,8 @@ dwarf2_evaluate_loc_desc (struct type *type, struct frame_info *frame,
 			  const gdb_byte *data, size_t size,
 			  struct dwarf2_per_cu_data *per_cu)
 {
-  return dwarf2_evaluate_loc_desc_full (type, frame, data, size, per_cu, 0);
+  return dwarf2_evaluate_loc_desc_full (type, frame, data, size, per_cu,
+					0, NULL);
 }
 
 /* Evaluates a dwarf expression and stores the result in VAL, expecting
diff --git a/gdb/dwarf2loc.h b/gdb/dwarf2loc.h
index d6cdbd2..d669d1e 100644
--- a/gdb/dwarf2loc.h
+++ b/gdb/dwarf2loc.h
@@ -77,6 +77,9 @@ extern const gdb_byte *dwarf2_fetch_constant_bytes (sect_offset,
 						    struct obstack *,
 						    LONGEST *);
 
+struct type *dwarf2_fetch_die_type_sect_off (sect_offset,
+					     struct dwarf2_per_cu_data *);
+
 struct type *dwarf2_get_die_type (cu_offset die_offset,
 				  struct dwarf2_per_cu_data *per_cu);
 
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 774ed73..bdaac19 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -20258,6 +20258,31 @@ dwarf2_fetch_constant_bytes (sect_offset offset,
   return result;
 }
 
+/* Return the type of the die at OFFSET in PER_CU.  Return NULL if no
+   valid type for this die is found.  */
+
+struct type *
+dwarf2_fetch_die_type_sect_off (sect_offset offset,
+				struct dwarf2_per_cu_data *per_cu)
+{
+  struct dwarf2_cu *cu;
+  struct die_info *die;
+
+  dw2_setup (per_cu->objfile);
+
+  if (per_cu->cu == NULL)
+    load_cu (per_cu);
+  cu = per_cu->cu;
+  if (!cu)
+    return NULL;
+
+  die = follow_die_offset (offset, per_cu->is_dwz, &cu);
+  if (!die)
+    return NULL;
+
+  return die_type (die, cu);
+}
+
 /* Return the type of the DIE at DIE_OFFSET in the CU named by
    PER_CU.  */
 
diff --git a/gdb/testsuite/gdb.dwarf2/nonvar-access.exp b/gdb/testsuite/gdb.dwarf2/nonvar-access.exp
index 3cb5c49..633c6b3 100644
--- a/gdb/testsuite/gdb.dwarf2/nonvar-access.exp
+++ b/gdb/testsuite/gdb.dwarf2/nonvar-access.exp
@@ -33,7 +33,7 @@ Dwarf::assemble $asm_file {
 	} {
 	    declare_labels int_type_label char_type_label \
 		struct_s_label struct_t_label array_a9_label \
-		char_ptr_label implicit_a_label
+		char_ptr_label implicit_a_label stack_b_label
 
 	    int_type_label: base_type {
 		{name "int"}
@@ -162,6 +162,23 @@ Dwarf::assemble $asm_file {
 			GNU_implicit_pointer $implicit_a_label 5
 		    } SPECIAL_expr}
 		}
+		# Stack-value location.
+		stack_b_label: DW_TAG_variable {
+		    {name def_stack_b}
+		    {type :$struct_t_label}
+		    {location {
+			const4u 0x1a2b3c4d
+			stack_value
+		    } SPECIAL_expr}
+		}
+		# Implicit pointer into stack value.
+		DW_TAG_variable {
+		    {name implicit_b_ptr}
+		    {type :$char_ptr_label}
+		    {location {
+			GNU_implicit_pointer $stack_b_label 1
+		    } SPECIAL_expr}
+		}
 	    }
 	}
     }
@@ -194,6 +211,13 @@ gdb_test "print/x def_implicit_a" \
 gdb_test "print/x def_implicit_a\[5\]" " = 0x56"
 gdb_test "print/x *(char (*)\[5\]) implicit_a_ptr" \
     " = \\{0x56, 0x67, 0x78, 0x89, 0x9a\\}"
+switch $endian {
+    big {set val "a = 52, b = 2833485"}
+    little {set val "a = 77, b = 857502"}
+}
+gdb_test "print def_stack_b" " = \\{$val\\}"
+switch $endian {big {set val 0x2b} little {set val 0x3c}}
+gdb_test "print/x *implicit_b_ptr" " = $val"
 
 # Byte-aligned fields, pieced together from DWARF stack values.
 gdb_test "print def_s" " = \\{a = 0, b = -1\\}"
-- 
2.5.0

Re: [PATCH] Big-endian targets: Don't ignore offset into DW_OP_stack_value

[Ulrich Weigand]

Andreas Arnez wrote:

> Recently I fixed a bug that caused a DW_OP_implicit_pointer with non-zero
> offset into a DW_OP_implicit_value to be handled incorrectly on big-endian
> targets.  GDB ignored the offset and copied the wrong bytes:
> 
>   https://sourceware.org/ml/gdb-patches/2017-01/msg00251.html
> 
> But there is still a similar issue when a DW_OP_implicit_pointer points
> into a DW_OP_stack_value instead; and again, the offset is ignored.  There
> is an important difference, though: While implicit values are treated like
> blocks of data and anchored at the lowest-addressed byte, stack values
> traditionally contain integer numbers and are anchored at the *least
> significant* byte.  Also, stack values do not come in varying sizes, but
> are cut down appropriately when used.  Thus, on big-endian targets the
> scenario looks like this (higher addresses shown right):
> 
>   |<- - - - - Stack value - - - - - - ->|
>                   |                     |
>                   |<- original object ->|
>                   |
>                   | offset ->|####|
> 			      ^^^^
>                               de-referenced
> 			      implicit pointer
> 
> (Note how the original object's size influences the position of the
> de-referenced implicit pointer within the stack value.  This is not the
> case for little-endian targets, where the original object starts at offset
> zero within the stack value.)
> 
> This patch implements the logic indicated in the above diagram and adds an
> appropriate test case.  A new function dwarf2_fetch_die_type_sect_off is
> added; it is used for retrieving the original object's type, so its size
> can be determined.  That type is passed to dwarf2_evaluate_loc_desc_full
> via a new parameter.

This makes sense to me.



> @@ -50,7 +50,8 @@ static struct value *dwarf2_evaluate_loc_desc_full (struct type *type,
>  						    const gdb_byte *data,
>  						    size_t size,
>  						    struct dwarf2_per_cu_data *per_cu,
> -						    LONGEST byte_offset);
> +						    LONGEST byte_offset,
> +						    struct type *orig_type);

Please update the comment to indicate the precise meaning of the ORIG_TYPE
parameter, and when it has to be specified and when it may be omitted.

Thinking about this more, maybe it would be clearer to swap around the
two types, and have the semantics of the function be something like:

/* Evaluate a location description, starting at DATA and with length
   SIZE, to find the current location of variable of TYPE in the
   context of FRAME.  If SUBOBJ_TYPE is non-NULL, return instead the
   location of the subobject of type SUBOBJ_TYPE at byte offset
   SUBOBJ_BYTE_OFFSET within the variable of type TYPE.  */


Otherwise the patch looks good to me.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: [PATCH] Big-endian targets: Don't ignore offset into DW_OP_stack_value

[Andreas Arnez]

On Mon, Mar 13 2017, Ulrich Weigand wrote:

> Andreas Arnez wrote:
>

[...]

>> @@ -50,7 +50,8 @@ static struct value *dwarf2_evaluate_loc_desc_full (struct type *type,
>>  						    const gdb_byte *data,
>>  						    size_t size,
>>  						    struct dwarf2_per_cu_data *per_cu,
>> -						    LONGEST byte_offset);
>> +						    LONGEST byte_offset,
>> +						    struct type *orig_type);
>
> Please update the comment to indicate the precise meaning of the ORIG_TYPE
> parameter, and when it has to be specified and when it may be omitted.
>
> Thinking about this more, maybe it would be clearer to swap around the
> two types, and have the semantics of the function be something like:
>
> /* Evaluate a location description, starting at DATA and with length
>    SIZE, to find the current location of variable of TYPE in the
>    context of FRAME.  If SUBOBJ_TYPE is non-NULL, return instead the
>    location of the subobject of type SUBOBJ_TYPE at byte offset
>    SUBOBJ_BYTE_OFFSET within the variable of type TYPE.  */

OK.  Of course this increases the amount of change a bit.  New patch
below.

Ready to apply?

--
Andreas

-- >8 --
Subject: [PATCH v2] Big-endian targets: Don't ignore offset into DW_OP_stack_value

Recently I fixed a bug that caused a DW_OP_implicit_pointer with non-zero
offset into a DW_OP_implicit_value to be handled incorrectly on big-endian
targets.  GDB ignored the offset and copied the wrong bytes:

  https://sourceware.org/ml/gdb-patches/2017-01/msg00251.html

But there is still a similar issue when a DW_OP_implicit_pointer points
into a DW_OP_stack_value instead; and again, the offset is ignored.  There
is an important difference, though: While implicit values are treated like
blocks of data and anchored at the lowest-addressed byte, stack values
traditionally contain integer numbers and are anchored at the *least
significant* byte.  Also, stack values do not come in varying sizes, but
are cut down appropriately when used.  Thus, on big-endian targets the
scenario looks like this (higher addresses shown right):

  |<- - - - - Stack value - - - - - - ->|
                  |                     |
                  |<- original object ->|
                  |
                  | offset ->|####|
			      ^^^^
                              de-referenced
			      implicit pointer

(Note how the original object's size influences the position of the
de-referenced implicit pointer within the stack value.  This is not the
case for little-endian targets, where the original object starts at offset
zero within the stack value.)

This patch implements the logic indicated in the above diagram and adds an
appropriate test case.  A new function dwarf2_fetch_die_type_sect_off is
added; it is used for retrieving the original object's type, so its size
can be determined.  That type is passed to dwarf2_evaluate_loc_desc_full
via a new parameter.

gdb/ChangeLog:

	* dwarf2loc.c (indirect_synthetic_pointer): Get data type of
	pointed-to DIE and pass it to dwarf2_evaluate_loc_desc_full.
	(dwarf2_evaluate_loc_desc_full): New parameter subobj_type; rename
	byte_offset to subobj_byte_offset.  Fix the handling of
	DWARF_VALUE_STACK on big-endian targets when coming via an
	implicit pointer.
	(dwarf2_evaluate_loc_desc): Adjust call to
	dwarf2_evaluate_loc_desc_full.
	* dwarf2loc.h (dwarf2_fetch_die_type_sect_off): New declaration.
	* dwarf2read.c (dwarf2_fetch_die_type_sect_off): New function.

gdb/testsuite/ChangeLog:

	* gdb.dwarf2/nonvar-access.exp: Add test for stack value location
	and implicit pointer into such a location.
---
 gdb/dwarf2loc.c                            | 97 +++++++++++++++++-------------
 gdb/dwarf2loc.h                            |  3 +
 gdb/dwarf2read.c                           | 25 ++++++++
 gdb/testsuite/gdb.dwarf2/nonvar-access.exp | 26 +++++++-
 4 files changed, 107 insertions(+), 44 deletions(-)

diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 4393c1f..900278e 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -50,7 +50,8 @@ static struct value *dwarf2_evaluate_loc_desc_full (struct type *type,
 						    const gdb_byte *data,
 						    size_t size,
 						    struct dwarf2_per_cu_data *per_cu,
-						    LONGEST byte_offset);
+						    struct type *subobj_type,
+						    LONGEST subobj_byte_offset);
 
 static struct call_site_parameter *dwarf_expr_reg_to_entry_parameter
     (struct frame_info *frame,
@@ -2163,12 +2164,16 @@ indirect_synthetic_pointer (sect_offset die, LONGEST byte_offset,
     = dwarf2_fetch_die_loc_sect_off (die, per_cu,
 				     get_frame_address_in_block_wrapper, frame);
 
+  /* Get type of pointed-to DIE.  */
+  struct type *orig_type = dwarf2_fetch_die_type_sect_off (die, per_cu);
+
   /* If pointed-to DIE has a DW_AT_location, evaluate it and return the
      resulting value.  Otherwise, it may have a DW_AT_const_value instead,
      or it may've been optimized out.  */
   if (baton.data != NULL)
-    return dwarf2_evaluate_loc_desc_full (TYPE_TARGET_TYPE (type), frame,
-					  baton.data, baton.size, baton.per_cu,
+    return dwarf2_evaluate_loc_desc_full (orig_type, frame, baton.data,
+					  baton.size, baton.per_cu,
+					  TYPE_TARGET_TYPE (type),
 					  byte_offset);
   else
     return fetch_const_value_from_synthetic_pointer (die, byte_offset, per_cu,
@@ -2327,23 +2332,30 @@ static const struct lval_funcs pieced_value_funcs = {
 
 /* Evaluate a location description, starting at DATA and with length
    SIZE, to find the current location of variable of TYPE in the
-   context of FRAME.  BYTE_OFFSET is applied after the contents are
-   computed.  */
+   context of FRAME.  If SUBOBJ_TYPE is non-NULL, return instead the
+   location of the subobject of type SUBOBJ_TYPE at byte offset
+   SUBOBJ_BYTE_OFFSET within the variable of type TYPE.  */
 
 static struct value *
 dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
 			       const gdb_byte *data, size_t size,
 			       struct dwarf2_per_cu_data *per_cu,
-			       LONGEST byte_offset)
+			       struct type *subobj_type,
+			       LONGEST subobj_byte_offset)
 {
   struct value *retval;
   struct objfile *objfile = dwarf2_per_cu_objfile (per_cu);
 
-  if (byte_offset < 0)
+  if (subobj_type == NULL)
+    {
+      subobj_type = type;
+      subobj_byte_offset = 0;
+    }
+  else if (subobj_byte_offset < 0)
     invalid_synthetic_pointer ();
 
   if (size == 0)
-    return allocate_optimized_out_value (type);
+    return allocate_optimized_out_value (subobj_type);
 
   dwarf_evaluate_loc_desc ctx;
   ctx.frame = frame;
@@ -2366,8 +2378,9 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
       if (ex.error == NOT_AVAILABLE_ERROR)
 	{
 	  free_values.free_to_mark ();
-	  retval = allocate_value (type);
-	  mark_value_bytes_unavailable (retval, 0, TYPE_LENGTH (type));
+	  retval = allocate_value (subobj_type);
+	  mark_value_bytes_unavailable (retval, 0,
+					TYPE_LENGTH (subobj_type));
 	  return retval;
 	}
       else if (ex.error == NO_ENTRY_VALUE_ERROR)
@@ -2375,7 +2388,7 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
 	  if (entry_values_debug)
 	    exception_print (gdb_stdout, ex);
 	  free_values.free_to_mark ();
-	  return allocate_optimized_out_value (type);
+	  return allocate_optimized_out_value (subobj_type);
 	}
       else
 	throw_exception (ex);
@@ -2390,7 +2403,7 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
 
       for (i = 0; i < ctx.num_pieces; ++i)
 	bit_size += ctx.pieces[i].size;
-      if (8 * (byte_offset + TYPE_LENGTH (type)) > bit_size)
+      if (8 * (subobj_byte_offset + TYPE_LENGTH (subobj_type)) > bit_size)
 	invalid_synthetic_pointer ();
 
       c = allocate_piece_closure (per_cu, ctx.num_pieces, ctx.pieces,
@@ -2398,8 +2411,9 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *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 (type, &pieced_value_funcs, c);
-      set_value_offset (retval, byte_offset);
+      retval = allocate_computed_value (subobj_type,
+					&pieced_value_funcs, c);
+      set_value_offset (retval, subobj_byte_offset);
     }
   else
     {
@@ -2412,10 +2426,10 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
 	      = longest_to_int (value_as_long (ctx.fetch (0)));
 	    int gdb_regnum = dwarf_reg_to_regnum_or_error (arch, dwarf_regnum);
 
-	    if (byte_offset != 0)
+	    if (subobj_byte_offset != 0)
 	      error (_("cannot use offset on synthetic pointer to register"));
 	    free_values.free_to_mark ();
-	    retval = value_from_register (type, gdb_regnum, frame);
+	    retval = value_from_register (subobj_type, gdb_regnum, frame);
 	    if (value_optimized_out (retval))
 	      {
 		struct value *tmp;
@@ -2426,8 +2440,9 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
 		   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 (type);
-		value_contents_copy (tmp, 0, retval, 0, TYPE_LENGTH (type));
+		tmp = allocate_value (subobj_type);
+		value_contents_copy (tmp, 0, retval, 0,
+				     TYPE_LENGTH (subobj_type));
 		retval = tmp;
 	      }
 	  }
@@ -2447,7 +2462,7 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
 	       the operation.  Therefore, we do the conversion here
 	       since the type is readily available.  */
 
-	    switch (TYPE_CODE (type))
+	    switch (TYPE_CODE (subobj_type))
 	      {
 		case TYPE_CODE_FUNC:
 		case TYPE_CODE_METHOD:
@@ -2460,7 +2475,8 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
 	    address = value_as_address (value_from_pointer (ptr_type, address));
 
 	    free_values.free_to_mark ();
-	    retval = value_at_lazy (type, address + byte_offset);
+	    retval = value_at_lazy (subobj_type,
+				    address + subobj_byte_offset);
 	    if (in_stack_memory)
 	      set_value_stack (retval, 1);
 	  }
@@ -2469,18 +2485,15 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
 	case DWARF_VALUE_STACK:
 	  {
 	    struct value *value = ctx.fetch (0);
-	    gdb_byte *contents;
-	    const gdb_byte *val_bytes;
 	    size_t n = TYPE_LENGTH (value_type (value));
+	    size_t len = TYPE_LENGTH (subobj_type);
+	    size_t max = type != NULL ? TYPE_LENGTH (type) : n;
+	    struct gdbarch *objfile_gdbarch = get_objfile_arch (objfile);
 	    struct cleanup *cleanup;
 
-	    if (byte_offset + TYPE_LENGTH (type) > n)
+	    if (subobj_byte_offset + len > max)
 	      invalid_synthetic_pointer ();
 
-	    val_bytes = value_contents_all (value);
-	    val_bytes += byte_offset;
-	    n -= byte_offset;
-
 	    /* Preserve VALUE because we are going to free values back
 	       to the mark, but we still need the value contents
 	       below.  */
@@ -2488,17 +2501,14 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
 	    free_values.free_to_mark ();
 	    cleanup = make_cleanup_value_free (value);
 
-	    retval = allocate_value (type);
-	    contents = value_contents_raw (retval);
-	    if (n > TYPE_LENGTH (type))
-	      {
-		struct gdbarch *objfile_gdbarch = get_objfile_arch (objfile);
+	    retval = allocate_value (subobj_type);
 
-		if (gdbarch_byte_order (objfile_gdbarch) == BFD_ENDIAN_BIG)
-		  val_bytes += n - TYPE_LENGTH (type);
-		n = TYPE_LENGTH (type);
-	      }
-	    memcpy (contents, val_bytes, n);
+	    /* 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);
 
 	    do_cleanups (cleanup);
 	  }
@@ -2507,21 +2517,21 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
 	case DWARF_VALUE_LITERAL:
 	  {
 	    bfd_byte *contents;
-	    size_t n = TYPE_LENGTH (type);
+	    size_t n = TYPE_LENGTH (subobj_type);
 
-	    if (byte_offset + n > ctx.len)
+	    if (subobj_byte_offset + n > ctx.len)
 	      invalid_synthetic_pointer ();
 
 	    free_values.free_to_mark ();
-	    retval = allocate_value (type);
+	    retval = allocate_value (subobj_type);
 	    contents = value_contents_raw (retval);
-	    memcpy (contents, ctx.data + byte_offset, n);
+	    memcpy (contents, ctx.data + subobj_byte_offset, n);
 	  }
 	  break;
 
 	case DWARF_VALUE_OPTIMIZED_OUT:
 	  free_values.free_to_mark ();
-	  retval = allocate_optimized_out_value (type);
+	  retval = allocate_optimized_out_value (subobj_type);
 	  break;
 
 	  /* DWARF_VALUE_IMPLICIT_POINTER was converted to a pieced
@@ -2547,7 +2557,8 @@ dwarf2_evaluate_loc_desc (struct type *type, struct frame_info *frame,
 			  const gdb_byte *data, size_t size,
 			  struct dwarf2_per_cu_data *per_cu)
 {
-  return dwarf2_evaluate_loc_desc_full (type, frame, data, size, per_cu, 0);
+  return dwarf2_evaluate_loc_desc_full (type, frame, data, size, per_cu,
+					NULL, 0);
 }
 
 /* Evaluates a dwarf expression and stores the result in VAL, expecting
diff --git a/gdb/dwarf2loc.h b/gdb/dwarf2loc.h
index 1f3e20e..9063b6e 100644
--- a/gdb/dwarf2loc.h
+++ b/gdb/dwarf2loc.h
@@ -79,6 +79,9 @@ extern const gdb_byte *dwarf2_fetch_constant_bytes (sect_offset,
 						    struct obstack *,
 						    LONGEST *);
 
+struct type *dwarf2_fetch_die_type_sect_off (sect_offset,
+					     struct dwarf2_per_cu_data *);
+
 struct type *dwarf2_get_die_type (cu_offset die_offset,
 				  struct dwarf2_per_cu_data *per_cu);
 
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 40b99d9..da70884 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -20775,6 +20775,31 @@ dwarf2_fetch_constant_bytes (sect_offset offset,
   return result;
 }
 
+/* Return the type of the die at OFFSET in PER_CU.  Return NULL if no
+   valid type for this die is found.  */
+
+struct type *
+dwarf2_fetch_die_type_sect_off (sect_offset offset,
+				struct dwarf2_per_cu_data *per_cu)
+{
+  struct dwarf2_cu *cu;
+  struct die_info *die;
+
+  dw2_setup (per_cu->objfile);
+
+  if (per_cu->cu == NULL)
+    load_cu (per_cu);
+  cu = per_cu->cu;
+  if (!cu)
+    return NULL;
+
+  die = follow_die_offset (offset, per_cu->is_dwz, &cu);
+  if (!die)
+    return NULL;
+
+  return die_type (die, cu);
+}
+
 /* Return the type of the DIE at DIE_OFFSET in the CU named by
    PER_CU.  */
 
diff --git a/gdb/testsuite/gdb.dwarf2/nonvar-access.exp b/gdb/testsuite/gdb.dwarf2/nonvar-access.exp
index 3cb5c49..633c6b3 100644
--- a/gdb/testsuite/gdb.dwarf2/nonvar-access.exp
+++ b/gdb/testsuite/gdb.dwarf2/nonvar-access.exp
@@ -33,7 +33,7 @@ Dwarf::assemble $asm_file {
 	} {
 	    declare_labels int_type_label char_type_label \
 		struct_s_label struct_t_label array_a9_label \
-		char_ptr_label implicit_a_label
+		char_ptr_label implicit_a_label stack_b_label
 
 	    int_type_label: base_type {
 		{name "int"}
@@ -162,6 +162,23 @@ Dwarf::assemble $asm_file {
 			GNU_implicit_pointer $implicit_a_label 5
 		    } SPECIAL_expr}
 		}
+		# Stack-value location.
+		stack_b_label: DW_TAG_variable {
+		    {name def_stack_b}
+		    {type :$struct_t_label}
+		    {location {
+			const4u 0x1a2b3c4d
+			stack_value
+		    } SPECIAL_expr}
+		}
+		# Implicit pointer into stack value.
+		DW_TAG_variable {
+		    {name implicit_b_ptr}
+		    {type :$char_ptr_label}
+		    {location {
+			GNU_implicit_pointer $stack_b_label 1
+		    } SPECIAL_expr}
+		}
 	    }
 	}
     }
@@ -194,6 +211,13 @@ gdb_test "print/x def_implicit_a" \
 gdb_test "print/x def_implicit_a\[5\]" " = 0x56"
 gdb_test "print/x *(char (*)\[5\]) implicit_a_ptr" \
     " = \\{0x56, 0x67, 0x78, 0x89, 0x9a\\}"
+switch $endian {
+    big {set val "a = 52, b = 2833485"}
+    little {set val "a = 77, b = 857502"}
+}
+gdb_test "print def_stack_b" " = \\{$val\\}"
+switch $endian {big {set val 0x2b} little {set val 0x3c}}
+gdb_test "print/x *implicit_b_ptr" " = $val"
 
 # Byte-aligned fields, pieced together from DWARF stack values.
 gdb_test "print def_s" " = \\{a = 0, b = -1\\}"
-- 
2.5.0

Re: [PATCH] Big-endian targets: Don't ignore offset into DW_OP_stack_value

[Ulrich Weigand]

Andreas Arnez wrote:
> On Mon, Mar 13 2017, Ulrich Weigand wrote:
> > Thinking about this more, maybe it would be clearer to swap around the
> > two types, and have the semantics of the function be something like:
> >
> > /* Evaluate a location description, starting at DATA and with length
> >    SIZE, to find the current location of variable of TYPE in the
> >    context of FRAME.  If SUBOBJ_TYPE is non-NULL, return instead the
> >    location of the subobject of type SUBOBJ_TYPE at byte offset
> >    SUBOBJ_BYTE_OFFSET within the variable of type TYPE.  */
> 
> OK.  Of course this increases the amount of change a bit.  New patch
> below.

Well, yes ... I still think it is clearer overall in the end.

> +  /* Get type of pointed-to DIE.  */
> +  struct type *orig_type = dwarf2_fetch_die_type_sect_off (die, per_cu);
> +
>    /* If pointed-to DIE has a DW_AT_location, evaluate it and return the
>       resulting value.  Otherwise, it may have a DW_AT_const_value instead,
>       or it may've been optimized out.  */
>    if (baton.data != NULL)
> -    return dwarf2_evaluate_loc_desc_full (TYPE_TARGET_TYPE (type), frame,
> -					  baton.data, baton.size, baton.per_cu,
> +    return dwarf2_evaluate_loc_desc_full (orig_type, frame, baton.data,

Can orig_type ever be NULL here?  It probably would not be a good idea
to pass NULL as the type argument.

> -	    if (byte_offset != 0)
> +	    if (subobj_byte_offset != 0)
>  	      error (_("cannot use offset on synthetic pointer to register"));

As an aside: now that we have the full object type, we may actually be able
to lift that restriction (do a value_from_register on the full type, and
then extract the subobject).  Might be interesting to look into as a
follow-on change ...

>  	    size_t n = TYPE_LENGTH (value_type (value));
> +	    size_t len = TYPE_LENGTH (subobj_type);
> +	    size_t max = type != NULL ? TYPE_LENGTH (type) : n;

As mentioned above, I don't think it makes sense to allow a NULL type here.

Otherwise, this looks good to me.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: [PATCH] Big-endian targets: Don't ignore offset into DW_OP_stack_value

[Andreas Arnez]

On Wed, Mar 15 2017, Ulrich Weigand wrote:

> Andreas Arnez wrote:

[...]

>> +  /* Get type of pointed-to DIE.  */
>> +  struct type *orig_type = dwarf2_fetch_die_type_sect_off (die, per_cu);
>> +
>>    /* If pointed-to DIE has a DW_AT_location, evaluate it and return the
>>       resulting value.  Otherwise, it may have a DW_AT_const_value instead,
>>       or it may've been optimized out.  */
>>    if (baton.data != NULL)
>> -    return dwarf2_evaluate_loc_desc_full (TYPE_TARGET_TYPE (type), frame,
>> -					  baton.data, baton.size, baton.per_cu,
>> +    return dwarf2_evaluate_loc_desc_full (orig_type, frame, baton.data,
>
> Can orig_type ever be NULL here?  It probably would not be a good idea
> to pass NULL as the type argument.

From looking at the code, it looks like it can.  But only for very
special cases, it seems.  For instance, in case of a missing DW_AT_TYPE
attribute for the pointed-to die we get the type "void" instead.

>
>> -	    if (byte_offset != 0)
>> +	    if (subobj_byte_offset != 0)
>>  	      error (_("cannot use offset on synthetic pointer to register"));
>
> As an aside: now that we have the full object type, we may actually be able
> to lift that restriction (do a value_from_register on the full type, and
> then extract the subobject).  Might be interesting to look into as a
> follow-on change ...

Right, I'll look into that.

>
>>  	    size_t n = TYPE_LENGTH (value_type (value));
>> +	    size_t len = TYPE_LENGTH (subobj_type);
>> +	    size_t max = type != NULL ? TYPE_LENGTH (type) : n;
>
> As mentioned above, I don't think it makes sense to allow a NULL type here.

OK.  I'll adjust the patch with the delta-patch below.  Is that ready to
apply then?

--
Andreas


-- >8 --
1 file changed, 3 insertions(+), 1 deletion(-)
gdb/dwarf2loc.c | 4 +++-

modified   gdb/dwarf2loc.c
@@ -2166,6 +2166,8 @@ indirect_synthetic_pointer (sect_offset die, LONGEST byte_offset,
 
   /* Get type of pointed-to DIE.  */
   struct type *orig_type = dwarf2_fetch_die_type_sect_off (die, per_cu);
+  if (orig_type == NULL)
+    invalid_synthetic_pointer ();
 
   /* If pointed-to DIE has a DW_AT_location, evaluate it and return the
      resulting value.  Otherwise, it may have a DW_AT_const_value instead,
@@ -2487,7 +2489,7 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
 	    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 != NULL ? TYPE_LENGTH (type) : n;
+	    size_t max = TYPE_LENGTH (type);
 	    struct gdbarch *objfile_gdbarch = get_objfile_arch (objfile);
 	    struct cleanup *cleanup;
 

Re: [PATCH] Big-endian targets: Don't ignore offset into DW_OP_stack_value

[Ulrich Weigand]

Andreas Arnez wrote:
> On Wed, Mar 15 2017, Ulrich Weigand wrote:
> > As mentioned above, I don't think it makes sense to allow a NULL type here.
> 
> OK.  I'll adjust the patch with the delta-patch below.  Is that ready to
> apply then?

Yes, this is OK.

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: [PATCH] Big-endian targets: Don't ignore offset into DW_OP_stack_value

[Andreas Arnez]

On Thu, Mar 16 2017, Ulrich Weigand wrote:

> Andreas Arnez wrote:
>> On Wed, Mar 15 2017, Ulrich Weigand wrote:
>> > As mentioned above, I don't think it makes sense to allow a NULL type here.
>> 
>> OK.  I'll adjust the patch with the delta-patch below.  Is that ready to
>> apply then?
>
> Yes, this is OK.

Thanks, pushed.

--
Andreas