* [PATCH] Pass address around within ada-valprint.c
@ 2013-07-30 10:40 Andrew Burgess
2013-08-06 10:41 ` ping: " Andrew Burgess
0 siblings, 1 reply; 2+ messages in thread
From: Andrew Burgess @ 2013-07-30 10:40 UTC (permalink / raw)
To: gdb-patches
I was trying to better understand ada-lang.c:coerce_unspec_val_to_type,
and after looking at the code for a while I tried experimentally
applying this patch to make all values lazy:
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index dc5f2b6..58c4ba1 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -568,15 +568,8 @@ coerce_unspec_val_to_type (struct value *val, struct type *
trying to allocate some memory for it. */
check_size (type);
- if (value_lazy (val)
- || TYPE_LENGTH (type) > TYPE_LENGTH (value_type (val)))
- result = allocate_value_lazy (type);
- else
- {
- result = allocate_value (type);
- memcpy (value_contents_raw (result), value_contents (val),
- TYPE_LENGTH (type));
- }
+ result = allocate_value_lazy (type);
+
set_value_component_location (result, val);
set_value_bitsize (result, value_bitsize (val));
set_value_bitpos (result, value_bitpos (val));
I know we don't want to actually apply this patch as it would
cause gdb to fetch the value contents from the inferior in cases
where we currently reuse the contents we already have to hand,
BUT, as an experiment, I don't see why it should not be fine.
Unfortunately I ran into a few regressions.... it turns out that
in some cases the address parameter on VAL is not set-up correctly [1].
This worries me, given the "|| TYPE_LENGTH (type) > TYPE_LENGTH
(value_type (val))" check, would it not be possible to create an
example where the address of VAL is unset, but the sizes of the types
involved force the creation of a lazy value? If we could make such
an example then it surely would fail.
The patch below fixes all the regressions that the above experiment
revealed, it's really just passing the address around inside
ada-valprint.c a little more, seems like a good thing to me, but let
me know what you think.
OK to apply?
Thanks,
Andrew
[1] Maybe there's a reason for this that I'm not seeing / understanding.
2013-07-30 Andrew Burgess <aburgess@broadcom.com>
* ada-valprint.c (print_record): Add address parameter, and pass
the address on to sub-functions.
(print_field_values): Add address parameter, and pass the address
on to sub-functions.
(ada_val_print_1): Pass address parameter on to sub-functions.
(print_variant_part): Pass address parameter on to sub-functions.
diff --git a/gdb/ada-valprint.c b/gdb/ada-valprint.c
index 4a04d28..6ec7764 100644
--- a/gdb/ada-valprint.c
+++ b/gdb/ada-valprint.c
@@ -34,14 +34,15 @@
#include "exceptions.h"
#include "objfiles.h"
-static void print_record (struct type *, const gdb_byte *, int,
+static void print_record (struct type *, const gdb_byte *,
+ int, CORE_ADDR,
struct ui_file *,
int,
const struct value *,
const struct value_print_options *);
static int print_field_values (struct type *, const gdb_byte *,
- int,
+ int, CORE_ADDR,
struct ui_file *, int,
const struct value *,
const struct value_print_options *,
@@ -658,7 +659,7 @@ ada_val_print_1 (struct type *type, const gdb_byte *valaddr,
struct value *mark = value_mark ();
struct value *val;
- val = value_from_contents_and_address (type, valaddr + offset, address);
+ val = value_from_contents_and_address (type, valaddr + offset, address + offset);
/* If this is a reference, coerce it now. This helps taking care
of the case where ADDRESS is meaningless because original_value
was not an lval. */
@@ -732,7 +733,7 @@ ada_val_print_1 (struct type *type, const gdb_byte *valaddr,
nonsense value. Actually, we could use the same
code regardless of lengths; I'm just avoiding a cast. */
struct value *v1
- = value_from_contents_and_address (type, valaddr + offset, 0);
+ = value_from_contents_and_address (type, valaddr + offset, address + offset);
struct value *v = value_cast (target_type, v1);
ada_val_print_1 (target_type,
@@ -850,7 +851,7 @@ ada_val_print_1 (struct type *type, const gdb_byte *valaddr,
}
else
{
- print_record (type, valaddr, offset_aligned,
+ print_record (type, valaddr, offset_aligned, address,
stream, recurse, original_value, options);
return;
}
@@ -916,6 +917,7 @@ ada_val_print_1 (struct type *type, const gdb_byte *valaddr,
static int
print_variant_part (struct type *type, int field_num,
const gdb_byte *valaddr, int offset,
+ CORE_ADDR address,
struct ui_file *stream, int recurse,
const struct value *val,
const struct value_print_options *options,
@@ -934,7 +936,7 @@ print_variant_part (struct type *type, int field_num,
valaddr,
offset + TYPE_FIELD_BITPOS (type, field_num) / HOST_CHAR_BIT
+ TYPE_FIELD_BITPOS (var_type, which) / HOST_CHAR_BIT,
- stream, recurse, val, options,
+ address, stream, recurse, val, options,
comma_needed, outer_type, outer_offset);
}
@@ -990,7 +992,7 @@ ada_value_print (struct value *val0, struct ui_file *stream,
static void
print_record (struct type *type, const gdb_byte *valaddr,
- int offset,
+ int offset, CORE_ADDR address,
struct ui_file *stream, int recurse,
const struct value *val,
const struct value_print_options *options)
@@ -999,7 +1001,7 @@ print_record (struct type *type, const gdb_byte *valaddr,
fprintf_filtered (stream, "(");
- if (print_field_values (type, valaddr, offset,
+ if (print_field_values (type, valaddr, offset, address,
stream, recurse, val, options,
0, type, offset) != 0 && options->prettyformat)
{
@@ -1027,7 +1029,8 @@ print_record (struct type *type, const gdb_byte *valaddr,
static int
print_field_values (struct type *type, const gdb_byte *valaddr,
- int offset, struct ui_file *stream, int recurse,
+ int offset, CORE_ADDR address,
+ struct ui_file *stream, int recurse,
const struct value *val,
const struct value_print_options *options,
int comma_needed,
@@ -1049,7 +1052,7 @@ print_field_values (struct type *type, const gdb_byte *valaddr,
valaddr,
(offset
+ TYPE_FIELD_BITPOS (type, i) / HOST_CHAR_BIT),
- stream, recurse, val, options,
+ address, stream, recurse, val, options,
comma_needed, type, offset);
continue;
}
@@ -1057,7 +1060,7 @@ print_field_values (struct type *type, const gdb_byte *valaddr,
{
comma_needed =
print_variant_part (type, i, valaddr,
- offset, stream, recurse, val,
+ offset, address, stream, recurse, val,
options, comma_needed,
outer_type, outer_offset);
continue;
@@ -1111,7 +1114,7 @@ print_field_values (struct type *type, const gdb_byte *valaddr,
opts.deref_ref = 0;
val_print (TYPE_FIELD_TYPE (type, i),
value_contents_for_printing (v),
- value_embedded_offset (v), 0,
+ value_embedded_offset (v), address,
stream, recurse + 1, v,
&opts, current_language);
}
@@ -1125,7 +1128,7 @@ print_field_values (struct type *type, const gdb_byte *valaddr,
valaddr,
(offset
+ TYPE_FIELD_BITPOS (type, i) / HOST_CHAR_BIT),
- 0, stream, recurse + 1, val, &opts);
+ address, stream, recurse + 1, val, &opts);
}
annotate_field_end ();
}
^ permalink raw reply [flat|nested] 2+ messages in thread
* ping: [PATCH] Pass address around within ada-valprint.c
2013-07-30 10:40 [PATCH] Pass address around within ada-valprint.c Andrew Burgess
@ 2013-08-06 10:41 ` Andrew Burgess
0 siblings, 0 replies; 2+ messages in thread
From: Andrew Burgess @ 2013-08-06 10:41 UTC (permalink / raw)
To: gdb-patches
Any feedback on the following?
On 30/07/2013 11:40 AM, Andrew Burgess wrote:
> I was trying to better understand ada-lang.c:coerce_unspec_val_to_type,
> and after looking at the code for a while I tried experimentally
> applying this patch to make all values lazy:
>
> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index dc5f2b6..58c4ba1 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -568,15 +568,8 @@ coerce_unspec_val_to_type (struct value *val,
> struct type *
> trying to allocate some memory for it. */
> check_size (type);
>
> - if (value_lazy (val)
> - || TYPE_LENGTH (type) > TYPE_LENGTH (value_type (val)))
> - result = allocate_value_lazy (type);
> - else
> - {
> - result = allocate_value (type);
> - memcpy (value_contents_raw (result), value_contents (val),
> - TYPE_LENGTH (type));
> - }
> + result = allocate_value_lazy (type);
> +
> set_value_component_location (result, val);
> set_value_bitsize (result, value_bitsize (val));
> set_value_bitpos (result, value_bitpos (val));
>
>
> I know we don't want to actually apply this patch as it would
> cause gdb to fetch the value contents from the inferior in cases
> where we currently reuse the contents we already have to hand,
> BUT, as an experiment, I don't see why it should not be fine.
>
> Unfortunately I ran into a few regressions.... it turns out that
> in some cases the address parameter on VAL is not set-up correctly [1].
>
> This worries me, given the "|| TYPE_LENGTH (type) > TYPE_LENGTH
> (value_type (val))" check, would it not be possible to create an
> example where the address of VAL is unset, but the sizes of the types
> involved force the creation of a lazy value? If we could make such
> an example then it surely would fail.
>
> The patch below fixes all the regressions that the above experiment
> revealed, it's really just passing the address around inside
> ada-valprint.c a little more, seems like a good thing to me, but let
> me know what you think.
>
> OK to apply?
>
>
> Thanks,
> Andrew
>
> [1] Maybe there's a reason for this that I'm not seeing / understanding.
>
>
> 2013-07-30 Andrew Burgess <aburgess@broadcom.com>
>
> * ada-valprint.c (print_record): Add address parameter, and pass
> the address on to sub-functions.
> (print_field_values): Add address parameter, and pass the address
> on to sub-functions.
> (ada_val_print_1): Pass address parameter on to sub-functions.
> (print_variant_part): Pass address parameter on to sub-functions.
>
>
> diff --git a/gdb/ada-valprint.c b/gdb/ada-valprint.c
> index 4a04d28..6ec7764 100644
> --- a/gdb/ada-valprint.c
> +++ b/gdb/ada-valprint.c
> @@ -34,14 +34,15 @@
> #include "exceptions.h"
> #include "objfiles.h"
>
> -static void print_record (struct type *, const gdb_byte *, int,
> +static void print_record (struct type *, const gdb_byte *,
> + int, CORE_ADDR,
> struct ui_file *,
> int,
> const struct value *,
> const struct value_print_options *);
>
> static int print_field_values (struct type *, const gdb_byte *,
> - int,
> + int, CORE_ADDR,
> struct ui_file *, int,
> const struct value *,
> const struct value_print_options *,
> @@ -658,7 +659,7 @@ ada_val_print_1 (struct type *type, const gdb_byte
> *valaddr,
> struct value *mark = value_mark ();
> struct value *val;
>
> - val = value_from_contents_and_address (type, valaddr + offset,
> address);
> + val = value_from_contents_and_address (type, valaddr + offset,
> address + offset);
> /* If this is a reference, coerce it now. This helps taking care
> of the case where ADDRESS is meaningless because original_value
> was not an lval. */
> @@ -732,7 +733,7 @@ ada_val_print_1 (struct type *type, const gdb_byte
> *valaddr,
> nonsense value. Actually, we could use the same
> code regardless of lengths; I'm just avoiding a cast. */
> struct value *v1
> - = value_from_contents_and_address (type, valaddr + offset, 0);
> + = value_from_contents_and_address (type, valaddr + offset,
> address + offset);
> struct value *v = value_cast (target_type, v1);
>
> ada_val_print_1 (target_type,
> @@ -850,7 +851,7 @@ ada_val_print_1 (struct type *type, const gdb_byte
> *valaddr,
> }
> else
> {
> - print_record (type, valaddr, offset_aligned,
> + print_record (type, valaddr, offset_aligned, address,
> stream, recurse, original_value, options);
> return;
> }
> @@ -916,6 +917,7 @@ ada_val_print_1 (struct type *type, const gdb_byte
> *valaddr,
> static int
> print_variant_part (struct type *type, int field_num,
> const gdb_byte *valaddr, int offset,
> + CORE_ADDR address,
> struct ui_file *stream, int recurse,
> const struct value *val,
> const struct value_print_options *options,
> @@ -934,7 +936,7 @@ print_variant_part (struct type *type, int field_num,
> valaddr,
> offset + TYPE_FIELD_BITPOS (type, field_num) / HOST_CHAR_BIT
> + TYPE_FIELD_BITPOS (var_type, which) / HOST_CHAR_BIT,
> - stream, recurse, val, options,
> + address, stream, recurse, val, options,
> comma_needed, outer_type, outer_offset);
> }
>
> @@ -990,7 +992,7 @@ ada_value_print (struct value *val0, struct
> ui_file *stream,
>
> static void
> print_record (struct type *type, const gdb_byte *valaddr,
> - int offset,
> + int offset, CORE_ADDR address,
> struct ui_file *stream, int recurse,
> const struct value *val,
> const struct value_print_options *options)
> @@ -999,7 +1001,7 @@ print_record (struct type *type, const gdb_byte
> *valaddr,
>
> fprintf_filtered (stream, "(");
>
> - if (print_field_values (type, valaddr, offset,
> + if (print_field_values (type, valaddr, offset, address,
> stream, recurse, val, options,
> 0, type, offset) != 0 && options->prettyformat)
> {
> @@ -1027,7 +1029,8 @@ print_record (struct type *type, const gdb_byte
> *valaddr,
>
> static int
> print_field_values (struct type *type, const gdb_byte *valaddr,
> - int offset, struct ui_file *stream, int recurse,
> + int offset, CORE_ADDR address,
> + struct ui_file *stream, int recurse,
> const struct value *val,
> const struct value_print_options *options,
> int comma_needed,
> @@ -1049,7 +1052,7 @@ print_field_values (struct type *type, const
> gdb_byte *valaddr,
> valaddr,
> (offset
> + TYPE_FIELD_BITPOS (type, i) / HOST_CHAR_BIT),
> - stream, recurse, val, options,
> + address, stream, recurse, val, options,
> comma_needed, type, offset);
> continue;
> }
> @@ -1057,7 +1060,7 @@ print_field_values (struct type *type, const
> gdb_byte *valaddr,
> {
> comma_needed =
> print_variant_part (type, i, valaddr,
> - offset, stream, recurse, val,
> + offset, address, stream, recurse, val,
> options, comma_needed,
> outer_type, outer_offset);
> continue;
> @@ -1111,7 +1114,7 @@ print_field_values (struct type *type, const
> gdb_byte *valaddr,
> opts.deref_ref = 0;
> val_print (TYPE_FIELD_TYPE (type, i),
> value_contents_for_printing (v),
> - value_embedded_offset (v), 0,
> + value_embedded_offset (v), address,
> stream, recurse + 1, v,
> &opts, current_language);
> }
> @@ -1125,7 +1128,7 @@ print_field_values (struct type *type, const
> gdb_byte *valaddr,
> valaddr,
> (offset
> + TYPE_FIELD_BITPOS (type, i) / HOST_CHAR_BIT),
> - 0, stream, recurse + 1, val, &opts);
> + address, stream, recurse + 1, val, &opts);
> }
> annotate_field_end ();
> }
>
>
>
>
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2013-08-06 10:41 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-30 10:40 [PATCH] Pass address around within ada-valprint.c Andrew Burgess
2013-08-06 10:41 ` ping: " Andrew Burgess
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).