public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Big-endian targets: don't ignore offset into DW_OP_implicit_value
@ 2017-01-12 19:24 Andreas Arnez
  2017-01-19 17:44 ` Yao Qi
  2017-01-25 22:12 ` [PATCH] Big-endian targets: don't ignore offset into DW_OP_implicit_value Yao Qi
  0 siblings, 2 replies; 12+ messages in thread
From: Andreas Arnez @ 2017-01-12 19:24 UTC (permalink / raw)
  To: gdb-patches; +Cc: Jan Kratochvil

When a variable's location is expressed as DW_OP_implicit_value, but the
given value is longer than needed, which bytes should be used?  GDB's
current logic was introduced with a patch from 2011 and uses the "least
significant" bytes:

  https://sourceware.org/ml/gdb-patches/2011-08/msg00123.html

Now consider a sub-value from such a location at a given offset, accessed
through DW_OP_implicit_pointer.  Which bytes should be used for that?  The
patch above *always* uses the last bytes on big-endian targets, ignoring
the offset.

E.g., given the code snippet

  const char foo[] = "Hello, world!";
  const char *a = &foo[0];
  const char *b = &foo[7];

assume that `foo' is described as DW_OP_implicit_value and `a' and `b'
each as DW_OP_implicit_pointer into that value.  Then with current GDB
`*a' and `*b' yield the same result -- the string's zero terminator.

This patch basically reverts the portion of the patch above that deals
with DW_OP_implicit_value.  This fixes the offset handling and also goes
back to dropping the last instead of the first bytes on big-endian targets
if the implicit value is longer than needed.  The latter aspect of the
change probably doesn't matter for actual programs, but simplifies the
logic.

The patch also cleans up the original code a bit and adds appropriate test
cases.

gdb/testsuite/ChangeLog:

	* gdb.dwarf2/dw2-op-stack-value.exp: Adjust expected result of
	taking a 2-byte value out of a 4-byte DWARF implicit value on
	big-endian targets.
	* gdb.dwarf2/nonvar-access.exp: Add more comments to existing
	logic.  Add test cases for DW_OP_implicit.
	* lib/dwarf.exp (_location): Add handling for
	DW_OP_implicit_value.

gdb/ChangeLog:

	* dwarf2loc.c (dwarf2_evaluate_loc_desc_full): For
	DWARF_VALUE_LITERAL, no longer ignore the offset on big-endian
	targets.  And if the implicit value is longer than needed, extract
	the first bytes instead of the "least significant" ones.
---
 gdb/dwarf2loc.c                                 | 19 +-----
 gdb/testsuite/gdb.dwarf2/dw2-op-stack-value.exp |  2 +-
 gdb/testsuite/gdb.dwarf2/nonvar-access.exp      | 78 ++++++++++++++++++++++++-
 gdb/testsuite/lib/dwarf.exp                     | 19 ++++++
 4 files changed, 99 insertions(+), 19 deletions(-)

diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 7fca76b..a592b66 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -2442,28 +2442,15 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
 	case DWARF_VALUE_LITERAL:
 	  {
 	    bfd_byte *contents;
-	    const bfd_byte *ldata;
-	    size_t n = ctx.len;
+	    size_t n = TYPE_LENGTH (type);
 
-	    if (byte_offset + TYPE_LENGTH (type) > n)
+	    if (byte_offset + n > ctx.len)
 	      invalid_synthetic_pointer ();
 
 	    free_values.free_to_mark ();
 	    retval = allocate_value (type);
 	    contents = value_contents_raw (retval);
-
-	    ldata = ctx.data + byte_offset;
-	    n -= byte_offset;
-
-	    if (n > TYPE_LENGTH (type))
-	      {
-		struct gdbarch *objfile_gdbarch = get_objfile_arch (objfile);
-
-		if (gdbarch_byte_order (objfile_gdbarch) == BFD_ENDIAN_BIG)
-		  ldata += n - TYPE_LENGTH (type);
-		n = TYPE_LENGTH (type);
-	      }
-	    memcpy (contents, ldata, n);
+	    memcpy (contents, ctx.data + byte_offset, n);
 	  }
 	  break;
 
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-op-stack-value.exp b/gdb/testsuite/gdb.dwarf2/dw2-op-stack-value.exp
index c28dcca..808f983 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-op-stack-value.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-op-stack-value.exp
@@ -45,7 +45,7 @@ gdb_test_multiple $test $test {
     -re ":\[ \t\]*0xaa551234\r\n$gdb_prompt $" {
 	# big endian
 	pass $test
-	gdb_test "p/x implicit4to2" " = 0x3344"
+	gdb_test "p/x implicit4to2" " = 0x1122"
 	gdb_test "p/x implicit4to4" " = 0x11223344"
     }
     -re ":\[ \t\]*0x\[0-9a-f\]{8}\r\n$gdb_prompt $" {
diff --git a/gdb/testsuite/gdb.dwarf2/nonvar-access.exp b/gdb/testsuite/gdb.dwarf2/nonvar-access.exp
index 506ff9e..f47df70 100644
--- a/gdb/testsuite/gdb.dwarf2/nonvar-access.exp
+++ b/gdb/testsuite/gdb.dwarf2/nonvar-access.exp
@@ -31,14 +31,23 @@ Dwarf::assemble $asm_file {
 	compile_unit {
 	    {DW_AT_name main.c}
 	} {
-	    declare_labels int_type_label short_type_label
-	    declare_labels struct_s_label struct_t_label
+	    declare_labels int_type_label char_type_label \
+		struct_s_label struct_t_label array_a9_label \
+		char_ptr_label implicit_a_label
 
 	    int_type_label: base_type {
 		{name "int"}
 		{encoding @DW_ATE_signed}
 		{byte_size 4 DW_FORM_sdata}
 	    }
+	    char_type_label: base_type {
+		{name "char"}
+		{encoding @DW_ATE_unsigned}
+		{byte_size 1 DW_FORM_sdata}
+	    }
+	    char_ptr_label: pointer_type {
+		{type :$char_type_label}
+	    }
 
 	    struct_s_label: structure_type {
 		{name s}
@@ -76,20 +85,32 @@ Dwarf::assemble $asm_file {
 		}
 	    }
 
+	    array_a9_label: array_type {
+		{type :$char_type_label}
+	    } {
+		subrange_type {
+		    {type :$int_type_label}
+		    {upper_bound 8 DW_FORM_udata}
+		}
+	    }
+
 	    DW_TAG_subprogram {
 		{name main}
 		{DW_AT_external 1 flag}
 		{low_pc [gdb_target_symbol main] DW_FORM_addr}
 		{high_pc [gdb_target_symbol main]+0x10000 DW_FORM_addr}
 	    } {
+		# Simple variable without location.
 		DW_TAG_variable {
 		    {name undef_int}
 		    {type :$int_type_label}
 		}
+		# Struct variable without location.
 		DW_TAG_variable {
 		    {name undef_s}
 		    {type :$struct_s_label}
 		}
+		# Composite location: byte-aligned pieces.
 		DW_TAG_variable {
 		    {name def_s}
 		    {type :$struct_s_label}
@@ -102,6 +123,7 @@ Dwarf::assemble $asm_file {
 			bit_piece 24 0
 		    } SPECIAL_expr}
 		}
+		# Composite location: non-byte-aligned pieces.
 		DW_TAG_variable {
 		    {name def_t}
 		    {type :$struct_t_label}
@@ -114,6 +136,32 @@ Dwarf::assemble $asm_file {
 			bit_piece 23 0
 		    } SPECIAL_expr}
 		}
+		# Implicit location: immediate value.
+		DW_TAG_variable {
+		    {name def_implicit_s}
+		    {type :$struct_s_label}
+		    {location {
+			implicit_value 0x12 0x34 0x56 0x78
+		    } SPECIAL_expr}
+		}
+		# Implicit location: immediate value for whole array, with
+		# excess bytes.
+		implicit_a_label: DW_TAG_variable {
+		    {name def_implicit_a}
+		    {type :$array_a9_label}
+		    {location {
+			implicit_value 0x01 0x12 0x23 0x34 0x45 \
+			    0x56 0x67 0x78 0x89 0x9a 0xab
+		    } SPECIAL_expr}
+		}
+		# Implicit pointer into immediate value.
+		DW_TAG_variable {
+		    {name implicit_a_ptr}
+		    {type :$char_ptr_label}
+		    {location {
+			GNU_implicit_pointer $implicit_a_label 5
+		    } SPECIAL_expr}
+		}
 	    }
 	}
     }
@@ -128,7 +176,33 @@ if ![runto_main] {
     return -1
 }
 
+# Determine endianness.
+set endian "little"
+gdb_test_multiple "show endian" "determine endianness" {
+    -re ".* (big|little) endian.*$gdb_prompt $" {
+	set endian $expect_out(1,string)
+	pass "endianness: $endian"
+    }
+}
+
+# Byte-aligned objects with simple location descriptions.
+switch $endian { big {set val 0x345678} little {set val 0x785634} }
+gdb_test "print/x def_implicit_s" " = \\{a = 0x12, b = $val\\}"
+gdb_test "print/x def_implicit_s.b" " = $val"
+gdb_test "print/x def_implicit_a" \
+    " = \\{0x1, 0x12, 0x23, 0x34, 0x45, 0x56, 0x67, 0x78, 0x89\\}"
+gdb_test "print/x def_implicit_a\[5\]" " = 0x56"
+gdb_test "print/x *(char (*)\[5\]) implicit_a_ptr" \
+    " = \\{0x56, 0x67, 0x78, 0x89, 0x9a\\}"
+
+# Byte-aligned fields, pieced together from DWARF stack values.
 gdb_test "print def_s" " = \\{a = 0, b = -1\\}"
+
+# Non-byte-aligned fields, pieced together from DWARF stack values.
 gdb_test "print def_t" " = \\{a = 0, b = -1\\}"
+
+# Simple variable without location.
 gdb_test "print undef_int" " = <optimized out>"
+
+# Member of a structure without location.
 gdb_test "print undef_s.a" " = <optimized out>"
diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index 9f5fa6c..6bd563a 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -906,6 +906,25 @@ namespace eval Dwarf {
 		    _op .2byte [lindex $line 1]
 		}
 
+		DW_OP_implicit_value {
+		    set l1 [new_label "value_start"]
+		    set l2 [new_label "value_end"]
+		    _op .uleb128 "$l2 - $l1"
+		    define_label $l1
+		    foreach value [lrange $line 1 end] {
+			switch -regexp -- $value {
+			    {^0x[[:xdigit:]]{2}$} {_op .byte $value}
+			    {^0x[[:xdigit:]]{4}$} {_op .2byte $value}
+			    {^0x[[:xdigit:]]{8}$} {_op .4byte $value}
+			    {^0x[[:xdigit:]]{16}$} {_op .8byte $value}
+			    default {
+				error "bad value '$value' in DW_OP_implicit_value"
+			    }
+			}
+		    }
+		    define_label $l2
+		}
+
 		DW_OP_GNU_implicit_pointer {
 		    if {[llength $line] != 3} {
 			error "usage: DW_OP_GNU_implicit_pointer LABEL OFFSET"
-- 
2.3.0

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Big-endian targets: don't ignore offset into DW_OP_implicit_value
  2017-01-12 19:24 [PATCH] Big-endian targets: don't ignore offset into DW_OP_implicit_value Andreas Arnez
@ 2017-01-19 17:44 ` Yao Qi
  2017-01-19 18:21   ` Andreas Arnez
  2017-01-25 22:12 ` [PATCH] Big-endian targets: don't ignore offset into DW_OP_implicit_value Yao Qi
  1 sibling, 1 reply; 12+ messages in thread
From: Yao Qi @ 2017-01-19 17:44 UTC (permalink / raw)
  To: Andreas Arnez; +Cc: gdb-patches, Jan Kratochvil

On 17-01-12 20:24:27, Andreas Arnez wrote:
> diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
> index 9f5fa6c..6bd563a 100644
> --- a/gdb/testsuite/lib/dwarf.exp
> +++ b/gdb/testsuite/lib/dwarf.exp
> @@ -906,6 +906,25 @@ namespace eval Dwarf {
>  		    _op .2byte [lindex $line 1]
>  		}
>  
> +		DW_OP_implicit_value {
> +		    set l1 [new_label "value_start"]
> +		    set l2 [new_label "value_end"]
> +		    _op .uleb128 "$l2 - $l1"
> +		    define_label $l1
> +		    foreach value [lrange $line 1 end] {
> +			switch -regexp -- $value {
> +			    {^0x[[:xdigit:]]{2}$} {_op .byte $value}

Nit: we could also match one digit to simplify the usage.  With your
patch, we need to use it like this,


+		    {DW_AT_location {
+			DW_OP_implicit_value 0x01 0x01 0x01 0x01
+		    } SPECIAL_expr}

but if we add "{^0x[[:xdigit:]]{2}$} {_op .byte $value}", the use
above can be simplified,

+                   {DW_AT_location {
+                       DW_OP_implicit_value 0x1 0x1 0x1 0x1
+                   } SPECIAL_expr}

> +			    {^0x[[:xdigit:]]{4}$} {_op .2byte $value}
> +			    {^0x[[:xdigit:]]{8}$} {_op .4byte $value}
> +			    {^0x[[:xdigit:]]{16}$} {_op .8byte $value}
> +			    default {
> +				error "bad value '$value' in DW_OP_implicit_value"
> +			    }
> +			}
> +		    }
> +		    define_label $l2
> +		}
> +

During the review, I am trying to use DW_OP_implicit_value in
gdb.dwarf2/implptr-64bit.exp, because I think we should use this new
added DW_OP as much as we can in existing test case.  I get some troubles
on using Dwarf::assemble for two cus.  I'll let you know once I resolve
the issues there.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Big-endian targets: don't ignore offset into DW_OP_implicit_value
  2017-01-19 17:44 ` Yao Qi
@ 2017-01-19 18:21   ` Andreas Arnez
  2017-01-24  9:42     ` [PATCH 0/2] Add DW_OP_implicit_value in dwarf assembler Yao Qi
  0 siblings, 1 reply; 12+ messages in thread
From: Andreas Arnez @ 2017-01-19 18:21 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches, Jan Kratochvil

On Thu, Jan 19 2017, Yao Qi wrote:

> Nit: we could also match one digit to simplify the usage.  With your
> patch, we need to use it like this,
>
>
> +		    {DW_AT_location {
> +			DW_OP_implicit_value 0x01 0x01 0x01 0x01
> +		    } SPECIAL_expr}

Right, or in this particular case you could say

      DW_OP_implicit_value 0x01010101

instead, because the number is endianness-symmetric ;-)

>
> but if we add "{^0x[[:xdigit:]]{2}$} {_op .byte $value}", the use
> above can be simplified,
>
> +                   {DW_AT_location {
> +                       DW_OP_implicit_value 0x1 0x1 0x1 0x1
> +                   } SPECIAL_expr}

You probably mean "{^0x[[:xdigit:]]{1,2}$} {_op .byte $value}"?  Sure,
will do.  The idea behind the existing logic is that the resulting
immediate value always has half as many bytes as there are hex digits.
But for single byte integers I agree that this is a bit picky and the
simplification is useful.  I would not extend this to multi-byte
integers, though, such as allowing 0x123456 or such.

> During the review, I am trying to use DW_OP_implicit_value in
> gdb.dwarf2/implptr-64bit.exp, because I think we should use this new
> added DW_OP as much as we can in existing test case.

Sounds good!

> I get some troubles on using Dwarf::assemble for two cus.  I'll let
> you know once I resolve the issues there.

OK, let's see how that goes.

--
Andreas

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 0/2] Add DW_OP_implicit_value in dwarf assembler
  2017-01-19 18:21   ` Andreas Arnez
@ 2017-01-24  9:42     ` Yao Qi
  2017-01-24  9:43       ` [PATCH 1/2] Handle DW_OP_GNU_implicit_pointer " Yao Qi
  2017-01-24  9:43       ` [PATCH 2/2] Use dwarf assembler in gdb.dwarf2/implptr-64bit.exp Yao Qi
  0 siblings, 2 replies; 12+ messages in thread
From: Yao Qi @ 2017-01-24  9:42 UTC (permalink / raw)
  To: gdb-patches; +Cc: arnez

Andreas posted a patch
https://sourceware.org/ml/gdb-patches/2017-01/msg00251.html and added
DW_OP_implicit_value in dwarf assembler.  I find DW_OP_implicit_value
can be used in gdb.dwarf2/implptr-64bit.exp, so that we can remove
implptr-64bit.S and use the generated .S file instead.

Patch 1 fixes a minor bug in dwarf assembler, and patch 2 does the
major work.  Run gdb.dwarf2/*.exp on i686-pc-linux-gnu.  There is
no change in the .S files generated by dwarf assembler.

*** BLURB HERE ***

Yao Qi (2):
  Handle DW_OP_GNU_implicit_pointer in dwarf assembler
  Use dwarf assembler in gdb.dwarf2/implptr-64bit.exp

 gdb/testsuite/gdb.dwarf2/implptr-64bit.S   | 226 -----------------------------
 gdb/testsuite/gdb.dwarf2/implptr-64bit.exp | 118 +++++++++++++--
 gdb/testsuite/lib/dwarf.exp                |  46 +++++-
 3 files changed, 153 insertions(+), 237 deletions(-)
 delete mode 100644 gdb/testsuite/gdb.dwarf2/implptr-64bit.S

-- 
1.9.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/2] Use dwarf assembler in gdb.dwarf2/implptr-64bit.exp
  2017-01-24  9:42     ` [PATCH 0/2] Add DW_OP_implicit_value in dwarf assembler Yao Qi
  2017-01-24  9:43       ` [PATCH 1/2] Handle DW_OP_GNU_implicit_pointer " Yao Qi
@ 2017-01-24  9:43       ` Yao Qi
  2017-01-24 19:21         ` Andreas Arnez
  1 sibling, 1 reply; 12+ messages in thread
From: Yao Qi @ 2017-01-24  9:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: arnez

This patch adds a DW_OP_implicit_value in dwarf assembler, and uses
dwarf assembler in implptr-64bit.exp.  Using dwarf assembler in
implptr-64bit.exp exposes some limitations in dwarf assembler,

 - some variables are not evaluated in the caller's context, so we
   can not pass variable to assembler, like this

       Dwarf::assemble $asm_file {

	cu {
	    version $dwarf_version
	    addr_size $addr_size
	    is_64 $is_64
	} {
	}

	and

	{DW_AT_type :$struct_label "DW_FORM_ref$ref_addr_size"}

   this limitation is fixed by adding "uplevel" and "subst".

 - dwarf assembler doesn't emit DW_FORM_ref_addr for label referencing.
   this limitation is fixed by adding a new character "%",

	{ type %$int_label }

   this means we want to emit DW_FORM_ref_addr for label referencing.

 - we can't set the form of label referencing offset in dwarf assembler.
   Nowadays, dwarf assembler guesses the form of labels, which is
   DW_FORM_ref4.  However, in implptr-64bit.exp, both DW_FORM_ref4
   and DW_FORM_ref8 is used (see REF_ADDR in implptr-64bit.S).  This
   patch adds the flexibility of setting the form of label reference.
   Both of them below are valid,

	{DW_AT_type :$struct_label}
	{DW_AT_type :$struct_label DW_FORM_ref8}

   the former form is the default DW_FORM_ref4.

I compared the .debug_info of objects without and with this patch
applied.  There is no changes except abbrev numbers.

gdb/testsuite:

2017-01-24  Andreas Arnez  <arnez@linux.vnet.ibm.com>
	    Yao Qi  <yao.qi@linaro.org>

	* gdb.dwarf2/implptr-64bit.exp: Use dwarf assembler.
	* gdb.dwarf2/implptr-64bit.S: Remove.
	* lib/dwarf.exp (Dwarf): Handle character "%".  Evaluate some
	variables in caller's context.
	(Dwarf::_location): Add handling for DW_OP_implicit_value.
---
 gdb/testsuite/gdb.dwarf2/implptr-64bit.S   | 226 -----------------------------
 gdb/testsuite/gdb.dwarf2/implptr-64bit.exp | 118 +++++++++++++--
 gdb/testsuite/lib/dwarf.exp                |  39 ++++-
 3 files changed, 147 insertions(+), 236 deletions(-)
 delete mode 100644 gdb/testsuite/gdb.dwarf2/implptr-64bit.S

diff --git a/gdb/testsuite/gdb.dwarf2/implptr-64bit.S b/gdb/testsuite/gdb.dwarf2/implptr-64bit.S
deleted file mode 100644
index 003bf20..0000000
--- a/gdb/testsuite/gdb.dwarf2/implptr-64bit.S
+++ /dev/null
@@ -1,226 +0,0 @@
-/* Copyright 2010-2017 Free Software Foundation, Inc.
-
-   This program is free software; you can redistribute it and/or modify
-   it under the terms of the GNU General Public License as published by
-   the Free Software Foundation; either version 3 of the License, or
-   (at your option) any later version.
-
-   This program is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-   GNU General Public License for more details.
-
-   You should have received a copy of the GNU General Public License
-   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
-
-	.section	.debug_info
-d:
-	/* Length of Compilation Unit Info */
-#if OFFSET_SIZE == 4
-# define OFFSET .4byte
-# define HEADER_LINE1
-# define HEADER_LINE2(END) .4byte	END - 1f
-#elif OFFSET_SIZE == 8
-# define OFFSET .8byte
-# define HEADER_LINE1 .4byte	0xffffffff
-# define HEADER_LINE2(END) .8byte	END - 1f
-#else
-# error
-#endif
-#if ADDR_SIZE == 4
-# define ADDR .4byte
-#elif ADDR_SIZE == 8
-# define ADDR .8byte
-#else
-# error
-#endif
-#if REF_ADDR_SIZE == 4
-# define REF_ADDR .4byte
-#elif REF_ADDR_SIZE == 8
-# define REF_ADDR .8byte
-#else
-# error
-#endif
-	
-#if TWO_CU
-# define END1 .Lcu_end_1
-#else
-# define END1 debug_end
-#endif
-
-	HEADER_LINE1
-	HEADER_LINE2(END1)
-
-1:
-	.2byte	DWARF_VERSION	/* DWARF version number */
-	OFFSET	.Ldebug_abbrev0	/* Offset Into Abbrev. Section */
-	.byte	ADDR_SIZE	/* Pointer Size (in bytes) */
-
-	.uleb128 0x1	/* (DIE (0xb) DW_TAG_compile_unit) */
-	.ascii "GNU C 4.4.3\0"	/* DW_AT_producer */
-	.byte	0x1	/* DW_AT_language */
-	.ascii "1.c\0"	/* DW_AT_name */
-
-.Ltype_int:
-	.uleb128 0x7	/* DW_TAG_base_type */
-	.byte	0x4	/* DW_AT_byte_size */
-	.byte	0x5	/* DW_AT_encoding */
-	.ascii "int\0"	/* DW_AT_name */
-
-.Ltype_struct:
-	.uleb128 0x2	/* DW_TAG_structure_type */
-	.ascii "s\0"	/* DW_AT_name */
-	.byte	4	/* DW_AT_byte_size */
-
-	.uleb128 0x3	/* DW_TAG_member */
-	.ascii "f\0"	/* DW_AT_name */
-	.4byte	.Ltype_int - d	/* DW_AT_type */
-	.byte	0	/* DW_AT_data_member_location */
-
-	.byte	0x0	/* end of children of DW_TAG_structure_type */
-
-.Ltype_structptr:
-	.uleb128 0x5	/* DW_TAG_pointer_type */
-	.byte	ADDR_SIZE	/* DW_AT_byte_size */
-	.4byte	.Ltype_struct - d	/* DW_AT_type */
-
-.Lvar_out:
-	.uleb128 0x4	/* (DW_TAG_variable) */
-	.ascii "v\0"	/* DW_AT_name */
-	.byte	2f - 1f	/* DW_AT_location: DW_FORM_block1 */
-1:
-	.byte	0x9e	/* DW_OP_implicit_value */
-	.uleb128  2f - 3f
-3:
-	.byte	1, 1, 1, 1
-2:
-	REF_ADDR	.Ltype_struct - d	/* DW_AT_type */
-
-#if TWO_CU
-	.byte	0x0	/* end of children of CU */
-.Lcu_end_1:
-
-	HEADER_LINE1
-	HEADER_LINE2 (debug_end)
-
-1:
-	.2byte	DWARF_VERSION	/* DWARF version number */
-	OFFSET	.Ldebug_abbrev0	/* Offset Into Abbrev. Section */
-	.byte	ADDR_SIZE	/* Pointer Size (in bytes) */
-
-	.uleb128 0x1	/* (DIE (0xb) DW_TAG_compile_unit) */
-	.ascii "GNU C 4.4.3\0"	/* DW_AT_producer */
-	.byte	0x1	/* DW_AT_language */
-	.ascii "1.c\0"	/* DW_AT_name */
-#endif
-
-	.uleb128	6			/* Abbrev: DW_TAG_subprogram */
-	.ascii		"main\0"		/* DW_AT_name */
-	ADDR		main			/* DW_AT_low_pc */
-	ADDR		main + 0x100		/* DW_AT_high_pc */
-	REF_ADDR	.Ltype_int - d		/* DW_AT_type */
-	.byte		1			/* DW_AT_external */
-
-	.uleb128 0x4	/* (DW_TAG_variable) */
-	.ascii "p\0"	/* DW_AT_name */
-	.byte	2f - 1f	/* DW_AT_location: DW_FORM_block1 */
-1:
-	.byte	0xf2	/* DW_OP_GNU_implicit_pointer */
-	REF_ADDR	.Lvar_out - d	/* referenced DIE */
-	.sleb128	0	/* offset */
-2:
-	REF_ADDR	.Ltype_structptr - d	/* DW_AT_type */
-
-	.byte	0x0	/* end of children of main */
-
-	.byte	0x0	/* end of children of CU */
-debug_end:
-
-	.section	.debug_abbrev
-.Ldebug_abbrev0:
-
-	.uleb128 0x1	/* (abbrev code) */
-	.uleb128 0x11	/* (TAG: DW_TAG_compile_unit) */
-	.byte	0x1	/* DW_children_yes */
-	.uleb128 0x25	/* (DW_AT_producer) */
-	.uleb128 0x8	/* (DW_FORM_string) */
-	.uleb128 0x13	/* (DW_AT_language) */
-	.uleb128 0xb	/* (DW_FORM_data1) */
-	.uleb128 0x3	/* (DW_AT_name) */
-	.uleb128 0x8	/* (DW_FORM_string) */
-	.byte	0x0
-	.byte	0x0
-
-	.uleb128 0x2	/* (abbrev code) */
-	.uleb128 0x13	/* (TAG: DW_TAG_structure_type) */
-	.byte	0x1	/* DW_children_yes */
-	.uleb128 0x3	/* (DW_AT_name) */
-	.uleb128 0x8	/* (DW_FORM_string) */
-	.uleb128 0xb	/* (DW_AT_byte_size) */
-	.uleb128 0xb	/* (DW_FORM_data1) */
-	.byte	0
-	.byte	0
-
-	.uleb128 0x3	/* (abbrev code) */
-	.uleb128 0xd	/* (TAG: DW_TAG_member) */
-	.byte	0	/* DW_children_no */
-	.uleb128 0x3	/* (DW_AT_name) */
-	.uleb128 0x8	/* (DW_FORM_string) */
-	.uleb128 0x49	/* (DW_AT_type) */
-	.uleb128 0x13	/* (DW_FORM_ref4) */
-	.uleb128 0x38	/* (DW_AT_data_member_location) */
-	.uleb128 0xb	/* (DW_FORM_data1) */
-	.byte	0
-	.byte	0
-
-	.uleb128 0x4	/* (abbrev code) */
-	.uleb128 0x34	/* (TAG: DW_TAG_variable) */
-	.byte	0x0	/* DW_children_yes */
-	.uleb128 0x3	/* (DW_AT_name) */
-	.uleb128 0x8	/* (DW_FORM_string) */
-	.uleb128 0x02	/* (DW_AT_location) */
-	.uleb128 0xa	/* (DW_FORM_block1) */
-	.uleb128 0x49	/* (DW_AT_type) */
-	.uleb128 0x10	/* (DW_FORM_ref_addr) */
-	.byte	0x0
-	.byte	0x0
-
-	.uleb128 0x5	/* (abbrev code) */
-	.uleb128 0xf	/* (TAG: DW_TAG_pointer_type) */
-	.byte	0x0	/* DW_children_no */
-	.uleb128 0xb	/* (DW_AT_byte_size) */
-	.uleb128 0xb	/* (DW_FORM_data1) */
-	.uleb128 0x49	/* (DW_AT_type) */
-	.uleb128 0x13	/* (DW_FORM_ref4) */
-	.byte	0x0
-	.byte	0x0
-
-	.uleb128	6			/* Abbrev code */
-	.uleb128	0x2e			/* DW_TAG_subprogram */
-	.byte		1			/* has_children */
-	.uleb128	0x3			/* DW_AT_name */
-	.uleb128	0x8			/* DW_FORM_string */
-	.uleb128	0x11			/* DW_AT_low_pc */
-	.uleb128	0x1			/* DW_FORM_addr */
-	.uleb128	0x12			/* DW_AT_high_pc */
-	.uleb128	0x1			/* DW_FORM_addr */
-	.uleb128	0x49			/* DW_AT_type */
-	.uleb128	0x10			/* DW_FORM_ref_addr */
-	.uleb128	0x3f			/* DW_AT_external */
-	.uleb128	0xc			/* DW_FORM_flag */
-	.byte		0x0			/* Terminator */
-	.byte		0x0			/* Terminator */
-
-	.uleb128 0x7	/* (abbrev code) */
-	.uleb128 0x24	/* (TAG: DW_TAG_base_type) */
-	.byte	0	/* DW_children_no */
-	.uleb128 0xb	/* (DW_AT_byte_size) */
-	.uleb128 0xb	/* (DW_FORM_data1) */
-	.uleb128 0x3e	/* (DW_AT_encoding) */
-	.uleb128 0xb	/* (DW_FORM_data1) */
-	.uleb128 0x3	/* (DW_AT_name) */
-	.uleb128 0x8	/* (DW_FORM_string) */
-	.byte	0
-	.byte	0
-
-	.byte	0x0
diff --git a/gdb/testsuite/gdb.dwarf2/implptr-64bit.exp b/gdb/testsuite/gdb.dwarf2/implptr-64bit.exp
index b4dcbde..9565579 100644
--- a/gdb/testsuite/gdb.dwarf2/implptr-64bit.exp
+++ b/gdb/testsuite/gdb.dwarf2/implptr-64bit.exp
@@ -19,21 +19,121 @@ if {![dwarf2_support]} {
     return 0  
 }
 
-standard_testfile .S
-set mainfile main.c
+standard_testfile main.c
 
 proc test { dwarf_version offset_size addr_size ref_addr_size two_cu } {
-    global testfile srcfile mainfile
+    global testfile srcfile
 
-    # 32-bit targets do not support any of the testcases; keep quiet there.
-    set opts {quiet}
-    foreach n { dwarf_version offset_size addr_size ref_addr_size two_cu } {
-	lappend opts "additional_flags=-D[string toupper $n]=[expr "\$$n"]"
+    set name "d${dwarf_version}o${offset_size}a${addr_size}r${ref_addr_size}t${two_cu}"
+
+    # Make some DWARF for the test.
+    set asm_file [standard_output_file ${testfile}-${name}.S]
+    Dwarf::assemble $asm_file {
+	upvar dwarf_version dwarf_version
+	upvar addr_size addr_size
+	upvar offset_size offset_size
+	upvar ref_addr_size ref_addr_size
+	upvar two_cu two_cu
+
+	set is_64 [expr { $offset_size == 4 ? 0 : 1 }]
+
+	cu {
+	    version $dwarf_version
+	    addr_size $addr_size
+	    is_64 $is_64
+	} {
+	    compile_unit {
+		{ producer "GNU C 4.4.3" }
+		{ language @DW_LANG_C89 }
+		{ name 1.c }
+	    } {
+		declare_labels struct_label variable_label int_label pointer_label
+
+		int_label: base_type {
+		    { byte_size 4 DW_FORM_sdata }
+		    { DW_AT_encoding @DW_ATE_signed }
+		    { name int }
+		}
+
+		struct_label: structure_type {
+		    { name s }
+		    { byte_size 4 sdata }
+		} {
+		    member {
+			{ name f }
+			{ type :$int_label }
+			{ data_member_location 0 data1 }
+		    }
+		}
+
+		pointer_label: pointer_type {
+		    { byte_size $Dwarf::_cu_addr_size sdata }
+		    { type  :$struct_label }
+		}
+
+		variable_label: variable {
+		    { name v }
+		    { location {
+			DW_OP_implicit_value 0x1 0x1 0x1 0x1
+		    } SPECIAL_expr}
+		    { type :$struct_label "DW_FORM_ref$ref_addr_size" }
+		}
+
+		if { !$two_cu } {
+		    subprogram {
+			{ name main }
+			{ low_pc main addr }
+			{ high_pc "main+0x100" addr }
+			{ type %$int_label }
+			{ external 1 flag }
+		    } {
+			variable {
+			    { name p }
+			    { location {
+				GNU_implicit_pointer $variable_label 0
+			    } SPECIAL_expr }
+			    { type :$pointer_label "DW_FORM_ref$ref_addr_size" }
+			}
+		    }
+		}
+	    }
+	}
+
+	if { $two_cu } {
+	    cu {
+		version $dwarf_version
+		addr_size $addr_size
+		is_64 $is_64
+	    } {
+		compile_unit {
+		    { producer "GNU C 4.4.3" }
+		    { language @DW_LANG_C89 }
+		    { name 1.c }
+		} {
+		    subprogram {
+			{ name main }
+			{ low_pc main addr }
+			{ high_pc "main+0x100" addr }
+			{ type %$int_label }
+			{ external 1 flag }
+		    } {
+			DW_TAG_variable {
+			    { name p }
+			    { location {
+				GNU_implicit_pointer $variable_label 0
+			    } SPECIAL_expr }
+			    { type %$pointer_label }
+			}
+		    }
+		}
+	    }
+	}
     }
 
-    set name "d${dwarf_version}o${offset_size}a${addr_size}r${ref_addr_size}t${two_cu}"
+    # 32-bit targets do not support any of the testcases; keep quiet there.
+    set opts {quiet}
     set executable ${testfile}-${name}
-    if [prepare_for_testing "failed to prepare" $executable "${srcfile} ${mainfile}" $opts] {
+    if [prepare_for_testing "failed to prepare" $executable "${asm_file} ${srcfile}" $opts] {
 	return -1
     }
 
diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index 1883c86..db51f5d 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -229,6 +229,8 @@ proc function_range { func src } {
 # * If VALUE starts with the ":" character, then it is a label
 #   reference.  The rest of VALUE is taken to be the name of a label,
 #   and DW_FORM_ref4 is used.  See 'new_label' and 'define_label'.
+# * If VALUE starts with the "%" character, then it is a label
+#   reference too, but DW_FORM_ref_addr is used.
 # * Otherwise, VALUE is taken to be a string and DW_FORM_string is
 #   used.  In order to prevent bugs where a numeric value is given but
 #   no form is specified, it is an error if the value looks like a number
@@ -559,6 +561,15 @@ namespace eval Dwarf {
 		return DW_FORM_ref4
 	    }
 
+	    % {
+		# Label reference, an offset from .debug_info.  Assuming
+		# .Lcu1_begin is on .debug_info.
+		set cu1_label [_compute_label "cu1_begin"]
+		set new_value "[string range $value 1 end] - $cu1_label"
+
+		return DW_FORM_ref_addr
+	    }
+
 	    default {
 		return DW_FORM_string
 	    }
@@ -654,7 +665,12 @@ namespace eval Dwarf {
 		_handle_macro_at_range $attr_value
 	    } else {
 		if {[llength $attr] > 2} {
-		    set attr_form [lindex $attr end]
+		    set attr_form [uplevel 2 [list subst [lindex $attr end]]]
+
+		    if { [string index $attr_value 0] == ":" } {
+			# It is a label, get its value.
+			_guess_form $attr_value attr_value
+		    }
 		} else {
 		    # If the value looks like an integer, a form is required.
 		    if [string is integer $attr_value] {
@@ -912,6 +928,26 @@ namespace eval Dwarf {
 		    _op .2byte [lindex $line 1]
 		}
 
+		DW_OP_implicit_value {
+		    set l1 [new_label "value_start"]
+		    set l2 [new_label "value_end"]
+		    _op .uleb128 "$l2 - $l1"
+		    define_label $l1
+		    foreach value [lrange $line 1 end] {
+			switch -regexp -- $value {
+			    {^0x[[:xdigit:]]{1}$} {_op .byte $value}
+			    {^0x[[:xdigit:]]{2}$} {_op .byte $value}
+			    {^0x[[:xdigit:]]{4}$} {_op .2byte $value}
+			    {^0x[[:xdigit:]]{8}$} {_op .4byte $value}
+			    {^0x[[:xdigit:]]{16}$} {_op .8byte $value}
+			    default {
+				error "bad value '$value' in DW_OP_implicit_value"
+			    }
+			}
+		    }
+		    define_label $l2
+		}
+
 		DW_OP_GNU_implicit_pointer {
 		    if {[llength $line] != 3} {
 			error "usage: DW_OP_GNU_implicit_pointer LABEL OFFSET"
@@ -981,6 +1017,7 @@ namespace eval Dwarf {
 	set _abbrev_section ".debug_abbrev"
 
 	foreach { name value } $options {
+	    set value [uplevel 1 "subst \"$value\""]
 	    switch -exact -- $name {
 		is_64 { set is_64 $value }
 		version { set _cu_version $value }
-- 
1.9.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] Handle DW_OP_GNU_implicit_pointer in dwarf assembler
  2017-01-24  9:42     ` [PATCH 0/2] Add DW_OP_implicit_value in dwarf assembler Yao Qi
@ 2017-01-24  9:43       ` Yao Qi
  2017-01-24  9:43       ` [PATCH 2/2] Use dwarf assembler in gdb.dwarf2/implptr-64bit.exp Yao Qi
  1 sibling, 0 replies; 12+ messages in thread
From: Yao Qi @ 2017-01-24  9:43 UTC (permalink / raw)
  To: gdb-patches; +Cc: arnez

DW_OP_GNU_implicit_pointer refers to a DIE with an offset of different
sizes in different dwarf versions.  In v2, the size is the pointer size,
while in v3 and above, it is the ref_addr size.  This patch fixes
dwarf assembler to emit the correct size of offset.  We've already fixed
this size issue in gdb,
https://sourceware.org/ml/gdb-patches/2011-09/msg00451.html

gdb/testsuite:

2017-01-24  Yao Qi  <yao.qi@linaro.org>

	* lib/dwarf.exp (Dwarf::_location): Handle
	DW_OP_GNU_implicit_pointer with proper size.
---
 gdb/testsuite/lib/dwarf.exp | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index f7e4236..1883c86 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -845,6 +845,7 @@ namespace eval Dwarf {
     proc _location {body} {
 	variable _constants
 	variable _cu_label
+	variable _cu_version
 	variable _cu_addr_size
 	variable _cu_offset_size
 
@@ -918,7 +919,11 @@ namespace eval Dwarf {
 
 		    # Here label is a section offset.
 		    set label [lindex $line 1]
-		    _op .${_cu_offset_size}byte $label
+		    if { $_cu_version == 2 } {
+			_op .${_cu_addr_size}byte $label
+		    } else {
+			_op .${_cu_offset_size}byte $label
+		    }
 		    _op .sleb128 [lindex $line 2]
 		}
 
-- 
1.9.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] Use dwarf assembler in gdb.dwarf2/implptr-64bit.exp
  2017-01-24  9:43       ` [PATCH 2/2] Use dwarf assembler in gdb.dwarf2/implptr-64bit.exp Yao Qi
@ 2017-01-24 19:21         ` Andreas Arnez
  2017-01-25 16:26           ` Yao Qi
  0 siblings, 1 reply; 12+ messages in thread
From: Andreas Arnez @ 2017-01-24 19:21 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On Tue, Jan 24 2017, Yao Qi wrote:

> diff --git a/gdb/testsuite/gdb.dwarf2/implptr-64bit.exp b/gdb/testsuite/gdb.dwarf2/implptr-64bit.exp
> index b4dcbde..9565579 100644
> --- a/gdb/testsuite/gdb.dwarf2/implptr-64bit.exp
> +++ b/gdb/testsuite/gdb.dwarf2/implptr-64bit.exp

Looks generally OK to me, except...

> +		variable_label: variable {

...this causes FAILs on one of my systems unless replacing the
"variable" above by "DW_TAG_variable".  Note that "variable" is a Tcl
command.

> +		    { name v }
> +		    { location {
> +			DW_OP_implicit_value 0x1 0x1 0x1 0x1
> +		    } SPECIAL_expr}
> +		    { type :$struct_label "DW_FORM_ref$ref_addr_size" }
> +		}
> +
> +		if { !$two_cu } {
> +		    subprogram {
> +			{ name main }
> +			{ low_pc main addr }
> +			{ high_pc "main+0x100" addr }
> +			{ type %$int_label }
> +			{ external 1 flag }
> +		    } {
> +			variable {

Same here.

> diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
> index 1883c86..db51f5d 100644
> --- a/gdb/testsuite/lib/dwarf.exp
> +++ b/gdb/testsuite/lib/dwarf.exp

Looks OK to me as well, except a nit:

> +			switch -regexp -- $value {
> +			    {^0x[[:xdigit:]]{1}$} {_op .byte $value}
> +			    {^0x[[:xdigit:]]{2}$} {_op .byte $value}

I'd rather replace the above two lines by a single one, to emphasize
that both cases are handled in the same way:

			    {^0x[[:xdigit:]]{1,2}$} {_op .byte $value}

--
Andreas

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] Use dwarf assembler in gdb.dwarf2/implptr-64bit.exp
  2017-01-24 19:21         ` Andreas Arnez
@ 2017-01-25 16:26           ` Yao Qi
  0 siblings, 0 replies; 12+ messages in thread
From: Yao Qi @ 2017-01-25 16:26 UTC (permalink / raw)
  To: Andreas Arnez; +Cc: gdb-patches

> 
> > +		variable_label: variable {
> 
> ...this causes FAILs on one of my systems unless replacing the
> "variable" above by "DW_TAG_variable".  Note that "variable" is a Tcl
> command.
>

Fixed.
 
> > +		    { name v }
> > +		    { location {
> > +			DW_OP_implicit_value 0x1 0x1 0x1 0x1
> > +		    } SPECIAL_expr}
> > +		    { type :$struct_label "DW_FORM_ref$ref_addr_size" }
> > +		}
> > +
> > +		if { !$two_cu } {
> > +		    subprogram {
> > +			{ name main }
> > +			{ low_pc main addr }
> > +			{ high_pc "main+0x100" addr }
> > +			{ type %$int_label }
> > +			{ external 1 flag }
> > +		    } {
> > +			variable {
> 
> Same here.
> 
> > diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
> > index 1883c86..db51f5d 100644
> > --- a/gdb/testsuite/lib/dwarf.exp
> > +++ b/gdb/testsuite/lib/dwarf.exp
> 
> Looks OK to me as well, except a nit:
> 
> > +			switch -regexp -- $value {
> > +			    {^0x[[:xdigit:]]{1}$} {_op .byte $value}
> > +			    {^0x[[:xdigit:]]{2}$} {_op .byte $value}
> 
> I'd rather replace the above two lines by a single one, to emphasize
> that both cases are handled in the same way:
> 
> 			    {^0x[[:xdigit:]]{1,2}$} {_op .byte $value}
> 

Fixed.  I pushed these two patches in.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Big-endian targets: don't ignore offset into DW_OP_implicit_value
  2017-01-12 19:24 [PATCH] Big-endian targets: don't ignore offset into DW_OP_implicit_value Andreas Arnez
  2017-01-19 17:44 ` Yao Qi
@ 2017-01-25 22:12 ` Yao Qi
  2017-01-27 19:35   ` Andreas Arnez
  1 sibling, 1 reply; 12+ messages in thread
From: Yao Qi @ 2017-01-25 22:12 UTC (permalink / raw)
  To: Andreas Arnez; +Cc: gdb-patches, Jan Kratochvil

On 17-01-12 20:24:27, Andreas Arnez wrote:
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-op-stack-value.exp b/gdb/testsuite/gdb.dwarf2/dw2-op-stack-value.exp
> index c28dcca..808f983 100644
> --- a/gdb/testsuite/gdb.dwarf2/dw2-op-stack-value.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-op-stack-value.exp
> @@ -45,7 +45,7 @@ gdb_test_multiple $test $test {
>      -re ":\[ \t\]*0xaa551234\r\n$gdb_prompt $" {
>  	# big endian
>  	pass $test
> -	gdb_test "p/x implicit4to2" " = 0x3344"
> +	gdb_test "p/x implicit4to2" " = 0x1122"
>  	gdb_test "p/x implicit4to4" " = 0x11223344"

It takes me a while to understand this.  I am wondering is it a valid
test case? how does compiler generate a DIE for a 2-byte variable
from a 4-byte implicit value.  DWARF spec isn't clear on this case to
me.  It has nothing to do with your patch, but I just raise this
question when I read your patch.

> +# Byte-aligned objects with simple location descriptions.
> +switch $endian { big {set val 0x345678} little {set val 0x785634} }
> +gdb_test "print/x def_implicit_s" " = \\{a = 0x12, b = $val\\}"
> +gdb_test "print/x def_implicit_s.b" " = $val"
> +gdb_test "print/x def_implicit_a" \
> +    " = \\{0x1, 0x12, 0x23, 0x34, 0x45, 0x56, 0x67, 0x78, 0x89\\}"

All these values are from debug information rather than inferior memory,
does it make sense to run these tests above with both big and little
endianess?

Otherwise, patch is good to me.

-- 
Yao 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Big-endian targets: don't ignore offset into DW_OP_implicit_value
  2017-01-25 22:12 ` [PATCH] Big-endian targets: don't ignore offset into DW_OP_implicit_value Yao Qi
@ 2017-01-27 19:35   ` Andreas Arnez
  2017-02-01  9:09     ` Andreas Arnez
  0 siblings, 1 reply; 12+ messages in thread
From: Andreas Arnez @ 2017-01-27 19:35 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches, Jan Kratochvil

On Wed, Jan 25 2017, Yao Qi wrote:

> On 17-01-12 20:24:27, Andreas Arnez wrote:
>> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-op-stack-value.exp b/gdb/testsuite/gdb.dwarf2/dw2-op-stack-value.exp
>> index c28dcca..808f983 100644
>> --- a/gdb/testsuite/gdb.dwarf2/dw2-op-stack-value.exp
>> +++ b/gdb/testsuite/gdb.dwarf2/dw2-op-stack-value.exp
>> @@ -45,7 +45,7 @@ gdb_test_multiple $test $test {
>>      -re ":\[ \t\]*0xaa551234\r\n$gdb_prompt $" {
>>  	# big endian
>>  	pass $test
>> -	gdb_test "p/x implicit4to2" " = 0x3344"
>> +	gdb_test "p/x implicit4to2" " = 0x1122"
>>  	gdb_test "p/x implicit4to4" " = 0x11223344"
>
> It takes me a while to understand this.  I am wondering is it a valid
> test case? how does compiler generate a DIE for a 2-byte variable
> from a 4-byte implicit value.

AFAIK never.  I don't know why a compiler should emit a larger immediate
value than necessary.  So this is an artificial test.

> DWARF spec isn't clear on this case to me.

Right, DWARF does not specify this.  Even so, we may want GDB to exhibit
defined behavior for such corner cases as well.  I probably wouldn't
have added such a test, but I didn't want remove it either, because it
could be considered useful in the sense that it tests GDB's
"implementation-defined" behavior for this case.

> It has nothing to do with your patch, but I just raise this
> question when I read your patch.
>
>> +# Byte-aligned objects with simple location descriptions.
>> +switch $endian { big {set val 0x345678} little {set val 0x785634} }
>> +gdb_test "print/x def_implicit_s" " = \\{a = 0x12, b = $val\\}"
>> +gdb_test "print/x def_implicit_s.b" " = $val"
>> +gdb_test "print/x def_implicit_a" \
>> +    " = \\{0x1, 0x12, 0x23, 0x34, 0x45, 0x56, 0x67, 0x78, 0x89\\}"
>
> All these values are from debug information rather than inferior memory,
> does it make sense to run these tests above with both big and little
> endianess?

I've tried, but I don't know to make it work.  Switching to the opposite
endianness affects more than just the byte order of variable contents;
the variables are not even found any more.  Any idea?

> Otherwise, patch is good to me.

Thanks!

--
Andreas

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Big-endian targets: don't ignore offset into DW_OP_implicit_value
  2017-01-27 19:35   ` Andreas Arnez
@ 2017-02-01  9:09     ` Andreas Arnez
  2017-02-01  9:16       ` Yao Qi
  0 siblings, 1 reply; 12+ messages in thread
From: Andreas Arnez @ 2017-02-01  9:09 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches, Jan Kratochvil

On Fri, Jan 27 2017, Andreas Arnez wrote:

> On Wed, Jan 25 2017, Yao Qi wrote:
>

[...]

>> All these values are from debug information rather than inferior memory,
>> does it make sense to run these tests above with both big and little
>> endianess?
>
> I've tried, but I don't know to make it work.  Switching to the opposite
> endianness affects more than just the byte order of variable contents;
> the variables are not even found any more.  Any idea?
>
>> Otherwise, patch is good to me.

Is it OK then to push the patch?  The logic for testing both little- and
big-endian byte order can still be added later, right?

--
Andreas

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Big-endian targets: don't ignore offset into DW_OP_implicit_value
  2017-02-01  9:09     ` Andreas Arnez
@ 2017-02-01  9:16       ` Yao Qi
  0 siblings, 0 replies; 12+ messages in thread
From: Yao Qi @ 2017-02-01  9:16 UTC (permalink / raw)
  To: Andreas Arnez; +Cc: gdb-patches, Jan Kratochvil

On Wed, Feb 1, 2017 at 9:09 AM, Andreas Arnez <arnez@linux.vnet.ibm.com> wrote:
>>
>> I've tried, but I don't know to make it work.  Switching to the opposite
>> endianness affects more than just the byte order of variable contents;
>> the variables are not even found any more.  Any idea?
>>
>>> Otherwise, patch is good to me.
>
> Is it OK then to push the patch?  The logic for testing both little- and
> big-endian byte order can still be added later, right?
>

Hi Andreas,
Yes, the patch can be pushed in.  I can take a look at the test for both
endianess later.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-02-01  9:16 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-12 19:24 [PATCH] Big-endian targets: don't ignore offset into DW_OP_implicit_value Andreas Arnez
2017-01-19 17:44 ` Yao Qi
2017-01-19 18:21   ` Andreas Arnez
2017-01-24  9:42     ` [PATCH 0/2] Add DW_OP_implicit_value in dwarf assembler Yao Qi
2017-01-24  9:43       ` [PATCH 1/2] Handle DW_OP_GNU_implicit_pointer " Yao Qi
2017-01-24  9:43       ` [PATCH 2/2] Use dwarf assembler in gdb.dwarf2/implptr-64bit.exp Yao Qi
2017-01-24 19:21         ` Andreas Arnez
2017-01-25 16:26           ` Yao Qi
2017-01-25 22:12 ` [PATCH] Big-endian targets: don't ignore offset into DW_OP_implicit_value Yao Qi
2017-01-27 19:35   ` Andreas Arnez
2017-02-01  9:09     ` Andreas Arnez
2017-02-01  9:16       ` Yao Qi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).