Re: [1/3] [PATCH] value_optimized_out and value_fetch_lazy

Andrew Burgess
2013-07-01T18:06:30Z

Sorry for the delay in replying.

On 19/06/2013 4:50 PM, Pedro Alves wrote:
> On 06/10/2013 11:24 AM, Andrew Burgess wrote:

>> I ran into an issue with gdb that appears to be caused by an incorrect
>> use of value_optimized_out.
>
> I'm finding the patch a bit hard to read though. Could you
> split it up? At least the move of value_fetch_lazy to value.c
> would be better done in its own. Pretty hard to reason
> about or even tell which were the value_fetch_lazy changes
> from the diff.
>

First stage moves value_fetch_lazy from valops.c to value.c.
The reason for the move is to allow access to the internals
of the value structure rather than having to call the
value_optimized_out function in later patches. At this
stage there should be no functional change though.

Ok to apply?

Thanks,
Andrew

gdb/ChangeLog

2013-07-01 Andrew Burgess <aburgess@broadcom.com>

* valops.c (value_fetch_lazy): Moved to value.c.
* value.c (value_fetch_lazy): Moved from valops.c.

diff --git a/gdb/valops.c b/gdb/valops.c
index 93c09d8..4161c2c 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -35,7 +35,6 @@
#include "dictionary.h"
#include "cp-support.h"
#include "dfp.h"
-#include "user-regs.h"
#include "tracepoint.h"
#include <errno.h>
#include "gdb_string.h"
@@ -947,167 +946,6 @@ value_at_lazy (struct type *type, CORE_ADDR addr)
return get_value_at (type, addr, 1);
}

-/* Called only from the value_contents and value_contents_all()
- macros, if the current data for a variable needs to be loaded into
- value_contents(VAL). Fetches the data from the user's process, and
- clears the lazy flag to indicate that the data in the buffer is
- valid.
-
- If the value is zero-length, we avoid calling read_memory, which
- would abort. We mark the value as fetched anyway -- all 0 bytes of
- it.
-
- This function returns a value because it is used in the
- value_contents macro as part of an expression, where a void would
- not work. The value is ignored. */
-
-int
-value_fetch_lazy (struct value *val)
-{
- gdb_assert (value_lazy (val));
- allocate_value_contents (val);
- if (value_bitsize (val))
- {
- /* To read a lazy bitfield, read the entire enclosing value. This
- prevents reading the same block of (possibly volatile) memory once
- per bitfield. It would be even better to read only the containing
- word, but we have no way to record that just specific bits of a
- value have been fetched. */
- struct type *type = check_typedef (value_type (val));
- enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
- struct value *parent = value_parent (val);
- LONGEST offset = value_offset (val);
- LONGEST num;
-
- if (!value_bits_valid (val,
- TARGET_CHAR_BIT * offset + value_bitpos (val),
- value_bitsize (val)))
- error (_("value has been optimized out"));
-
- if (!unpack_value_bits_as_long (value_type (val),
- value_contents_for_printing (parent),
- offset,
- value_bitpos (val),
- value_bitsize (val), parent, &num))
- mark_value_bytes_unavailable (val,
- value_embedded_offset (val),
- TYPE_LENGTH (type));
- else
- store_signed_integer (value_contents_raw (val), TYPE_LENGTH (type),
- byte_order, num);
- }
- else if (VALUE_LVAL (val) == lval_memory)
- {
- CORE_ADDR addr = value_address (val);
- struct type *type = check_typedef (value_enclosing_type (val));
-
- if (TYPE_LENGTH (type))
- read_value_memory (val, 0, value_stack (val),
- addr, value_contents_all_raw (val),
- TYPE_LENGTH (type));
- }
- else if (VALUE_LVAL (val) == lval_register)
- {
- struct frame_info *frame;
- int regnum;
- struct type *type = check_typedef (value_type (val));
- struct value *new_val = val, *mark = value_mark ();
-
- /* Offsets are not supported here; lazy register values must
- refer to the entire register. */
- gdb_assert (value_offset (val) == 0);
-
- while (VALUE_LVAL (new_val) == lval_register && value_lazy (new_val))
- {
- frame = frame_find_by_id (VALUE_FRAME_ID (new_val));
- regnum = VALUE_REGNUM (new_val);
-
- gdb_assert (frame != NULL);
-
- /* Convertible register routines are used for multi-register
- values and for interpretation in different types
- (e.g. float or int from a double register). Lazy
- register values should have the register's natural type,
- so they do not apply. */
- gdb_assert (!gdbarch_convert_register_p (get_frame_arch (frame),
- regnum, type));
-
- new_val = get_frame_register_value (frame, regnum);
- }
-
- /* If it's still lazy (for instance, a saved register on the
- stack), fetch it. */
- if (value_lazy (new_val))
- value_fetch_lazy (new_val);
-
- /* If the register was not saved, mark it optimized out. */
- if (value_optimized_out (new_val))
- set_value_optimized_out (val, 1);
- else
- {
- set_value_lazy (val, 0);
- value_contents_copy (val, value_embedded_offset (val),
- new_val, value_embedded_offset (new_val),
- TYPE_LENGTH (type));
- }
-
- if (frame_debug)
- {
- struct gdbarch *gdbarch;
- frame = frame_find_by_id (VALUE_FRAME_ID (val));
- regnum = VALUE_REGNUM (val);
- gdbarch = get_frame_arch (frame);
-
- fprintf_unfiltered (gdb_stdlog,
- "{ value_fetch_lazy "
- "(frame=%d,regnum=%d(%s),...) ",
- frame_relative_level (frame), regnum,
- user_reg_map_regnum_to_name (gdbarch, regnum));
-
- fprintf_unfiltered (gdb_stdlog, "->");
- if (value_optimized_out (new_val))
- fprintf_unfiltered (gdb_stdlog, " optimized out");
- else
- {
- int i;
- const gdb_byte *buf = value_contents (new_val);
-
- if (VALUE_LVAL (new_val) == lval_register)
- fprintf_unfiltered (gdb_stdlog, " register=%d",
- VALUE_REGNUM (new_val));
- else if (VALUE_LVAL (new_val) == lval_memory)
- fprintf_unfiltered (gdb_stdlog, " address=%s",
- paddress (gdbarch,
- value_address (new_val)));
- else
- fprintf_unfiltered (gdb_stdlog, " computed");
-
- fprintf_unfiltered (gdb_stdlog, " bytes=");
- fprintf_unfiltered (gdb_stdlog, "[");
- for (i = 0; i < register_size (gdbarch, regnum); i++)
- fprintf_unfiltered (gdb_stdlog, "%02x", buf[i]);
- fprintf_unfiltered (gdb_stdlog, "]");
- }
-
- fprintf_unfiltered (gdb_stdlog, " }\n");
- }
-
- /* Dispose of the intermediate values. This prevents
- watchpoints from trying to watch the saved frame pointer. */
- value_free_to_mark (mark);
- }
- else if (VALUE_LVAL (val) == lval_computed
- && value_computed_funcs (val)->read != NULL)
- value_computed_funcs (val)->read (val);
- else if (value_optimized_out (val))
- /* Keep it optimized out. */;
- else
- internal_error (__FILE__, __LINE__, _("Unexpected lazy value type."));
-
- set_value_lazy (val, 0);
- return 0;
-}
-
void
read_value_memory (struct value *val, int embedded_offset,
int stack, CORE_ADDR memaddr,
diff --git a/gdb/value.c b/gdb/value.c
index fae8b98..8547590 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -42,6 +42,7 @@
#include <ctype.h>
#include "tracepoint.h"
#include "cp-abi.h"
+#include "user-regs.h"

/* Prototypes for exported functions. */

@@ -3373,6 +3374,167 @@ value_initialized (struct value *val)
return val->initialized;
}

+/* Called only from the value_contents and value_contents_all()
+ macros, if the current data for a variable needs to be loaded into
+ value_contents(VAL). Fetches the data from the user's process, and
+ clears the lazy flag to indicate that the data in the buffer is
+ valid.
+
+ If the value is zero-length, we avoid calling read_memory, which
+ would abort. We mark the value as fetched anyway -- all 0 bytes of
+ it.
+
+ This function returns a value because it is used in the
+ value_contents macro as part of an expression, where a void would
+ not work. The value is ignored. */
+
+int
+value_fetch_lazy (struct value *val)
+{
+ gdb_assert (value_lazy (val));
+ allocate_value_contents (val);
+ if (value_bitsize (val))
+ {
+ /* To read a lazy bitfield, read the entire enclosing value. This
+ prevents reading the same block of (possibly volatile) memory once
+ per bitfield. It would be even better to read only the containing
+ word, but we have no way to record that just specific bits of a
+ value have been fetched. */
+ struct type *type = check_typedef (value_type (val));
+ enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
+ struct value *parent = value_parent (val);
+ LONGEST offset = value_offset (val);
+ LONGEST num;
+
+ if (!value_bits_valid (val,
+ TARGET_CHAR_BIT * offset + value_bitpos (val),
+ value_bitsize (val)))
+ error (_("value has been optimized out"));
+
+ if (!unpack_value_bits_as_long (value_type (val),
+ value_contents_for_printing (parent),
+ offset,
+ value_bitpos (val),
+ value_bitsize (val), parent, &num))
+ mark_value_bytes_unavailable (val,
+ value_embedded_offset (val),
+ TYPE_LENGTH (type));
+ else
+ store_signed_integer (value_contents_raw (val), TYPE_LENGTH (type),
+ byte_order, num);
+ }
+ else if (VALUE_LVAL (val) == lval_memory)
+ {
+ CORE_ADDR addr = value_address (val);
+ struct type *type = check_typedef (value_enclosing_type (val));
+
+ if (TYPE_LENGTH (type))
+ read_value_memory (val, 0, value_stack (val),
+ addr, value_contents_all_raw (val),
+ TYPE_LENGTH (type));
+ }
+ else if (VALUE_LVAL (val) == lval_register)
+ {
+ struct frame_info *frame;
+ int regnum;
+ struct type *type = check_typedef (value_type (val));
+ struct value *new_val = val, *mark = value_mark ();
+
+ /* Offsets are not supported here; lazy register values must
+ refer to the entire register. */
+ gdb_assert (value_offset (val) == 0);
+
+ while (VALUE_LVAL (new_val) == lval_register && value_lazy (new_val))
+ {
+ frame = frame_find_by_id (VALUE_FRAME_ID (new_val));
+ regnum = VALUE_REGNUM (new_val);
+
+ gdb_assert (frame != NULL);
+
+ /* Convertible register routines are used for multi-register
+ values and for interpretation in different types
+ (e.g. float or int from a double register). Lazy
+ register values should have the register's natural type,
+ so they do not apply. */
+ gdb_assert (!gdbarch_convert_register_p (get_frame_arch (frame),
+ regnum, type));
+
+ new_val = get_frame_register_value (frame, regnum);
+ }
+
+ /* If it's still lazy (for instance, a saved register on the
+ stack), fetch it. */
+ if (value_lazy (new_val))
+ value_fetch_lazy (new_val);
+
+ /* If the register was not saved, mark it optimized out. */
+ if (value_optimized_out (new_val))
+ set_value_optimized_out (val, 1);
+ else
+ {
+ set_value_lazy (val, 0);
+ value_contents_copy (val, value_embedded_offset (val),
+ new_val, value_embedded_offset (new_val),
+ TYPE_LENGTH (type));
+ }
+
+ if (frame_debug)
+ {
+ struct gdbarch *gdbarch;
+ frame = frame_find_by_id (VALUE_FRAME_ID (val));
+ regnum = VALUE_REGNUM (val);
+ gdbarch = get_frame_arch (frame);
+
+ fprintf_unfiltered (gdb_stdlog,
+ "{ value_fetch_lazy "
+ "(frame=%d,regnum=%d(%s),...) ",
+ frame_relative_level (frame), regnum,
+ user_reg_map_regnum_to_name (gdbarch, regnum));
+
+ fprintf_unfiltered (gdb_stdlog, "->");
+ if (value_optimized_out (new_val))
+ fprintf_unfiltered (gdb_stdlog, " optimized out");
+ else
+ {
+ int i;
+ const gdb_byte *buf = value_contents (new_val);
+
+ if (VALUE_LVAL (new_val) == lval_register)
+ fprintf_unfiltered (gdb_stdlog, " register=%d",
+ VALUE_REGNUM (new_val));
+ else if (VALUE_LVAL (new_val) == lval_memory)
+ fprintf_unfiltered (gdb_stdlog, " address=%s",
+ paddress (gdbarch,
+ value_address (new_val)));
+ else
+ fprintf_unfiltered (gdb_stdlog, " computed");
+
+ fprintf_unfiltered (gdb_stdlog, " bytes=");
+ fprintf_unfiltered (gdb_stdlog, "[");
+ for (i = 0; i < register_size (gdbarch, regnum); i++)
+ fprintf_unfiltered (gdb_stdlog, "%02x", buf[i]);
+ fprintf_unfiltered (gdb_stdlog, "]");
+ }
+
+ fprintf_unfiltered (gdb_stdlog, " }\n");
+ }
+
+ /* Dispose of the intermediate values. This prevents
+ watchpoints from trying to watch the saved frame pointer. */
+ value_free_to_mark (mark);
+ }
+ else if (VALUE_LVAL (val) == lval_computed
+ && value_computed_funcs (val)->read != NULL)
+ value_computed_funcs (val)->read (val);
+ else if (value_optimized_out (val))
+ /* Keep it optimized out. */;
+ else
+ internal_error (__FILE__, __LINE__, _("Unexpected lazy value type."));
+
+ set_value_lazy (val, 0);
+ return 0;
+}
+
void
_initialize_values (void)
{