Big-endian targets: don't ignore offset into DW_OP_implicit_value
Commit Message
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(-)
Comments
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.
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
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
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.
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
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
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.
@@ -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;
@@ -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 $" {
@@ -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>"
@@ -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"