* [PATCH 1/3] Move computed value's frame id to piece_closure
2016-11-25 10:07 ` [PATCH 0/3] regnum and next_frame_id are only used for lval_register Yao Qi
@ 2016-11-25 10:07 ` Yao Qi
2016-11-25 11:48 ` Ulrich Weigand
2016-11-25 10:07 ` [PATCH 3/3] Restrict checking value.lval on using address Yao Qi
2016-11-25 10:07 ` [PATCH 2/3] Adjust Value.location for lval_register Yao Qi
2 siblings, 1 reply; 25+ messages in thread
From: Yao Qi @ 2016-11-25 10:07 UTC (permalink / raw)
To: gdb-patches
Nowadays, we set computed value's frame id, which is a misuse to me.
The computed value itself doesn't care about frame id, but function
value_computed_funcs (val)->read (or read_pieced_value) cares about
which frame the register is relative to, so 'struct piece_closure' is
a better place to fit frame id.
This patch adds a frame id in 'struct piece_closure', and use it
instead of using computed value's frame id.
gdb:
2016-11-24 Yao Qi <yao.qi@linaro.org>
* dwarf2loc.c (struct piece_closure) <frame_id>: New field.
(allocate_piece_closure): Add new parameter 'frame' and set
closure's frame_id field accordingly.
(read_pieced_value): Get frame from closure instead of value.
(dwarf2_evaluate_loc_desc_full): Remove code getting frame id.
Don't set value's frame id.
---
gdb/dwarf2loc.c | 25 ++++++++++++-------------
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 44dceda..872d033 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1464,6 +1464,10 @@ struct piece_closure
/* The pieces themselves. */
struct dwarf_expr_piece *pieces;
+
+ /* Frame ID of frame to which a register value is relative, used
+ only by DWARF_VALUE_REGISTER. */
+ struct frame_id frame_id;
};
/* Allocate a closure for a value formed from separately-described
@@ -1472,7 +1476,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)
+ int addr_size, struct frame_info *frame)
{
struct piece_closure *c = XCNEW (struct piece_closure);
int i;
@@ -1482,6 +1486,10 @@ allocate_piece_closure (struct dwarf2_per_cu_data *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;
+ else
+ c->frame_id = get_frame_id (get_next_frame_sentinel_okay (frame));
memcpy (c->pieces, pieces, n_pieces * sizeof (struct dwarf_expr_piece));
for (i = 0; i < n_pieces; ++i)
@@ -1672,18 +1680,12 @@ read_pieced_value (struct value *v)
gdb_byte *contents;
struct piece_closure *c
= (struct piece_closure *) value_computed_closure (v);
- struct frame_info *frame;
size_t type_len;
size_t buffer_size = 0;
std::vector<gdb_byte> buffer;
int bits_big_endian
= gdbarch_bits_big_endian (get_type_arch (value_type (v)));
- /* VALUE_FRAME_ID is used instead of VALUE_NEXT_FRAME_ID here
- because FRAME is passed to get_frame_register_bytes(), which
- does its own "->next" operation. */
- frame = frame_find_by_id (VALUE_FRAME_ID (v));
-
if (value_type (v) != value_enclosing_type (v))
internal_error (__FILE__, __LINE__,
_("Should not be able to create a lazy value with "
@@ -1743,6 +1745,8 @@ read_pieced_value (struct value *v)
{
case DWARF_VALUE_REGISTER:
{
+ struct frame_info *frame
+ = frame_find_by_id (get_prev_frame_id_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 optim, unavail;
@@ -2311,10 +2315,6 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
if (ctx.num_pieces > 0)
{
struct piece_closure *c;
- struct frame_id frame_id
- = frame == NULL
- ? null_frame_id
- : get_frame_id (get_next_frame_sentinel_okay (frame));
ULONGEST bit_size = 0;
int i;
@@ -2324,12 +2324,11 @@ 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);
+ ctx.addr_size, frame);
/* We must clean up the value chain after creating the piece
closure but before allocating the result. */
do_cleanups (value_chain);
retval = allocate_computed_value (type, &pieced_value_funcs, c);
- VALUE_NEXT_FRAME_ID (retval) = frame_id;
set_value_offset (retval, byte_offset);
}
else
--
1.9.1
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] Move computed value's frame id to piece_closure
2016-11-25 10:07 ` [PATCH 1/3] Move computed value's frame id to piece_closure Yao Qi
@ 2016-11-25 11:48 ` Ulrich Weigand
2016-11-28 17:20 ` Yao Qi
0 siblings, 1 reply; 25+ messages in thread
From: Ulrich Weigand @ 2016-11-25 11:48 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
Yao Qi wrote:
> + if (frame == NULL)
> + c->frame_id = null_frame_id;
> + else
> + c->frame_id = get_frame_id (get_next_frame_sentinel_okay (frame));
[...]
> + struct frame_info *frame
> + = frame_find_by_id (get_prev_frame_id_by_id (c->frame_id));
I guess now that this is a separate field, there's no longer a reason
to go via next-frame / prev-frame here. It looks like this was introduced
when VALUE_FRAME_ID was changed to VALUE_NEXT_FRAME_ID, but this isn't
really necessary for the piece logic here. I think this can simply be:
c->frame_id = get_frame_id (frame);
[...]
struct frame_info *frame = frame_find_by_id (c->frame_id);
instead.
Otherwise, this looks good to me.
Bye,
Ulrich
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] Move computed value's frame id to piece_closure
2016-11-25 11:48 ` Ulrich Weigand
@ 2016-11-28 17:20 ` Yao Qi
0 siblings, 0 replies; 25+ messages in thread
From: Yao Qi @ 2016-11-28 17:20 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: gdb-patches
On Fri, Nov 25, 2016 at 12:48:36PM +0100, Ulrich Weigand wrote:
> Yao Qi wrote:
>
> > + if (frame == NULL)
> > + c->frame_id = null_frame_id;
> > + else
> > + c->frame_id = get_frame_id (get_next_frame_sentinel_okay (frame));
> [...]
> > + struct frame_info *frame
> > + = frame_find_by_id (get_prev_frame_id_by_id (c->frame_id));
>
> I guess now that this is a separate field, there's no longer a reason
> to go via next-frame / prev-frame here. It looks like this was introduced
> when VALUE_FRAME_ID was changed to VALUE_NEXT_FRAME_ID, but this isn't
> really necessary for the piece logic here. I think this can simply be:
>
> c->frame_id = get_frame_id (frame);
> [...]
> struct frame_info *frame = frame_find_by_id (c->frame_id);
>
> instead.
>
> Otherwise, this looks good to me.
>
Patch below is pushed in.
--
Yao (é½å°§)
From ee40d8d45213caf0cfb63e603f0fd5a58532e751 Mon Sep 17 00:00:00 2001
From: Yao Qi <yao.qi@linaro.org>
Date: Mon, 28 Nov 2016 17:09:26 +0000
Subject: [PATCH] Move computed value's frame id to piece_closure
Nowadays, we set computed value's frame id, which is a misuse to me.
The computed value itself doesn't care about frame id, but function
value_computed_funcs (val)->read (or read_pieced_value) cares about
which frame the register is relative to, so 'struct piece_closure' is
a better place to fit frame id.
This patch adds a frame id in 'struct piece_closure', and use it
instead of using computed value's frame id.
gdb:
2016-11-28 Yao Qi <yao.qi@linaro.org>
* dwarf2loc.c (struct piece_closure) <frame_id>: New field.
(allocate_piece_closure): Add new parameter 'frame' and set
closure's frame_id field accordingly.
(read_pieced_value): Get frame from closure instead of value.
(dwarf2_evaluate_loc_desc_full): Remove code getting frame id.
Don't set value's frame id.
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 128f654..aaf88e4 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1465,6 +1465,10 @@ struct piece_closure
/* The pieces themselves. */
struct dwarf_expr_piece *pieces;
+
+ /* Frame ID of frame to which a register value is relative, used
+ only by DWARF_VALUE_REGISTER. */
+ struct frame_id frame_id;
};
/* Allocate a closure for a value formed from separately-described
@@ -1473,7 +1477,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)
+ int addr_size, struct frame_info *frame)
{
struct piece_closure *c = XCNEW (struct piece_closure);
int i;
@@ -1483,6 +1487,10 @@ allocate_piece_closure (struct dwarf2_per_cu_data *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;
+ else
+ c->frame_id = get_frame_id (frame);
memcpy (c->pieces, pieces, n_pieces * sizeof (struct dwarf_expr_piece));
for (i = 0; i < n_pieces; ++i)
@@ -1731,18 +1739,12 @@ read_pieced_value (struct value *v)
gdb_byte *contents;
struct piece_closure *c
= (struct piece_closure *) value_computed_closure (v);
- struct frame_info *frame;
size_t type_len;
size_t buffer_size = 0;
std::vector<gdb_byte> buffer;
int bits_big_endian
= gdbarch_bits_big_endian (get_type_arch (value_type (v)));
- /* VALUE_FRAME_ID is used instead of VALUE_NEXT_FRAME_ID here
- because FRAME is passed to get_frame_register_bytes(), which
- does its own "->next" operation. */
- frame = frame_find_by_id (VALUE_FRAME_ID (v));
-
if (value_type (v) != value_enclosing_type (v))
internal_error (__FILE__, __LINE__,
_("Should not be able to create a lazy value with "
@@ -1802,6 +1804,7 @@ read_pieced_value (struct value *v)
{
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);
int optim, unavail;
@@ -2370,10 +2373,6 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
if (ctx.num_pieces > 0)
{
struct piece_closure *c;
- struct frame_id frame_id
- = frame == NULL
- ? null_frame_id
- : get_frame_id (get_next_frame_sentinel_okay (frame));
ULONGEST bit_size = 0;
int i;
@@ -2383,12 +2382,11 @@ 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);
+ ctx.addr_size, frame);
/* We must clean up the value chain after creating the piece
closure but before allocating the result. */
do_cleanups (value_chain);
retval = allocate_computed_value (type, &pieced_value_funcs, c);
- VALUE_NEXT_FRAME_ID (retval) = frame_id;
set_value_offset (retval, byte_offset);
}
else
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 3/3] Restrict checking value.lval on using address
2016-11-25 10:07 ` [PATCH 0/3] regnum and next_frame_id are only used for lval_register Yao Qi
2016-11-25 10:07 ` [PATCH 1/3] Move computed value's frame id to piece_closure Yao Qi
@ 2016-11-25 10:07 ` Yao Qi
2016-11-25 11:52 ` Ulrich Weigand
2016-11-25 10:07 ` [PATCH 2/3] Adjust Value.location for lval_register Yao Qi
2 siblings, 1 reply; 25+ messages in thread
From: Yao Qi @ 2016-11-25 10:07 UTC (permalink / raw)
To: gdb-patches
With the previous change, value.location.address is only valid for
lval_memory. This patch restrict some checking on value.lval on
using address. Since we have a check on VALUE_VAL in
set_value_address, we need to set VALUE_VAL properly before
set_value_address too.
gdb:
2016-11-25 Yao Qi <yao.qi@linaro.org>
* ada-lang.c (ensure_lval): Call set_value_address after setting
VALUE_LVAL.
* elfread.c (elf_gnu_ifunc_resolve_addr): Set VALUE_LVAL to
lval_memory.
(elf_gnu_ifunc_resolver_return_stop): Likewise.
* value.c (value_fn_field): Likewise.
(value_from_contents_and_address_unresolved): Likewise.
(value_from_contents_and_address): Likewise.
(value_address): Check value->lval isn't
lval_memory.
(value_raw_address): Likewise.
(set_value_address): Assert value->lval is lval_memory.
---
gdb/ada-lang.c | 2 +-
gdb/elfread.c | 2 ++
gdb/value.c | 17 ++++++-----------
3 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 0647a9b..33591af 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -4473,8 +4473,8 @@ ensure_lval (struct value *val)
const CORE_ADDR addr =
value_as_long (value_allocate_space_in_inferior (len));
- set_value_address (val, addr);
VALUE_LVAL (val) = lval_memory;
+ set_value_address (val, addr);
write_memory (addr, value_contents (val), len);
}
diff --git a/gdb/elfread.c b/gdb/elfread.c
index e49af6d..c6d0fdb 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -879,6 +879,7 @@ elf_gnu_ifunc_resolve_addr (struct gdbarch *gdbarch, CORE_ADDR pc)
name_at_pc = NULL;
function = allocate_value (func_func_type);
+ VALUE_LVAL (function) = lval_memory;
set_value_address (function, pc);
/* STT_GNU_IFUNC resolver functions usually receive the HWCAP vector as
@@ -992,6 +993,7 @@ elf_gnu_ifunc_resolver_return_stop (struct breakpoint *b)
gdb_assert (b->loc->next == NULL);
func_func = allocate_value (func_func_type);
+ VALUE_LVAL (func_func) = lval_memory;
set_value_address (func_func, b->loc->related_address);
value = allocate_value (value_type);
diff --git a/gdb/value.c b/gdb/value.c
index eff5462..9fb5fe1 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -1539,9 +1539,7 @@ value_lval_const (const struct value *value)
CORE_ADDR
value_address (const struct value *value)
{
- if (value->lval == lval_internalvar
- || value->lval == lval_internalvar_component
- || value->lval == lval_xcallable)
+ if (value->lval != lval_memory)
return 0;
if (value->parent != NULL)
return value_address (value->parent) + value->offset;
@@ -1557,9 +1555,7 @@ value_address (const struct value *value)
CORE_ADDR
value_raw_address (const struct value *value)
{
- if (value->lval == lval_internalvar
- || value->lval == lval_internalvar_component
- || value->lval == lval_xcallable)
+ if (value->lval != lval_memory)
return 0;
return value->location.address;
}
@@ -1567,9 +1563,7 @@ value_raw_address (const struct value *value)
void
set_value_address (struct value *value, CORE_ADDR addr)
{
- gdb_assert (value->lval != lval_internalvar
- && value->lval != lval_internalvar_component
- && value->lval != lval_xcallable);
+ gdb_assert (value->lval == lval_memory);
value->location.address = addr;
}
@@ -3268,6 +3262,7 @@ value_fn_field (struct value **arg1p, struct fn_field *f,
}
v = allocate_value (ftype);
+ VALUE_LVAL (v) = lval_memory;
if (sym)
{
set_value_address (v, BLOCK_START (SYMBOL_BLOCK_VALUE (sym)));
@@ -3654,8 +3649,8 @@ value_from_contents_and_address_unresolved (struct type *type,
v = allocate_value_lazy (type);
else
v = value_from_contents (type, valaddr);
- set_value_address (v, address);
VALUE_LVAL (v) = lval_memory;
+ set_value_address (v, address);
return v;
}
@@ -3680,8 +3675,8 @@ value_from_contents_and_address (struct type *type,
if (TYPE_DATA_LOCATION (resolved_type_no_typedef) != NULL
&& TYPE_DATA_LOCATION_KIND (resolved_type_no_typedef) == PROP_CONST)
address = TYPE_DATA_LOCATION_ADDR (resolved_type_no_typedef);
- set_value_address (v, address);
VALUE_LVAL (v) = lval_memory;
+ set_value_address (v, address);
return v;
}
--
1.9.1
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/3] Restrict checking value.lval on using address
2016-11-25 10:07 ` [PATCH 3/3] Restrict checking value.lval on using address Yao Qi
@ 2016-11-25 11:52 ` Ulrich Weigand
2016-11-28 17:22 ` Yao Qi
0 siblings, 1 reply; 25+ messages in thread
From: Ulrich Weigand @ 2016-11-25 11:52 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
Yao Qi wrote:
> * ada-lang.c (ensure_lval): Call set_value_address after setting
> VALUE_LVAL.
> * elfread.c (elf_gnu_ifunc_resolve_addr): Set VALUE_LVAL to
> lval_memory.
> (elf_gnu_ifunc_resolver_return_stop): Likewise.
> * value.c (value_fn_field): Likewise.
> (value_from_contents_and_address_unresolved): Likewise.
> (value_from_contents_and_address): Likewise.
> (value_address): Check value->lval isn't
> lval_memory.
> (value_raw_address): Likewise.
> (set_value_address): Assert value->lval is lval_memory.
Looks good to me.
Thanks,
Ulrich
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/3] Restrict checking value.lval on using address
2016-11-25 11:52 ` Ulrich Weigand
@ 2016-11-28 17:22 ` Yao Qi
0 siblings, 0 replies; 25+ messages in thread
From: Yao Qi @ 2016-11-28 17:22 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: gdb-patches
On Fri, Nov 25, 2016 at 12:52:10PM +0100, Ulrich Weigand wrote:
> Yao Qi wrote:
>
> > * ada-lang.c (ensure_lval): Call set_value_address after setting
> > VALUE_LVAL.
> > * elfread.c (elf_gnu_ifunc_resolve_addr): Set VALUE_LVAL to
> > lval_memory.
> > (elf_gnu_ifunc_resolver_return_stop): Likewise.
> > * value.c (value_fn_field): Likewise.
> > (value_from_contents_and_address_unresolved): Likewise.
> > (value_from_contents_and_address): Likewise.
> > (value_address): Check value->lval isn't
> > lval_memory.
> > (value_raw_address): Likewise.
> > (set_value_address): Assert value->lval is lval_memory.
>
> Looks good to me.
>
Patch is pushed in.
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 2/3] Adjust Value.location for lval_register
2016-11-25 10:07 ` [PATCH 0/3] regnum and next_frame_id are only used for lval_register Yao Qi
2016-11-25 10:07 ` [PATCH 1/3] Move computed value's frame id to piece_closure Yao Qi
2016-11-25 10:07 ` [PATCH 3/3] Restrict checking value.lval on using address Yao Qi
@ 2016-11-25 10:07 ` Yao Qi
2016-11-25 11:51 ` Ulrich Weigand
2 siblings, 1 reply; 25+ messages in thread
From: Yao Qi @ 2016-11-25 10:07 UTC (permalink / raw)
To: gdb-patches
value.regnum and value.next_frame_id are only used for lval_register,
so this patch moves them to union value.location. As a result, when
we copy value, only copy location, don't need to copy regnum and
next_frame_id.
gdb:
2016-11-23 Yao Qi <yao.qi@linaro.org>
* valops.c (value_slice): Don't set frame id of slice.
* value.c (struct value) <regnum, next_frame_id>: Move them to...
(struct value) <location>: ... here. Update comments.
(allocate_value_lazy): Don't set frame id and regnum.
(deprecated_value_next_frame_id_hack): Adjust.
(deprecated_value_regnum_hack): Adjust.
(value_copy): Don't copy frame id and regnu.
(value_primitive_field): Likewise.
(value_from_component): Likewise.
---
gdb/valops.c | 1 -
gdb/value.c | 40 +++++++++++++++-------------------------
2 files changed, 15 insertions(+), 26 deletions(-)
diff --git a/gdb/valops.c b/gdb/valops.c
index 8a45641..3a7550d 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -3827,7 +3827,6 @@ value_slice (struct value *array, int lowbound, int length)
}
set_value_component_location (slice, array);
- VALUE_NEXT_FRAME_ID (slice) = VALUE_NEXT_FRAME_ID (array);
set_value_offset (slice, value_offset (array) + offset);
}
diff --git a/gdb/value.c b/gdb/value.c
index 8d33501..eff5462 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -205,17 +205,23 @@ struct value
/* If the value has been released. */
unsigned int released : 1;
- /* Register number if the value is from a register. */
- short regnum;
-
/* Location of value (if lval). */
union
{
- /* If lval == lval_memory, this is the address in the inferior.
- If lval == lval_register, this is the byte offset into the
- registers structure. */
+ /* If lval == lval_memory, this is the address in the inferior */
CORE_ADDR address;
+ /*If lval == lval_register, the value is from a register. */
+ struct
+ {
+ /* Register number. */
+ short regnum;
+ /* Frame ID of "next" frame to which a register value is relative.
+ If the register value is found relative to frame F, then the
+ frame id of F->next will be stored in next_frame_id. */
+ struct frame_id next_frame_id;
+ } reg;
+
/* Pointer to internal variable. */
struct internalvar *internalvar;
@@ -237,9 +243,7 @@ struct value
/* Describes offset of a value within lval of a structure in target
addressable memory units. If lval == lval_memory, this is an offset to
- the address. If lval == lval_register, this is a further offset from
- location.address within the registers structure. Note also the member
- embedded_offset below. */
+ the address. Note also the member embedded_offset below. */
LONGEST offset;
/* Only used for bitfields; number of bits contained in them. */
@@ -262,12 +266,6 @@ struct value
bitfields. */
struct value *parent;
- /* Frame ID of "next" frame to which a register value is relative. A
- register value is indicated when the lval enum (above) is set to
- lval_register. So, if the register value is found relative to frame F,
- then the frame id of F->next will be stored in next_frame_id. */
- struct frame_id next_frame_id;
-
/* Type of the value. */
struct type *type;
@@ -945,11 +943,9 @@ allocate_value_lazy (struct type *type)
val->enclosing_type = type;
VALUE_LVAL (val) = not_lval;
val->location.address = 0;
- VALUE_NEXT_FRAME_ID (val) = null_frame_id;
val->offset = 0;
val->bitpos = 0;
val->bitsize = 0;
- VALUE_REGNUM (val) = -1;
val->lazy = 1;
val->embedded_offset = 0;
val->pointed_to_offset = 0;
@@ -1586,13 +1582,13 @@ deprecated_value_internalvar_hack (struct value *value)
struct frame_id *
deprecated_value_next_frame_id_hack (struct value *value)
{
- return &value->next_frame_id;
+ return &value->location.reg.next_frame_id;
}
short *
deprecated_value_regnum_hack (struct value *value)
{
- return &value->regnum;
+ return &value->location.reg.regnum;
}
int
@@ -1788,8 +1784,6 @@ value_copy (struct value *arg)
val->offset = arg->offset;
val->bitpos = arg->bitpos;
val->bitsize = arg->bitsize;
- VALUE_NEXT_FRAME_ID (val) = VALUE_NEXT_FRAME_ID (arg);
- VALUE_REGNUM (val) = VALUE_REGNUM (arg);
val->lazy = arg->lazy;
val->embedded_offset = value_embedded_offset (arg);
val->pointed_to_offset = arg->pointed_to_offset;
@@ -3229,8 +3223,6 @@ value_primitive_field (struct value *arg1, LONGEST offset,
+ value_embedded_offset (arg1));
}
set_value_component_location (v, arg1);
- VALUE_REGNUM (v) = VALUE_REGNUM (arg1);
- VALUE_NEXT_FRAME_ID (v) = VALUE_NEXT_FRAME_ID (arg1);
return v;
}
@@ -3814,8 +3806,6 @@ value_from_component (struct value *whole, struct type *type, LONGEST offset)
}
v->offset = value_offset (whole) + offset + value_embedded_offset (whole);
set_value_component_location (v, whole);
- VALUE_REGNUM (v) = VALUE_REGNUM (whole);
- VALUE_NEXT_FRAME_ID (v) = VALUE_NEXT_FRAME_ID (whole);
return v;
}
--
1.9.1
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/3] Adjust Value.location for lval_register
2016-11-25 10:07 ` [PATCH 2/3] Adjust Value.location for lval_register Yao Qi
@ 2016-11-25 11:51 ` Ulrich Weigand
2016-11-25 11:57 ` Yao Qi
0 siblings, 1 reply; 25+ messages in thread
From: Ulrich Weigand @ 2016-11-25 11:51 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
Yao Qi wrote:
> + /* Register number. */
> + short regnum;
Now this is no longer space-constrained (the location union is two pointers
in size anyway), so this can just be an "int" like regnums are elsewhere.
> + /* Frame ID of "next" frame to which a register value is relative.
> + If the register value is found relative to frame F, then the
> + frame id of F->next will be stored in next_frame_id. */
> + struct frame_id next_frame_id;
> + } reg;
> /* Describes offset of a value within lval of a structure in target
> addressable memory units. If lval == lval_memory, this is an offset to
> - the address. If lval == lval_register, this is a further offset from
> - location.address within the registers structure. Note also the member
> - embedded_offset below. */
> + the address. Note also the member embedded_offset below. */
> LONGEST offset;
Hmm, I think we recently had the discussion that *any* values should allow
using an offset. The comment should probably reflect this.
Otherwise, this looks good to me.
Bye,
Ulrich
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/3] Adjust Value.location for lval_register
2016-11-25 11:51 ` Ulrich Weigand
@ 2016-11-25 11:57 ` Yao Qi
2016-11-25 12:10 ` Ulrich Weigand
0 siblings, 1 reply; 25+ messages in thread
From: Yao Qi @ 2016-11-25 11:57 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: gdb-patches
On Fri, Nov 25, 2016 at 12:51:30PM +0100, Ulrich Weigand wrote:
>
>
> > /* Describes offset of a value within lval of a structure in target
> > addressable memory units. If lval == lval_memory, this is an offset to
> > - the address. If lval == lval_register, this is a further offset from
> > - location.address within the registers structure. Note also the member
> > - embedded_offset below. */
> > + the address. Note also the member embedded_offset below. */
> > LONGEST offset;
>
> Hmm, I think we recently had the discussion that *any* values should allow
> using an offset. The comment should probably reflect this.
>
How about "Describes offset of a value within lval of a structure. Note
also the member embedded_offset below."?
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/3] Adjust Value.location for lval_register
2016-11-25 11:57 ` Yao Qi
@ 2016-11-25 12:10 ` Ulrich Weigand
2016-11-28 17:22 ` Yao Qi
0 siblings, 1 reply; 25+ messages in thread
From: Ulrich Weigand @ 2016-11-25 12:10 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
> On Fri, Nov 25, 2016 at 12:51:30PM +0100, Ulrich Weigand wrote:
> >
> >
> > > /* Describes offset of a value within lval of a structure in target
> > > addressable memory units. If lval == lval_memory, this is an offset to
> > > - the address. If lval == lval_register, this is a further offset from
> > > - location.address within the registers structure. Note also the member
> > > - embedded_offset below. */
> > > + the address. Note also the member embedded_offset below. */
> > > LONGEST offset;
> >
> > Hmm, I think we recently had the discussion that *any* values should allow
> > using an offset. The comment should probably reflect this.
> >
>
> How about "Describes offset of a value within lval of a structure. Note
> also the member embedded_offset below."?
I think we should leave in the "in target addressable memory units" to clarify
that this is a *byte* offset, not a bit offset.
Otherwise, looks good to me.
Bye,
Ulrich
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/3] Adjust Value.location for lval_register
2016-11-25 12:10 ` Ulrich Weigand
@ 2016-11-28 17:22 ` Yao Qi
0 siblings, 0 replies; 25+ messages in thread
From: Yao Qi @ 2016-11-28 17:22 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: gdb-patches
On Fri, Nov 25, 2016 at 01:10:02PM +0100, Ulrich Weigand wrote:
> > On Fri, Nov 25, 2016 at 12:51:30PM +0100, Ulrich Weigand wrote:
> > >
> > >
> > > > /* Describes offset of a value within lval of a structure in target
> > > > addressable memory units. If lval == lval_memory, this is an offset to
> > > > - the address. If lval == lval_register, this is a further offset from
> > > > - location.address within the registers structure. Note also the member
> > > > - embedded_offset below. */
> > > > + the address. Note also the member embedded_offset below. */
> > > > LONGEST offset;
> > >
> > > Hmm, I think we recently had the discussion that *any* values should allow
> > > using an offset. The comment should probably reflect this.
> > >
> >
> > How about "Describes offset of a value within lval of a structure. Note
> > also the member embedded_offset below."?
>
> I think we should leave in the "in target addressable memory units" to clarify
> that this is a *byte* offset, not a bit offset.
>
> Otherwise, looks good to me.
>
OK, patch below is pushed in.
--
Yao (é½å°§)
From 7dc54575d91a2b41f6c3e838eec44a7017a24436 Mon Sep 17 00:00:00 2001
From: Yao Qi <yao.qi@linaro.org>
Date: Mon, 28 Nov 2016 17:09:26 +0000
Subject: [PATCH] Adjust Value.location for lval_register
value.regnum and value.next_frame_id are only used for lval_register,
so this patch moves them to union value.location. As a result, when
we copy value, only copy location, don't need to copy regnum and
next_frame_id.
This patch also changes regnum's type to int as there is no space
constraint, so update deprecated_value_regnum_hack return type too.
gdb:
2016-11-28 Yao Qi <yao.qi@linaro.org>
* valops.c (value_slice): Don't set frame id of slice.
* value.c (struct value) <regnum, next_frame_id>: Move them to...
(struct value) <location>: ... here. Update comments.
(allocate_value_lazy): Don't set frame id and regnum.
(deprecated_value_next_frame_id_hack): Adjust.
(deprecated_value_regnum_hack): Adjust.
(value_copy): Don't copy frame id and regnu.
(value_primitive_field): Likewise.
(value_from_component): Likewise.
(deprecated_value_regnum_hack): Return int *.
* value.h (deprecated_value_regnum_hack): Update declaration.
diff --git a/gdb/valops.c b/gdb/valops.c
index 8a45641..3a7550d 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -3827,7 +3827,6 @@ value_slice (struct value *array, int lowbound, int length)
}
set_value_component_location (slice, array);
- VALUE_NEXT_FRAME_ID (slice) = VALUE_NEXT_FRAME_ID (array);
set_value_offset (slice, value_offset (array) + offset);
}
diff --git a/gdb/value.c b/gdb/value.c
index 8d33501..13a0bb9 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -205,17 +205,23 @@ struct value
/* If the value has been released. */
unsigned int released : 1;
- /* Register number if the value is from a register. */
- short regnum;
-
/* Location of value (if lval). */
union
{
- /* If lval == lval_memory, this is the address in the inferior.
- If lval == lval_register, this is the byte offset into the
- registers structure. */
+ /* If lval == lval_memory, this is the address in the inferior */
CORE_ADDR address;
+ /*If lval == lval_register, the value is from a register. */
+ struct
+ {
+ /* Register number. */
+ int regnum;
+ /* Frame ID of "next" frame to which a register value is relative.
+ If the register value is found relative to frame F, then the
+ frame id of F->next will be stored in next_frame_id. */
+ struct frame_id next_frame_id;
+ } reg;
+
/* Pointer to internal variable. */
struct internalvar *internalvar;
@@ -236,10 +242,8 @@ struct value
} location;
/* Describes offset of a value within lval of a structure in target
- addressable memory units. If lval == lval_memory, this is an offset to
- the address. If lval == lval_register, this is a further offset from
- location.address within the registers structure. Note also the member
- embedded_offset below. */
+ addressable memory units. Note also the member embedded_offset
+ below. */
LONGEST offset;
/* Only used for bitfields; number of bits contained in them. */
@@ -262,12 +266,6 @@ struct value
bitfields. */
struct value *parent;
- /* Frame ID of "next" frame to which a register value is relative. A
- register value is indicated when the lval enum (above) is set to
- lval_register. So, if the register value is found relative to frame F,
- then the frame id of F->next will be stored in next_frame_id. */
- struct frame_id next_frame_id;
-
/* Type of the value. */
struct type *type;
@@ -945,11 +943,9 @@ allocate_value_lazy (struct type *type)
val->enclosing_type = type;
VALUE_LVAL (val) = not_lval;
val->location.address = 0;
- VALUE_NEXT_FRAME_ID (val) = null_frame_id;
val->offset = 0;
val->bitpos = 0;
val->bitsize = 0;
- VALUE_REGNUM (val) = -1;
val->lazy = 1;
val->embedded_offset = 0;
val->pointed_to_offset = 0;
@@ -1586,13 +1582,13 @@ deprecated_value_internalvar_hack (struct value *value)
struct frame_id *
deprecated_value_next_frame_id_hack (struct value *value)
{
- return &value->next_frame_id;
+ return &value->location.reg.next_frame_id;
}
-short *
+int *
deprecated_value_regnum_hack (struct value *value)
{
- return &value->regnum;
+ return &value->location.reg.regnum;
}
int
@@ -1788,8 +1784,6 @@ value_copy (struct value *arg)
val->offset = arg->offset;
val->bitpos = arg->bitpos;
val->bitsize = arg->bitsize;
- VALUE_NEXT_FRAME_ID (val) = VALUE_NEXT_FRAME_ID (arg);
- VALUE_REGNUM (val) = VALUE_REGNUM (arg);
val->lazy = arg->lazy;
val->embedded_offset = value_embedded_offset (arg);
val->pointed_to_offset = arg->pointed_to_offset;
@@ -3229,8 +3223,6 @@ value_primitive_field (struct value *arg1, LONGEST offset,
+ value_embedded_offset (arg1));
}
set_value_component_location (v, arg1);
- VALUE_REGNUM (v) = VALUE_REGNUM (arg1);
- VALUE_NEXT_FRAME_ID (v) = VALUE_NEXT_FRAME_ID (arg1);
return v;
}
@@ -3814,8 +3806,6 @@ value_from_component (struct value *whole, struct type *type, LONGEST offset)
}
v->offset = value_offset (whole) + offset + value_embedded_offset (whole);
set_value_component_location (v, whole);
- VALUE_REGNUM (v) = VALUE_REGNUM (whole);
- VALUE_NEXT_FRAME_ID (v) = VALUE_NEXT_FRAME_ID (whole);
return v;
}
diff --git a/gdb/value.h b/gdb/value.h
index 281b5a8..f776323 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -448,7 +448,7 @@ extern struct frame_id *deprecated_value_next_frame_id_hack (struct value *);
#define VALUE_FRAME_ID(val) (get_prev_frame_id_by_id (VALUE_NEXT_FRAME_ID (val)))
/* Register number if the value is from a register. */
-extern short *deprecated_value_regnum_hack (struct value *);
+extern int *deprecated_value_regnum_hack (struct value *);
#define VALUE_REGNUM(val) (*deprecated_value_regnum_hack (val))
/* Return value after lval_funcs->coerce_ref (after check_typedef). Return
^ permalink raw reply [flat|nested] 25+ messages in thread