* [PATCH v2 01/19] Add test for modifiable DWARF locations
2017-05-09 17:47 [PATCH v2 00/19] Various DWARF piece fixes Andreas Arnez
@ 2017-05-09 17:47 ` Andreas Arnez
2017-05-11 21:22 ` Yao Qi
2017-05-09 17:48 ` [PATCH v2 02/19] write_pieced_value: Fix size capping logic Andreas Arnez
` (19 subsequent siblings)
20 siblings, 1 reply; 56+ messages in thread
From: Andreas Arnez @ 2017-05-09 17:47 UTC (permalink / raw)
To: gdb-patches
This adds a test for read/write access to variables with various types of
DWARF locations. It uses register- and memory locations and composite
locations with register- and memory pieces.
Since the new test calls gdb_test_no_output with commands that contain
braces, it is necessary for string_to_regexp to quote braces as well.
This was not done before.
gdb/testsuite/ChangeLog:
* gdb.dwarf2/var-access.c: New file.
* gdb.dwarf2/var-access.exp: New test.
* lib/gdb-utils.exp (string_to_regexp): Quote braces as well.
---
gdb/testsuite/gdb.dwarf2/var-access.c | 25 ++++
gdb/testsuite/gdb.dwarf2/var-access.exp | 197 ++++++++++++++++++++++++++++++++
gdb/testsuite/lib/gdb-utils.exp | 2 +-
3 files changed, 223 insertions(+), 1 deletion(-)
create mode 100644 gdb/testsuite/gdb.dwarf2/var-access.c
create mode 100644 gdb/testsuite/gdb.dwarf2/var-access.exp
diff --git a/gdb/testsuite/gdb.dwarf2/var-access.c b/gdb/testsuite/gdb.dwarf2/var-access.c
new file mode 100644
index 0000000..108be82
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/var-access.c
@@ -0,0 +1,25 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 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/>. */
+
+char buf[] = {0, 1, 2, 3, 4, 5, 6, 7};
+
+int
+main (void)
+{
+ asm volatile ("main_label: .globl main_label");
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.dwarf2/var-access.exp b/gdb/testsuite/gdb.dwarf2/var-access.exp
new file mode 100644
index 0000000..a52327d
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/var-access.exp
@@ -0,0 +1,197 @@
+# Copyright 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/>.
+
+# Test reading/writing variables with non-trivial DWARF locations. In
+# particular the test uses register- and memory locations as well as
+# composite locations with register- and memory pieces.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+ return 0
+}
+
+# Choose suitable integer registers for the test.
+
+set dwarf_regnum {0 1}
+
+if { [istarget "aarch64*-*-*"] } {
+ set regname {x0 x1}
+} elseif { [istarget "arm*-*-*"]
+ || [istarget "s390*-*-*" ]
+ || [istarget "powerpc*-*-*"]
+ || [istarget "rs6000*-*-aix*"] } {
+ set regname {r0 r1}
+} elseif { [istarget "i?86-*-*"] } {
+ set regname {eax edx}
+} elseif { [istarget "x86_64-*-*"] } {
+ set regname {rax rdx}
+} else {
+ verbose "Skipping tests for accessing DWARF-described variables."
+ return
+}
+
+standard_testfile .c ${gdb_test_file_name}-dw.S
+
+# Make some DWARF for the test.
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+ global srcdir subdir srcfile
+ global dwarf_regnum regname
+
+ set buf_var [gdb_target_symbol buf]
+
+ cu {} {
+ DW_TAG_compile_unit {
+ {DW_AT_name var-pieces-dw.c}
+ {DW_AT_comp_dir /tmp}
+ } {
+ declare_labels char_type_label
+ declare_labels int_type_label short_type_label
+ declare_labels array_a8_label struct_s_label
+
+ # char
+ char_type_label: base_type {
+ {name "char"}
+ {encoding @DW_ATE_unsigned_char}
+ {byte_size 1 DW_FORM_sdata}
+ }
+
+ # int
+ int_type_label: base_type {
+ {name "int"}
+ {encoding @DW_ATE_signed}
+ {byte_size 4 DW_FORM_sdata}
+ }
+
+ # char [8]
+ array_a8_label: array_type {
+ {type :$char_type_label}
+ } {
+ subrange_type {
+ {type :$int_type_label}
+ {upper_bound 7 DW_FORM_udata}
+ }
+ }
+
+ # struct s { char a, b, c, d; };
+ struct_s_label: structure_type {
+ {name "s"}
+ {byte_size 4 DW_FORM_sdata}
+ } {
+ member {
+ {name "a"}
+ {type :$char_type_label}
+ {data_member_location 0 DW_FORM_udata}
+ }
+ member {
+ {name "b"}
+ {type :$char_type_label}
+ {data_member_location 1 DW_FORM_udata}
+ }
+ member {
+ {name "c"}
+ {type :$char_type_label}
+ {data_member_location 2 DW_FORM_udata}
+ }
+ member {
+ {name "d"}
+ {type :$char_type_label}
+ {data_member_location 3 DW_FORM_udata}
+ }
+ }
+
+ DW_TAG_subprogram {
+ {MACRO_AT_func { main ${srcdir}/${subdir}/${srcfile} }}
+ {DW_AT_external 1 flag}
+ } {
+ # Simple memory location.
+ DW_TAG_variable {
+ {name "a"}
+ {type :$array_a8_label}
+ {location {
+ addr $buf_var
+ } SPECIAL_expr}
+ }
+ # Memory pieces: two bytes from &buf[2], and two bytes
+ # from &buf[0].
+ DW_TAG_variable {
+ {name "s1"}
+ {type :$struct_s_label}
+ {location {
+ addr $buf_var
+ plus_uconst 2
+ piece 2
+ addr $buf_var
+ piece 2
+ } SPECIAL_expr}
+ }
+ # Register- and memory pieces: one byte each from r0,
+ # &buf[4], r1, and &buf[5].
+ DW_TAG_variable {
+ {name "s2"}
+ {type :$struct_s_label}
+ {location {
+ regx [lindex $dwarf_regnum 0]
+ piece 1
+ addr "$buf_var + 4"
+ piece 1
+ regx [lindex $dwarf_regnum 1]
+ piece 1
+ addr "$buf_var + 5"
+ piece 1
+ } SPECIAL_expr}
+ }
+ }
+ }
+ }
+}
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} \
+ [list $srcfile $asm_file] {nodebug}] } {
+ return -1
+}
+
+if ![runto_main] {
+ return -1
+}
+
+# Byte-aligned memory pieces.
+gdb_test "print/d s1" " = \\{a = 2, b = 3, c = 0, d = 1\\}" \
+ "s1 == re-ordered buf"
+gdb_test_no_output "set var s1.a = 63"
+gdb_test "print/d s1" " = \\{a = 63, b = 3, c = 0, d = 1\\}" \
+ "verify s1.a"
+gdb_test "print/d a" " = \\{0, 1, 63, 3, 4, 5, 6, 7\\}" \
+ "verify s1.a through a"
+
+# Byte-aligned register- and memory pieces.
+gdb_test_no_output "set var \$[lindex $regname 0] = 81" \
+ "init reg for s2.a"
+gdb_test_no_output "set var \$[lindex $regname 1] = 28" \
+ "init reg for s2.c"
+gdb_test "print/d s2" " = \\{a = 81, b = 4, c = 28, d = 5\\}" \
+ "initialized s2 from mem and regs"
+gdb_test_no_output "set var s2.c += s2.a + s2.b - s2.d"
+gdb_test "print/d s2" " = \\{a = 81, b = 4, c = 108, d = 5\\}" \
+ "verify s2.c"
+gdb_test "print/d \$[lindex $regname 1]" " = 108" \
+ "verify s2.c through reg"
+gdb_test_no_output "set var s2 = {191, 73, 231, 123}" \
+ "re-initialize s2"
+gdb_test "print/d s2" " = \\{a = 191, b = 73, c = 231, d = 123\\}" \
+ "verify re-initialized s2"
diff --git a/gdb/testsuite/lib/gdb-utils.exp b/gdb/testsuite/lib/gdb-utils.exp
index ff1b24a..37abb14 100644
--- a/gdb/testsuite/lib/gdb-utils.exp
+++ b/gdb/testsuite/lib/gdb-utils.exp
@@ -34,6 +34,6 @@ proc gdb_init_commands {} {
proc string_to_regexp {str} {
set result $str
- regsub -all {[]*+.|()^$\[\\]} $str {\\&} result
+ regsub -all {[]*+.|(){}^$\[\\]} $str {\\&} result
return $result
}
--
2.5.0
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v2 01/19] Add test for modifiable DWARF locations
2017-05-09 17:47 ` [PATCH v2 01/19] Add test for modifiable DWARF locations Andreas Arnez
@ 2017-05-11 21:22 ` Yao Qi
0 siblings, 0 replies; 56+ messages in thread
From: Yao Qi @ 2017-05-11 21:22 UTC (permalink / raw)
To: Andreas Arnez; +Cc: gdb-patches
On 17-05-09 19:45:57, Andreas Arnez wrote:
> This adds a test for read/write access to variables with various types of
> DWARF locations. It uses register- and memory locations and composite
> locations with register- and memory pieces.
>
> Since the new test calls gdb_test_no_output with commands that contain
> braces, it is necessary for string_to_regexp to quote braces as well.
> This was not done before.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.dwarf2/var-access.c: New file.
> * gdb.dwarf2/var-access.exp: New test.
> * lib/gdb-utils.exp (string_to_regexp): Quote braces as well.
LGTM.
--
Yao
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v2 02/19] write_pieced_value: Fix size capping logic
2017-05-09 17:47 [PATCH v2 00/19] Various DWARF piece fixes Andreas Arnez
2017-05-09 17:47 ` [PATCH v2 01/19] Add test for modifiable DWARF locations Andreas Arnez
@ 2017-05-09 17:48 ` Andreas Arnez
2017-05-11 21:26 ` Yao Qi
2017-05-09 17:49 ` [PATCH v2 04/19] Remove addr_size field from struct piece_closure Andreas Arnez
` (18 subsequent siblings)
20 siblings, 1 reply; 56+ messages in thread
From: Andreas Arnez @ 2017-05-09 17:48 UTC (permalink / raw)
To: gdb-patches
A field f in a structure composed of DWARF pieces may be located in
multiple pieces, where the first and last of those may contain bits from
other fields as well. So when writing to f, the beginning of the first
and the end of the last of those pieces may have to be skipped. But the
logic in write_pieced_value for handling one of those pieces is flawed
when the first and last piece are the same, i.e., f is contained in a
single piece:
< - - - - - - - - - piece_size - - - - - - - - - ->
+-------------------------------------------------+
| skipped_bits | f_bits | / / / / / / / / / / |
+-------------------------------------------------+
The current logic determines the size of the sub-piece to operate on by
limiting the piece size to the bit size of f and then subtracting the
skipped bits:
min (piece_size, f_bits) - skipped_bits
Instead of:
min (piece_size - skipped_bits, f_bits)
So the resulting sub-piece size is corrupted, leading to wrong handling of
this piece in write_pieced_value.
Note that the same bug was already found in read_pieced_value and fixed
there (but not in write_pieced_value), see PR 15391.
This patch swaps the calculations, bringing them into the same (correct)
order as in read_pieced_value.
gdb/ChangeLog:
* dwarf2loc.c (write_pieced_value): Fix order of calculations for
size capping.
gdb/testsuite/ChangeLog:
* gdb.dwarf2/var-pieces.exp: Add test case for modifying a
variable at nonzero offset.
---
gdb/dwarf2loc.c | 4 ++--
gdb/testsuite/gdb.dwarf2/var-access.exp | 5 +++++
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 127167d..2a45a79 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1964,8 +1964,6 @@ write_pieced_value (struct value *to, struct value *from)
bits_to_skip -= this_size_bits;
continue;
}
- if (this_size_bits > type_len - offset)
- this_size_bits = type_len - offset;
if (bits_to_skip > 0)
{
dest_offset_bits = bits_to_skip;
@@ -1978,6 +1976,8 @@ write_pieced_value (struct value *to, struct value *from)
dest_offset_bits = 0;
source_offset_bits = offset;
}
+ if (this_size_bits > type_len - offset)
+ this_size_bits = type_len - offset;
this_size = (this_size_bits + source_offset_bits % 8 + 7) / 8;
source_offset = source_offset_bits / 8;
diff --git a/gdb/testsuite/gdb.dwarf2/var-access.exp b/gdb/testsuite/gdb.dwarf2/var-access.exp
index a52327d..bd92a44 100644
--- a/gdb/testsuite/gdb.dwarf2/var-access.exp
+++ b/gdb/testsuite/gdb.dwarf2/var-access.exp
@@ -178,6 +178,11 @@ gdb_test "print/d s1" " = \\{a = 63, b = 3, c = 0, d = 1\\}" \
"verify s1.a"
gdb_test "print/d a" " = \\{0, 1, 63, 3, 4, 5, 6, 7\\}" \
"verify s1.a through a"
+gdb_test_no_output "set var s1.b = 42"
+gdb_test "print/d s1" " = \\{a = 63, b = 42, c = 0, d = 1\\}" \
+ "verify s1.b"
+gdb_test "print/d a" " = \\{0, 1, 63, 42, 4, 5, 6, 7\\}" \
+ "verify s1.b through a"
# Byte-aligned register- and memory pieces.
gdb_test_no_output "set var \$[lindex $regname 0] = 81" \
--
2.5.0
^ permalink raw reply [flat|nested] 56+ messages in thread* [PATCH v2 04/19] Remove addr_size field from struct piece_closure
2017-05-09 17:47 [PATCH v2 00/19] Various DWARF piece fixes Andreas Arnez
2017-05-09 17:47 ` [PATCH v2 01/19] Add test for modifiable DWARF locations Andreas Arnez
2017-05-09 17:48 ` [PATCH v2 02/19] write_pieced_value: Fix size capping logic Andreas Arnez
@ 2017-05-09 17:49 ` Andreas Arnez
2017-05-11 21:29 ` Yao Qi
2017-05-09 17:49 ` [PATCH v2 03/19] PR gdb/21226: Take DWARF stack value pieces from LSB end Andreas Arnez
` (17 subsequent siblings)
20 siblings, 1 reply; 56+ messages in thread
From: Andreas Arnez @ 2017-05-09 17:49 UTC (permalink / raw)
To: gdb-patches
The addr_size field in the piece_closure data structure is a relic from
before introducing the typed DWARF stack. It is obsolete now. This patch
removes it.
gdb/ChangeLog:
* dwarf2loc.c (struct piece_closure) <addr_size>: Remove field.
(allocate_piece_closure): Drop addr_size parameter.
(dwarf2_evaluate_loc_desc_full): Adjust call to
allocate_piece_closure.
---
gdb/dwarf2loc.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 061ec6d..8f99844 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1486,9 +1486,6 @@ struct piece_closure
/* The number of pieces used to describe this variable. */
int n_pieces;
- /* The target address size, used only for DWARF_VALUE_STACK. */
- int addr_size;
-
/* The pieces themselves. */
struct dwarf_expr_piece *pieces;
@@ -1503,7 +1500,7 @@ struct piece_closure
static struct piece_closure *
allocate_piece_closure (struct dwarf2_per_cu_data *per_cu,
int n_pieces, struct dwarf_expr_piece *pieces,
- int addr_size, struct frame_info *frame)
+ struct frame_info *frame)
{
struct piece_closure *c = XCNEW (struct piece_closure);
int i;
@@ -1511,7 +1508,6 @@ allocate_piece_closure (struct dwarf2_per_cu_data *per_cu,
c->refc = 1;
c->per_cu = per_cu;
c->n_pieces = n_pieces;
- c->addr_size = addr_size;
c->pieces = XCNEWVEC (struct dwarf_expr_piece, n_pieces);
if (frame == NULL)
c->frame_id = null_frame_id;
@@ -2417,7 +2413,7 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
invalid_synthetic_pointer ();
c = allocate_piece_closure (per_cu, ctx.num_pieces, ctx.pieces,
- ctx.addr_size, frame);
+ frame);
/* We must clean up the value chain after creating the piece
closure but before allocating the result. */
free_values.free_to_mark ();
--
2.5.0
^ permalink raw reply [flat|nested] 56+ messages in thread* [PATCH v2 03/19] PR gdb/21226: Take DWARF stack value pieces from LSB end
2017-05-09 17:47 [PATCH v2 00/19] Various DWARF piece fixes Andreas Arnez
` (2 preceding siblings ...)
2017-05-09 17:49 ` [PATCH v2 04/19] Remove addr_size field from struct piece_closure Andreas Arnez
@ 2017-05-09 17:49 ` Andreas Arnez
2017-05-15 9:32 ` Yao Qi
2017-05-09 17:50 ` [PATCH v2 05/19] gdb/testsuite: Add "get_endianness" convenience proc Andreas Arnez
` (16 subsequent siblings)
20 siblings, 1 reply; 56+ messages in thread
From: Andreas Arnez @ 2017-05-09 17:49 UTC (permalink / raw)
To: gdb-patches
When taking a DW_OP_piece or DW_OP_bit_piece from a DW_OP_stack_value, the
existing logic always takes the piece from the lowest-addressed end, which
is wrong on big-endian targets. The DWARF standard states that the
"DW_OP_bit_piece operation describes a sequence of bits using the least
significant bits of that value", and this also matches the current logic
in GCC. For instance, the GCC guality test case pr54970.c fails on s390x
because of this.
This fix adjusts the piece accordingly on big-endian targets. It is
assumed that:
* DW_OP_piece shall take the piece from the LSB end as well;
* pieces reaching outside the stack value bits are considered undefined,
and a zero value can be used instead.
gdb/ChangeLog:
PR gdb/21226
* dwarf2loc.c (read_pieced_value): Anchor stack value pieces at
the LSB end, independent of endianness.
gdb/testsuite/ChangeLog:
PR gdb/21226
* gdb.dwarf2/nonvar-access.exp: Add checks for verifying that
stack value pieces are taken from the LSB end.
---
gdb/dwarf2loc.c | 46 +++++++++++++++++-------------
gdb/testsuite/gdb.dwarf2/nonvar-access.exp | 21 ++++++++++++--
2 files changed, 44 insertions(+), 23 deletions(-)
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 2a45a79..061ec6d 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1858,6 +1858,10 @@ read_pieced_value (struct value *v)
if (unavail)
mark_value_bits_unavailable (v, offset, this_size_bits);
}
+
+ copy_bitwise (contents, dest_offset_bits,
+ intermediate_buffer, source_offset_bits % 8,
+ this_size_bits, bits_big_endian);
}
break;
@@ -1866,26 +1870,30 @@ read_pieced_value (struct value *v)
p->v.mem.in_stack_memory,
p->v.mem.addr + source_offset,
buffer.data (), this_size);
+ copy_bitwise (contents, dest_offset_bits,
+ intermediate_buffer, source_offset_bits % 8,
+ this_size_bits, bits_big_endian);
break;
case DWARF_VALUE_STACK:
{
- size_t n = this_size;
+ struct objfile *objfile = dwarf2_per_cu_objfile (c->per_cu);
+ struct gdbarch *objfile_gdbarch = get_objfile_arch (objfile);
+ ULONGEST stack_value_size_bits
+ = 8 * TYPE_LENGTH (value_type (p->v.value));
- if (n > c->addr_size - source_offset)
- n = (c->addr_size >= source_offset
- ? c->addr_size - source_offset
- : 0);
- if (n == 0)
- {
- /* Nothing. */
- }
- else
- {
- const gdb_byte *val_bytes = value_contents_all (p->v.value);
+ /* Use zeroes if piece reaches beyond stack value. */
+ if (p->size > stack_value_size_bits)
+ break;
- intermediate_buffer = val_bytes + source_offset;
- }
+ /* Piece is anchored at least significant bit end. */
+ if (gdbarch_byte_order (objfile_gdbarch) == BFD_ENDIAN_BIG)
+ source_offset_bits += stack_value_size_bits - p->size;
+
+ copy_bitwise (contents, dest_offset_bits,
+ value_contents_all (p->v.value),
+ source_offset_bits,
+ this_size_bits, bits_big_endian);
}
break;
@@ -1899,6 +1907,10 @@ read_pieced_value (struct value *v)
: 0);
if (n != 0)
intermediate_buffer = p->v.literal.data + source_offset;
+
+ copy_bitwise (contents, dest_offset_bits,
+ intermediate_buffer, source_offset_bits % 8,
+ this_size_bits, bits_big_endian);
}
break;
@@ -1915,12 +1927,6 @@ read_pieced_value (struct value *v)
internal_error (__FILE__, __LINE__, _("invalid location type"));
}
- if (p->location != DWARF_VALUE_OPTIMIZED_OUT
- && p->location != DWARF_VALUE_IMPLICIT_POINTER)
- copy_bitwise (contents, dest_offset_bits,
- intermediate_buffer, source_offset_bits % 8,
- this_size_bits, bits_big_endian);
-
offset += this_size_bits;
}
}
diff --git a/gdb/testsuite/gdb.dwarf2/nonvar-access.exp b/gdb/testsuite/gdb.dwarf2/nonvar-access.exp
index 99f63cc..5406029 100644
--- a/gdb/testsuite/gdb.dwarf2/nonvar-access.exp
+++ b/gdb/testsuite/gdb.dwarf2/nonvar-access.exp
@@ -128,14 +128,26 @@ Dwarf::assemble $asm_file {
{name def_t}
{type :$struct_t_label}
{location {
- const1u 0
+ const2s -184
stack_value
bit_piece 9 0
- const1s -1
+ const4u 1752286
stack_value
bit_piece 23 0
} SPECIAL_expr}
}
+ # Composite location with some empty pieces.
+ DW_TAG_variable {
+ {name part_def_a}
+ {type :$array_a9_label}
+ {location {
+ piece 3
+ const4u 0xf1927314
+ stack_value
+ piece 4
+ piece 2
+ } SPECIAL_expr}
+ }
# Implicit location: immediate value.
DW_TAG_variable {
{name def_implicit_s}
@@ -221,9 +233,12 @@ 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\\}"
+switch $endian { big {set val 0x92} little {set val 0x73} }
+gdb_test "print/x part_def_a\[4\]" " = $val"
+gdb_test "print/x part_def_a\[8\]" " = <optimized out>"
# Non-byte-aligned fields, pieced together from DWARF stack values.
-gdb_test "print def_t" " = \\{a = 0, b = -1\\}"
+gdb_test "print def_t" " = \\{a = -184, b = 1752286\\}"
# Simple variable without location.
gdb_test "print undef_int" " = <optimized out>"
--
2.5.0
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v2 03/19] PR gdb/21226: Take DWARF stack value pieces from LSB end
2017-05-09 17:49 ` [PATCH v2 03/19] PR gdb/21226: Take DWARF stack value pieces from LSB end Andreas Arnez
@ 2017-05-15 9:32 ` Yao Qi
2017-05-15 16:35 ` Andreas Arnez
0 siblings, 1 reply; 56+ messages in thread
From: Yao Qi @ 2017-05-15 9:32 UTC (permalink / raw)
To: Andreas Arnez; +Cc: gdb-patches
Andreas Arnez <arnez@linux.vnet.ibm.com> writes:
> * DW_OP_piece shall take the piece from the LSB end as well;
>
My understanding is that this is true for a given arch or ABI. This
change will affect other big-endian architectures, like mips.
> @@ -1866,26 +1870,30 @@ read_pieced_value (struct value *v)
> p->v.mem.in_stack_memory,
> p->v.mem.addr + source_offset,
> buffer.data (), this_size);
> + copy_bitwise (contents, dest_offset_bits,
> + intermediate_buffer, source_offset_bits % 8,
> + this_size_bits, bits_big_endian);
> break;
>
> case DWARF_VALUE_STACK:
> {
> - size_t n = this_size;
> + struct objfile *objfile = dwarf2_per_cu_objfile (c->per_cu);
> + struct gdbarch *objfile_gdbarch = get_objfile_arch (objfile);
> + ULONGEST stack_value_size_bits
> + = 8 * TYPE_LENGTH (value_type (p->v.value));
>
> - if (n > c->addr_size - source_offset)
> - n = (c->addr_size >= source_offset
> - ? c->addr_size - source_offset
> - : 0);
> - if (n == 0)
> - {
> - /* Nothing. */
> - }
> - else
> - {
> - const gdb_byte *val_bytes = value_contents_all (p->v.value);
> + /* Use zeroes if piece reaches beyond stack value. */
> + if (p->size > stack_value_size_bits)
> + break;
Does this indicate something wrong in DWARF producer? Does GDB need to
emit a complaint?
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v2 03/19] PR gdb/21226: Take DWARF stack value pieces from LSB end
2017-05-15 9:32 ` Yao Qi
@ 2017-05-15 16:35 ` Andreas Arnez
2017-05-16 7:53 ` Yao Qi
0 siblings, 1 reply; 56+ messages in thread
From: Andreas Arnez @ 2017-05-15 16:35 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On Mon, May 15 2017, Yao Qi wrote:
> Andreas Arnez <arnez@linux.vnet.ibm.com> writes:
>
>> * DW_OP_piece shall take the piece from the LSB end as well;
>>
>
> My understanding is that this is true for a given arch or ABI.
Well, if a DWARF stack value is of some integral type, the LSB end is
well-defined, independently from the ABI. I don't know whether any
DWARF consumer emits something like a 3-byte DW_OP_piece of an 8-byte
floating-point stack value, and what it would mean by that. My best
guess is that it would refer to the same 3 bytes of the stack value's
in-memory representation as if the value *was* of integral type.
Or do you refer to the DWARF definition of DW_OP_piece? -- "If the piece
is located in a register, but does not occupy the entire register, the
placement of the piece within that register is defined by the ABI."
Unfortunately nothing is said about pieces from something else but
registers; thus I phrased my assumption above. I can't verify the
assumption for all DWARF producers, but I'm pretty sure that it holds
for GCC and LLVM.
> This change will affect other big-endian architectures, like mips.
Right, the change affects (hopefully fixes) all big-endian
architectures, because stack value pieces were taken from the
lowest-addressed end so far, where they should have been taken from the
LSB end instead. These two placement rules happen to be the same on
little-endian architectures, which are consequently unaffected by the
change.
>
>> @@ -1866,26 +1870,30 @@ read_pieced_value (struct value *v)
>> p->v.mem.in_stack_memory,
>> p->v.mem.addr + source_offset,
>> buffer.data (), this_size);
>> + copy_bitwise (contents, dest_offset_bits,
>> + intermediate_buffer, source_offset_bits % 8,
>> + this_size_bits, bits_big_endian);
>> break;
>>
>> case DWARF_VALUE_STACK:
>> {
>> - size_t n = this_size;
>> + struct objfile *objfile = dwarf2_per_cu_objfile (c->per_cu);
>> + struct gdbarch *objfile_gdbarch = get_objfile_arch (objfile);
>> + ULONGEST stack_value_size_bits
>> + = 8 * TYPE_LENGTH (value_type (p->v.value));
>>
>> - if (n > c->addr_size - source_offset)
>> - n = (c->addr_size >= source_offset
>> - ? c->addr_size - source_offset
>> - : 0);
>> - if (n == 0)
>> - {
>> - /* Nothing. */
>> - }
>> - else
>> - {
>> - const gdb_byte *val_bytes = value_contents_all (p->v.value);
>> + /* Use zeroes if piece reaches beyond stack value. */
>> + if (p->size > stack_value_size_bits)
>> + break;
>
> Does this indicate something wrong in DWARF producer? Does GDB need to
> emit a complaint?
I don't know any DWARF producer that would emit such a DWARF piece.
Whether it should be considered valid or not is unclear. Probably not.
But if we emit a complaint for such a piece, then all pieces will become
unusable just because GDB can't deal with that one piece. Before the
change, no complaint was emitted either; not sure if the author had
another reason for that.
--
Andreas
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v2 03/19] PR gdb/21226: Take DWARF stack value pieces from LSB end
2017-05-15 16:35 ` Andreas Arnez
@ 2017-05-16 7:53 ` Yao Qi
[not found] ` <m34lwlf2cq.fsf@oc1027705133.ibm.com>
0 siblings, 1 reply; 56+ messages in thread
From: Yao Qi @ 2017-05-16 7:53 UTC (permalink / raw)
To: Andreas Arnez; +Cc: gdb-patches
Andreas Arnez <arnez@linux.vnet.ibm.com> writes:
>>> - const gdb_byte *val_bytes = value_contents_all (p->v.value);
>>> + /* Use zeroes if piece reaches beyond stack value. */
>>> + if (p->size > stack_value_size_bits)
>>> + break;
>>
>> Does this indicate something wrong in DWARF producer? Does GDB need to
>> emit a complaint?
>
> I don't know any DWARF producer that would emit such a DWARF piece.
> Whether it should be considered valid or not is unclear. Probably not.
> But if we emit a complaint for such a piece, then all pieces will become
> unusable just because GDB can't deal with that one piece. Before the
> change, no complaint was emitted either; not sure if the author had
> another reason for that.
Oh, I thought it is you who add this check and "break" in this patch. I
can't find such logic in the original code.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v2 05/19] gdb/testsuite: Add "get_endianness" convenience proc
2017-05-09 17:47 [PATCH v2 00/19] Various DWARF piece fixes Andreas Arnez
` (3 preceding siblings ...)
2017-05-09 17:49 ` [PATCH v2 03/19] PR gdb/21226: Take DWARF stack value pieces from LSB end Andreas Arnez
@ 2017-05-09 17:50 ` Andreas Arnez
2017-05-11 21:32 ` Yao Qi
2017-05-09 17:51 ` [PATCH v2 06/19] read/write_pieced_value: Respect value parent's offset Andreas Arnez
` (15 subsequent siblings)
20 siblings, 1 reply; 56+ messages in thread
From: Andreas Arnez @ 2017-05-09 17:50 UTC (permalink / raw)
To: gdb-patches
The test suite contains multiple instances of determining the target's
endianness with GDB's "show endian" command. This patch replaces these by
an invocation of a new convenience proc 'get_endianness'.
gdb/testsuite/ChangeLog:
* lib/gdb.exp (get_endianness): New proc.
* gdb.arch/aarch64-fp.exp: Use it.
* gdb.arch/altivec-regs.exp: Likewise.
* gdb.arch/e500-regs.exp: Likewise.
* gdb.arch/vsx-regs.exp: Likewise.
* gdb.base/dump.exp: Likewise.
* gdb.base/funcargs.exp: Likewise.
* gdb.base/gnu_vector.exp: Likewise.
* gdb.dwarf2/formdata16.exp: Likewise.
* gdb.dwarf2/implptrpiece.exp: Likewise.
* gdb.dwarf2/nonvar-access.exp: Likewise.
* gdb.python/py-inferior.exp: Likewise.
* gdb.trace/unavailable-dwarf-piece.exp: Likewise.
---
gdb/testsuite/gdb.arch/aarch64-fp.exp | 9 +--------
gdb/testsuite/gdb.arch/altivec-regs.exp | 12 +-----------
gdb/testsuite/gdb.arch/e500-regs.exp | 12 +-----------
gdb/testsuite/gdb.arch/vsx-regs.exp | 12 +-----------
gdb/testsuite/gdb.base/dump.exp | 7 +------
gdb/testsuite/gdb.base/funcargs.exp | 12 +++---------
gdb/testsuite/gdb.base/gnu_vector.exp | 7 +------
gdb/testsuite/gdb.dwarf2/formdata16.exp | 9 +--------
gdb/testsuite/gdb.dwarf2/implptrpiece.exp | 10 ++--------
gdb/testsuite/gdb.dwarf2/nonvar-access.exp | 10 ++--------
gdb/testsuite/gdb.python/py-inferior.exp | 12 +++---------
gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp | 8 +-------
gdb/testsuite/lib/gdb.exp | 13 +++++++++++++
13 files changed, 31 insertions(+), 102 deletions(-)
diff --git a/gdb/testsuite/gdb.arch/aarch64-fp.exp b/gdb/testsuite/gdb.arch/aarch64-fp.exp
index 2faf121..483c8f1 100644
--- a/gdb/testsuite/gdb.arch/aarch64-fp.exp
+++ b/gdb/testsuite/gdb.arch/aarch64-fp.exp
@@ -34,14 +34,7 @@ if ![runto_main] {
return -1
}
-set endianness "little"
-set test "show endian"
-gdb_test_multiple $test $test {
- -re "(.* )(big|little)( endian.*)$gdb_prompt $" {
- set endianness $expect_out(2,string)
- pass "endianness"
- }
-}
+set endianness [get_endianness]
gdb_test "break ${srcfile}:[gdb_get_line_number "return"]" \
"Breakpoint $decimal at 0x\[0-9a-fA-F\]+: file .*${srcfile}.*\\\." \
diff --git a/gdb/testsuite/gdb.arch/altivec-regs.exp b/gdb/testsuite/gdb.arch/altivec-regs.exp
index 680b512..3cf487b 100644
--- a/gdb/testsuite/gdb.arch/altivec-regs.exp
+++ b/gdb/testsuite/gdb.arch/altivec-regs.exp
@@ -77,17 +77,7 @@ gdb_test "set \$vrsave = 1" "" ""
gdb_test "next" "" ""
-set endianness ""
-set msg "detect endianness"
-gdb_test_multiple "show endian" "$msg" {
- -re "(The target endianness is set automatically .currently )(big|little)( endian.*)$gdb_prompt $" {
- pass "$msg"
- set endianness $expect_out(2,string)
- }
- -re ".*$gdb_prompt $" {
- fail "$msg"
- }
-}
+set endianness [get_endianness]
# And then read the AltiVec registers back, to see that
# a) the register write above worked, and
diff --git a/gdb/testsuite/gdb.arch/e500-regs.exp b/gdb/testsuite/gdb.arch/e500-regs.exp
index 8bf405a..bab6e7d 100644
--- a/gdb/testsuite/gdb.arch/e500-regs.exp
+++ b/gdb/testsuite/gdb.arch/e500-regs.exp
@@ -59,17 +59,7 @@ for {set i 0} {$i < 32} {incr i 1} {
#gdb_test "next" "" ""
-send_gdb "show endian\n"
-gdb_expect {
- -re "(The target endianness is set automatically .currently )(big|little)( endian.*)$gdb_prompt $" {
- pass "endianness"
- set endianness $expect_out(2,string)
- }
- -re ".*$gdb_prompt $" {
- fail "couldn't get endianness"
- }
- timeout { fail "(timeout) endianness" }
-}
+set endianness [get_endianness]
# And then read the E500 registers back, to see that
# a) the register write above worked, and
diff --git a/gdb/testsuite/gdb.arch/vsx-regs.exp b/gdb/testsuite/gdb.arch/vsx-regs.exp
index bedbcaa..ab01c67 100644
--- a/gdb/testsuite/gdb.arch/vsx-regs.exp
+++ b/gdb/testsuite/gdb.arch/vsx-regs.exp
@@ -56,17 +56,7 @@ if ![runto_main] then {
gdb_suppress_tests
}
-set endianness ""
-set msg "detect endianness"
-gdb_test_multiple "show endian" "$msg" {
- -re "(The target endianness is set automatically .currently )(big|little)( endian.*)$gdb_prompt $" {
- pass "$msg"
- set endianness $expect_out(2,string)
- }
- -re ".*$gdb_prompt $" {
- fail "$msg"
- }
-}
+set endianness [get_endianness]
# Data sets used throughout the test
diff --git a/gdb/testsuite/gdb.base/dump.exp b/gdb/testsuite/gdb.base/dump.exp
index e67e1aa..a9625d7 100644
--- a/gdb/testsuite/gdb.base/dump.exp
+++ b/gdb/testsuite/gdb.base/dump.exp
@@ -101,12 +101,7 @@ if { ! [ runto checkpoint1 ] } then {
# Get the endianness for the later use with endianless formats.
-gdb_test_multiple "show endian" "show endian" {
- -re ".* (big|little) endian.*$gdb_prompt $" {
- set endian $expect_out(1,string)
- pass "endianness: $endian"
- }
-}
+set endian [get_endianness]
# Now generate some dump files.
diff --git a/gdb/testsuite/gdb.base/funcargs.exp b/gdb/testsuite/gdb.base/funcargs.exp
index 5852f12..c48b882 100644
--- a/gdb/testsuite/gdb.base/funcargs.exp
+++ b/gdb/testsuite/gdb.base/funcargs.exp
@@ -1139,15 +1139,9 @@ gdb_test_multiple "print sizeof (int)" "getting sizeof int" {
}
}
-gdb_test_multiple "show endian" "getting target endian" {
- -re ".*little endian.*$gdb_prompt $" {
- set target_bigendian_p 0
- # pass silently
- }
- -re ".*big endian.*$gdb_prompt $" {
- set target_bigendian_p 1
- # pass silently
- }
+switch [get_endianness] {
+ little { set target_bigendian_p 0 }
+ big { set target_bigendian_p 1 }
}
# Perform tests
diff --git a/gdb/testsuite/gdb.base/gnu_vector.exp b/gdb/testsuite/gdb.base/gnu_vector.exp
index 1e0e46c..44b1405 100644
--- a/gdb/testsuite/gdb.base/gnu_vector.exp
+++ b/gdb/testsuite/gdb.base/gnu_vector.exp
@@ -48,12 +48,7 @@ if { ![runto main] } {
}
# Get endianess for the scalar->vector casts
-gdb_test_multiple "show endian" "show endian" {
- -re ".* (big|little) endian.*$gdb_prompt $" {
- set endian $expect_out(1,string)
- pass "endianness: $endian"
- }
-}
+set endian [get_endianness]
# Test printing of character vector types
gdb_test "print c4" "\\\$$decimal = \\{1, 2, 3, 4\\}"
diff --git a/gdb/testsuite/gdb.dwarf2/formdata16.exp b/gdb/testsuite/gdb.dwarf2/formdata16.exp
index bba2015..036bf25 100644
--- a/gdb/testsuite/gdb.dwarf2/formdata16.exp
+++ b/gdb/testsuite/gdb.dwarf2/formdata16.exp
@@ -28,14 +28,7 @@ if [prepare_for_testing "failed to prepare for endianness test" ${testfile} ${sr
return -1
}
-set endianness "little"
-set test "show endian"
-gdb_test_multiple $test $test {
- -re "(.* )(big|little)( endian.*)\r\n$gdb_prompt $" {
- set endianness $expect_out(2,string)
- pass "endianness"
- }
-}
+set endianness [get_endianness]
set high "0x123456789abcdef0"
set low "0x0fedcba987654321"
diff --git a/gdb/testsuite/gdb.dwarf2/implptrpiece.exp b/gdb/testsuite/gdb.dwarf2/implptrpiece.exp
index ef483c0..ac3ee78 100644
--- a/gdb/testsuite/gdb.dwarf2/implptrpiece.exp
+++ b/gdb/testsuite/gdb.dwarf2/implptrpiece.exp
@@ -119,14 +119,8 @@ 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"
- }
-}
+# Determine byte order.
+set endian [get_endianness]
# Access the second byte of s through an implicit pointer to the third
# byte of s, using a negative offset. Compare that to the second byte of
diff --git a/gdb/testsuite/gdb.dwarf2/nonvar-access.exp b/gdb/testsuite/gdb.dwarf2/nonvar-access.exp
index 5406029..b7d952a 100644
--- a/gdb/testsuite/gdb.dwarf2/nonvar-access.exp
+++ b/gdb/testsuite/gdb.dwarf2/nonvar-access.exp
@@ -205,14 +205,8 @@ 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"
- }
-}
+# Determine byte order.
+set endian [get_endianness]
# Byte-aligned objects with simple location descriptions.
switch $endian { big {set val 0x345678} little {set val 0x785634} }
diff --git a/gdb/testsuite/gdb.python/py-inferior.exp b/gdb/testsuite/gdb.python/py-inferior.exp
index adb5df3..c2ea2c2 100644
--- a/gdb/testsuite/gdb.python/py-inferior.exp
+++ b/gdb/testsuite/gdb.python/py-inferior.exp
@@ -30,15 +30,9 @@ clean_restart ${testfile}
# Skip all tests if Python scripting is not enabled.
if { [skip_python_tests] } { continue }
-gdb_test_multiple "show endian" "getting target endian" {
- -re ".*little endian.*$gdb_prompt $" {
- set python_pack_char "<"
- # pass silently
- }
- -re ".*big endian.*$gdb_prompt $" {
- set python_pack_char ">"
- # pass silently
- }
+switch [get_endianness] {
+ little { set python_pack_char "<" }
+ big { set python_pack_char ">" }
}
# The following tests require execution.
diff --git a/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp b/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
index 3b14bea..d8a75d4 100644
--- a/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
+++ b/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
@@ -325,13 +325,7 @@ with_test_prefix "tracing bar" {
gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*"
gdb_test_no_output "tstop"
- set endian ""
- gdb_test_multiple "show endian" "show endian" {
- -re ".* (big|little) endian.*$gdb_prompt $" {
- set endian $expect_out(1,string)
- pass "endianness: $endian"
- }
- }
+ set endian [get_endianness]
gdb_test "tfind 0" "Found trace frame 0, tracepoint .*"
if { $endian == "little" } {
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 6633d24..a74f974 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -5671,6 +5671,19 @@ proc set_remotetimeout { timeout } {
}
}
+# Get the target's current endianness and return it.
+proc get_endianness { } {
+ global gdb_prompt
+
+ gdb_test_multiple "show endian" "determine endianness" {
+ -re ".* (little|big) endian.*\r\n$gdb_prompt $" {
+ # Pass silently.
+ return $expect_out(1,string)
+ }
+ }
+ return "little"
+}
+
# ROOT and FULL are file names. Returns the relative path from ROOT
# to FULL. Note that FULL must be in a subdirectory of ROOT.
# For example, given ROOT = /usr/bin and FULL = /usr/bin/ls, this
--
2.5.0
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v2 05/19] gdb/testsuite: Add "get_endianness" convenience proc
2017-05-09 17:50 ` [PATCH v2 05/19] gdb/testsuite: Add "get_endianness" convenience proc Andreas Arnez
@ 2017-05-11 21:32 ` Yao Qi
0 siblings, 0 replies; 56+ messages in thread
From: Yao Qi @ 2017-05-11 21:32 UTC (permalink / raw)
To: Andreas Arnez; +Cc: gdb-patches
On 17-05-09 19:46:01, Andreas Arnez wrote:
> The test suite contains multiple instances of determining the target's
> endianness with GDB's "show endian" command. This patch replaces these by
> an invocation of a new convenience proc 'get_endianness'.
>
> gdb/testsuite/ChangeLog:
>
> * lib/gdb.exp (get_endianness): New proc.
> * gdb.arch/aarch64-fp.exp: Use it.
> * gdb.arch/altivec-regs.exp: Likewise.
> * gdb.arch/e500-regs.exp: Likewise.
> * gdb.arch/vsx-regs.exp: Likewise.
> * gdb.base/dump.exp: Likewise.
> * gdb.base/funcargs.exp: Likewise.
> * gdb.base/gnu_vector.exp: Likewise.
> * gdb.dwarf2/formdata16.exp: Likewise.
> * gdb.dwarf2/implptrpiece.exp: Likewise.
> * gdb.dwarf2/nonvar-access.exp: Likewise.
> * gdb.python/py-inferior.exp: Likewise.
> * gdb.trace/unavailable-dwarf-piece.exp: Likewise.
LGTM.
--
Yao
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v2 06/19] read/write_pieced_value: Respect value parent's offset
2017-05-09 17:47 [PATCH v2 00/19] Various DWARF piece fixes Andreas Arnez
` (4 preceding siblings ...)
2017-05-09 17:50 ` [PATCH v2 05/19] gdb/testsuite: Add "get_endianness" convenience proc Andreas Arnez
@ 2017-05-09 17:51 ` Andreas Arnez
2017-05-16 8:18 ` Yao Qi
2017-05-09 17:51 ` [PATCH v2 07/19] write_pieced_value: Fix copy/paste error in size calculation Andreas Arnez
` (14 subsequent siblings)
20 siblings, 1 reply; 56+ messages in thread
From: Andreas Arnez @ 2017-05-09 17:51 UTC (permalink / raw)
To: gdb-patches
In the case of targeting a bit-field, read_pieced_value and
write_pieced_value calculate the number of bits preceding the bit-field
without considering the relative offset of the value's parent. This is
relevant for a structure variable like this:
struct s {
uint64_t foo;
struct {
uint32_t bar;
uint32_t bf : 10; /* <-- target bit-field */
} baz;
} s;
In this scenario, if 'val' is a GDB value representing s.baz.bf,
val->parent represents the whole s.baz structure, and the following holds:
- value_offset (val) == sizeof s.baz.bar == 4
- value_offset (val->parent) == sizeof s.foo == 8
The current logic would only use value_offset(val), resulting in the wrong
offset into the target value. This is fixed.
gdb/ChangeLog:
* dwarf2loc.c (read_pieced_value): Respect parent value's offset
when targeting a bit-field.
(write_pieced_value): Likewise.
---
gdb/dwarf2loc.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 8f99844..6ce2993 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1776,7 +1776,8 @@ read_pieced_value (struct value *v)
bits_to_skip = 8 * value_offset (v);
if (value_bitsize (v))
{
- bits_to_skip += value_bitpos (v);
+ bits_to_skip += (8 * value_offset (value_parent (v))
+ + value_bitpos (v));
type_len = value_bitsize (v);
}
else
@@ -1946,7 +1947,8 @@ write_pieced_value (struct value *to, struct value *from)
bits_to_skip = 8 * value_offset (to);
if (value_bitsize (to))
{
- bits_to_skip += value_bitpos (to);
+ bits_to_skip += (8 * value_offset (value_parent (to))
+ + value_bitpos (to));
type_len = value_bitsize (to);
}
else
--
2.5.0
^ permalink raw reply [flat|nested] 56+ messages in thread* [PATCH v2 07/19] write_pieced_value: Fix copy/paste error in size calculation
2017-05-09 17:47 [PATCH v2 00/19] Various DWARF piece fixes Andreas Arnez
` (5 preceding siblings ...)
2017-05-09 17:51 ` [PATCH v2 06/19] read/write_pieced_value: Respect value parent's offset Andreas Arnez
@ 2017-05-09 17:51 ` Andreas Arnez
2017-05-16 8:29 ` Yao Qi
2017-05-09 17:52 ` [PATCH v2 08/19] write_pieced_value: Include transfer size in byte-wise check Andreas Arnez
` (13 subsequent siblings)
20 siblings, 1 reply; 56+ messages in thread
From: Andreas Arnez @ 2017-05-09 17:51 UTC (permalink / raw)
To: gdb-patches
In write_pieced_value, the number of bytes containing a portion of the
bit-field in a given piece is calculated with the wrong starting offset;
thus the result may be off by one. This bug was probably introduced when
copying this logic from read_pieced_value. Fix it.
gdb/ChangeLog:
* dwarf2loc.c (write_pieced_value): Fix copy/paste error in the
calculation of this_size.
---
gdb/dwarf2loc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 6ce2993..1122c8a 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1983,7 +1983,7 @@ write_pieced_value (struct value *to, struct value *from)
if (this_size_bits > type_len - offset)
this_size_bits = type_len - offset;
- this_size = (this_size_bits + source_offset_bits % 8 + 7) / 8;
+ this_size = (this_size_bits + dest_offset_bits % 8 + 7) / 8;
source_offset = source_offset_bits / 8;
dest_offset = dest_offset_bits / 8;
if (dest_offset_bits % 8 == 0 && source_offset_bits % 8 == 0)
--
2.5.0
^ permalink raw reply [flat|nested] 56+ messages in thread* [PATCH v2 08/19] write_pieced_value: Include transfer size in byte-wise check
2017-05-09 17:47 [PATCH v2 00/19] Various DWARF piece fixes Andreas Arnez
` (6 preceding siblings ...)
2017-05-09 17:51 ` [PATCH v2 07/19] write_pieced_value: Fix copy/paste error in size calculation Andreas Arnez
@ 2017-05-09 17:52 ` Andreas Arnez
2017-05-16 8:32 ` Yao Qi
2017-05-09 17:53 ` [PATCH v2 10/19] write_pieced_value: Transfer least significant bits into bit-field Andreas Arnez
` (12 subsequent siblings)
20 siblings, 1 reply; 56+ messages in thread
From: Andreas Arnez @ 2017-05-09 17:52 UTC (permalink / raw)
To: gdb-patches
In write_pieced_value, when checking whether the data can be transferred
byte-wise, the current logic verifies the source- and destination offsets
to be byte-aligned, but not the transfer size. This is fixed.
gdb/ChangeLog:
* dwarf2loc.c (write_pieced_value): Include transfer size in
byte-wise check.
---
gdb/dwarf2loc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 1122c8a..8b5cbc5 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1986,7 +1986,8 @@ write_pieced_value (struct value *to, struct value *from)
this_size = (this_size_bits + dest_offset_bits % 8 + 7) / 8;
source_offset = source_offset_bits / 8;
dest_offset = dest_offset_bits / 8;
- if (dest_offset_bits % 8 == 0 && source_offset_bits % 8 == 0)
+ if (dest_offset_bits % 8 == 0 && source_offset_bits % 8 == 0
+ && this_size_bits % 8 == 0)
{
source_buffer = contents + source_offset;
need_bitwise = 0;
--
2.5.0
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v2 08/19] write_pieced_value: Include transfer size in byte-wise check
2017-05-09 17:52 ` [PATCH v2 08/19] write_pieced_value: Include transfer size in byte-wise check Andreas Arnez
@ 2017-05-16 8:32 ` Yao Qi
2017-05-16 13:45 ` Andreas Arnez
0 siblings, 1 reply; 56+ messages in thread
From: Yao Qi @ 2017-05-16 8:32 UTC (permalink / raw)
To: Andreas Arnez; +Cc: gdb-patches
Andreas Arnez <arnez@linux.vnet.ibm.com> writes:
> In write_pieced_value, when checking whether the data can be transferred
> byte-wise, the current logic verifies the source- and destination offsets
> to be byte-aligned, but not the transfer size. This is fixed.
>
> ---
> gdb/dwarf2loc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
> index 1122c8a..8b5cbc5 100644
> --- a/gdb/dwarf2loc.c
> +++ b/gdb/dwarf2loc.c
> @@ -1986,7 +1986,8 @@ write_pieced_value (struct value *to, struct value *from)
> this_size = (this_size_bits + dest_offset_bits % 8 + 7) / 8;
> source_offset = source_offset_bits / 8;
> dest_offset = dest_offset_bits / 8;
> - if (dest_offset_bits % 8 == 0 && source_offset_bits % 8 == 0)
> + if (dest_offset_bits % 8 == 0 && source_offset_bits % 8 == 0
> + && this_size_bits % 8 == 0)
Can you add a comment here saying that "Check whether the data can be
transferred byte-wise."?
Otherwise, patch is good tome.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH v2 08/19] write_pieced_value: Include transfer size in byte-wise check
2017-05-16 8:32 ` Yao Qi
@ 2017-05-16 13:45 ` Andreas Arnez
0 siblings, 0 replies; 56+ messages in thread
From: Andreas Arnez @ 2017-05-16 13:45 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On Tue, May 16 2017, Yao Qi wrote:
> Andreas Arnez <arnez@linux.vnet.ibm.com> writes:
>
>> In write_pieced_value, when checking whether the data can be transferred
>> byte-wise, the current logic verifies the source- and destination offsets
>> to be byte-aligned, but not the transfer size. This is fixed.
>>
>
>> ---
>> gdb/dwarf2loc.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
>> index 1122c8a..8b5cbc5 100644
>> --- a/gdb/dwarf2loc.c
>> +++ b/gdb/dwarf2loc.c
>> @@ -1986,7 +1986,8 @@ write_pieced_value (struct value *to, struct value *from)
>> this_size = (this_size_bits + dest_offset_bits % 8 + 7) / 8;
>> source_offset = source_offset_bits / 8;
>> dest_offset = dest_offset_bits / 8;
>> - if (dest_offset_bits % 8 == 0 && source_offset_bits % 8 == 0)
>> + if (dest_offset_bits % 8 == 0 && source_offset_bits % 8 == 0
>> + && this_size_bits % 8 == 0)
>
> Can you add a comment here saying that "Check whether the data can be
> transferred byte-wise."?
OK.
> Otherwise, patch is good tome.
Thanks!
--
Andreas
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v2 10/19] write_pieced_value: Transfer least significant bits into bit-field
2017-05-09 17:47 [PATCH v2 00/19] Various DWARF piece fixes Andreas Arnez
` (7 preceding siblings ...)
2017-05-09 17:52 ` [PATCH v2 08/19] write_pieced_value: Include transfer size in byte-wise check Andreas Arnez
@ 2017-05-09 17:53 ` Andreas Arnez
2017-05-16 9:14 ` Yao Qi
2017-05-09 17:53 ` [PATCH v2 09/19] write_pieced_value: Fix buffer offset for memory pieces Andreas Arnez
` (11 subsequent siblings)
20 siblings, 1 reply; 56+ messages in thread
From: Andreas Arnez @ 2017-05-09 17:53 UTC (permalink / raw)
To: gdb-patches
On big-endian targets, when targeting a bit-field, write_pieced_value
currently transfers the source value's *most* significant bits to the
target value, instead of its least significant bits. This is fixed.
In particular the fix adjusts the initial value of 'offset', which can now
potentially be nonzero. Thus the variable 'type_len' is renamed to
'max_offset', to avoid confusion. And for consistency, the affected logic
that was mirrored in read_pieced_value is changed there in the same way.
gdb/ChangeLog:
* dwarf2loc.c (write_pieced_value): When writing to a bit-field,
transfer the source value's least significant bits, instead of its
lowest-addressed ones. Rename type_len to max_offset.
(read_pieced_value): Mirror above changes to write_pieced_value as
applicable.
---
gdb/dwarf2loc.c | 70 +++++++++++++++++++++++++--------------------------------
1 file changed, 31 insertions(+), 39 deletions(-)
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index e1e1562..da0e8c3 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1756,12 +1756,11 @@ static void
read_pieced_value (struct value *v)
{
int i;
- long offset = 0;
+ LONGEST offset = 0, max_offset;
ULONGEST bits_to_skip;
gdb_byte *contents;
struct piece_closure *c
= (struct piece_closure *) value_computed_closure (v);
- size_t type_len;
size_t buffer_size = 0;
std::vector<gdb_byte> buffer;
int bits_big_endian
@@ -1778,12 +1777,12 @@ read_pieced_value (struct value *v)
{
bits_to_skip += (8 * value_offset (value_parent (v))
+ value_bitpos (v));
- type_len = value_bitsize (v);
+ max_offset = value_bitsize (v);
}
else
- type_len = 8 * TYPE_LENGTH (value_type (v));
+ max_offset = 8 * TYPE_LENGTH (value_type (v));
- for (i = 0; i < c->n_pieces && offset < type_len; i++)
+ for (i = 0; i < c->n_pieces && offset < max_offset; i++)
{
struct dwarf_expr_piece *p = &c->pieces[i];
size_t this_size, this_size_bits;
@@ -1798,20 +1797,13 @@ read_pieced_value (struct value *v)
bits_to_skip -= this_size_bits;
continue;
}
- if (bits_to_skip > 0)
- {
- dest_offset_bits = 0;
- source_offset_bits = bits_to_skip;
- this_size_bits -= bits_to_skip;
- bits_to_skip = 0;
- }
- else
- {
- dest_offset_bits = offset;
- source_offset_bits = 0;
- }
- if (this_size_bits > type_len - offset)
- this_size_bits = type_len - offset;
+ source_offset_bits = bits_to_skip;
+ this_size_bits -= bits_to_skip;
+ bits_to_skip = 0;
+ dest_offset_bits = offset;
+
+ if (this_size_bits > max_offset - offset)
+ this_size_bits = max_offset - offset;
this_size = (this_size_bits + source_offset_bits % 8 + 7) / 8;
source_offset = source_offset_bits / 8;
@@ -1932,12 +1924,11 @@ static void
write_pieced_value (struct value *to, struct value *from)
{
int i;
- long offset = 0;
ULONGEST bits_to_skip;
+ LONGEST offset = 0, max_offset;
const gdb_byte *contents;
struct piece_closure *c
= (struct piece_closure *) value_computed_closure (to);
- size_t type_len;
size_t buffer_size = 0;
std::vector<gdb_byte> buffer;
int bits_big_endian
@@ -1949,12 +1940,20 @@ write_pieced_value (struct value *to, struct value *from)
{
bits_to_skip += (8 * value_offset (value_parent (to))
+ value_bitpos (to));
- type_len = value_bitsize (to);
- }
+ /* Use the least significant bits of FROM. */
+ if (gdbarch_byte_order (get_type_arch (value_type (from)))
+ == BFD_ENDIAN_BIG)
+ {
+ max_offset = 8 * TYPE_LENGTH (value_type (from));
+ offset = max_offset - value_bitsize (to);
+ }
+ else
+ max_offset = value_bitsize (to);
+ }
else
- type_len = 8 * TYPE_LENGTH (value_type (to));
+ max_offset = 8 * TYPE_LENGTH (value_type (to));
- for (i = 0; i < c->n_pieces && offset < type_len; i++)
+ for (i = 0; i < c->n_pieces && offset < max_offset; i++)
{
struct dwarf_expr_piece *p = &c->pieces[i];
size_t this_size_bits, this_size;
@@ -1968,20 +1967,13 @@ write_pieced_value (struct value *to, struct value *from)
bits_to_skip -= this_size_bits;
continue;
}
- if (bits_to_skip > 0)
- {
- dest_offset_bits = bits_to_skip;
- source_offset_bits = 0;
- this_size_bits -= bits_to_skip;
- bits_to_skip = 0;
- }
- else
- {
- dest_offset_bits = 0;
- source_offset_bits = offset;
- }
- if (this_size_bits > type_len - offset)
- this_size_bits = type_len - offset;
+ dest_offset_bits = bits_to_skip;
+ this_size_bits -= bits_to_skip;
+ bits_to_skip = 0;
+ source_offset_bits = offset;
+
+ if (this_size_bits > max_offset - offset)
+ this_size_bits = max_offset - offset;
this_size = (this_size_bits + dest_offset_bits % 8 + 7) / 8;
source_offset = source_offset_bits / 8;
--
2.5.0
^ permalink raw reply [flat|nested] 56+ messages in thread* [PATCH v2 09/19] write_pieced_value: Fix buffer offset for memory pieces
2017-05-09 17:47 [PATCH v2 00/19] Various DWARF piece fixes Andreas Arnez
` (8 preceding siblings ...)
2017-05-09 17:53 ` [PATCH v2 10/19] write_pieced_value: Transfer least significant bits into bit-field Andreas Arnez
@ 2017-05-09 17:53 ` Andreas Arnez
2017-05-16 8:46 ` Yao Qi
2017-05-09 17:54 ` [PATCH v2 11/19] Add DWARF piece test cases for bit-field access Andreas Arnez
` (10 subsequent siblings)
20 siblings, 1 reply; 56+ messages in thread
From: Andreas Arnez @ 2017-05-09 17:53 UTC (permalink / raw)
To: gdb-patches
In write_pieced_value, when transferring the data to target memory via a
buffer, the bit offset within the target value is not reduced to its
sub-byte fraction before using it as a bit offset into the buffer. This
is fixed.
gdb/ChangeLog:
* dwarf2loc.c (write_pieced_value): In DWARF_VALUE_MEMORY,
truncate full bytes from dest_offset_bits before using it as an
offset into the buffer.
---
gdb/dwarf2loc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 8b5cbc5..e1e1562 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -2056,7 +2056,7 @@ write_pieced_value (struct value *to, struct value *from)
read_memory (p->v.mem.addr + dest_offset, buffer.data (), 1);
read_memory (p->v.mem.addr + dest_offset + this_size - 1,
&buffer[this_size - 1], 1);
- copy_bitwise (buffer.data (), dest_offset_bits,
+ copy_bitwise (buffer.data (), dest_offset_bits % 8,
contents, source_offset_bits,
this_size_bits,
bits_big_endian);
--
2.5.0
^ permalink raw reply [flat|nested] 56+ messages in thread* [PATCH v2 11/19] Add DWARF piece test cases for bit-field access
2017-05-09 17:47 [PATCH v2 00/19] Various DWARF piece fixes Andreas Arnez
` (9 preceding siblings ...)
2017-05-09 17:53 ` [PATCH v2 09/19] write_pieced_value: Fix buffer offset for memory pieces Andreas Arnez
@ 2017-05-09 17:54 ` Andreas Arnez
2017-05-16 13:52 ` Yao Qi
2017-05-09 17:55 ` [PATCH v2 13/19] Fix handling of DWARF register pieces on big-endian targets Andreas Arnez
` (9 subsequent siblings)
20 siblings, 1 reply; 56+ messages in thread
From: Andreas Arnez @ 2017-05-09 17:54 UTC (permalink / raw)
To: gdb-patches
This verifies some of the previous fixes to the logic in
write_pieced_value when accessing bit-fields.
gdb/testsuite/ChangeLog:
* gdb.dwarf2/var-access.exp: Add tests for accessing bit-fields
located in one or more DWARF pieces.
---
gdb/testsuite/gdb.dwarf2/var-access.exp | 81 ++++++++++++++++++++++++++++++++-
1 file changed, 80 insertions(+), 1 deletion(-)
diff --git a/gdb/testsuite/gdb.dwarf2/var-access.exp b/gdb/testsuite/gdb.dwarf2/var-access.exp
index bd92a44..3ba2c08 100644
--- a/gdb/testsuite/gdb.dwarf2/var-access.exp
+++ b/gdb/testsuite/gdb.dwarf2/var-access.exp
@@ -62,7 +62,8 @@ Dwarf::assemble $asm_file {
} {
declare_labels char_type_label
declare_labels int_type_label short_type_label
- declare_labels array_a8_label struct_s_label
+ declare_labels array_a8_label struct_s_label struct_t_label
+ declare_labels struct_st_label
# char
char_type_label: base_type {
@@ -115,6 +116,51 @@ Dwarf::assemble $asm_file {
}
}
+ # struct t { int u, x:9, y:13, z:10; };
+ struct_t_label: structure_type {
+ {name "t"}
+ {byte_size 8 DW_FORM_sdata}
+ } {
+ member {
+ {name "u"}
+ {type :$int_type_label}
+ }
+ member {
+ {name "x"}
+ {type :$int_type_label}
+ {data_member_location 4 DW_FORM_udata}
+ {bit_size 9 DW_FORM_udata}
+ }
+ member {
+ {name "y"}
+ {type :$int_type_label}
+ {data_bit_offset 41 DW_FORM_udata}
+ {bit_size 13 DW_FORM_udata}
+ }
+ member {
+ {name "z"}
+ {type :$int_type_label}
+ {data_bit_offset 54 DW_FORM_udata}
+ {bit_size 10 DW_FORM_udata}
+ }
+ }
+
+ # struct st { struct s s; struct t t; };
+ struct_st_label: structure_type {
+ {name "st"}
+ {byte_size 12 DW_FORM_udata}
+ } {
+ member {
+ {name "s"}
+ {type :$struct_s_label}
+ }
+ member {
+ {name "t"}
+ {type :$struct_t_label}
+ {data_member_location 4 DW_FORM_udata}
+ }
+ }
+
DW_TAG_subprogram {
{MACRO_AT_func { main ${srcdir}/${subdir}/${srcfile} }}
{DW_AT_external 1 flag}
@@ -156,6 +202,19 @@ Dwarf::assemble $asm_file {
piece 1
} SPECIAL_expr}
}
+ # Memory pieces for bitfield access: 8 bytes optimized
+ # out, 3 bytes from &buf[3], and 1 byte from &buf[1].
+ DW_TAG_variable {
+ {name "st1"}
+ {type :$struct_st_label}
+ {location {
+ piece 8
+ addr "$buf_var + 3"
+ piece 3
+ addr "$buf_var + 1"
+ piece 1
+ } SPECIAL_expr}
+ }
}
}
}
@@ -170,6 +229,9 @@ if ![runto_main] {
return -1
}
+# Determine byte order.
+set endian [get_endianness]
+
# Byte-aligned memory pieces.
gdb_test "print/d s1" " = \\{a = 2, b = 3, c = 0, d = 1\\}" \
"s1 == re-ordered buf"
@@ -200,3 +262,20 @@ gdb_test_no_output "set var s2 = {191, 73, 231, 123}" \
"re-initialize s2"
gdb_test "print/d s2" " = \\{a = 191, b = 73, c = 231, d = 123\\}" \
"verify re-initialized s2"
+
+# Unaligned bitfield access through byte-aligned pieces.
+gdb_test_no_output "set var a = { 0 }"
+gdb_test_no_output "set var st1.t.x = -7"
+gdb_test_no_output "set var st1.t.z = 340"
+gdb_test_no_output "set var st1.t.y = 1234"
+gdb_test "print st1.t" " = \\{u = <optimized out>, x = -7, y = 1234, z = 340\\}" \
+ "verify st1.t"
+switch $endian {
+ little {set val "0x55, 0x0, 0xf9, 0xa5, 0x9"}
+ big {set val "0x54, 0x0, 0xfc, 0x93, 0x49"}
+}
+# | -- | z:2-9 | -- | x:0-7 | x:8 y:0-6 | y:7-12 z:0-1 | -- | -- |
+# \_______________________________________________/
+# val
+gdb_test "print/x a" " = \\{0x0, ${val}, 0x0, 0x0\\}" \
+ "verify st1 through a"
--
2.5.0
^ permalink raw reply [flat|nested] 56+ messages in thread* [PATCH v2 13/19] Fix handling of DWARF register pieces on big-endian targets
2017-05-09 17:47 [PATCH v2 00/19] Various DWARF piece fixes Andreas Arnez
` (10 preceding siblings ...)
2017-05-09 17:54 ` [PATCH v2 11/19] Add DWARF piece test cases for bit-field access Andreas Arnez
@ 2017-05-09 17:55 ` Andreas Arnez
2017-06-12 13:12 ` Yao Qi
2017-05-09 17:55 ` [PATCH v2 12/19] read/write_pieced_value: Drop 'buffer_size' variable Andreas Arnez
` (8 subsequent siblings)
20 siblings, 1 reply; 56+ messages in thread
From: Andreas Arnez @ 2017-05-09 17:55 UTC (permalink / raw)
To: gdb-patches
For big-endian targets the logic in read/write_pieced_value tries to take
a register piece from the LSB end. This requires offsets and sizes to be
adjusted accordingly, and that's where the current implementation has some
issues:
* The formulas for recalculating the bit- and byte-offsets into the
register are wrong. They just happen to yield correct results if
everything is byte-aligned and the piece's last byte belongs to the
given value.
* After recalculating the bit offset into the register, the number of
bytes to be copied from the register is not recalculated. Of course
this does not matter if everything (particularly the piece size) is
byte-aligned.
These issues are fixed. The size calculation is performed with a new
helper function bits_to_bytes().
gdb/ChangeLog:
* dwarf2loc.c (bits_to_bytes): New function.
(read_pieced_value): Fix offset calculations for register pieces
on big-endian targets.
(write_pieced_value): Likewise.
gdb/testsuite/ChangeLog:
* gdb.dwarf2/var-access.exp: Add test for non-byte-aligned
register pieces.
---
gdb/dwarf2loc.c | 56 ++++++++++++++++++++-------------
gdb/testsuite/gdb.dwarf2/var-access.exp | 25 +++++++++++++++
2 files changed, 60 insertions(+), 21 deletions(-)
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 5c29cf6..62462f0 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1752,6 +1752,15 @@ copy_bitwise_tests (void)
#endif /* GDB_SELF_TEST */
+/* Return the number of bytes overlapping a contiguous chunk of N_BITS
+ bits whose first bit is located at bit offset START. */
+
+static size_t
+bits_to_bytes (ULONGEST start, ULONGEST n_bits)
+{
+ return (start % 8 + n_bits + 7) / 8;
+}
+
static void
read_pieced_value (struct value *v)
{
@@ -1804,7 +1813,7 @@ read_pieced_value (struct value *v)
if (this_size_bits > max_offset - offset)
this_size_bits = max_offset - offset;
- this_size = (this_size_bits + source_offset_bits % 8 + 7) / 8;
+ this_size = bits_to_bytes (source_offset_bits, this_size_bits);
buffer.reserve (this_size);
source_offset = source_offset_bits / 8;
intermediate_buffer = buffer.data ();
@@ -1817,20 +1826,20 @@ read_pieced_value (struct value *v)
struct frame_info *frame = frame_find_by_id (c->frame_id);
struct gdbarch *arch = get_frame_arch (frame);
int gdb_regnum = dwarf_reg_to_regnum_or_error (arch, p->v.regno);
+ ULONGEST reg_bits = 8 * register_size (arch, gdb_regnum);
int optim, unavail;
- LONGEST reg_offset = source_offset;
if (gdbarch_byte_order (arch) == BFD_ENDIAN_BIG
- && this_size < register_size (arch, gdb_regnum))
+ && p->size < reg_bits)
{
/* Big-endian, and we want less than full size. */
- reg_offset = register_size (arch, gdb_regnum) - this_size;
- /* We want the lower-order THIS_SIZE_BITS of the bytes
- we extract from the register. */
- source_offset_bits += 8 * this_size - this_size_bits;
+ source_offset_bits += reg_bits - p->size;
}
+ this_size = bits_to_bytes (source_offset_bits, this_size_bits);
+ buffer.reserve (this_size);
- if (!get_frame_register_bytes (frame, gdb_regnum, reg_offset,
+ if (!get_frame_register_bytes (frame, gdb_regnum,
+ source_offset_bits / 8,
this_size, buffer.data (),
&optim, &unavail))
{
@@ -1844,7 +1853,7 @@ read_pieced_value (struct value *v)
}
copy_bitwise (contents, dest_offset_bits,
- intermediate_buffer, source_offset_bits % 8,
+ buffer.data (), source_offset_bits % 8,
this_size_bits, bits_big_endian);
}
break;
@@ -1969,7 +1978,7 @@ write_pieced_value (struct value *to, struct value *from)
if (this_size_bits > max_offset - offset)
this_size_bits = max_offset - offset;
- this_size = (this_size_bits + dest_offset_bits % 8 + 7) / 8;
+ this_size = bits_to_bytes (dest_offset_bits, this_size_bits);
source_offset = source_offset_bits / 8;
dest_offset = dest_offset_bits / 8;
if (dest_offset_bits % 8 == 0 && source_offset_bits % 8 == 0
@@ -1992,20 +2001,25 @@ write_pieced_value (struct value *to, struct value *from)
struct frame_info *frame = frame_find_by_id (c->frame_id);
struct gdbarch *arch = get_frame_arch (frame);
int gdb_regnum = dwarf_reg_to_regnum_or_error (arch, p->v.regno);
- int reg_offset = dest_offset;
+ ULONGEST reg_bits = 8 * register_size (arch, gdb_regnum);
if (gdbarch_byte_order (arch) == BFD_ENDIAN_BIG
- && this_size <= register_size (arch, gdb_regnum))
+ && p->size <= reg_bits)
{
/* Big-endian, and we want less than full size. */
- reg_offset = register_size (arch, gdb_regnum) - this_size;
+ dest_offset_bits += reg_bits - p->size;
}
+ this_size = bits_to_bytes (dest_offset_bits, this_size_bits);
+ buffer.reserve (this_size);
- if (need_bitwise)
+ if (dest_offset_bits % 8 != 0 || this_size_bits % 8 != 0)
{
+ /* Data is copied non-byte-aligned into the register.
+ Need some bits from original register value. */
int optim, unavail;
- if (!get_frame_register_bytes (frame, gdb_regnum, reg_offset,
+ if (!get_frame_register_bytes (frame, gdb_regnum,
+ dest_offset_bits / 8,
this_size, buffer.data (),
&optim, &unavail))
{
@@ -2020,14 +2034,14 @@ write_pieced_value (struct value *to, struct value *from)
"bitfield; containing word "
"is unavailable"));
}
- copy_bitwise (buffer.data (), dest_offset_bits,
- contents, source_offset_bits,
- this_size_bits,
- bits_big_endian);
}
- put_frame_register_bytes (frame, gdb_regnum, reg_offset,
- this_size, source_buffer);
+ copy_bitwise (buffer.data (), dest_offset_bits % 8,
+ contents, source_offset_bits,
+ this_size_bits, bits_big_endian);
+ put_frame_register_bytes (frame, gdb_regnum,
+ dest_offset_bits / 8,
+ this_size, buffer.data ());
}
break;
case DWARF_VALUE_MEMORY:
diff --git a/gdb/testsuite/gdb.dwarf2/var-access.exp b/gdb/testsuite/gdb.dwarf2/var-access.exp
index 3ba2c08..c533b8d 100644
--- a/gdb/testsuite/gdb.dwarf2/var-access.exp
+++ b/gdb/testsuite/gdb.dwarf2/var-access.exp
@@ -215,6 +215,19 @@ Dwarf::assemble $asm_file {
piece 1
} SPECIAL_expr}
}
+ # Register pieces for bitfield access: 4 bytes optimized
+ # out, 3 bytes from r0, and 1 byte from r1.
+ DW_TAG_variable {
+ {name "t2"}
+ {type :$struct_t_label}
+ {location {
+ piece 4
+ regx [lindex $dwarf_regnum 0]
+ piece 3
+ regx [lindex $dwarf_regnum 1]
+ piece 1
+ } SPECIAL_expr}
+ }
}
}
}
@@ -279,3 +292,15 @@ switch $endian {
# val
gdb_test "print/x a" " = \\{0x0, ${val}, 0x0, 0x0\\}" \
"verify st1 through a"
+
+switch $endian { big {set val 0x7ffc} little {set val 0x3ffe00} }
+gdb_test_no_output "set var \$[lindex $regname 0] = $val" \
+ "init t2, first piece"
+gdb_test_no_output "set var \$[lindex $regname 1] = 0" \
+ "init t2, second piece"
+gdb_test "print/d t2" " = \\{u = <optimized out>, x = 0, y = -1, z = 0\\}" \
+ "initialized t2 from regs"
+gdb_test_no_output "set var t2.y = 2641"
+gdb_test_no_output "set var t2.z = -400"
+gdb_test_no_output "set var t2.x = 200"
+gdb_test "print t2.x + t2.y + t2.z" " = 2441"
--
2.5.0
^ permalink raw reply [flat|nested] 56+ messages in thread* [PATCH v2 12/19] read/write_pieced_value: Drop 'buffer_size' variable
2017-05-09 17:47 [PATCH v2 00/19] Various DWARF piece fixes Andreas Arnez
` (11 preceding siblings ...)
2017-05-09 17:55 ` [PATCH v2 13/19] Fix handling of DWARF register pieces on big-endian targets Andreas Arnez
@ 2017-05-09 17:55 ` Andreas Arnez
2017-05-16 14:08 ` Yao Qi
2017-05-09 17:56 ` [PATCH v2 14/19] read/write_pieced_value: Improve logic for buffer allocation Andreas Arnez
` (7 subsequent siblings)
20 siblings, 1 reply; 56+ messages in thread
From: Andreas Arnez @ 2017-05-09 17:55 UTC (permalink / raw)
To: gdb-patches
When the variable 'buffer_size' in read_pieced_value and
write_pieced_value was introduced, it was needed for tracking the buffer's
allocated size. Now that the buffer's data type has been changed to a
std::vector, the variable is no longer necessary; so remove it.
gdb/ChangeLog:
* dwarf2loc.c (read_pieced_value): Remove buffer_size variable.
(write_pieced_value): Likewise.
---
gdb/dwarf2loc.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index da0e8c3..5c29cf6 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1761,7 +1761,6 @@ read_pieced_value (struct value *v)
gdb_byte *contents;
struct piece_closure *c
= (struct piece_closure *) value_computed_closure (v);
- size_t buffer_size = 0;
std::vector<gdb_byte> buffer;
int bits_big_endian
= gdbarch_bits_big_endian (get_type_arch (value_type (v)));
@@ -1806,12 +1805,8 @@ read_pieced_value (struct value *v)
this_size_bits = max_offset - offset;
this_size = (this_size_bits + source_offset_bits % 8 + 7) / 8;
+ buffer.reserve (this_size);
source_offset = source_offset_bits / 8;
- if (buffer_size < this_size)
- {
- buffer_size = this_size;
- buffer.reserve (buffer_size);
- }
intermediate_buffer = buffer.data ();
/* Copy from the source to DEST_BUFFER. */
@@ -1929,7 +1924,6 @@ write_pieced_value (struct value *to, struct value *from)
const gdb_byte *contents;
struct piece_closure *c
= (struct piece_closure *) value_computed_closure (to);
- size_t buffer_size = 0;
std::vector<gdb_byte> buffer;
int bits_big_endian
= gdbarch_bits_big_endian (get_type_arch (value_type (to)));
@@ -1986,11 +1980,7 @@ write_pieced_value (struct value *to, struct value *from)
}
else
{
- if (buffer_size < this_size)
- {
- buffer_size = this_size;
- buffer.reserve (buffer_size);
- }
+ buffer.reserve (this_size);
source_buffer = buffer.data ();
need_bitwise = 1;
}
--
2.5.0
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v2 12/19] read/write_pieced_value: Drop 'buffer_size' variable
2017-05-09 17:55 ` [PATCH v2 12/19] read/write_pieced_value: Drop 'buffer_size' variable Andreas Arnez
@ 2017-05-16 14:08 ` Yao Qi
2017-05-16 17:51 ` Andreas Arnez
0 siblings, 1 reply; 56+ messages in thread
From: Yao Qi @ 2017-05-16 14:08 UTC (permalink / raw)
To: Andreas Arnez; +Cc: gdb-patches
Andreas Arnez <arnez@linux.vnet.ibm.com> writes:
Patch is good to me, a nit below,
> @@ -1806,12 +1805,8 @@ read_pieced_value (struct value *v)
> this_size_bits = max_offset - offset;
>
> this_size = (this_size_bits + source_offset_bits % 8 + 7) / 8;
> + buffer.reserve (this_size);
I noticed that buffer is only used within the for loop, so probably we
can move it into the loop, and do "std::vector<gdb_byte> buffer (this_size);"
This change makes troubles for you to rebase your following patches, so
if you want to change it, you can do it in another patch, 20/19, for
example.
> source_offset = source_offset_bits / 8;
> - if (buffer_size < this_size)
> - {
> - buffer_size = this_size;
> - buffer.reserve (buffer_size);
> - }
> intermediate_buffer = buffer.data ();
>
> /* Copy from the source to DEST_BUFFER. */
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v2 12/19] read/write_pieced_value: Drop 'buffer_size' variable
2017-05-16 14:08 ` Yao Qi
@ 2017-05-16 17:51 ` Andreas Arnez
0 siblings, 0 replies; 56+ messages in thread
From: Andreas Arnez @ 2017-05-16 17:51 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On Tue, May 16 2017, Yao Qi wrote:
> Andreas Arnez <arnez@linux.vnet.ibm.com> writes:
>
> Patch is good to me, a nit below,
>
>> @@ -1806,12 +1805,8 @@ read_pieced_value (struct value *v)
>> this_size_bits = max_offset - offset;
>>
>> this_size = (this_size_bits + source_offset_bits % 8 + 7) / 8;
>> + buffer.reserve (this_size);
>
> I noticed that buffer is only used within the for loop, so probably we
> can move it into the loop, and do "std::vector<gdb_byte> buffer (this_size);"
I thought there might be a slight performance benefit in keeping the
buffer variable across loop iterations and just '.reserve' more space
when needed. This was probably the original intention as well.
--
Andreas
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v2 14/19] read/write_pieced_value: Improve logic for buffer allocation
2017-05-09 17:47 [PATCH v2 00/19] Various DWARF piece fixes Andreas Arnez
` (12 preceding siblings ...)
2017-05-09 17:55 ` [PATCH v2 12/19] read/write_pieced_value: Drop 'buffer_size' variable Andreas Arnez
@ 2017-05-09 17:56 ` Andreas Arnez
2017-06-12 13:28 ` Yao Qi
2017-06-12 19:40 ` Simon Marchi
2017-05-09 17:57 ` [PATCH v2 15/19] Respect piece offset for DW_OP_bit_piece Andreas Arnez
` (6 subsequent siblings)
20 siblings, 2 replies; 56+ messages in thread
From: Andreas Arnez @ 2017-05-09 17:56 UTC (permalink / raw)
To: gdb-patches
So far the main loop in read_pieced_value and write_pieced_value is
structured like this:
(1) Prepare a buffer and some variables we may need;
(2) depending on the DWARF piece type to be handled, use the buffer and
the prepared variables, ignore them, or even recalculate them.
This approach reduces readability and may also lead to unnecessary copying
of data. This patch moves the preparations to the places where sufficient
information is available and removes some of the variables involved.
gdb/ChangeLog:
* dwarf2loc.c (read_pieced_value): Move the buffer allocation and
some other preparations to the places where sufficient information
is available.
(write_pieced_value): Likewise.
---
gdb/dwarf2loc.c | 108 ++++++++++++++++++++++++++++----------------------------
1 file changed, 54 insertions(+), 54 deletions(-)
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 62462f0..3255274 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1794,8 +1794,7 @@ read_pieced_value (struct value *v)
{
struct dwarf_expr_piece *p = &c->pieces[i];
size_t this_size, this_size_bits;
- long dest_offset_bits, source_offset_bits, source_offset;
- const gdb_byte *intermediate_buffer;
+ long dest_offset_bits, source_offset_bits;
/* Compute size, source, and destination offsets for copying, in
bits. */
@@ -1813,11 +1812,6 @@ read_pieced_value (struct value *v)
if (this_size_bits > max_offset - offset)
this_size_bits = max_offset - offset;
- this_size = bits_to_bytes (source_offset_bits, this_size_bits);
- buffer.reserve (this_size);
- source_offset = source_offset_bits / 8;
- intermediate_buffer = buffer.data ();
-
/* Copy from the source to DEST_BUFFER. */
switch (p->location)
{
@@ -1843,13 +1837,11 @@ read_pieced_value (struct value *v)
this_size, buffer.data (),
&optim, &unavail))
{
- /* Just so garbage doesn't ever shine through. */
- memset (buffer.data (), 0, this_size);
-
if (optim)
mark_value_bits_optimized_out (v, offset, this_size_bits);
if (unavail)
mark_value_bits_unavailable (v, offset, this_size_bits);
+ break;
}
copy_bitwise (contents, dest_offset_bits,
@@ -1859,12 +1851,15 @@ read_pieced_value (struct value *v)
break;
case DWARF_VALUE_MEMORY:
+ this_size = bits_to_bytes (source_offset_bits, this_size_bits);
+ buffer.reserve (this_size);
+
read_value_memory (v, offset,
p->v.mem.in_stack_memory,
- p->v.mem.addr + source_offset,
+ p->v.mem.addr + source_offset_bits / 8,
buffer.data (), this_size);
copy_bitwise (contents, dest_offset_bits,
- intermediate_buffer, source_offset_bits % 8,
+ buffer.data (), source_offset_bits % 8,
this_size_bits, bits_big_endian);
break;
@@ -1892,18 +1887,18 @@ read_pieced_value (struct value *v)
case DWARF_VALUE_LITERAL:
{
- size_t n = this_size;
+ ULONGEST literal_size_bits = 8 * p->v.literal.length;
+ size_t n = this_size_bits;
- if (n > p->v.literal.length - source_offset)
- n = (p->v.literal.length >= source_offset
- ? p->v.literal.length - source_offset
- : 0);
- if (n != 0)
- intermediate_buffer = p->v.literal.data + source_offset;
+ /* Cut off at the end of the implicit value. */
+ if (source_offset_bits >= literal_size_bits)
+ break;
+ if (n > literal_size_bits - source_offset_bits)
+ n = literal_size_bits - source_offset_bits;
copy_bitwise (contents, dest_offset_bits,
- intermediate_buffer, source_offset_bits % 8,
- this_size_bits, bits_big_endian);
+ p->v.literal.data, source_offset_bits,
+ n, bits_big_endian);
}
break;
@@ -1960,9 +1955,7 @@ write_pieced_value (struct value *to, struct value *from)
{
struct dwarf_expr_piece *p = &c->pieces[i];
size_t this_size_bits, this_size;
- long dest_offset_bits, source_offset_bits, dest_offset, source_offset;
- int need_bitwise;
- const gdb_byte *source_buffer;
+ long dest_offset_bits, source_offset_bits;
this_size_bits = p->size;
if (bits_to_skip > 0 && bits_to_skip >= this_size_bits)
@@ -1978,22 +1971,6 @@ write_pieced_value (struct value *to, struct value *from)
if (this_size_bits > max_offset - offset)
this_size_bits = max_offset - offset;
- this_size = bits_to_bytes (dest_offset_bits, this_size_bits);
- source_offset = source_offset_bits / 8;
- dest_offset = dest_offset_bits / 8;
- if (dest_offset_bits % 8 == 0 && source_offset_bits % 8 == 0
- && this_size_bits % 8 == 0)
- {
- source_buffer = contents + source_offset;
- need_bitwise = 0;
- }
- else
- {
- buffer.reserve (this_size);
- source_buffer = buffer.data ();
- need_bitwise = 1;
- }
-
switch (p->location)
{
case DWARF_VALUE_REGISTER:
@@ -2045,21 +2022,44 @@ write_pieced_value (struct value *to, struct value *from)
}
break;
case DWARF_VALUE_MEMORY:
- if (need_bitwise)
- {
- /* Only the first and last bytes can possibly have any
- bits reused. */
- read_memory (p->v.mem.addr + dest_offset, buffer.data (), 1);
- read_memory (p->v.mem.addr + dest_offset + this_size - 1,
- &buffer[this_size - 1], 1);
- copy_bitwise (buffer.data (), dest_offset_bits % 8,
- contents, source_offset_bits,
- this_size_bits,
- bits_big_endian);
- }
+ {
+ CORE_ADDR start_addr = p->v.mem.addr + dest_offset_bits / 8;
- write_memory (p->v.mem.addr + dest_offset,
- source_buffer, this_size);
+ if (dest_offset_bits % 8 == 0 && this_size_bits % 8 == 0
+ && source_offset_bits % 8 == 0)
+ {
+ /* Everything is byte-aligned; no buffer needed. */
+ write_memory (start_addr,
+ contents + source_offset_bits / 8,
+ this_size_bits / 8);
+ break;
+ }
+
+ this_size = bits_to_bytes (dest_offset_bits, this_size_bits);
+ buffer.reserve (this_size);
+
+ if (dest_offset_bits % 8 != 0 || this_size_bits % 8 != 0)
+ {
+ if (this_size <= 8)
+ {
+ /* Perform a single read for small sizes. */
+ read_memory (start_addr, buffer.data (), this_size);
+ }
+ else
+ {
+ /* Only the first and last bytes can possibly have any
+ bits reused. */
+ read_memory (start_addr, buffer.data (), 1);
+ read_memory (start_addr + this_size - 1,
+ &buffer[this_size - 1], 1);
+ }
+ }
+
+ copy_bitwise (buffer.data (), dest_offset_bits % 8,
+ contents, source_offset_bits,
+ this_size_bits, bits_big_endian);
+ write_memory (start_addr, buffer.data (), this_size);
+ }
break;
default:
mark_value_bytes_optimized_out (to, 0, TYPE_LENGTH (value_type (to)));
--
2.5.0
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v2 14/19] read/write_pieced_value: Improve logic for buffer allocation
2017-05-09 17:56 ` [PATCH v2 14/19] read/write_pieced_value: Improve logic for buffer allocation Andreas Arnez
@ 2017-06-12 13:28 ` Yao Qi
2017-06-12 19:40 ` Simon Marchi
1 sibling, 0 replies; 56+ messages in thread
From: Yao Qi @ 2017-06-12 13:28 UTC (permalink / raw)
To: Andreas Arnez; +Cc: gdb-patches
Andreas Arnez <arnez@linux.vnet.ibm.com> writes:
> gdb/ChangeLog:
>
> * dwarf2loc.c (read_pieced_value): Move the buffer allocation and
> some other preparations to the places where sufficient information
> is available.
> (write_pieced_value): Likewise.
Patch is good to me.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH v2 14/19] read/write_pieced_value: Improve logic for buffer allocation
2017-05-09 17:56 ` [PATCH v2 14/19] read/write_pieced_value: Improve logic for buffer allocation Andreas Arnez
2017-06-12 13:28 ` Yao Qi
@ 2017-06-12 19:40 ` Simon Marchi
2017-06-13 12:10 ` Andreas Arnez
1 sibling, 1 reply; 56+ messages in thread
From: Simon Marchi @ 2017-06-12 19:40 UTC (permalink / raw)
To: Andreas Arnez; +Cc: gdb-patches
On 2017-05-09 19:46, Andreas Arnez wrote:
> @@ -2045,21 +2022,44 @@ write_pieced_value (struct value *to, struct
> value *from)
> }
> break;
> case DWARF_VALUE_MEMORY:
> - if (need_bitwise)
> - {
> - /* Only the first and last bytes can possibly have any
> - bits reused. */
> - read_memory (p->v.mem.addr + dest_offset, buffer.data (), 1);
> - read_memory (p->v.mem.addr + dest_offset + this_size - 1,
> - &buffer[this_size - 1], 1);
> - copy_bitwise (buffer.data (), dest_offset_bits % 8,
> - contents, source_offset_bits,
> - this_size_bits,
> - bits_big_endian);
> - }
> + {
> + CORE_ADDR start_addr = p->v.mem.addr + dest_offset_bits / 8;
>
> - write_memory (p->v.mem.addr + dest_offset,
> - source_buffer, this_size);
> + if (dest_offset_bits % 8 == 0 && this_size_bits % 8 == 0
> + && source_offset_bits % 8 == 0)
> + {
> + /* Everything is byte-aligned; no buffer needed. */
> + write_memory (start_addr,
> + contents + source_offset_bits / 8,
> + this_size_bits / 8);
> + break;
> + }
> +
> + this_size = bits_to_bytes (dest_offset_bits, this_size_bits);
> + buffer.reserve (this_size);
Reading Pedro's patch about byte_vector:
https://sourceware.org/ml/gdb-patches/2017-06/msg00339.html
reminded me of this, when I read the patch I didn't think it was a
problem. I think this should use resize instead of reserve.
Simon
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v2 14/19] read/write_pieced_value: Improve logic for buffer allocation
2017-06-12 19:40 ` Simon Marchi
@ 2017-06-13 12:10 ` Andreas Arnez
2017-06-13 12:18 ` Pedro Alves
0 siblings, 1 reply; 56+ messages in thread
From: Andreas Arnez @ 2017-06-13 12:10 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches, Pedro Alves
On Mon, Jun 12 2017, Simon Marchi wrote:
> On 2017-05-09 19:46, Andreas Arnez wrote:
>> @@ -2045,21 +2022,44 @@ write_pieced_value (struct value *to, struct
>> value *from)
>> }
>> break;
>> case DWARF_VALUE_MEMORY:
[...]
> + this_size = bits_to_bytes (dest_offset_bits, this_size_bits);
>> + buffer.reserve (this_size);
>
> Reading Pedro's patch about byte_vector:
>
> https://sourceware.org/ml/gdb-patches/2017-06/msg00339.html
>
> reminded me of this, when I read the patch I didn't think it was a
> problem. I think this should use resize instead of reserve.
OK, I'll wait for Pedro's patch to go in and then push the series with
that change.
--
Andreas
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH v2 14/19] read/write_pieced_value: Improve logic for buffer allocation
2017-06-13 12:10 ` Andreas Arnez
@ 2017-06-13 12:18 ` Pedro Alves
2017-06-13 14:41 ` Andreas Arnez
0 siblings, 1 reply; 56+ messages in thread
From: Pedro Alves @ 2017-06-13 12:18 UTC (permalink / raw)
To: Andreas Arnez, Simon Marchi; +Cc: gdb-patches
On 06/13/2017 01:10 PM, Andreas Arnez wrote:
> On Mon, Jun 12 2017, Simon Marchi wrote:
>
>> On 2017-05-09 19:46, Andreas Arnez wrote:
>>> @@ -2045,21 +2022,44 @@ write_pieced_value (struct value *to, struct
>>> value *from)
>>> }
>>> break;
>>> case DWARF_VALUE_MEMORY:
>
> [...]
>
>> + this_size = bits_to_bytes (dest_offset_bits, this_size_bits);
>>> + buffer.reserve (this_size);
>>
>> Reading Pedro's patch about byte_vector:
>>
>> https://sourceware.org/ml/gdb-patches/2017-06/msg00339.html
>>
>> reminded me of this, when I read the patch I didn't think it was a
>> problem. I think this should use resize instead of reserve.
>
> OK, I'll wait for Pedro's patch to go in and then push the series with
> that change.
Please feel free to push you series in before mine is in.
I think it'll be much easier for me to handle the merge
conflict than for you.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH v2 14/19] read/write_pieced_value: Improve logic for buffer allocation
2017-06-13 12:18 ` Pedro Alves
@ 2017-06-13 14:41 ` Andreas Arnez
0 siblings, 0 replies; 56+ messages in thread
From: Andreas Arnez @ 2017-06-13 14:41 UTC (permalink / raw)
To: Pedro Alves; +Cc: Simon Marchi, gdb-patches
On Tue, Jun 13 2017, Pedro Alves wrote:
> On 06/13/2017 01:10 PM, Andreas Arnez wrote:
>> On Mon, Jun 12 2017, Simon Marchi wrote:
>>
>>> On 2017-05-09 19:46, Andreas Arnez wrote:
>>>> @@ -2045,21 +2022,44 @@ write_pieced_value (struct value *to, struct
>>>> value *from)
>>>> }
>>>> break;
>>>> case DWARF_VALUE_MEMORY:
>>
>> [...]
>>
>>> + this_size = bits_to_bytes (dest_offset_bits, this_size_bits);
>>>> + buffer.reserve (this_size);
>>>
>>> Reading Pedro's patch about byte_vector:
>>>
>>> https://sourceware.org/ml/gdb-patches/2017-06/msg00339.html
>>>
>>> reminded me of this, when I read the patch I didn't think it was a
>>> problem. I think this should use resize instead of reserve.
>>
>> OK, I'll wait for Pedro's patch to go in and then push the series with
>> that change.
>
> Please feel free to push you series in before mine is in.
> I think it'll be much easier for me to handle the merge
> conflict than for you.
Thanks, that works for me ;-)
I just pushed the series.
--
Andreas
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v2 15/19] Respect piece offset for DW_OP_bit_piece
2017-05-09 17:47 [PATCH v2 00/19] Various DWARF piece fixes Andreas Arnez
` (13 preceding siblings ...)
2017-05-09 17:56 ` [PATCH v2 14/19] read/write_pieced_value: Improve logic for buffer allocation Andreas Arnez
@ 2017-05-09 17:57 ` Andreas Arnez
2017-05-16 21:08 ` Yao Qi
2017-05-09 17:58 ` [PATCH v2 17/19] Fix bit-/byte-offset mismatch in parameter to read_value_memory Andreas Arnez
` (5 subsequent siblings)
20 siblings, 1 reply; 56+ messages in thread
From: Andreas Arnez @ 2017-05-09 17:57 UTC (permalink / raw)
To: gdb-patches
So far GDB ignores the piece offset of all kinds of DWARF bit
pieces (DW_OP_bit_piece) and treats such pieces as if the offset was zero.
This is fixed, and an appropriate test is added.
gdb/ChangeLog:
* dwarf2loc.c (read_pieced_value): Respect the piece offset, as
given by DW_OP_bit_piece.
(write_pieced_value): Likewise.
Andreas Arnez <arnez@linux.vnet.ibm.com>
* gdb.dwarf2/var-access.exp: Add test for composite location with
nonzero piece offsets.
---
gdb/dwarf2loc.c | 25 ++++++++++++++++-----
gdb/testsuite/gdb.dwarf2/var-access.exp | 39 +++++++++++++++++++++++++++++++++
2 files changed, 58 insertions(+), 6 deletions(-)
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 3255274..23901f0e7 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1824,11 +1824,14 @@ read_pieced_value (struct value *v)
int optim, unavail;
if (gdbarch_byte_order (arch) == BFD_ENDIAN_BIG
- && p->size < reg_bits)
+ && p->offset + p->size < reg_bits)
{
/* Big-endian, and we want less than full size. */
- source_offset_bits += reg_bits - p->size;
+ source_offset_bits += reg_bits - (p->offset + p->size);
}
+ else
+ source_offset_bits += p->offset;
+
this_size = bits_to_bytes (source_offset_bits, this_size_bits);
buffer.reserve (this_size);
@@ -1851,6 +1854,7 @@ read_pieced_value (struct value *v)
break;
case DWARF_VALUE_MEMORY:
+ source_offset_bits += p->offset;
this_size = bits_to_bytes (source_offset_bits, this_size_bits);
buffer.reserve (this_size);
@@ -1871,12 +1875,15 @@ read_pieced_value (struct value *v)
= 8 * TYPE_LENGTH (value_type (p->v.value));
/* Use zeroes if piece reaches beyond stack value. */
- if (p->size > stack_value_size_bits)
+ if (p->offset + p->size > stack_value_size_bits)
break;
/* Piece is anchored at least significant bit end. */
if (gdbarch_byte_order (objfile_gdbarch) == BFD_ENDIAN_BIG)
- source_offset_bits += stack_value_size_bits - p->size;
+ source_offset_bits += (stack_value_size_bits
+ - p->offset - p->size);
+ else
+ source_offset_bits += p->offset;
copy_bitwise (contents, dest_offset_bits,
value_contents_all (p->v.value),
@@ -1891,6 +1898,7 @@ read_pieced_value (struct value *v)
size_t n = this_size_bits;
/* Cut off at the end of the implicit value. */
+ source_offset_bits += p->offset;
if (source_offset_bits >= literal_size_bits)
break;
if (n > literal_size_bits - source_offset_bits)
@@ -1981,11 +1989,14 @@ write_pieced_value (struct value *to, struct value *from)
ULONGEST reg_bits = 8 * register_size (arch, gdb_regnum);
if (gdbarch_byte_order (arch) == BFD_ENDIAN_BIG
- && p->size <= reg_bits)
+ && p->offset + p->size < reg_bits)
{
/* Big-endian, and we want less than full size. */
- dest_offset_bits += reg_bits - p->size;
+ dest_offset_bits += reg_bits - (p->offset + p->size);
}
+ else
+ dest_offset_bits += p->offset;
+
this_size = bits_to_bytes (dest_offset_bits, this_size_bits);
buffer.reserve (this_size);
@@ -2023,6 +2034,8 @@ write_pieced_value (struct value *to, struct value *from)
break;
case DWARF_VALUE_MEMORY:
{
+ dest_offset_bits += p->offset;
+
CORE_ADDR start_addr = p->v.mem.addr + dest_offset_bits / 8;
if (dest_offset_bits % 8 == 0 && this_size_bits % 8 == 0
diff --git a/gdb/testsuite/gdb.dwarf2/var-access.exp b/gdb/testsuite/gdb.dwarf2/var-access.exp
index c533b8d..07516fc 100644
--- a/gdb/testsuite/gdb.dwarf2/var-access.exp
+++ b/gdb/testsuite/gdb.dwarf2/var-access.exp
@@ -228,6 +228,24 @@ Dwarf::assemble $asm_file {
piece 1
} SPECIAL_expr}
}
+ # One piece per bitfield, using piece offsets: 32 bits of
+ # an implicit value, 9 bits of a stack value, 13 bits of
+ # r0, and 10 bits of buf.
+ DW_TAG_variable {
+ {name "t3"}
+ {type :$struct_t_label}
+ {location {
+ implicit_value 0x12 0x34 0x56 0x78 0x9a
+ bit_piece 32 4
+ const2s -280
+ stack_value
+ bit_piece 9 2
+ regx [lindex $dwarf_regnum 0]
+ bit_piece 13 14
+ addr $buf_var
+ bit_piece 10 42
+ } SPECIAL_expr}
+ }
}
}
}
@@ -304,3 +322,24 @@ gdb_test_no_output "set var t2.y = 2641"
gdb_test_no_output "set var t2.z = -400"
gdb_test_no_output "set var t2.x = 200"
gdb_test "print t2.x + t2.y + t2.z" " = 2441"
+
+# Bitfield access through pieces with nonzero piece offsets.
+gdb_test_no_output "set var \$[lindex $regname 0] = 0xa8000" \
+ "init reg for t3.y"
+gdb_test_no_output "set var *(char \[2\] *) (a + 5) = { 70, 82 }" \
+ "init mem for t3.z"
+switch $endian {
+ little {set val "u = -1484430527, x = -70, y = 42, z = 145"}
+ big {set val "u = 591751049, x = -70, y = 42, z = 101"}
+}
+gdb_test "print t3" " = \\{$val\\}" \
+ "initialized t3 from reg and mem"
+gdb_test_no_output "set var t3.y = -1" \
+ "overwrite t3.y"
+gdb_test "print/x \$[lindex $regname 0]" " = 0x7ffc000" \
+ "verify t3.y through reg"
+gdb_test_no_output "set var t3.z = -614" \
+ "overwrite t3.z"
+switch $endian {big {set val "0x59, 0xa2"} little {set val "0x6a, 0x56"}}
+gdb_test "print/x *(char \[2\] *) (a + 5)" " = \\{$val\\}" \
+ "verify t3.z through mem"
--
2.5.0
^ permalink raw reply [flat|nested] 56+ messages in thread* [PATCH v2 17/19] Fix bit-/byte-offset mismatch in parameter to read_value_memory
2017-05-09 17:47 [PATCH v2 00/19] Various DWARF piece fixes Andreas Arnez
` (14 preceding siblings ...)
2017-05-09 17:57 ` [PATCH v2 15/19] Respect piece offset for DW_OP_bit_piece Andreas Arnez
@ 2017-05-09 17:58 ` Andreas Arnez
2017-05-30 19:59 ` Simon Marchi
2017-05-09 17:58 ` [PATCH v2 16/19] read/write_pieced_value: Remove unnecessary variable copies Andreas Arnez
` (4 subsequent siblings)
20 siblings, 1 reply; 56+ messages in thread
From: Andreas Arnez @ 2017-05-09 17:58 UTC (permalink / raw)
To: gdb-patches
The function read_value_memory accepts a parameter embedded_offset and
expects it to represent the byte offset into the given value. However,
the only invocation with a possibly non-zero embedded_offset happens in
read_pieced_value, where a bit offset is passed instead.
Adjust the implementation of read_value_memory to meet the caller's
expectation. This implicitly fixes the invocation in read_pieced_value.
gdb/ChangeLog:
* valops.c (read_value_memory): Change embedded_offset to
represent a bit offset instead of a byte offset.
* value.h (read_value_memory): Adjust comment.
---
gdb/valops.c | 7 ++++---
gdb/value.h | 9 ++++-----
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/gdb/valops.c b/gdb/valops.c
index 93ae6cf..8675e6c 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -958,7 +958,7 @@ value_at_lazy (struct type *type, CORE_ADDR addr)
}
void
-read_value_memory (struct value *val, LONGEST embedded_offset,
+read_value_memory (struct value *val, LONGEST bit_offset,
int stack, CORE_ADDR memaddr,
gdb_byte *buffer, size_t length)
{
@@ -984,8 +984,9 @@ read_value_memory (struct value *val, LONGEST embedded_offset,
if (status == TARGET_XFER_OK)
/* nothing */;
else if (status == TARGET_XFER_UNAVAILABLE)
- mark_value_bytes_unavailable (val, embedded_offset + xfered_total,
- xfered_partial);
+ mark_value_bits_unavailable (val, (xfered_total * HOST_CHAR_BIT
+ + bit_offset),
+ xfered_partial * HOST_CHAR_BIT);
else if (status == TARGET_XFER_EOF)
memory_error (TARGET_XFER_E_IO, memaddr + xfered_total);
else
diff --git a/gdb/value.h b/gdb/value.h
index a1d1609..fb7f13d 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -581,12 +581,11 @@ extern int value_contents_eq (const struct value *val1, LONGEST offset1,
/* Read LENGTH addressable memory units starting at MEMADDR into BUFFER,
which is (or will be copied to) VAL's contents buffer offset by
- EMBEDDED_OFFSET (that is, to &VAL->contents[EMBEDDED_OFFSET]).
- Marks value contents ranges as unavailable if the corresponding
- memory is likewise unavailable. STACK indicates whether the memory
- is known to be stack memory. */
+ BIT_OFFSET bits. Marks value contents ranges as unavailable if
+ the corresponding memory is likewise unavailable. STACK indicates
+ whether the memory is known to be stack memory. */
-extern void read_value_memory (struct value *val, LONGEST embedded_offset,
+extern void read_value_memory (struct value *val, LONGEST bit_offset,
int stack, CORE_ADDR memaddr,
gdb_byte *buffer, size_t length);
--
2.5.0
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v2 17/19] Fix bit-/byte-offset mismatch in parameter to read_value_memory
2017-05-09 17:58 ` [PATCH v2 17/19] Fix bit-/byte-offset mismatch in parameter to read_value_memory Andreas Arnez
@ 2017-05-30 19:59 ` Simon Marchi
2017-05-31 14:02 ` Andreas Arnez
0 siblings, 1 reply; 56+ messages in thread
From: Simon Marchi @ 2017-05-30 19:59 UTC (permalink / raw)
To: Andreas Arnez; +Cc: gdb-patches
On 2017-05-09 19:46, Andreas Arnez wrote:
> @@ -984,8 +984,9 @@ read_value_memory (struct value *val, LONGEST
> embedded_offset,
> if (status == TARGET_XFER_OK)
> /* nothing */;
> else if (status == TARGET_XFER_UNAVAILABLE)
> - mark_value_bytes_unavailable (val, embedded_offset + xfered_total,
> - xfered_partial);
> + mark_value_bits_unavailable (val, (xfered_total * HOST_CHAR_BIT
> + + bit_offset),
> + xfered_partial * HOST_CHAR_BIT);
Since it's readily available, please use the unit_size variable here
instead of HOST_CHAR_BIT.
LGTM with that change.
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH v2 17/19] Fix bit-/byte-offset mismatch in parameter to read_value_memory
2017-05-30 19:59 ` Simon Marchi
@ 2017-05-31 14:02 ` Andreas Arnez
2017-05-31 14:30 ` Simon Marchi
0 siblings, 1 reply; 56+ messages in thread
From: Andreas Arnez @ 2017-05-31 14:02 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
On Tue, May 30 2017, Simon Marchi wrote:
> On 2017-05-09 19:46, Andreas Arnez wrote:
>> @@ -984,8 +984,9 @@ read_value_memory (struct value *val, LONGEST
>> embedded_offset,
>> if (status == TARGET_XFER_OK)
>> /* nothing */;
>> else if (status == TARGET_XFER_UNAVAILABLE)
>> - mark_value_bytes_unavailable (val, embedded_offset + xfered_total,
>> - xfered_partial);
>> + mark_value_bits_unavailable (val, (xfered_total * HOST_CHAR_BIT
>> + + bit_offset),
>> + xfered_partial * HOST_CHAR_BIT);
>
> Since it's readily available, please use the unit_size variable here
> instead of HOST_CHAR_BIT.
Hm, I thought unit_size represents the number of *bytes* stored at the
same target address. But we need an offset in *bits* here. Maybe you
mean that xfered_total should have been multiplied by unit_size even
before my patch? (Is it even correct that xfered_partial measures
addressable target units? Note that to_xfer_partial is documented to
"[...] transfer up to LEN 8-bit bytes of the target's OBJECT.")
>
> LGTM with that change.
Thanks!
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH v2 17/19] Fix bit-/byte-offset mismatch in parameter to read_value_memory
2017-05-31 14:02 ` Andreas Arnez
@ 2017-05-31 14:30 ` Simon Marchi
0 siblings, 0 replies; 56+ messages in thread
From: Simon Marchi @ 2017-05-31 14:30 UTC (permalink / raw)
To: Andreas Arnez; +Cc: gdb-patches
On 2017-05-31 16:02, Andreas Arnez wrote:
> On Tue, May 30 2017, Simon Marchi wrote:
>
>> On 2017-05-09 19:46, Andreas Arnez wrote:
>>> @@ -984,8 +984,9 @@ read_value_memory (struct value *val, LONGEST
>>> embedded_offset,
>>> if (status == TARGET_XFER_OK)
>>> /* nothing */;
>>> else if (status == TARGET_XFER_UNAVAILABLE)
>>> - mark_value_bytes_unavailable (val, embedded_offset + xfered_total,
>>> - xfered_partial);
>>> + mark_value_bits_unavailable (val, (xfered_total * HOST_CHAR_BIT
>>> + + bit_offset),
>>> + xfered_partial * HOST_CHAR_BIT);
>>
>> Since it's readily available, please use the unit_size variable here
>> instead of HOST_CHAR_BIT.
>
> Hm, I thought unit_size represents the number of *bytes* stored at the
> same target address. But we need an offset in *bits* here. Maybe you
> mean that xfered_total should have been multiplied by unit_size even
> before my patch?
Ah yes sorry, it should be "* unit_size * 8", since the unit size is
specifically the number of 8-bit units.
And yes, the multiplication with unit_size "should" have been there
before. In our GDB port that uses 16-bit bytes, we don't really deal
with unavailable bytes, so we haven't adapted that particular code. In
the end, I don't really mind if you keep "* HOST_CHAR_BIT": if we ever
need to deal with unavailable data on a non 8-bit bytes platform, we'll
need to adjust many places, so it doesn't really matter if this one uses
the unit size or not right now. You can forget about this comment :).
> (Is it even correct that xfered_partial measures
> addressable target units? Note that to_xfer_partial is documented to
> "[...] transfer up to LEN 8-bit bytes of the target's OBJECT.")
Yes that's the intent, and that's how it is in our GDB port. As for the
documentation of to_xfer_partial, that's probably an oversight.
target_read and target_write have the right doc.
Simon
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v2 16/19] read/write_pieced_value: Remove unnecessary variable copies
2017-05-09 17:47 [PATCH v2 00/19] Various DWARF piece fixes Andreas Arnez
` (15 preceding siblings ...)
2017-05-09 17:58 ` [PATCH v2 17/19] Fix bit-/byte-offset mismatch in parameter to read_value_memory Andreas Arnez
@ 2017-05-09 17:58 ` Andreas Arnez
2017-06-12 13:50 ` Yao Qi
2017-05-09 17:59 ` [PATCH v2 18/19] write_pieced_value: Notify memory_changed observers Andreas Arnez
` (3 subsequent siblings)
20 siblings, 1 reply; 56+ messages in thread
From: Andreas Arnez @ 2017-05-09 17:58 UTC (permalink / raw)
To: gdb-patches
In read_pieced_value's main loop, the variables `dest_offset_bits' and
`source_offset_bits' are basically just copies of `offset' and
`bits_to_skip', respectively. In write_pieced_value the copies are
reversed. This is not very helpful when trying to keep the logic between
these functions in sync. Since the copies are unnecessary, this patch
just removes them.
gdb/ChangeLog:
* dwarf2loc.c (read_pieced_value): Remove unnecessary variables
dest_offset_bits and source_offset_bits.
(write_pieced_value): Likewise.
---
gdb/dwarf2loc.c | 120 ++++++++++++++++++++++++--------------------------------
1 file changed, 52 insertions(+), 68 deletions(-)
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 23901f0e7..ed5b29c 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1790,25 +1790,16 @@ read_pieced_value (struct value *v)
else
max_offset = 8 * TYPE_LENGTH (value_type (v));
- for (i = 0; i < c->n_pieces && offset < max_offset; i++)
+ /* Advance to the first non-skipped piece. */
+ for (i = 0; i < c->n_pieces && bits_to_skip >= c->pieces[i].size; i++)
+ bits_to_skip -= c->pieces[i].size;
+
+ for (; i < c->n_pieces && offset < max_offset; i++)
{
struct dwarf_expr_piece *p = &c->pieces[i];
size_t this_size, this_size_bits;
- long dest_offset_bits, source_offset_bits;
-
- /* Compute size, source, and destination offsets for copying, in
- bits. */
- this_size_bits = p->size;
- if (bits_to_skip > 0 && bits_to_skip >= this_size_bits)
- {
- bits_to_skip -= this_size_bits;
- continue;
- }
- source_offset_bits = bits_to_skip;
- this_size_bits -= bits_to_skip;
- bits_to_skip = 0;
- dest_offset_bits = offset;
+ this_size_bits = p->size - bits_to_skip;
if (this_size_bits > max_offset - offset)
this_size_bits = max_offset - offset;
@@ -1827,16 +1818,16 @@ read_pieced_value (struct value *v)
&& p->offset + p->size < reg_bits)
{
/* Big-endian, and we want less than full size. */
- source_offset_bits += reg_bits - (p->offset + p->size);
+ bits_to_skip += reg_bits - (p->offset + p->size);
}
else
- source_offset_bits += p->offset;
+ bits_to_skip += p->offset;
- this_size = bits_to_bytes (source_offset_bits, this_size_bits);
+ this_size = bits_to_bytes (bits_to_skip, this_size_bits);
buffer.reserve (this_size);
if (!get_frame_register_bytes (frame, gdb_regnum,
- source_offset_bits / 8,
+ bits_to_skip / 8,
this_size, buffer.data (),
&optim, &unavail))
{
@@ -1846,24 +1837,23 @@ read_pieced_value (struct value *v)
mark_value_bits_unavailable (v, offset, this_size_bits);
break;
}
-
- copy_bitwise (contents, dest_offset_bits,
- buffer.data (), source_offset_bits % 8,
+ copy_bitwise (contents, offset,
+ buffer.data (), bits_to_skip % 8,
this_size_bits, bits_big_endian);
}
break;
case DWARF_VALUE_MEMORY:
- source_offset_bits += p->offset;
- this_size = bits_to_bytes (source_offset_bits, this_size_bits);
+ bits_to_skip += p->offset;
+ this_size = bits_to_bytes (bits_to_skip, this_size_bits);
buffer.reserve (this_size);
read_value_memory (v, offset,
p->v.mem.in_stack_memory,
- p->v.mem.addr + source_offset_bits / 8,
+ p->v.mem.addr + bits_to_skip / 8,
buffer.data (), this_size);
- copy_bitwise (contents, dest_offset_bits,
- buffer.data (), source_offset_bits % 8,
+ copy_bitwise (contents, offset,
+ buffer.data (), bits_to_skip % 8,
this_size_bits, bits_big_endian);
break;
@@ -1880,14 +1870,13 @@ read_pieced_value (struct value *v)
/* Piece is anchored at least significant bit end. */
if (gdbarch_byte_order (objfile_gdbarch) == BFD_ENDIAN_BIG)
- source_offset_bits += (stack_value_size_bits
- - p->offset - p->size);
+ bits_to_skip += stack_value_size_bits - p->offset - p->size;
else
- source_offset_bits += p->offset;
+ bits_to_skip += p->offset;
- copy_bitwise (contents, dest_offset_bits,
+ copy_bitwise (contents, offset,
value_contents_all (p->v.value),
- source_offset_bits,
+ bits_to_skip,
this_size_bits, bits_big_endian);
}
break;
@@ -1898,14 +1887,14 @@ read_pieced_value (struct value *v)
size_t n = this_size_bits;
/* Cut off at the end of the implicit value. */
- source_offset_bits += p->offset;
- if (source_offset_bits >= literal_size_bits)
+ bits_to_skip += p->offset;
+ if (bits_to_skip >= literal_size_bits)
break;
- if (n > literal_size_bits - source_offset_bits)
- n = literal_size_bits - source_offset_bits;
+ if (n > literal_size_bits - bits_to_skip)
+ n = literal_size_bits - bits_to_skip;
- copy_bitwise (contents, dest_offset_bits,
- p->v.literal.data, source_offset_bits,
+ copy_bitwise (contents, offset,
+ p->v.literal.data, bits_to_skip,
n, bits_big_endian);
}
break;
@@ -1924,6 +1913,7 @@ read_pieced_value (struct value *v)
}
offset += this_size_bits;
+ bits_to_skip = 0;
}
}
@@ -1959,23 +1949,16 @@ write_pieced_value (struct value *to, struct value *from)
else
max_offset = 8 * TYPE_LENGTH (value_type (to));
- for (i = 0; i < c->n_pieces && offset < max_offset; i++)
+ /* Advance to the first non-skipped piece. */
+ for (i = 0; i < c->n_pieces && bits_to_skip >= c->pieces[i].size; i++)
+ bits_to_skip -= c->pieces[i].size;
+
+ for (; i < c->n_pieces && offset < max_offset; i++)
{
struct dwarf_expr_piece *p = &c->pieces[i];
size_t this_size_bits, this_size;
- long dest_offset_bits, source_offset_bits;
-
- this_size_bits = p->size;
- if (bits_to_skip > 0 && bits_to_skip >= this_size_bits)
- {
- bits_to_skip -= this_size_bits;
- continue;
- }
- dest_offset_bits = bits_to_skip;
- this_size_bits -= bits_to_skip;
- bits_to_skip = 0;
- source_offset_bits = offset;
+ this_size_bits = p->size - bits_to_skip;
if (this_size_bits > max_offset - offset)
this_size_bits = max_offset - offset;
@@ -1992,22 +1975,22 @@ write_pieced_value (struct value *to, struct value *from)
&& p->offset + p->size < reg_bits)
{
/* Big-endian, and we want less than full size. */
- dest_offset_bits += reg_bits - (p->offset + p->size);
+ bits_to_skip += reg_bits - (p->offset + p->size);
}
else
- dest_offset_bits += p->offset;
+ bits_to_skip += p->offset;
- this_size = bits_to_bytes (dest_offset_bits, this_size_bits);
+ this_size = bits_to_bytes (bits_to_skip, this_size_bits);
buffer.reserve (this_size);
- if (dest_offset_bits % 8 != 0 || this_size_bits % 8 != 0)
+ if (bits_to_skip % 8 != 0 || this_size_bits % 8 != 0)
{
/* Data is copied non-byte-aligned into the register.
Need some bits from original register value. */
int optim, unavail;
if (!get_frame_register_bytes (frame, gdb_regnum,
- dest_offset_bits / 8,
+ bits_to_skip / 8,
this_size, buffer.data (),
&optim, &unavail))
{
@@ -2024,34 +2007,34 @@ write_pieced_value (struct value *to, struct value *from)
}
}
- copy_bitwise (buffer.data (), dest_offset_bits % 8,
- contents, source_offset_bits,
+ copy_bitwise (buffer.data (), bits_to_skip % 8,
+ contents, offset,
this_size_bits, bits_big_endian);
put_frame_register_bytes (frame, gdb_regnum,
- dest_offset_bits / 8,
+ bits_to_skip / 8,
this_size, buffer.data ());
}
break;
case DWARF_VALUE_MEMORY:
{
- dest_offset_bits += p->offset;
+ bits_to_skip += p->offset;
- CORE_ADDR start_addr = p->v.mem.addr + dest_offset_bits / 8;
+ CORE_ADDR start_addr = p->v.mem.addr + bits_to_skip / 8;
- if (dest_offset_bits % 8 == 0 && this_size_bits % 8 == 0
- && source_offset_bits % 8 == 0)
+ if (bits_to_skip % 8 == 0 && this_size_bits % 8 == 0
+ && offset % 8 == 0)
{
/* Everything is byte-aligned; no buffer needed. */
write_memory (start_addr,
- contents + source_offset_bits / 8,
+ contents + offset / 8,
this_size_bits / 8);
break;
}
- this_size = bits_to_bytes (dest_offset_bits, this_size_bits);
+ this_size = bits_to_bytes (bits_to_skip, this_size_bits);
buffer.reserve (this_size);
- if (dest_offset_bits % 8 != 0 || this_size_bits % 8 != 0)
+ if (bits_to_skip % 8 != 0 || this_size_bits % 8 != 0)
{
if (this_size <= 8)
{
@@ -2068,8 +2051,8 @@ write_pieced_value (struct value *to, struct value *from)
}
}
- copy_bitwise (buffer.data (), dest_offset_bits % 8,
- contents, source_offset_bits,
+ copy_bitwise (buffer.data (), bits_to_skip % 8,
+ contents, offset,
this_size_bits, bits_big_endian);
write_memory (start_addr, buffer.data (), this_size);
}
@@ -2079,6 +2062,7 @@ write_pieced_value (struct value *to, struct value *from)
break;
}
offset += this_size_bits;
+ bits_to_skip = 0;
}
}
--
2.5.0
^ permalink raw reply [flat|nested] 56+ messages in thread* [PATCH v2 18/19] write_pieced_value: Notify memory_changed observers
2017-05-09 17:47 [PATCH v2 00/19] Various DWARF piece fixes Andreas Arnez
` (16 preceding siblings ...)
2017-05-09 17:58 ` [PATCH v2 16/19] read/write_pieced_value: Remove unnecessary variable copies Andreas Arnez
@ 2017-05-09 17:59 ` Andreas Arnez
2017-05-16 21:12 ` Yao Qi
2017-05-09 18:00 ` [PATCH v2 19/19] read/write_pieced_value: Merge into one function Andreas Arnez
` (2 subsequent siblings)
20 siblings, 1 reply; 56+ messages in thread
From: Andreas Arnez @ 2017-05-09 17:59 UTC (permalink / raw)
To: gdb-patches
So far write_pieced_value uses write_memory when writing memory pieces to
the target. However, this is a case where GDB potentially overwrites a
watchpoint value. In such a case write_memory_with_notification should be
used instead, so that memory_changed observers get notified.
gdb/ChangeLog:
* dwarf2loc.c (write_pieced_value): When writing the data for a
memory piece, use write_memory_with_notification instead of
write_memory.
---
gdb/dwarf2loc.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index ed5b29c..94175ef 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -2025,9 +2025,9 @@ write_pieced_value (struct value *to, struct value *from)
&& offset % 8 == 0)
{
/* Everything is byte-aligned; no buffer needed. */
- write_memory (start_addr,
- contents + offset / 8,
- this_size_bits / 8);
+ write_memory_with_notification (start_addr,
+ contents + offset / 8,
+ this_size_bits / 8);
break;
}
@@ -2054,7 +2054,8 @@ write_pieced_value (struct value *to, struct value *from)
copy_bitwise (buffer.data (), bits_to_skip % 8,
contents, offset,
this_size_bits, bits_big_endian);
- write_memory (start_addr, buffer.data (), this_size);
+ write_memory_with_notification (start_addr, buffer.data (),
+ this_size);
}
break;
default:
--
2.5.0
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v2 18/19] write_pieced_value: Notify memory_changed observers
2017-05-09 17:59 ` [PATCH v2 18/19] write_pieced_value: Notify memory_changed observers Andreas Arnez
@ 2017-05-16 21:12 ` Yao Qi
0 siblings, 0 replies; 56+ messages in thread
From: Yao Qi @ 2017-05-16 21:12 UTC (permalink / raw)
To: Andreas Arnez; +Cc: gdb-patches
On 17-05-09 19:46:14, Andreas Arnez wrote:
> So far write_pieced_value uses write_memory when writing memory pieces to
> the target. However, this is a case where GDB potentially overwrites a
> watchpoint value. In such a case write_memory_with_notification should be
> used instead, so that memory_changed observers get notified.
>
> gdb/ChangeLog:
>
> * dwarf2loc.c (write_pieced_value): When writing the data for a
> memory piece, use write_memory_with_notification instead of
> write_memory.
LGTM.
--
Yao
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v2 19/19] read/write_pieced_value: Merge into one function
2017-05-09 17:47 [PATCH v2 00/19] Various DWARF piece fixes Andreas Arnez
` (17 preceding siblings ...)
2017-05-09 17:59 ` [PATCH v2 18/19] write_pieced_value: Notify memory_changed observers Andreas Arnez
@ 2017-05-09 18:00 ` Andreas Arnez
2017-06-12 13:57 ` Yao Qi
2017-05-30 16:42 ` [ping] [PATCH v2 00/19] Various DWARF piece fixes Andreas Arnez
2017-05-30 20:44 ` Simon Marchi
20 siblings, 1 reply; 56+ messages in thread
From: Andreas Arnez @ 2017-05-09 18:00 UTC (permalink / raw)
To: gdb-patches
Since read_pieced_value and write_pieced_value share significant logic,
this patch merges them into a single function rw_pieced_value.
gdb/ChangeLog:
* dwarf2loc.c (rw_pieced_value): New. Merge logic from...
(read_pieced_value, write_pieced_value): ...here. Reduce to
wrappers that just call rw_pieced_value.
---
gdb/dwarf2loc.c | 357 +++++++++++++++++++++++++++-----------------------------
1 file changed, 175 insertions(+), 182 deletions(-)
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 94175ef..beb11f5 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1761,31 +1761,55 @@ bits_to_bytes (ULONGEST start, ULONGEST n_bits)
return (start % 8 + n_bits + 7) / 8;
}
+/* Read or write a pieced value V. If FROM != NULL, operate in "write
+ mode": copy FROM into the pieces comprising V. If FROM == NULL,
+ operate in "read mode": fetch the contents of the (lazy) value V by
+ composing it from its pieces. */
+
static void
-read_pieced_value (struct value *v)
+rw_pieced_value (struct value *v, struct value *from)
{
int i;
LONGEST offset = 0, max_offset;
ULONGEST bits_to_skip;
- gdb_byte *contents;
+ gdb_byte *v_contents;
+ const gdb_byte *from_contents;
struct piece_closure *c
= (struct piece_closure *) value_computed_closure (v);
std::vector<gdb_byte> buffer;
int bits_big_endian
= gdbarch_bits_big_endian (get_type_arch (value_type (v)));
- if (value_type (v) != value_enclosing_type (v))
- internal_error (__FILE__, __LINE__,
- _("Should not be able to create a lazy value with "
- "an enclosing type"));
+ if (from != NULL)
+ {
+ from_contents = value_contents (from);
+ v_contents = NULL;
+ }
+ else
+ {
+ if (value_type (v) != value_enclosing_type (v))
+ internal_error (__FILE__, __LINE__,
+ _("Should not be able to create a lazy value with "
+ "an enclosing type"));
+ v_contents = value_contents_raw (v);
+ from_contents = NULL;
+ }
- contents = value_contents_raw (v);
bits_to_skip = 8 * value_offset (v);
if (value_bitsize (v))
{
bits_to_skip += (8 * value_offset (value_parent (v))
+ value_bitpos (v));
- max_offset = value_bitsize (v);
+ if (from != NULL
+ && (gdbarch_byte_order (get_type_arch (value_type (from)))
+ == BFD_ENDIAN_BIG))
+ {
+ /* Use the least significant bits of FROM. */
+ max_offset = 8 * TYPE_LENGTH (value_type (from));
+ offset = max_offset - value_bitsize (v);
+ }
+ else
+ max_offset = value_bitsize (v);
}
else
max_offset = 8 * TYPE_LENGTH (value_type (v));
@@ -1797,13 +1821,12 @@ read_pieced_value (struct value *v)
for (; i < c->n_pieces && offset < max_offset; i++)
{
struct dwarf_expr_piece *p = &c->pieces[i];
- size_t this_size, this_size_bits;
+ size_t this_size_bits, this_size;
this_size_bits = p->size - bits_to_skip;
if (this_size_bits > max_offset - offset)
this_size_bits = max_offset - offset;
- /* Copy from the source to DEST_BUFFER. */
switch (p->location)
{
case DWARF_VALUE_REGISTER:
@@ -1826,39 +1849,134 @@ read_pieced_value (struct value *v)
this_size = bits_to_bytes (bits_to_skip, this_size_bits);
buffer.reserve (this_size);
- if (!get_frame_register_bytes (frame, gdb_regnum,
- bits_to_skip / 8,
- this_size, buffer.data (),
- &optim, &unavail))
+ if (from == NULL)
{
- if (optim)
- mark_value_bits_optimized_out (v, offset, this_size_bits);
- if (unavail)
- mark_value_bits_unavailable (v, offset, this_size_bits);
- break;
+ /* Read mode. */
+ if (!get_frame_register_bytes (frame, gdb_regnum,
+ bits_to_skip / 8,
+ this_size, buffer.data (),
+ &optim, &unavail))
+ {
+ if (optim)
+ mark_value_bits_optimized_out (v, offset,
+ this_size_bits);
+ if (unavail)
+ mark_value_bits_unavailable (v, offset,
+ this_size_bits);
+ break;
+ }
+
+ copy_bitwise (v_contents, offset,
+ buffer.data (), bits_to_skip % 8,
+ this_size_bits, bits_big_endian);
+ }
+ else
+ {
+ /* Write mode. */
+ if (bits_to_skip % 8 != 0 || this_size_bits % 8 != 0)
+ {
+ /* Data is copied non-byte-aligned into the register.
+ Need some bits from original register value. */
+ get_frame_register_bytes (frame, gdb_regnum,
+ bits_to_skip / 8,
+ this_size, buffer.data (),
+ &optim, &unavail);
+ if (optim)
+ throw_error (OPTIMIZED_OUT_ERROR,
+ _("Can't do read-modify-write to "
+ "update bitfield; containing word "
+ "has been optimized out"));
+ if (unavail)
+ throw_error (NOT_AVAILABLE_ERROR,
+ _("Can't do read-modify-write to "
+ "update bitfield; containing word "
+ "is unavailable"));
+ }
+
+ copy_bitwise (buffer.data (), bits_to_skip % 8,
+ from_contents, offset,
+ this_size_bits, bits_big_endian);
+ put_frame_register_bytes (frame, gdb_regnum,
+ bits_to_skip / 8,
+ this_size, buffer.data ());
}
- copy_bitwise (contents, offset,
- buffer.data (), bits_to_skip % 8,
- this_size_bits, bits_big_endian);
}
break;
case DWARF_VALUE_MEMORY:
- bits_to_skip += p->offset;
- this_size = bits_to_bytes (bits_to_skip, this_size_bits);
- buffer.reserve (this_size);
-
- read_value_memory (v, offset,
- p->v.mem.in_stack_memory,
- p->v.mem.addr + bits_to_skip / 8,
- buffer.data (), this_size);
- copy_bitwise (contents, offset,
- buffer.data (), bits_to_skip % 8,
- this_size_bits, bits_big_endian);
+ {
+ bits_to_skip += p->offset;
+
+ CORE_ADDR start_addr = p->v.mem.addr + bits_to_skip / 8;
+
+ if (bits_to_skip % 8 == 0 && this_size_bits % 8 == 0
+ && offset % 8 == 0)
+ {
+ /* Everything is byte-aligned; no buffer needed. */
+ if (from != NULL)
+ write_memory_with_notification (start_addr,
+ (from_contents
+ + offset / 8),
+ this_size_bits / 8);
+ else
+ read_value_memory (v, offset,
+ p->v.mem.in_stack_memory,
+ p->v.mem.addr + bits_to_skip / 8,
+ v_contents + offset / 8,
+ this_size_bits / 8);
+ break;
+ }
+
+ this_size = bits_to_bytes (bits_to_skip, this_size_bits);
+ buffer.reserve (this_size);
+
+ if (from == NULL)
+ {
+ /* Read mode. */
+ read_value_memory (v, offset,
+ p->v.mem.in_stack_memory,
+ p->v.mem.addr + bits_to_skip / 8,
+ buffer.data (), this_size);
+ copy_bitwise (v_contents, offset,
+ buffer.data (), bits_to_skip % 8,
+ this_size_bits, bits_big_endian);
+ break;
+ }
+
+ /* Write mode. */
+ if (bits_to_skip % 8 != 0 || this_size_bits % 8 != 0)
+ {
+ if (this_size <= 8)
+ {
+ /* Perform a single read for small sizes. */
+ read_memory (start_addr, buffer.data (), this_size);
+ }
+ else
+ {
+ /* Only the first and last bytes can possibly have any
+ bits reused. */
+ read_memory (start_addr, buffer.data (), 1);
+ read_memory (start_addr + this_size - 1,
+ &buffer[this_size - 1], 1);
+ }
+ }
+
+ copy_bitwise (buffer.data (), bits_to_skip % 8,
+ from_contents, offset,
+ this_size_bits, bits_big_endian);
+ write_memory_with_notification (start_addr, buffer.data (),
+ this_size);
+ }
break;
case DWARF_VALUE_STACK:
{
+ if (from != NULL)
+ {
+ mark_value_bits_optimized_out (v, offset, this_size_bits);
+ break;
+ }
+
struct objfile *objfile = dwarf2_per_cu_objfile (c->per_cu);
struct gdbarch *objfile_gdbarch = get_objfile_arch (objfile);
ULONGEST stack_value_size_bits
@@ -1874,7 +1992,7 @@ read_pieced_value (struct value *v)
else
bits_to_skip += p->offset;
- copy_bitwise (contents, offset,
+ copy_bitwise (v_contents, offset,
value_contents_all (p->v.value),
bits_to_skip,
this_size_bits, bits_big_endian);
@@ -1883,6 +2001,12 @@ read_pieced_value (struct value *v)
case DWARF_VALUE_LITERAL:
{
+ if (from != NULL)
+ {
+ mark_value_bits_optimized_out (v, offset, this_size_bits);
+ break;
+ }
+
ULONGEST literal_size_bits = 8 * p->v.literal.length;
size_t n = this_size_bits;
@@ -1893,15 +2017,21 @@ read_pieced_value (struct value *v)
if (n > literal_size_bits - bits_to_skip)
n = literal_size_bits - bits_to_skip;
- copy_bitwise (contents, offset,
+ copy_bitwise (v_contents, offset,
p->v.literal.data, bits_to_skip,
n, bits_big_endian);
}
break;
- /* These bits show up as zeros -- but do not cause the value
- to be considered optimized-out. */
case DWARF_VALUE_IMPLICIT_POINTER:
+ if (from != NULL)
+ {
+ mark_value_bits_optimized_out (v, offset, this_size_bits);
+ break;
+ }
+
+ /* These bits show up as zeros -- but do not cause the value to
+ be considered optimized-out. */
break;
case DWARF_VALUE_OPTIMIZED_OUT:
@@ -1917,154 +2047,17 @@ read_pieced_value (struct value *v)
}
}
+
static void
-write_pieced_value (struct value *to, struct value *from)
+read_pieced_value (struct value *v)
{
- int i;
- ULONGEST bits_to_skip;
- LONGEST offset = 0, max_offset;
- const gdb_byte *contents;
- struct piece_closure *c
- = (struct piece_closure *) value_computed_closure (to);
- std::vector<gdb_byte> buffer;
- int bits_big_endian
- = gdbarch_bits_big_endian (get_type_arch (value_type (to)));
-
- contents = value_contents (from);
- bits_to_skip = 8 * value_offset (to);
- if (value_bitsize (to))
- {
- bits_to_skip += (8 * value_offset (value_parent (to))
- + value_bitpos (to));
- /* Use the least significant bits of FROM. */
- if (gdbarch_byte_order (get_type_arch (value_type (from)))
- == BFD_ENDIAN_BIG)
- {
- max_offset = 8 * TYPE_LENGTH (value_type (from));
- offset = max_offset - value_bitsize (to);
- }
- else
- max_offset = value_bitsize (to);
- }
- else
- max_offset = 8 * TYPE_LENGTH (value_type (to));
-
- /* Advance to the first non-skipped piece. */
- for (i = 0; i < c->n_pieces && bits_to_skip >= c->pieces[i].size; i++)
- bits_to_skip -= c->pieces[i].size;
-
- for (; i < c->n_pieces && offset < max_offset; i++)
- {
- struct dwarf_expr_piece *p = &c->pieces[i];
- size_t this_size_bits, this_size;
-
- this_size_bits = p->size - bits_to_skip;
- if (this_size_bits > max_offset - offset)
- this_size_bits = max_offset - offset;
-
- switch (p->location)
- {
- case DWARF_VALUE_REGISTER:
- {
- struct frame_info *frame = frame_find_by_id (c->frame_id);
- struct gdbarch *arch = get_frame_arch (frame);
- int gdb_regnum = dwarf_reg_to_regnum_or_error (arch, p->v.regno);
- ULONGEST reg_bits = 8 * register_size (arch, gdb_regnum);
-
- if (gdbarch_byte_order (arch) == BFD_ENDIAN_BIG
- && p->offset + p->size < reg_bits)
- {
- /* Big-endian, and we want less than full size. */
- bits_to_skip += reg_bits - (p->offset + p->size);
- }
- else
- bits_to_skip += p->offset;
-
- this_size = bits_to_bytes (bits_to_skip, this_size_bits);
- buffer.reserve (this_size);
-
- if (bits_to_skip % 8 != 0 || this_size_bits % 8 != 0)
- {
- /* Data is copied non-byte-aligned into the register.
- Need some bits from original register value. */
- int optim, unavail;
-
- if (!get_frame_register_bytes (frame, gdb_regnum,
- bits_to_skip / 8,
- this_size, buffer.data (),
- &optim, &unavail))
- {
- if (optim)
- throw_error (OPTIMIZED_OUT_ERROR,
- _("Can't do read-modify-write to "
- "update bitfield; containing word "
- "has been optimized out"));
- if (unavail)
- throw_error (NOT_AVAILABLE_ERROR,
- _("Can't do read-modify-write to update "
- "bitfield; containing word "
- "is unavailable"));
- }
- }
-
- copy_bitwise (buffer.data (), bits_to_skip % 8,
- contents, offset,
- this_size_bits, bits_big_endian);
- put_frame_register_bytes (frame, gdb_regnum,
- bits_to_skip / 8,
- this_size, buffer.data ());
- }
- break;
- case DWARF_VALUE_MEMORY:
- {
- bits_to_skip += p->offset;
-
- CORE_ADDR start_addr = p->v.mem.addr + bits_to_skip / 8;
-
- if (bits_to_skip % 8 == 0 && this_size_bits % 8 == 0
- && offset % 8 == 0)
- {
- /* Everything is byte-aligned; no buffer needed. */
- write_memory_with_notification (start_addr,
- contents + offset / 8,
- this_size_bits / 8);
- break;
- }
-
- this_size = bits_to_bytes (bits_to_skip, this_size_bits);
- buffer.reserve (this_size);
-
- if (bits_to_skip % 8 != 0 || this_size_bits % 8 != 0)
- {
- if (this_size <= 8)
- {
- /* Perform a single read for small sizes. */
- read_memory (start_addr, buffer.data (), this_size);
- }
- else
- {
- /* Only the first and last bytes can possibly have any
- bits reused. */
- read_memory (start_addr, buffer.data (), 1);
- read_memory (start_addr + this_size - 1,
- &buffer[this_size - 1], 1);
- }
- }
+ rw_pieced_value (v, NULL);
+}
- copy_bitwise (buffer.data (), bits_to_skip % 8,
- contents, offset,
- this_size_bits, bits_big_endian);
- write_memory_with_notification (start_addr, buffer.data (),
- this_size);
- }
- break;
- default:
- mark_value_bytes_optimized_out (to, 0, TYPE_LENGTH (value_type (to)));
- break;
- }
- offset += this_size_bits;
- bits_to_skip = 0;
- }
+static void
+write_pieced_value (struct value *to, struct value *from)
+{
+ rw_pieced_value (to, from);
}
/* An implementation of an lval_funcs method to see whether a value is
--
2.5.0
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v2 19/19] read/write_pieced_value: Merge into one function
2017-05-09 18:00 ` [PATCH v2 19/19] read/write_pieced_value: Merge into one function Andreas Arnez
@ 2017-06-12 13:57 ` Yao Qi
2017-06-12 14:34 ` Andreas Arnez
0 siblings, 1 reply; 56+ messages in thread
From: Yao Qi @ 2017-06-12 13:57 UTC (permalink / raw)
To: Andreas Arnez; +Cc: gdb-patches
Andreas Arnez <arnez@linux.vnet.ibm.com> writes:
> + if (from == NULL)
> + {
> + /* Read mode. */
> + read_value_memory (v, offset,
> + p->v.mem.in_stack_memory,
> + p->v.mem.addr + bits_to_skip / 8,
> + buffer.data (), this_size);
> + copy_bitwise (v_contents, offset,
> + buffer.data (), bits_to_skip % 8,
> + this_size_bits, bits_big_endian);
> + break;
> + }
> +
> + /* Write mode. */
I feel it is more clear to add "else" here, like
if (from == NULL)
{
/* Read mode. */
}
else
{
/* Write mode. */
}
then, we don't need the "break" above.
> + if (bits_to_skip % 8 != 0 || this_size_bits % 8 != 0)
> + {
> + if (this_size <= 8)
> + {
> + /* Perform a single read for small sizes. */
> + read_memory (start_addr, buffer.data (), this_size);
> + }
> + else
> + {
> + /* Only the first and last bytes can possibly have any
> + bits reused. */
> + read_memory (start_addr, buffer.data (), 1);
> + read_memory (start_addr + this_size - 1,
> + &buffer[this_size - 1], 1);
> + }
> + }
> +
> + copy_bitwise (buffer.data (), bits_to_skip % 8,
> + from_contents, offset,
> + this_size_bits, bits_big_endian);
> + write_memory_with_notification (start_addr, buffer.data (),
> + this_size);
> + }
> break;
Otherwise, the patch is good to me.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v2 19/19] read/write_pieced_value: Merge into one function
2017-06-12 13:57 ` Yao Qi
@ 2017-06-12 14:34 ` Andreas Arnez
2017-06-13 9:17 ` Yao Qi
0 siblings, 1 reply; 56+ messages in thread
From: Andreas Arnez @ 2017-06-12 14:34 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On Mon, Jun 12 2017, Yao Qi wrote:
> Andreas Arnez <arnez@linux.vnet.ibm.com> writes:
>
>> + if (from == NULL)
>> + {
>> + /* Read mode. */
>> + read_value_memory (v, offset,
>> + p->v.mem.in_stack_memory,
>> + p->v.mem.addr + bits_to_skip / 8,
>> + buffer.data (), this_size);
>> + copy_bitwise (v_contents, offset,
>> + buffer.data (), bits_to_skip % 8,
>> + this_size_bits, bits_big_endian);
>> + break;
>> + }
>> +
>> + /* Write mode. */
>
> I feel it is more clear to add "else" here, like
>
> if (from == NULL)
> {
> /* Read mode. */
> }
> else
> {
> /* Write mode. */
> }
>
> then, we don't need the "break" above.
Yeah, I agree. Will change.
Thanks for reviewing so far! I think the only patch you haven't
approved yet is patch #17 "Fix bit-/byte-offset mismatch in parameter to
read_value_memory". Do you want to have a look at that as well?
--
Andreas
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v2 19/19] read/write_pieced_value: Merge into one function
2017-06-12 14:34 ` Andreas Arnez
@ 2017-06-13 9:17 ` Yao Qi
0 siblings, 0 replies; 56+ messages in thread
From: Yao Qi @ 2017-06-13 9:17 UTC (permalink / raw)
To: Andreas Arnez; +Cc: gdb-patches
Andreas Arnez <arnez@linux.vnet.ibm.com> writes:
> Thanks for reviewing so far! I think the only patch you haven't
> approved yet is patch #17 "Fix bit-/byte-offset mismatch in parameter to
> read_value_memory". Do you want to have a look at that as well?
Simon approved that patch.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 56+ messages in thread
* [ping] [PATCH v2 00/19] Various DWARF piece fixes
2017-05-09 17:47 [PATCH v2 00/19] Various DWARF piece fixes Andreas Arnez
` (18 preceding siblings ...)
2017-05-09 18:00 ` [PATCH v2 19/19] read/write_pieced_value: Merge into one function Andreas Arnez
@ 2017-05-30 16:42 ` Andreas Arnez
2017-05-30 20:44 ` Simon Marchi
20 siblings, 0 replies; 56+ messages in thread
From: Andreas Arnez @ 2017-05-30 16:42 UTC (permalink / raw)
To: gdb-patches
Ping:
https://sourceware.org/ml/gdb-patches/2017-05/msg00215.html
ON Tue, May 09 2017, Andreas Arnez wrote:
> This series fixes various issues with DWARF piece handling.
> Specifically it almost rewrites the functions read_pieced_value and
> write_pieced_value in multiple steps. Test cases are added as well.
So far I've received feedback from Yao, and for v1 also from Simon.
Thanks again for that! Now, if I tracked correctly, the patches
check-marked with [X] are approved:
- [X] Add test for modifiable DWARF locations
- [X] write_pieced_value: Fix size capping logic
- [X] PR gdb/21226: Take DWARF stack value pieces from LSB end
- [X] Remove addr_size field from struct piece_closure
- [X] gdb/testsuite: Add "get_endianness" convenience proc
- [X] read/write_pieced_value: Respect value parent's offset
- [X] write_pieced_value: Fix copy/paste error in size calculation
- [X] write_pieced_value: Include transfer size in byte-wise check
- [X] write_pieced_value: Fix buffer offset for memory pieces
- [X] write_pieced_value: Transfer least significant bits into bit-field
- [X] Add DWARF piece test cases for bit-field access
- [X] read/write_pieced_value: Drop 'buffer_size' variable
- [ ] Fix handling of DWARF register pieces on big-endian targets
- [ ] read/write_pieced_value: Improve logic for buffer allocation
- [X] Respect piece offset for DW_OP_bit_piece
- [ ] read/write_pieced_value: Remove unnecessary variable copies
- [ ] Fix bit-/byte-offset mismatch in parameter to read_value_memory
- [X] write_pieced_value: Notify memory_changed observers
- [ ] read/write_pieced_value: Merge into one function
For the 5 non-check-marked patches I haven't received feedback yet.
--
Andreas
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v2 00/19] Various DWARF piece fixes
2017-05-09 17:47 [PATCH v2 00/19] Various DWARF piece fixes Andreas Arnez
` (19 preceding siblings ...)
2017-05-30 16:42 ` [ping] [PATCH v2 00/19] Various DWARF piece fixes Andreas Arnez
@ 2017-05-30 20:44 ` Simon Marchi
2017-05-31 14:24 ` Andreas Arnez
20 siblings, 1 reply; 56+ messages in thread
From: Simon Marchi @ 2017-05-30 20:44 UTC (permalink / raw)
To: Andreas Arnez; +Cc: gdb-patches, qiyaoltc
On 2017-05-09 19:45, Andreas Arnez wrote:
> This series fixes various issues with DWARF piece handling.
> Specifically it almost rewrites the functions read_pieced_value and
> write_pieced_value in multiple steps. Test cases are added as well.
>
> Version 1 is here:
>
> https://sourceware.org/ml/gdb-patches/2017-04/msg00177.html
>
> Changes from version 1 include:
>
> * Split up some patches further.
>
> * Added two more fixes for memory pieces.
>
> * Added a "merge" patch at the end.
>
> * Introduced get_endianness convenience proc for test suite.
>
> * Minor test case improvement.
>
> * Comment- and some general readability improvements.
Hi Andreas,
I went over the patches that Yao hasn't replied on, namely 13, 14, 16,
17 and 19, it all looks good to me (note the little comment on #17).
Since it's some tricky code, I wasn't always able to convince myself of
the correctness by just looking at it. But I went through the test
cases by hand, when applicable, and it gives me enough confidence that
the code is correct (or at least more correct than the current code :)).
I'm not sure I like the merge of the two functions, already scary by
themselves, in one scarier monster, but I understand the downsides of
having two separate functions, so I'm ok to go with it.
I'll let Yao decide if he wants to go himself through the remaining
patches.
Simon
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v2 00/19] Various DWARF piece fixes
2017-05-30 20:44 ` Simon Marchi
@ 2017-05-31 14:24 ` Andreas Arnez
2017-06-12 11:38 ` Andreas Arnez
0 siblings, 1 reply; 56+ messages in thread
From: Andreas Arnez @ 2017-05-31 14:24 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches, qiyaoltc
On Tue, May 30 2017, Simon Marchi wrote:
> I went over the patches that Yao hasn't replied on, namely 13, 14, 16, 17
> and 19, it all looks good to me (note the little comment on #17). Since
> it's some tricky code, I wasn't always able to convince myself of the
> correctness by just looking at it. But I went through the test cases by
> hand, when applicable, and it gives me enough confidence that the code is
> correct (or at least more correct than the current code :)).
Thanks a lot for looking at this!
> I'm not sure I like the merge of the two functions, already scary by
> themselves, in one scarier monster, but I understand the downsides of
> having two separate functions, so I'm ok to go with it.
Yeah, it's a trade-off.
> I'll let Yao decide if he wants to go himself through the remaining
> patches.
OK, I guess I'll wait a few more days for further feedback from Yao (or
others).
--
Andreas
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH v2 00/19] Various DWARF piece fixes
2017-05-31 14:24 ` Andreas Arnez
@ 2017-06-12 11:38 ` Andreas Arnez
0 siblings, 0 replies; 56+ messages in thread
From: Andreas Arnez @ 2017-06-12 11:38 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches, qiyaoltc
On Wed, May 31 2017, Andreas Arnez wrote:
> On Tue, May 30 2017, Simon Marchi wrote:
>
>> I went over the patches that Yao hasn't replied on, namely 13, 14, 16, 17
>> and 19, it all looks good to me (note the little comment on #17). Since
>> it's some tricky code, I wasn't always able to convince myself of the
>> correctness by just looking at it. But I went through the test cases by
>> hand, when applicable, and it gives me enough confidence that the code is
>> correct (or at least more correct than the current code :)).
>
> Thanks a lot for looking at this!
>
>> I'm not sure I like the merge of the two functions, already scary by
>> themselves, in one scarier monster, but I understand the downsides of
>> having two separate functions, so I'm ok to go with it.
>
> Yeah, it's a trade-off.
>
>> I'll let Yao decide if he wants to go himself through the remaining
>> patches.
>
> OK, I guess I'll wait a few more days for further feedback from Yao (or
> others).
OK, I've waited a few days by now... Is this ready to push?
https://sourceware.org/ml/gdb-patches/2017-05/msg00215.html
--
Andreas
^ permalink raw reply [flat|nested] 56+ messages in thread