This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
ping: [PATCH] Pass address around within ada-valprint.c
- From: "Andrew Burgess" <aburgess at broadcom dot com>
- To: gdb-patches at sourceware dot org
- Date: Tue, 6 Aug 2013 11:40:50 +0100
- Subject: ping: [PATCH] Pass address around within ada-valprint.c
- References: <51F7981D dot 7010600 at broadcom dot com>
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 ();
> }
>
>
>
>
>