* [PATCH] value_optimized_out and value_fetch_lazy
@ 2013-06-10 12:46 Andrew Burgess
2013-06-19 15:55 ` Pedro Alves
0 siblings, 1 reply; 14+ messages in thread
From: Andrew Burgess @ 2013-06-10 12:46 UTC (permalink / raw)
To: gdb-patches
Hi,
I ran into an issue with gdb that appears to be caused by an incorrect
use of value_optimized_out.
Currently, if value_optimized_out returns true, this means we know the
value is optimized out, if value_optimized_out returns false this
means the value might, or might not, be optimized out; if the value
is currently lazy then we don't know it has been optimized out, and
so the answer from value_optimized_out is false.
I believe that currently, not every caller of value_optimized_out
follows these rules, and some cases if value_optimized_out returns
false, they treat the value as though it is NOT optimized out.
The patch below changes value_optimized_out so that if the value
is lazy it will be loaded, this should ensure that by the time
value_optimized_out returns, the value will be correct. I've fixed
the one place I could see where we were obviously taking care to
resolve lazy values and I've also made a few tweaks to some of the
other value code in order to prevent recursion while loading lazy
values, and to try and keep values lazy as much as possible.
Feedback welcome, or ok to commit?
Thanks,
Andrew
gdb/ChangeLog
2013-06-07 Andrew Burgess <aburgess@broadcom.com>
* stack.c (read_frame_arg): No longer need to fetch lazy values,
checking for optimized out will ensure lazy values are loaded.
* valops.c (value_fetch_lazy): Moved to value.c.
* value.c (value_fetch_lazy): Moved from valops.c, also use
optimized out flag rather than calling optimized_out method, to
avoid recursion.
(value_optimized_out): If the value is not already marked
optimized out, and is lazy then fetch it so we can know for sure
if the value is optimized out.
(allocate_optimized_out_value): If we create a value that we know
is optimized out, then mark the value as no longer lazy.
(value_primitive_field): Move optimized out check later to later
in the function after we have loaded any lazy values.
gdb/testsuite/ChangeLog
2013-06-07 Andrew Burgess <aburgess@broadcom.com>
* gdb.dwarf2/dw2-reg-undefined.exp: New file.
* gdb.dwarf2/dw2-reg-undefined.c: Likewise.
* gdb.dwarf2/dw2-reg-undefined.S: Likewise.
diff --git a/gdb/stack.c b/gdb/stack.c
index a4b392e..e844331 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -359,10 +359,6 @@ read_frame_arg (struct symbol *sym, struct frame_info *frame,
{
struct type *type = value_type (val);
- if (!value_optimized_out (val) && value_lazy (val))
- value_fetch_lazy (val);
- if (!value_optimized_out (val) && value_lazy (entryval))
- value_fetch_lazy (entryval);
if (!value_optimized_out (val)
&& value_available_contents_eq (val, 0, entryval, 0,
TYPE_LENGTH (type)))
diff --git a/gdb/valops.c b/gdb/valops.c
index 93c09d8..787b799 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -947,167 +947,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 ee3c998..fae21d6 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. */
@@ -743,6 +744,8 @@ allocate_optimized_out_value (struct type *type)
{
struct value *retval = allocate_value_lazy (type);
+ /* It's no longer lazy if we know it's optimized out. */
+ set_value_lazy (retval, 0);
set_value_optimized_out (retval, 1);
return retval;
@@ -1055,6 +1058,9 @@ value_contents_equal (struct value *val1, struct value *val2)
int
value_optimized_out (struct value *value)
{
+ if (!value->optimized_out && value->lazy)
+ value_fetch_lazy (value);
+
return value->optimized_out;
}
@@ -2629,9 +2635,7 @@ value_primitive_field (struct value *arg1, int offset,
description correctly. */
check_typedef (type);
- if (value_optimized_out (arg1))
- v = allocate_optimized_out_value (type);
- else if (TYPE_FIELD_BITSIZE (arg_type, fieldno))
+ if (TYPE_FIELD_BITSIZE (arg_type, fieldno))
{
/* Handle packed fields.
@@ -2645,19 +2649,24 @@ value_primitive_field (struct value *arg1, int offset,
int bitpos = TYPE_FIELD_BITPOS (arg_type, fieldno);
int container_bitsize = TYPE_LENGTH (type) * 8;
- v = allocate_value_lazy (type);
- v->bitsize = TYPE_FIELD_BITSIZE (arg_type, fieldno);
- if ((bitpos % container_bitsize) + v->bitsize <= container_bitsize
- && TYPE_LENGTH (type) <= (int) sizeof (LONGEST))
- v->bitpos = bitpos % container_bitsize;
+ if (arg1->optimized_out)
+ v = allocate_optimized_out_value (type);
else
- v->bitpos = bitpos % 8;
- v->offset = (value_embedded_offset (arg1)
- + offset
- + (bitpos - v->bitpos) / 8);
- set_value_parent (v, arg1);
- if (!value_lazy (arg1))
- value_fetch_lazy (v);
+ {
+ v = allocate_value_lazy (type);
+ v->bitsize = TYPE_FIELD_BITSIZE (arg_type, fieldno);
+ if ((bitpos % container_bitsize) + v->bitsize <= container_bitsize
+ && TYPE_LENGTH (type) <= (int) sizeof (LONGEST))
+ v->bitpos = bitpos % container_bitsize;
+ else
+ v->bitpos = bitpos % 8;
+ v->offset = (value_embedded_offset (arg1)
+ + offset
+ + (bitpos - v->bitpos) / 8);
+ set_value_parent (v, arg1);
+ if (!value_lazy (arg1))
+ value_fetch_lazy (v);
+ }
}
else if (fieldno < TYPE_N_BASECLASSES (arg_type))
{
@@ -2670,29 +2679,34 @@ value_primitive_field (struct value *arg1, int offset,
if (VALUE_LVAL (arg1) == lval_register && value_lazy (arg1))
value_fetch_lazy (arg1);
- /* We special case virtual inheritance here because this
- requires access to the contents, which we would rather avoid
- for references to ordinary fields of unavailable values. */
- if (BASETYPE_VIA_VIRTUAL (arg_type, fieldno))
- boffset = baseclass_offset (arg_type, fieldno,
- value_contents (arg1),
- value_embedded_offset (arg1),
- value_address (arg1),
- arg1);
- else
- boffset = TYPE_FIELD_BITPOS (arg_type, fieldno) / 8;
-
- if (value_lazy (arg1))
- v = allocate_value_lazy (value_enclosing_type (arg1));
+ if (arg1->optimized_out)
+ v = allocate_optimized_out_value (type);
else
{
- v = allocate_value (value_enclosing_type (arg1));
- value_contents_copy_raw (v, 0, arg1, 0,
- TYPE_LENGTH (value_enclosing_type (arg1)));
+ /* We special case virtual inheritance here because this
+ requires access to the contents, which we would rather avoid
+ for references to ordinary fields of unavailable values. */
+ if (BASETYPE_VIA_VIRTUAL (arg_type, fieldno))
+ boffset = baseclass_offset (arg_type, fieldno,
+ value_contents (arg1),
+ value_embedded_offset (arg1),
+ value_address (arg1),
+ arg1);
+ else
+ boffset = TYPE_FIELD_BITPOS (arg_type, fieldno) / 8;
+
+ if (value_lazy (arg1))
+ v = allocate_value_lazy (value_enclosing_type (arg1));
+ else
+ {
+ v = allocate_value (value_enclosing_type (arg1));
+ value_contents_copy_raw (v, 0, arg1, 0,
+ TYPE_LENGTH (value_enclosing_type (arg1)));
+ }
+ v->type = type;
+ v->offset = value_offset (arg1);
+ v->embedded_offset = offset + value_embedded_offset (arg1) + boffset;
}
- v->type = type;
- v->offset = value_offset (arg1);
- v->embedded_offset = offset + value_embedded_offset (arg1) + boffset;
}
else
{
@@ -2703,7 +2717,9 @@ value_primitive_field (struct value *arg1, int offset,
if (VALUE_LVAL (arg1) == lval_register && value_lazy (arg1))
value_fetch_lazy (arg1);
- if (value_lazy (arg1))
+ if (arg1->optimized_out)
+ v = allocate_optimized_out_value (type);
+ else if (value_lazy (arg1))
v = allocate_value_lazy (type);
else
{
@@ -3375,6 +3391,168 @@ 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 (val->optimized_out)
+ /* Keep it optimized out. */;
+ else
+ internal_error (__FILE__, __LINE__, _("Unexpected lazy value type."));
+
+ set_value_lazy (val, 0);
+ return 0;
+}
+
void
_initialize_values (void)
{
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.S b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.S
new file mode 100644
index 0000000..54b5af1
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.S
@@ -0,0 +1,505 @@
+ /* The FDE entry for "stop_frame" in the .debug_frame section has
+ been hand modified to mark a set of registers as undefined.
+ Otherwise this file is as generated by gcc 4.7.2 for x86_64. */
+ .file "dw2-reg-undefined.c"
+ .text
+.Ltext0:
+ .globl stop_frame
+ .type stop_frame, @function
+stop_frame:
+.LFB0:
+ .file 1 "dw2-reg-undefined.c"
+ .loc 1 3 0
+ pushq %rbp
+.LCFI0:
+ movq %rsp, %rbp
+.LCFI1:
+ .loc 1 6 0
+ popq %rbp
+.LCFI2:
+ ret
+.LFE0:
+ .size stop_frame, .-stop_frame
+ .globl first_frame
+ .type first_frame, @function
+first_frame:
+.LFB1:
+ .loc 1 10 0
+ pushq %rbp
+.LCFI3:
+ movq %rsp, %rbp
+.LCFI4:
+ .loc 1 11 0
+ movl $0, %eax
+ call stop_frame
+ .loc 1 12 0
+ popq %rbp
+.LCFI5:
+ ret
+.LFE1:
+ .size first_frame, .-first_frame
+ .globl main
+ .type main, @function
+main:
+.LFB2:
+ .loc 1 16 0
+ pushq %rbp
+.LCFI6:
+ movq %rsp, %rbp
+.LCFI7:
+ .loc 1 17 0
+ movl $0, %eax
+ call first_frame
+ .loc 1 19 0
+ movl $0, %eax
+ .loc 1 20 0
+ popq %rbp
+.LCFI8:
+ ret
+.LFE2:
+ .size main, .-main
+ .section .debug_frame,"",@progbits
+.Lframe0:
+ .long .LECIE0-.LSCIE0
+.LSCIE0:
+ .long 0xffffffff
+ .byte 0x1
+ .string ""
+ .uleb128 0x1
+ .sleb128 -8
+ .byte 0x10
+ .byte 0xc
+ .uleb128 0x7
+ .uleb128 0x8
+ .byte 0x90
+ .uleb128 0x1
+ .align 8
+.LECIE0:
+ /* This FDE entry, for stop_frame was modified to mark
+ registers 0 -> 6 as being undefined. */
+.LSFDE0:
+ .long .LEFDE0-.LASFDE0 /* FDE length (uword) */
+.LASFDE0:
+ .long .Lframe0 /* CIE Pointer (uword) */
+ .quad .LFB0 /* Initial location (addr) */
+ .quad .LFE0-.LFB0 /* Address range (addr) */
+ /* Instructions... */
+ .byte 0x7 /* DW_CFA_undefined */
+ .uleb128 0x0 /* ULEB128 register */
+ .byte 0x7 /* DW_CFA_undefined */
+ .uleb128 0x1 /* ULEB128 register */
+ .byte 0x7 /* DW_CFA_undefined */
+ .uleb128 0x2 /* ULEB128 register */
+ .byte 0x7 /* DW_CFA_undefined */
+ .uleb128 0x3 /* ULEB128 register */
+ .byte 0x7 /* DW_CFA_undefined */
+ .uleb128 0x4 /* ULEB128 register */
+ .byte 0x7 /* DW_CFA_undefined */
+ .uleb128 0x5 /* ULEB128 register */
+ .byte 0x7 /* DW_CFA_undefined */
+ .uleb128 0x6 /* ULEB128 register */
+ .byte 0x7 /* DW_CFA_undefined */
+ .uleb128 0x7 /* ULEB128 register */
+ .byte 0x4 /* DW_CFA_advance_loc4 */
+ .long .LCFI0-.LFB0 /* 4-byte offset */
+ .byte 0xe /* DW_CFA_def_cfa_offset */
+ .uleb128 0x10 /* ULEB128 offset */
+ .byte 0x86 /* DW_CFA_offset (reg = 6) */
+ .uleb128 0x2 /* ULEB128 offset */
+ .byte 0x4 /* DW_CFA_advance_loc4 */
+ .long .LCFI1-.LCFI0 /* 4-byte offset */
+ .byte 0xd /* DW_CFA_def_cfa_register */
+ .uleb128 0x6 /* ULEB128 register */
+ .byte 0x4 /* DW_CFA_advance_loc4 */
+ .long .LCFI2-.LCFI1 /* 4-byte offset */
+ .byte 0xc /* DW_CFA_def_cfa */
+ .uleb128 0x7 /* ULEB128 register */
+ .uleb128 0x8 /* ULEB128 offset */
+
+ .align 8
+ /* End of modified frame. */
+.LEFDE0:
+.LSFDE2:
+ .long .LEFDE2-.LASFDE2
+.LASFDE2:
+ .long .Lframe0
+ .quad .LFB1
+ .quad .LFE1-.LFB1
+ .byte 0x4
+ .long .LCFI3-.LFB1
+ .byte 0xe
+ .uleb128 0x10
+ .byte 0x86
+ .uleb128 0x2
+ .byte 0x4
+ .long .LCFI4-.LCFI3
+ .byte 0xd
+ .uleb128 0x6
+ .byte 0x4
+ .long .LCFI5-.LCFI4
+ .byte 0xc
+ .uleb128 0x7
+ .uleb128 0x8
+ .align 8
+.LEFDE2:
+.LSFDE4:
+ .long .LEFDE4-.LASFDE4
+.LASFDE4:
+ .long .Lframe0
+ .quad .LFB2
+ .quad .LFE2-.LFB2
+ .byte 0x4
+ .long .LCFI6-.LFB2
+ .byte 0xe
+ .uleb128 0x10
+ .byte 0x86
+ .uleb128 0x2
+ .byte 0x4
+ .long .LCFI7-.LCFI6
+ .byte 0xd
+ .uleb128 0x6
+ .byte 0x4
+ .long .LCFI8-.LCFI7
+ .byte 0xc
+ .uleb128 0x7
+ .uleb128 0x8
+ .align 8
+.LEFDE4:
+ .section .eh_frame,"a",@progbits
+.Lframe1:
+ .long .LECIE1-.LSCIE1
+.LSCIE1:
+ .long 0
+ .byte 0x1
+ .string "zR"
+ .uleb128 0x1
+ .sleb128 -8
+ .byte 0x10
+ .uleb128 0x1
+ .byte 0x3
+ .byte 0xc
+ .uleb128 0x7
+ .uleb128 0x8
+ .byte 0x90
+ .uleb128 0x1
+ .align 8
+.LECIE1:
+.LSFDE7:
+ .long .LEFDE7-.LASFDE7
+.LASFDE7:
+ .long .LASFDE7-.Lframe1
+ .long .LFB0
+ .long .LFE0-.LFB0
+ .uleb128 0
+ .byte 0x4
+ .long .LCFI0-.LFB0
+ .byte 0xe
+ .uleb128 0x10
+ .byte 0x86
+ .uleb128 0x2
+ .byte 0x4
+ .long .LCFI1-.LCFI0
+ .byte 0xd
+ .uleb128 0x6
+ .byte 0x4
+ .long .LCFI2-.LCFI1
+ .byte 0xc
+ .uleb128 0x7
+ .uleb128 0x8
+ .align 8
+.LEFDE7:
+.LSFDE9:
+ .long .LEFDE9-.LASFDE9
+.LASFDE9:
+ .long .LASFDE9-.Lframe1
+ .long .LFB1
+ .long .LFE1-.LFB1
+ .uleb128 0
+ .byte 0x4
+ .long .LCFI3-.LFB1
+ .byte 0xe
+ .uleb128 0x10
+ .byte 0x86
+ .uleb128 0x2
+ .byte 0x4
+ .long .LCFI4-.LCFI3
+ .byte 0xd
+ .uleb128 0x6
+ .byte 0x4
+ .long .LCFI5-.LCFI4
+ .byte 0xc
+ .uleb128 0x7
+ .uleb128 0x8
+ .align 8
+.LEFDE9:
+.LSFDE11:
+ .long .LEFDE11-.LASFDE11
+.LASFDE11:
+ .long .LASFDE11-.Lframe1
+ .long .LFB2
+ .long .LFE2-.LFB2
+ .uleb128 0
+ .byte 0x4
+ .long .LCFI6-.LFB2
+ .byte 0xe
+ .uleb128 0x10
+ .byte 0x86
+ .uleb128 0x2
+ .byte 0x4
+ .long .LCFI7-.LCFI6
+ .byte 0xd
+ .uleb128 0x6
+ .byte 0x4
+ .long .LCFI8-.LCFI7
+ .byte 0xc
+ .uleb128 0x7
+ .uleb128 0x8
+ .align 8
+.LEFDE11:
+ .text
+.Letext0:
+ .section .debug_info,"",@progbits
+.Ldebug_info0:
+ .long 0x8c
+ .value 0x2
+ .long .Ldebug_abbrev0
+ .byte 0x8
+ .uleb128 0x1
+ .long .LASF2
+ .byte 0x1
+ .long .LASF3
+ .long .LASF4
+ .quad .Ltext0
+ .quad .Letext0
+ .long .Ldebug_line0
+ .uleb128 0x2
+ .byte 0x1
+ .long .LASF0
+ .byte 0x1
+ .byte 0x2
+ .quad .LFB0
+ .quad .LFE0
+ .long .LLST0
+ .byte 0x1
+ .uleb128 0x3
+ .byte 0x1
+ .long .LASF1
+ .byte 0x1
+ .byte 0x9
+ .quad .LFB1
+ .quad .LFE1
+ .long .LLST1
+ .byte 0x1
+ .uleb128 0x4
+ .byte 0x1
+ .long .LASF5
+ .byte 0x1
+ .byte 0xf
+ .long 0x88
+ .quad .LFB2
+ .quad .LFE2
+ .long .LLST2
+ .byte 0x1
+ .uleb128 0x5
+ .byte 0x4
+ .byte 0x5
+ .string "int"
+ .byte 0
+ .section .debug_abbrev,"",@progbits
+.Ldebug_abbrev0:
+ .uleb128 0x1
+ .uleb128 0x11
+ .byte 0x1
+ .uleb128 0x25
+ .uleb128 0xe
+ .uleb128 0x13
+ .uleb128 0xb
+ .uleb128 0x3
+ .uleb128 0xe
+ .uleb128 0x1b
+ .uleb128 0xe
+ .uleb128 0x11
+ .uleb128 0x1
+ .uleb128 0x12
+ .uleb128 0x1
+ .uleb128 0x10
+ .uleb128 0x6
+ .byte 0
+ .byte 0
+ .uleb128 0x2
+ .uleb128 0x2e
+ .byte 0
+ .uleb128 0x3f
+ .uleb128 0xc
+ .uleb128 0x3
+ .uleb128 0xe
+ .uleb128 0x3a
+ .uleb128 0xb
+ .uleb128 0x3b
+ .uleb128 0xb
+ .uleb128 0x11
+ .uleb128 0x1
+ .uleb128 0x12
+ .uleb128 0x1
+ .uleb128 0x40
+ .uleb128 0x6
+ .uleb128 0x2117
+ .uleb128 0xc
+ .byte 0
+ .byte 0
+ .uleb128 0x3
+ .uleb128 0x2e
+ .byte 0
+ .uleb128 0x3f
+ .uleb128 0xc
+ .uleb128 0x3
+ .uleb128 0xe
+ .uleb128 0x3a
+ .uleb128 0xb
+ .uleb128 0x3b
+ .uleb128 0xb
+ .uleb128 0x11
+ .uleb128 0x1
+ .uleb128 0x12
+ .uleb128 0x1
+ .uleb128 0x40
+ .uleb128 0x6
+ .uleb128 0x2116
+ .uleb128 0xc
+ .byte 0
+ .byte 0
+ .uleb128 0x4
+ .uleb128 0x2e
+ .byte 0
+ .uleb128 0x3f
+ .uleb128 0xc
+ .uleb128 0x3
+ .uleb128 0xe
+ .uleb128 0x3a
+ .uleb128 0xb
+ .uleb128 0x3b
+ .uleb128 0xb
+ .uleb128 0x49
+ .uleb128 0x13
+ .uleb128 0x11
+ .uleb128 0x1
+ .uleb128 0x12
+ .uleb128 0x1
+ .uleb128 0x40
+ .uleb128 0x6
+ .uleb128 0x2116
+ .uleb128 0xc
+ .byte 0
+ .byte 0
+ .uleb128 0x5
+ .uleb128 0x24
+ .byte 0
+ .uleb128 0xb
+ .uleb128 0xb
+ .uleb128 0x3e
+ .uleb128 0xb
+ .uleb128 0x3
+ .uleb128 0x8
+ .byte 0
+ .byte 0
+ .byte 0
+ .section .debug_loc,"",@progbits
+.Ldebug_loc0:
+.LLST0:
+ .quad .LFB0-.Ltext0
+ .quad .LCFI0-.Ltext0
+ .value 0x2
+ .byte 0x77
+ .sleb128 8
+ .quad .LCFI0-.Ltext0
+ .quad .LCFI1-.Ltext0
+ .value 0x2
+ .byte 0x77
+ .sleb128 16
+ .quad .LCFI1-.Ltext0
+ .quad .LCFI2-.Ltext0
+ .value 0x2
+ .byte 0x76
+ .sleb128 16
+ .quad .LCFI2-.Ltext0
+ .quad .LFE0-.Ltext0
+ .value 0x2
+ .byte 0x77
+ .sleb128 8
+ .quad 0
+ .quad 0
+.LLST1:
+ .quad .LFB1-.Ltext0
+ .quad .LCFI3-.Ltext0
+ .value 0x2
+ .byte 0x77
+ .sleb128 8
+ .quad .LCFI3-.Ltext0
+ .quad .LCFI4-.Ltext0
+ .value 0x2
+ .byte 0x77
+ .sleb128 16
+ .quad .LCFI4-.Ltext0
+ .quad .LCFI5-.Ltext0
+ .value 0x2
+ .byte 0x76
+ .sleb128 16
+ .quad .LCFI5-.Ltext0
+ .quad .LFE1-.Ltext0
+ .value 0x2
+ .byte 0x77
+ .sleb128 8
+ .quad 0
+ .quad 0
+.LLST2:
+ .quad .LFB2-.Ltext0
+ .quad .LCFI6-.Ltext0
+ .value 0x2
+ .byte 0x77
+ .sleb128 8
+ .quad .LCFI6-.Ltext0
+ .quad .LCFI7-.Ltext0
+ .value 0x2
+ .byte 0x77
+ .sleb128 16
+ .quad .LCFI7-.Ltext0
+ .quad .LCFI8-.Ltext0
+ .value 0x2
+ .byte 0x76
+ .sleb128 16
+ .quad .LCFI8-.Ltext0
+ .quad .LFE2-.Ltext0
+ .value 0x2
+ .byte 0x77
+ .sleb128 8
+ .quad 0
+ .quad 0
+ .section .debug_aranges,"",@progbits
+ .long 0x2c
+ .value 0x2
+ .long .Ldebug_info0
+ .byte 0x8
+ .byte 0
+ .value 0
+ .value 0
+ .quad .Ltext0
+ .quad .Letext0-.Ltext0
+ .quad 0
+ .quad 0
+ .section .debug_line,"",@progbits
+.Ldebug_line0:
+ .section .debug_str,"MS",@progbits,1
+.LASF0:
+ .string "stop_frame"
+.LASF3:
+ .string "dw2-reg-undefined.c"
+.LASF2:
+ .string "GNU C 4.7.2"
+.LASF4:
+ .string "/projects/firepath_work/aburgess/tmp"
+.LASF1:
+ .string "first_frame"
+.LASF5:
+ .string "main"
+ .ident "GCC: (GNU) 4.7.2"
+ .section .note.GNU-stack,"",@progbits
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.c b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.c
new file mode 100644
index 0000000..2101ff9
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.c
@@ -0,0 +1,20 @@
+void
+stop_frame ()
+{
+ /* The debug information for this frame is modified in the accompanying
+ .S file, to mark a set of registers as being undefined. */
+}
+
+void
+first_frame ()
+{
+ stop_frame ();
+}
+
+int
+main ()
+{
+ first_frame ();
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
new file mode 100644
index 0000000..468cd21
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
@@ -0,0 +1,60 @@
+# Copyright 2008-2013 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/>.
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+ return 0
+}
+
+# This test can only be run on x86_64 targets.
+if {![istarget "x86_64-*-*"] || ![is_lp64_target]} {
+ return 0
+}
+
+standard_testfile .S
+
+if { [prepare_for_testing $testfile.exp $testfile $srcfile {nodebug}] } {
+ return -1
+}
+
+if ![runto stop_frame] {
+ perror "Failed to stop in stop_frame"
+ return -1
+}
+
+gdb_test "bt" "#0 (0x\[0-9a-f\]+ in )?stop_frame \[^\r\n\]*\r\n#1 \[^\r\n\]*first_frame \[^\r\n\]*\r\n#2 \[^\r\n\]*main\[^\r\n\]*" \
+ "backtrace from stop_frame"
+
+set value_pattern "0x\[0-9a-f\]+\\s+\[0-9\]+"
+set opt_out_pattern "\\*value not available\\*"
+
+for {set f 0} {$f < 3} {incr f} {
+ if {${f} == 0} {
+ set pattern_rax_rbx_rcx ${value_pattern}
+ set pattern_r8_r9 ${value_pattern}
+ } else {
+ set pattern_rax_rbx_rcx ${opt_out_pattern}
+ set pattern_r8_r9 ${value_pattern}
+ }
+
+ # Select frame.
+ gdb_test "frame ${f}" "#${f}.*" "Switch to frame ${f}"
+
+ # Display register values.
+ gdb_test "info registers rax rbx rcx r8 r9" "rax\\s+${pattern_rax_rbx_rcx}\\s*\r\nrbx\\s+${pattern_rax_rbx_rcx}\\s*\r\nrcx\\s+${pattern_rax_rbx_rcx}\\s*\r\nr8\\s+${pattern_r8_r9}\\s*\r\nr9\\s+${pattern_r8_r9}\\s*" \
+ "Check values of rax, rbx, rcx, r8, r9 in frame ${f}"
+}
+
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] value_optimized_out and value_fetch_lazy
2013-06-10 12:46 [PATCH] value_optimized_out and value_fetch_lazy Andrew Burgess
@ 2013-06-19 15:55 ` Pedro Alves
2013-07-01 18:06 ` [3/3] " Andrew Burgess
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Pedro Alves @ 2013-06-19 15:55 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
On 06/10/2013 11:24 AM, Andrew Burgess wrote:
> Hi,
>
> I ran into an issue with gdb that appears to be caused by an incorrect
> use of value_optimized_out.
>
> Currently, if value_optimized_out returns true, this means we know the
> value is optimized out, if value_optimized_out returns false this
> means the value might, or might not, be optimized out; if the value
> is currently lazy then we don't know it has been optimized out, and
> so the answer from value_optimized_out is false.
Looks like the main culprit would be frame_register_unwind.
>
> I believe that currently, not every caller of value_optimized_out
> follows these rules, and some cases if value_optimized_out returns
> false, they treat the value as though it is NOT optimized out.
>
> The patch below changes value_optimized_out so that if the value
> is lazy it will be loaded, this should ensure that by the time
> value_optimized_out returns, the value will be correct. I've fixed
> the one place I could see where we were obviously taking care to
> resolve lazy values and I've also made a few tweaks to some of the
> other value code in order to prevent recursion while loading lazy
> values, and to try and keep values lazy as much as possible.
>
> Feedback welcome, or ok to commit?
I agree with this. It might be possible to factor out the bits
of value_fetch_lazy that handle optimized out values, and use it
from value_optimized_out, avoiding fetching the value's contents,
but given the case I believe where it could most matter, backtracing,
in frame_register_unwind, already does:
*optimizedp = value_optimized_out (value);
*unavailablep = !value_entirely_available (value);
and value_entirely_available does already:
int
value_entirely_available (struct value *value)
{
/* We can only tell whether the whole value is available when we try
to read it. */
if (value->lazy)
value_fetch_lazy (value);
... it's super fine with me to make the code correct first,
and consider optimizing value_optimized_out's internals at some
other point.
(while at it, it seems value_entirely_available doesn't really
need to call value_fetch_lazy for optimized out values.)
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.
Can you explain a little better the value_primitive_field
changes? Is that code motion mainly an optimization that could
be done separately, or is it necessary for correctness? IOW, is
that something that could be split out too?
Please add copyright headers to the tests. I also noticed
the patch adds spurious trailing whitespace:
$ git am /tmp/mbox
Applying: value_optimized_out and value_fetch_lazy
/home/pedro/gdb/mygit/src/.git/rebase-apply/patch:609: trailing whitespace.
.uleb128 0x7 /* ULEB128 register */
/home/pedro/gdb/mygit/src/.git/rebase-apply/patch:1032: trailing whitespace.
int
/home/pedro/gdb/mygit/src/.git/rebase-apply/patch:1036: trailing whitespace.
/home/pedro/gdb/mygit/src/.git/rebase-apply/patch:1104: new blank line at EOF.
+
warning: 4 lines add whitespace errors.
--
Pedro Alves
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [2/3] [PATCH] value_optimized_out and value_fetch_lazy
2013-06-19 15:55 ` Pedro Alves
2013-07-01 18:06 ` [3/3] " Andrew Burgess
2013-07-01 18:06 ` [1/3] " Andrew Burgess
@ 2013-07-01 18:06 ` Andrew Burgess
2013-07-03 18:43 ` Pedro Alves
2 siblings, 1 reply; 14+ messages in thread
From: Andrew Burgess @ 2013-07-01 18:06 UTC (permalink / raw)
To: gdb-patches; +Cc: Pedro Alves
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.
>>
>> Currently, if value_optimized_out returns true, this means we know the
>> value is optimized out, if value_optimized_out returns false this
>> means the value might, or might not, be optimized out; if the value
>> is currently lazy then we don't know it has been optimized out, and
>> so the answer from value_optimized_out is false.
>
> Can you explain a little better the value_primitive_field
> changes? Is that code motion mainly an optimization that could
> be done separately, or is it necessary for correctness? IOW, is
> that something that could be split out too?
The changes in value_primitive_field are functional. I add some nested
blocks so the churn looks worse than it really is.
In the old world value_primitive_field the code looks roughly like this:
if (value_optimized_out (arg1))
v = allocate_optimized_out_value (type); /* CASE 1 */
else if (TYPE_FIELD_BITSIZE (arg_type, fieldno))
/* CASE 2 */
else if (fieldno < TYPE_N_BASECLASSES (arg_type))
/* CASE 3 */
else
/* CASE 4 */
Now, consider ARG1 to value_primitive_field has the following
properties, (a) optimised out but not yet marked optimized out,
(b) lval_memory, an (c) lazy. We'll not hit "CASE 1", but, if
we hit "CASE 2" then we will return another lazy value, same for
"CASE 3", and the same for "CASE 4". However, with my change we
decide at the first call to value_optimized_out that the value
is gone, and so land in "CASE 1".
Also note, in the old world we initially case value_optimized_out,
which might return false, but really mean true once we've done a
value_fetch_lazy, later, in "CASE 3" and "CASE 4" we called
value_fetch_lazy, but then failed to take any special action if
the value has now become optimized out.
When I initially wrote my patch I did not change
value_primitive_field, and some tests did regress, and the new
output was less helpful then the current behaviour.
My patch then does 2 things, first, I add in the extra checks
for has this value become optimized out after we call
value_fetch_lazy, this is needed for correctness, as a result of
this first change, only "CASE 2" does not handle optimized out
value, so I add code into "CASE 2" to handle optimized out values.
Now, I no longer need the initial check for optimized out values, I
can leave the values as lazy for as long as possible, only fetching
them if they are lazy and lval_register, this restores the original
behaviour, but, I believe is more correct.
>
> Please add copyright headers to the tests. I also noticed
> the patch adds spurious trailing whitespace:
These issues should be fixed now.
Here's the improved patch, ok to apply?
Thanks,
Andrew
gdb/ChangeLog
2013-07-01 Andrew Burgess <aburgess@broadcom.com>
* stack.c (read_frame_arg): No longer need to fetch lazy values,
checking for optimized out will ensure lazy values are loaded.
* value.c (value_optimized_out): If the value is not already marked
optimized out, and is lazy then fetch it so we can know for sure
if the value is optimized out.
(value_primitive_field): Move optimized out check later to later in
the function after we have loaded any lazy values.
(value_fetch_lazy): Use optimized out flag directly rather than
calling optimized_out method to avoid triggering recursion.
diff --git a/gdb/stack.c b/gdb/stack.c
index 08431bb..2462931 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -359,10 +359,6 @@ read_frame_arg (struct symbol *sym, struct frame_info *frame,
{
struct type *type = value_type (val);
- if (!value_optimized_out (val) && value_lazy (val))
- value_fetch_lazy (val);
- if (!value_optimized_out (val) && value_lazy (entryval))
- value_fetch_lazy (entryval);
if (!value_optimized_out (val)
&& value_available_contents_eq (val, 0, entryval, 0,
TYPE_LENGTH (type)))
diff --git a/gdb/value.c b/gdb/value.c
index 61fd4a1..e3a60dd 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -1054,6 +1054,9 @@ value_contents_equal (struct value *val1, struct value *val2)
int
value_optimized_out (struct value *value)
{
+ if (!value->optimized_out && value->lazy)
+ value_fetch_lazy (value);
+
return value->optimized_out;
}
@@ -2630,9 +2633,7 @@ value_primitive_field (struct value *arg1, int offset,
description correctly. */
check_typedef (type);
- if (value_optimized_out (arg1))
- v = allocate_optimized_out_value (type);
- else if (TYPE_FIELD_BITSIZE (arg_type, fieldno))
+ if (TYPE_FIELD_BITSIZE (arg_type, fieldno))
{
/* Handle packed fields.
@@ -2646,19 +2647,24 @@ value_primitive_field (struct value *arg1, int offset,
int bitpos = TYPE_FIELD_BITPOS (arg_type, fieldno);
int container_bitsize = TYPE_LENGTH (type) * 8;
- v = allocate_value_lazy (type);
- v->bitsize = TYPE_FIELD_BITSIZE (arg_type, fieldno);
- if ((bitpos % container_bitsize) + v->bitsize <= container_bitsize
- && TYPE_LENGTH (type) <= (int) sizeof (LONGEST))
- v->bitpos = bitpos % container_bitsize;
+ if (arg1->optimized_out)
+ v = allocate_optimized_out_value (type);
else
- v->bitpos = bitpos % 8;
- v->offset = (value_embedded_offset (arg1)
- + offset
- + (bitpos - v->bitpos) / 8);
- set_value_parent (v, arg1);
- if (!value_lazy (arg1))
- value_fetch_lazy (v);
+ {
+ v = allocate_value_lazy (type);
+ v->bitsize = TYPE_FIELD_BITSIZE (arg_type, fieldno);
+ if ((bitpos % container_bitsize) + v->bitsize <= container_bitsize
+ && TYPE_LENGTH (type) <= (int) sizeof (LONGEST))
+ v->bitpos = bitpos % container_bitsize;
+ else
+ v->bitpos = bitpos % 8;
+ v->offset = (value_embedded_offset (arg1)
+ + offset
+ + (bitpos - v->bitpos) / 8);
+ set_value_parent (v, arg1);
+ if (!value_lazy (arg1))
+ value_fetch_lazy (v);
+ }
}
else if (fieldno < TYPE_N_BASECLASSES (arg_type))
{
@@ -2671,29 +2677,34 @@ value_primitive_field (struct value *arg1, int offset,
if (VALUE_LVAL (arg1) == lval_register && value_lazy (arg1))
value_fetch_lazy (arg1);
- /* We special case virtual inheritance here because this
- requires access to the contents, which we would rather avoid
- for references to ordinary fields of unavailable values. */
- if (BASETYPE_VIA_VIRTUAL (arg_type, fieldno))
- boffset = baseclass_offset (arg_type, fieldno,
- value_contents (arg1),
- value_embedded_offset (arg1),
- value_address (arg1),
- arg1);
- else
- boffset = TYPE_FIELD_BITPOS (arg_type, fieldno) / 8;
-
- if (value_lazy (arg1))
- v = allocate_value_lazy (value_enclosing_type (arg1));
+ if (arg1->optimized_out)
+ v = allocate_optimized_out_value (type);
else
{
- v = allocate_value (value_enclosing_type (arg1));
- value_contents_copy_raw (v, 0, arg1, 0,
- TYPE_LENGTH (value_enclosing_type (arg1)));
+ /* We special case virtual inheritance here because this
+ requires access to the contents, which we would rather avoid
+ for references to ordinary fields of unavailable values. */
+ if (BASETYPE_VIA_VIRTUAL (arg_type, fieldno))
+ boffset = baseclass_offset (arg_type, fieldno,
+ value_contents (arg1),
+ value_embedded_offset (arg1),
+ value_address (arg1),
+ arg1);
+ else
+ boffset = TYPE_FIELD_BITPOS (arg_type, fieldno) / 8;
+
+ if (value_lazy (arg1))
+ v = allocate_value_lazy (value_enclosing_type (arg1));
+ else
+ {
+ v = allocate_value (value_enclosing_type (arg1));
+ value_contents_copy_raw (v, 0, arg1, 0,
+ TYPE_LENGTH (value_enclosing_type (arg1)));
+ }
+ v->type = type;
+ v->offset = value_offset (arg1);
+ v->embedded_offset = offset + value_embedded_offset (arg1) + boffset;
}
- v->type = type;
- v->offset = value_offset (arg1);
- v->embedded_offset = offset + value_embedded_offset (arg1) + boffset;
}
else
{
@@ -2704,7 +2715,9 @@ value_primitive_field (struct value *arg1, int offset,
if (VALUE_LVAL (arg1) == lval_register && value_lazy (arg1))
value_fetch_lazy (arg1);
- if (value_lazy (arg1))
+ if (arg1->optimized_out)
+ v = allocate_optimized_out_value (type);
+ else if (value_lazy (arg1))
v = allocate_value_lazy (type);
else
{
@@ -3528,7 +3541,7 @@ value_fetch_lazy (struct value *val)
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))
+ else if (val->optimized_out)
/* Keep it optimized out. */;
else
internal_error (__FILE__, __LINE__, _("Unexpected lazy value type."));
gdb/testsuite/ChangeLog
2013-07-01 Andrew Burgess <aburgess@broadcom.com>
* gdb.dwarf2/dw2-reg-undefined.exp: New file.
* gdb.dwarf2/dw2-reg-undefined.c: Likewise.
* gdb.dwarf2/dw2-reg-undefined.S: Likewise.
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.S b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.S
new file mode 100644
index 0000000..ae071d9
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.S
@@ -0,0 +1,522 @@
+/*
+ Copyright 2013 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/>. */
+
+ /* The FDE entry for "stop_frame" in the .debug_frame section has
+ been hand modified to mark a set of registers as undefined.
+ Otherwise this file is as generated by gcc 4.7.2 for x86_64. */
+ .file "dw2-reg-undefined.c"
+ .text
+.Ltext0:
+ .globl stop_frame
+ .type stop_frame, @function
+stop_frame:
+.LFB0:
+ .file 1 "dw2-reg-undefined.c"
+ .loc 1 19 0
+ pushq %rbp
+.LCFI0:
+ movq %rsp, %rbp
+.LCFI1:
+ .loc 1 22 0
+ popq %rbp
+.LCFI2:
+ ret
+.LFE0:
+ .size stop_frame, .-stop_frame
+ .globl first_frame
+ .type first_frame, @function
+first_frame:
+.LFB1:
+ .loc 1 26 0
+ pushq %rbp
+.LCFI3:
+ movq %rsp, %rbp
+.LCFI4:
+ .loc 1 27 0
+ movl $0, %eax
+ call stop_frame
+ .loc 1 28 0
+ popq %rbp
+.LCFI5:
+ ret
+.LFE1:
+ .size first_frame, .-first_frame
+ .globl main
+ .type main, @function
+main:
+.LFB2:
+ .loc 1 32 0
+ pushq %rbp
+.LCFI6:
+ movq %rsp, %rbp
+.LCFI7:
+ .loc 1 33 0
+ movl $0, %eax
+ call first_frame
+ .loc 1 35 0
+ movl $0, %eax
+ .loc 1 36 0
+ popq %rbp
+.LCFI8:
+ ret
+.LFE2:
+ .size main, .-main
+ .section .debug_frame,"",@progbits
+.Lframe0:
+ .long .LECIE0-.LSCIE0
+.LSCIE0:
+ .long 0xffffffff
+ .byte 0x1
+ .string ""
+ .uleb128 0x1
+ .sleb128 -8
+ .byte 0x10
+ .byte 0xc
+ .uleb128 0x7
+ .uleb128 0x8
+ .byte 0x90
+ .uleb128 0x1
+ .align 8
+.LECIE0:
+ /* This FDE entry, for stop_frame was modified to mark
+ registers 0 -> 6 as being undefined. */
+.LSFDE0:
+ .long .LEFDE0-.LASFDE0
+.LASFDE0:
+ .long .Lframe0
+ .quad .LFB0
+ .quad .LFE0-.LFB0
+
+ /* START OF NEW CONTENT. */
+ .byte 0x7 /* DW_CFA_undefined */
+ .uleb128 0x0 /* ULEB128 register */
+ .byte 0x7 /* DW_CFA_undefined */
+ .uleb128 0x1 /* ULEB128 register */
+ .byte 0x7 /* DW_CFA_undefined */
+ .uleb128 0x2 /* ULEB128 register */
+ .byte 0x7 /* DW_CFA_undefined */
+ .uleb128 0x3 /* ULEB128 register */
+ .byte 0x7 /* DW_CFA_undefined */
+ .uleb128 0x4 /* ULEB128 register */
+ .byte 0x7 /* DW_CFA_undefined */
+ .uleb128 0x5 /* ULEB128 register */
+ .byte 0x7 /* DW_CFA_undefined */
+ .uleb128 0x6 /* ULEB128 register */
+ .byte 0x7 /* DW_CFA_undefined */
+ .uleb128 0x7 /* ULEB128 register */
+ /* END OF NEW CONTENT. */
+
+ .byte 0x4
+ .long .LCFI0-.LFB0
+ .byte 0xe
+ .uleb128 0x10
+ .byte 0x86
+ .uleb128 0x2
+ .byte 0x4
+ .long .LCFI1-.LCFI0
+ .byte 0xd
+ .uleb128 0x6
+ .byte 0x4
+ .long .LCFI2-.LCFI1
+ .byte 0xc
+ .uleb128 0x7
+ .uleb128 0x8
+ .align 8
+.LEFDE0:
+.LSFDE2:
+ .long .LEFDE2-.LASFDE2
+.LASFDE2:
+ .long .Lframe0
+ .quad .LFB1
+ .quad .LFE1-.LFB1
+ .byte 0x4
+ .long .LCFI3-.LFB1
+ .byte 0xe
+ .uleb128 0x10
+ .byte 0x86
+ .uleb128 0x2
+ .byte 0x4
+ .long .LCFI4-.LCFI3
+ .byte 0xd
+ .uleb128 0x6
+ .byte 0x4
+ .long .LCFI5-.LCFI4
+ .byte 0xc
+ .uleb128 0x7
+ .uleb128 0x8
+ .align 8
+.LEFDE2:
+.LSFDE4:
+ .long .LEFDE4-.LASFDE4
+.LASFDE4:
+ .long .Lframe0
+ .quad .LFB2
+ .quad .LFE2-.LFB2
+ .byte 0x4
+ .long .LCFI6-.LFB2
+ .byte 0xe
+ .uleb128 0x10
+ .byte 0x86
+ .uleb128 0x2
+ .byte 0x4
+ .long .LCFI7-.LCFI6
+ .byte 0xd
+ .uleb128 0x6
+ .byte 0x4
+ .long .LCFI8-.LCFI7
+ .byte 0xc
+ .uleb128 0x7
+ .uleb128 0x8
+ .align 8
+.LEFDE4:
+ .section .eh_frame,"a",@progbits
+.Lframe1:
+ .long .LECIE1-.LSCIE1
+.LSCIE1:
+ .long 0
+ .byte 0x1
+ .string "zR"
+ .uleb128 0x1
+ .sleb128 -8
+ .byte 0x10
+ .uleb128 0x1
+ .byte 0x3
+ .byte 0xc
+ .uleb128 0x7
+ .uleb128 0x8
+ .byte 0x90
+ .uleb128 0x1
+ .align 8
+.LECIE1:
+.LSFDE7:
+ .long .LEFDE7-.LASFDE7
+.LASFDE7:
+ .long .LASFDE7-.Lframe1
+ .long .LFB0
+ .long .LFE0-.LFB0
+ .uleb128 0
+ .byte 0x4
+ .long .LCFI0-.LFB0
+ .byte 0xe
+ .uleb128 0x10
+ .byte 0x86
+ .uleb128 0x2
+ .byte 0x4
+ .long .LCFI1-.LCFI0
+ .byte 0xd
+ .uleb128 0x6
+ .byte 0x4
+ .long .LCFI2-.LCFI1
+ .byte 0xc
+ .uleb128 0x7
+ .uleb128 0x8
+ .align 8
+.LEFDE7:
+.LSFDE9:
+ .long .LEFDE9-.LASFDE9
+.LASFDE9:
+ .long .LASFDE9-.Lframe1
+ .long .LFB1
+ .long .LFE1-.LFB1
+ .uleb128 0
+ .byte 0x4
+ .long .LCFI3-.LFB1
+ .byte 0xe
+ .uleb128 0x10
+ .byte 0x86
+ .uleb128 0x2
+ .byte 0x4
+ .long .LCFI4-.LCFI3
+ .byte 0xd
+ .uleb128 0x6
+ .byte 0x4
+ .long .LCFI5-.LCFI4
+ .byte 0xc
+ .uleb128 0x7
+ .uleb128 0x8
+ .align 8
+.LEFDE9:
+.LSFDE11:
+ .long .LEFDE11-.LASFDE11
+.LASFDE11:
+ .long .LASFDE11-.Lframe1
+ .long .LFB2
+ .long .LFE2-.LFB2
+ .uleb128 0
+ .byte 0x4
+ .long .LCFI6-.LFB2
+ .byte 0xe
+ .uleb128 0x10
+ .byte 0x86
+ .uleb128 0x2
+ .byte 0x4
+ .long .LCFI7-.LCFI6
+ .byte 0xd
+ .uleb128 0x6
+ .byte 0x4
+ .long .LCFI8-.LCFI7
+ .byte 0xc
+ .uleb128 0x7
+ .uleb128 0x8
+ .align 8
+.LEFDE11:
+ .text
+.Letext0:
+ .section .debug_info,"",@progbits
+.Ldebug_info0:
+ .long 0x8c
+ .value 0x2
+ .long .Ldebug_abbrev0
+ .byte 0x8
+ .uleb128 0x1
+ .long .LASF2
+ .byte 0x1
+ .long .LASF3
+ .long .LASF4
+ .quad .Ltext0
+ .quad .Letext0
+ .long .Ldebug_line0
+ .uleb128 0x2
+ .byte 0x1
+ .long .LASF0
+ .byte 0x1
+ .byte 0x12
+ .quad .LFB0
+ .quad .LFE0
+ .long .LLST0
+ .byte 0x1
+ .uleb128 0x3
+ .byte 0x1
+ .long .LASF1
+ .byte 0x1
+ .byte 0x19
+ .quad .LFB1
+ .quad .LFE1
+ .long .LLST1
+ .byte 0x1
+ .uleb128 0x4
+ .byte 0x1
+ .long .LASF5
+ .byte 0x1
+ .byte 0x1f
+ .long 0x88
+ .quad .LFB2
+ .quad .LFE2
+ .long .LLST2
+ .byte 0x1
+ .uleb128 0x5
+ .byte 0x4
+ .byte 0x5
+ .string "int"
+ .byte 0
+ .section .debug_abbrev,"",@progbits
+.Ldebug_abbrev0:
+ .uleb128 0x1
+ .uleb128 0x11
+ .byte 0x1
+ .uleb128 0x25
+ .uleb128 0xe
+ .uleb128 0x13
+ .uleb128 0xb
+ .uleb128 0x3
+ .uleb128 0xe
+ .uleb128 0x1b
+ .uleb128 0xe
+ .uleb128 0x11
+ .uleb128 0x1
+ .uleb128 0x12
+ .uleb128 0x1
+ .uleb128 0x10
+ .uleb128 0x6
+ .byte 0
+ .byte 0
+ .uleb128 0x2
+ .uleb128 0x2e
+ .byte 0
+ .uleb128 0x3f
+ .uleb128 0xc
+ .uleb128 0x3
+ .uleb128 0xe
+ .uleb128 0x3a
+ .uleb128 0xb
+ .uleb128 0x3b
+ .uleb128 0xb
+ .uleb128 0x11
+ .uleb128 0x1
+ .uleb128 0x12
+ .uleb128 0x1
+ .uleb128 0x40
+ .uleb128 0x6
+ .uleb128 0x2117
+ .uleb128 0xc
+ .byte 0
+ .byte 0
+ .uleb128 0x3
+ .uleb128 0x2e
+ .byte 0
+ .uleb128 0x3f
+ .uleb128 0xc
+ .uleb128 0x3
+ .uleb128 0xe
+ .uleb128 0x3a
+ .uleb128 0xb
+ .uleb128 0x3b
+ .uleb128 0xb
+ .uleb128 0x11
+ .uleb128 0x1
+ .uleb128 0x12
+ .uleb128 0x1
+ .uleb128 0x40
+ .uleb128 0x6
+ .uleb128 0x2116
+ .uleb128 0xc
+ .byte 0
+ .byte 0
+ .uleb128 0x4
+ .uleb128 0x2e
+ .byte 0
+ .uleb128 0x3f
+ .uleb128 0xc
+ .uleb128 0x3
+ .uleb128 0xe
+ .uleb128 0x3a
+ .uleb128 0xb
+ .uleb128 0x3b
+ .uleb128 0xb
+ .uleb128 0x49
+ .uleb128 0x13
+ .uleb128 0x11
+ .uleb128 0x1
+ .uleb128 0x12
+ .uleb128 0x1
+ .uleb128 0x40
+ .uleb128 0x6
+ .uleb128 0x2116
+ .uleb128 0xc
+ .byte 0
+ .byte 0
+ .uleb128 0x5
+ .uleb128 0x24
+ .byte 0
+ .uleb128 0xb
+ .uleb128 0xb
+ .uleb128 0x3e
+ .uleb128 0xb
+ .uleb128 0x3
+ .uleb128 0x8
+ .byte 0
+ .byte 0
+ .byte 0
+ .section .debug_loc,"",@progbits
+.Ldebug_loc0:
+.LLST0:
+ .quad .LFB0-.Ltext0
+ .quad .LCFI0-.Ltext0
+ .value 0x2
+ .byte 0x77
+ .sleb128 8
+ .quad .LCFI0-.Ltext0
+ .quad .LCFI1-.Ltext0
+ .value 0x2
+ .byte 0x77
+ .sleb128 16
+ .quad .LCFI1-.Ltext0
+ .quad .LCFI2-.Ltext0
+ .value 0x2
+ .byte 0x76
+ .sleb128 16
+ .quad .LCFI2-.Ltext0
+ .quad .LFE0-.Ltext0
+ .value 0x2
+ .byte 0x77
+ .sleb128 8
+ .quad 0
+ .quad 0
+.LLST1:
+ .quad .LFB1-.Ltext0
+ .quad .LCFI3-.Ltext0
+ .value 0x2
+ .byte 0x77
+ .sleb128 8
+ .quad .LCFI3-.Ltext0
+ .quad .LCFI4-.Ltext0
+ .value 0x2
+ .byte 0x77
+ .sleb128 16
+ .quad .LCFI4-.Ltext0
+ .quad .LCFI5-.Ltext0
+ .value 0x2
+ .byte 0x76
+ .sleb128 16
+ .quad .LCFI5-.Ltext0
+ .quad .LFE1-.Ltext0
+ .value 0x2
+ .byte 0x77
+ .sleb128 8
+ .quad 0
+ .quad 0
+.LLST2:
+ .quad .LFB2-.Ltext0
+ .quad .LCFI6-.Ltext0
+ .value 0x2
+ .byte 0x77
+ .sleb128 8
+ .quad .LCFI6-.Ltext0
+ .quad .LCFI7-.Ltext0
+ .value 0x2
+ .byte 0x77
+ .sleb128 16
+ .quad .LCFI7-.Ltext0
+ .quad .LCFI8-.Ltext0
+ .value 0x2
+ .byte 0x76
+ .sleb128 16
+ .quad .LCFI8-.Ltext0
+ .quad .LFE2-.Ltext0
+ .value 0x2
+ .byte 0x77
+ .sleb128 8
+ .quad 0
+ .quad 0
+ .section .debug_aranges,"",@progbits
+ .long 0x2c
+ .value 0x2
+ .long .Ldebug_info0
+ .byte 0x8
+ .byte 0
+ .value 0
+ .value 0
+ .quad .Ltext0
+ .quad .Letext0-.Ltext0
+ .quad 0
+ .quad 0
+ .section .debug_line,"",@progbits
+.Ldebug_line0:
+ .section .debug_str,"MS",@progbits,1
+.LASF0:
+ .string "stop_frame"
+.LASF3:
+ .string "dw2-reg-undefined.c"
+.LASF2:
+ .string "GNU C 4.7.2"
+.LASF1:
+ .string "first_frame"
+.LASF5:
+ .string "main"
+.LASF4:
+ .string "/home/username/src/gdb/testsuite/gdb.dwarf2"
+ .ident "GCC: (GNU) 4.7.2"
+ .section .note.GNU-stack,"",@progbits
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.c b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.c
new file mode 100644
index 0000000..6a177c5
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.c
@@ -0,0 +1,36 @@
+/*
+ Copyright 2013 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/>. */
+
+void
+stop_frame ()
+{
+ /* The debug information for this frame is modified in the accompanying
+ .S file, to mark a set of registers as being undefined. */
+}
+
+void
+first_frame ()
+{
+ stop_frame ();
+}
+
+int
+main ()
+{
+ first_frame ();
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
new file mode 100644
index 0000000..7b7b4d1
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
@@ -0,0 +1,59 @@
+# Copyright 2013 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/>.
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+ return 0
+}
+
+# This test can only be run on x86_64 targets.
+if {![istarget "x86_64-*-*"] || ![is_lp64_target]} {
+ return 0
+}
+
+standard_testfile .S
+
+if { [prepare_for_testing $testfile.exp $testfile $srcfile {nodebug}] } {
+ return -1
+}
+
+if ![runto stop_frame] {
+ perror "Failed to stop in stop_frame"
+ return -1
+}
+
+gdb_test "bt" "#0 (0x\[0-9a-f\]+ in )?stop_frame \[^\r\n\]*\r\n#1 \[^\r\n\]*first_frame \[^\r\n\]*\r\n#2 \[^\r\n\]*main\[^\r\n\]*" \
+ "backtrace from stop_frame"
+
+set value_pattern "0x\[0-9a-f\]+\\s+\[0-9\]+"
+set opt_out_pattern "\\*value not available\\*"
+
+for {set f 0} {$f < 3} {incr f} {
+ if {${f} == 0} {
+ set pattern_rax_rbx_rcx ${value_pattern}
+ set pattern_r8_r9 ${value_pattern}
+ } else {
+ set pattern_rax_rbx_rcx ${opt_out_pattern}
+ set pattern_r8_r9 ${value_pattern}
+ }
+
+ # Select frame.
+ gdb_test "frame ${f}" "#${f}.*" "Switch to frame ${f}"
+
+ # Display register values.
+ gdb_test "info registers rax rbx rcx r8 r9" "rax\\s+${pattern_rax_rbx_rcx}\\s*\r\nrbx\\s+${pattern_rax_rbx_rcx}\\s*\r\nrcx\\s+${pattern_rax_rbx_rcx}\\s*\r\nr8\\s+${pattern_r8_r9}\\s*\r\nr9\\s+${pattern_r8_r9}\\s*" \
+ "Check values of rax, rbx, rcx, r8, r9 in frame ${f}"
+}
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [1/3] [PATCH] value_optimized_out and value_fetch_lazy
2013-06-19 15:55 ` Pedro Alves
2013-07-01 18:06 ` [3/3] " Andrew Burgess
@ 2013-07-01 18:06 ` Andrew Burgess
2013-07-03 18:42 ` Pedro Alves
2013-07-01 18:06 ` [2/3] " Andrew Burgess
2 siblings, 1 reply; 14+ messages in thread
From: Andrew Burgess @ 2013-07-01 18:06 UTC (permalink / raw)
To: gdb-patches; +Cc: Pedro Alves
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)
{
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [3/3] [PATCH] value_optimized_out and value_fetch_lazy
2013-06-19 15:55 ` Pedro Alves
@ 2013-07-01 18:06 ` Andrew Burgess
2013-07-03 19:57 ` Pedro Alves
2013-07-01 18:06 ` [1/3] " Andrew Burgess
2013-07-01 18:06 ` [2/3] " Andrew Burgess
2 siblings, 1 reply; 14+ messages in thread
From: Andrew Burgess @ 2013-07-01 18:06 UTC (permalink / raw)
To: gdb-patches; +Cc: Pedro Alves
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?
This tiny patch notices that when we mark a value as optimized out
we can also mark the value as no longer lazy.
This patch is not required, but felt like a good thing to me, not
sure if everyone will agree though.
Should I apply?
Thanks
Andrew
gdb/ChangeLog
2013-07-01 Andrew Burgess <aburgess@broadcom.com>
* value.c (set_value_optimized_out): A value that is optimized out
is no longer lazy.
diff --git a/gdb/value.c b/gdb/value.c
index 8547590..61fd4a1 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -1061,6 +1061,8 @@ void
set_value_optimized_out (struct value *value, int val)
{
value->optimized_out = val;
+ if (val)
+ set_value_lazy (value, 0);
}
int
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [1/3] [PATCH] value_optimized_out and value_fetch_lazy
2013-07-01 18:06 ` [1/3] " Andrew Burgess
@ 2013-07-03 18:42 ` Pedro Alves
2013-07-04 9:51 ` Andrew Burgess
0 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2013-07-03 18:42 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
On 07/01/2013 07:06 PM, Andrew Burgess wrote:
> 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.
This is OK, but should mention the "user-regs.h" include
move too.
Thanks,
--
Pedro Alves
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [2/3] [PATCH] value_optimized_out and value_fetch_lazy
2013-07-01 18:06 ` [2/3] " Andrew Burgess
@ 2013-07-03 18:43 ` Pedro Alves
2013-07-04 11:23 ` Andrew Burgess
0 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2013-07-03 18:43 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
On 07/01/2013 07:06 PM, Andrew Burgess wrote:
> On 19/06/2013 4:50 PM, Pedro Alves wrote:
>> Can you explain a little better the value_primitive_field
>> changes? Is that code motion mainly an optimization that could
>> be done separately, or is it necessary for correctness? IOW, is
>> that something that could be split out too?
>
> The changes in value_primitive_field are functional. I add some nested
> blocks so the churn looks worse than it really is.
>
> In the old world value_primitive_field the code looks roughly like this:
>
> if (value_optimized_out (arg1))
> v = allocate_optimized_out_value (type); /* CASE 1 */
> else if (TYPE_FIELD_BITSIZE (arg_type, fieldno))
> /* CASE 2 */
> else if (fieldno < TYPE_N_BASECLASSES (arg_type))
> /* CASE 3 */
> else
> /* CASE 4 */
>
> Now, consider ARG1 to value_primitive_field has the following
> properties, (a) optimised out but not yet marked optimized out,
> (b) lval_memory, an (c) lazy. We'll not hit "CASE 1", but, if
> we hit "CASE 2" then we will return another lazy value, same for
> "CASE 3", and the same for "CASE 4". However, with my change we
> decide at the first call to value_optimized_out that the value
> is gone, and so land in "CASE 1".
>
> Also note, in the old world we initially case value_optimized_out,
> which might return false, but really mean true once we've done a
> value_fetch_lazy, later, in "CASE 3" and "CASE 4" we called
> value_fetch_lazy, but then failed to take any special action if
> the value has now become optimized out.
>
> When I initially wrote my patch I did not change
> value_primitive_field, and some tests did regress, and the new
> output was less helpful then the current behaviour.
Alright, I tried that out myself, and I see it now:
@@ -8106 +8106 @@
-PASS: gdb.base/exprs.exp: print null_t_struct && null_t_struct->v_int_member == 0
+FAIL: gdb.base/exprs.exp: print null_t_struct && null_t_struct->v_int_member == 0
@@ -9291 +9291 @@
-PASS: gdb.base/longest-types.exp: print &f->buf
+FAIL: gdb.base/longest-types.exp: print &f->buf (timeout)
@@ -10975,2 +10975,2 @@
-PASS: gdb.base/ptype.exp: ptype v_t_struct_p.v_float_member
-PASS: gdb.base/ptype.exp: ptype v_t_struct_p->v_float_member
+FAIL: gdb.base/ptype.exp: ptype v_t_struct_p.v_float_member
+FAIL: gdb.base/ptype.exp: ptype v_t_struct_p->v_float_member
@@ -16927,2 +16927,2 @@
-PASS: gdb.cp/inherit.exp: ptype g_B.A::a
-PASS: gdb.cp/inherit.exp: ptype g_B.A::x
+FAIL: gdb.cp/inherit.exp: ptype g_B.A::a
+FAIL: gdb.cp/inherit.exp: ptype g_B.A::x
@@ -16931,2 +16931,2 @@
-PASS: gdb.cp/inherit.exp: ptype g_C.A::a
-PASS: gdb.cp/inherit.exp: ptype g_C.A::x
+FAIL: gdb.cp/inherit.exp: ptype g_C.A::a
+FAIL: gdb.cp/inherit.exp: ptype g_C.A::x
The FAILs are of the form:
(gdb) PASS: gdb.base/exprs.exp: print & (void) v_char
print null_t_struct && null_t_struct->v_int_member == 0
Cannot access memory at address 0x0
(gdb) FAIL: gdb.base/exprs.exp: print null_t_struct && null_t_struct->v_int_member == 0
...
(gdb) PASS: gdb.base/ptype.exp: ptype v_struct1->v_float_member
ptype v_t_struct_p.v_float_member
Cannot access memory at address 0x0
(gdb) FAIL: gdb.base/ptype.exp: ptype v_t_struct_p.v_float_member
>
> My patch then does 2 things, first, I add in the extra checks
> for has this value become optimized out after we call
> value_fetch_lazy, this is needed for correctness, as a result of
> this first change, only "CASE 2" does not handle optimized out
> value, so I add code into "CASE 2" to handle optimized out values.
> Now, I no longer need the initial check for optimized out values, I
> can leave the values as lazy for as long as possible, only fetching
> them if they are lazy and lval_register, this restores the original
> behaviour, but, I believe is more correct.
Here's that part of the patch with "diff -w" (ignore all whitespace):
diff --git c/gdb/value.c w/gdb/value.c
index e5cf1d7..970fb66 100644
--- c/gdb/value.c
+++ w/gdb/value.c
@@ -2631,9 +2631,7 @@ value_primitive_field (struct value *arg1, int offset,
description correctly. */
check_typedef (type);
- if (value_optimized_out (arg1))
- v = allocate_optimized_out_value (type);
- else if (TYPE_FIELD_BITSIZE (arg_type, fieldno))
+ if (TYPE_FIELD_BITSIZE (arg_type, fieldno))
{
/* Handle packed fields.
@@ -2647,6 +2645,10 @@ value_primitive_field (struct value *arg1, int offset,
int bitpos = TYPE_FIELD_BITPOS (arg_type, fieldno);
int container_bitsize = TYPE_LENGTH (type) * 8;
+ if (arg1->optimized_out)
+ v = allocate_optimized_out_value (type);
+ else
+ {
v = allocate_value_lazy (type);
v->bitsize = TYPE_FIELD_BITSIZE (arg_type, fieldno);
if ((bitpos % container_bitsize) + v->bitsize <= container_bitsize
@@ -2661,6 +2663,7 @@ value_primitive_field (struct value *arg1, int offset,
if (!value_lazy (arg1))
value_fetch_lazy (v);
}
+ }
else if (fieldno < TYPE_N_BASECLASSES (arg_type))
{
/* This field is actually a base subobject, so preserve the
@@ -2672,6 +2675,10 @@ value_primitive_field (struct value *arg1, int offset,
if (VALUE_LVAL (arg1) == lval_register && value_lazy (arg1))
value_fetch_lazy (arg1);
+ if (arg1->optimized_out)
+ v = allocate_optimized_out_value (type);
+ else
+ {
/* We special case virtual inheritance here because this
requires access to the contents, which we would rather avoid
for references to ordinary fields of unavailable values. */
@@ -2696,6 +2703,7 @@ value_primitive_field (struct value *arg1, int offset,
v->offset = value_offset (arg1);
v->embedded_offset = offset + value_embedded_offset (arg1) + boffset;
}
+ }
else
{
/* Plain old data member */
@@ -2705,7 +2713,9 @@ value_primitive_field (struct value *arg1, int offset,
if (VALUE_LVAL (arg1) == lval_register && value_lazy (arg1))
value_fetch_lazy (arg1);
- if (value_lazy (arg1))
+ if (arg1->optimized_out)
+ v = allocate_optimized_out_value (type);
+ else if (value_lazy (arg1))
v = allocate_value_lazy (type);
else
{
Makes sense to me now.
> Here's the improved patch, ok to apply?
OK, with ...
> gdb/ChangeLog
>
> 2013-07-01 Andrew Burgess <aburgess@broadcom.com>
>
> * stack.c (read_frame_arg): No longer need to fetch lazy values,
> checking for optimized out will ensure lazy values are loaded.
Write:
* stack.c (read_frame_arg): No longer fetch lazy values.
> * value.c (value_optimized_out): If the value is not already marked
> optimized out, and is lazy then fetch it so we can know for sure
> if the value is optimized out.
Write:
* value.c (value_optimized_out): If the value is not already marked
optimized out, and is lazy then fetch it.
and put the "so we can know for sure if the value is optimized out." comment
in the sources.
> (value_primitive_field): Move optimized out check later to later in
> the function after we have loaded any lazy values.
"later to later" sounds like a later too much. It'd be great to have a
comment in the sources about this detail.
> (value_fetch_lazy): Use optimized out flag directly rather than
> calling optimized_out method to avoid triggering recursion.
Write:
(value_fetch_lazy): Use optimized out flag directly rather than
calling optimized_out method.
and put the "to avoid triggering recursion." comment in the sources.
[ ChangeLog's mention "what's". "why's" go into sources and commit logs. ]
Thanks,
--
Pedro Alves
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [3/3] [PATCH] value_optimized_out and value_fetch_lazy
2013-07-01 18:06 ` [3/3] " Andrew Burgess
@ 2013-07-03 19:57 ` Pedro Alves
2013-07-04 16:41 ` Andrew Burgess
0 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2013-07-03 19:57 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
On 07/01/2013 07:06 PM, Andrew Burgess wrote:
> 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?
>
> This tiny patch notices that when we mark a value as optimized out
> we can also mark the value as no longer lazy.
> This patch is not required, but felt like a good thing to me, not
> sure if everyone will agree though.
>
> Should I apply?
>
> Thanks
> Andrew
>
>
>
>
> gdb/ChangeLog
>
> 2013-07-01 Andrew Burgess <aburgess@broadcom.com>
>
> * value.c (set_value_optimized_out): A value that is optimized out
> is no longer lazy.
Hmm.
I tried going a step further. If the value is optimized
out, then we don't really need its contents buffer.
That is:
gdb/value.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/gdb/value.c b/gdb/value.c
index 970fb66..50f8889 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -690,6 +690,8 @@ allocate_value_lazy (struct type *type)
void
allocate_value_contents (struct value *val)
{
+ gdb_assert (!val->optimized_out);
+
if (!val->contents)
val->contents = (gdb_byte *) xzalloc (TYPE_LENGTH (val->enclosing_type));
}
@@ -1064,6 +1066,13 @@ void
set_value_optimized_out (struct value *value, int val)
{
value->optimized_out = val;
+
+ if (val)
+ {
+ set_value_lazy (value, 0);
+ xfree (value->contents);
+ value->contents = NULL;
+ }
}
and then ran the testsuite. Sure enough, that catches a gdb crash.
In gdb.dwarf2/dw2-op-out-param.exp:
#0 0x000000000056bed0 in extract_signed_integer (addr=0x0, len=8, byte_order=BFD_ENDIAN_LITTLE) at ../../src/gdb/findvar.c:78
#1 0x000000000057e761 in unpack_long (type=0x1793f80, valaddr=0x0) at ../../src/gdb/value.c:2453
#2 0x000000000059baae in print_scalar_formatted (valaddr=0x0, type=0x1793f80, options=0x7fffdb2376c0, size=0, stream=0x1929a00) at ../../src/gdb/printcmd.c:404
#3 0x000000000059763b in val_print_scalar_formatted (type=0x1793f80, valaddr=0x0, embedded_offset=0, val=0x19dfcd0, options=0x7fffdb2376c0, size=0, stream=0x1929a00)
at ../../src/gdb/valprint.c:970
#4 0x00000000006d1f2f in c_val_print (type=0x1793f80, valaddr=0x0, embedded_offset=0, address=9225056, stream=0x1929a00, recurse=3, original_value=0x19dfcd0, options=
0x7fffdb237860) at ../../src/gdb/c-valprint.c:407
#5 0x0000000000596f4a in val_print (type=0x1793f80, valaddr=0x0, embedded_offset=0, address=9225056, stream=0x1929a00, recurse=3, val=0x19dfcd0, options=0x7fffdb237940,
language=0x8d34a0) at ../../src/gdb/valprint.c:778
#6 0x00000000006d3491 in cp_print_value_fields (type=0x1796110, real_type=0x1796110, valaddr=0x0, offset=0, address=9225056, stream=0x1929a00, recurse=2, val=0x19dfcd0,
options=0x7fffdb237d40, dont_print_vb=0x0, dont_print_statmem=0) at ../../src/gdb/cp-valprint.c:363
#7 0x00000000006d37ef in cp_print_value_fields_rtti (type=0x1796110, valaddr=0x0, offset=0, address=9225056, stream=0x1929a00, recurse=2, val=0x19dfcd0, options=
0x7fffdb237d40, dont_print_vb=0x0, dont_print_statmem=0) at ../../src/gdb/cp-valprint.c:458
#8 0x00000000006d1e45 in c_val_print (type=0x1796110, valaddr=0x0, embedded_offset=0, address=9225056, stream=0x1929a00, recurse=2, original_value=0x19dfcd0, options=
0x7fffdb237d40) at ../../src/gdb/c-valprint.c:393
#9 0x0000000000596f4a in val_print (type=0x1796110, valaddr=0x0, embedded_offset=0, address=9225056, stream=0x1929a00, recurse=2, val=0x19dfcd0, options=0x7fffdb237e60,
language=0x8d34a0) at ../../src/gdb/valprint.c:778
#10 0x0000000000597157 in common_val_print (val=0x19dfcd0, stream=0x1929a00, recurse=2, options=0x7fffdb237e60, language=0x8d34a0) at ../../src/gdb/valprint.c:840
#11 0x00000000005d9ea6 in print_frame_arg (arg=0x7fffdb237f30) at ../../src/gdb/stack.c:284
#12 0x00000000005daa49 in print_frame_args (func=0x1796af0, frame=0x1b5b800, num=-1, stream=0x168fd10) at ../../src/gdb/stack.c:645
#13 0x00000000005db9df in print_frame (frame=0x1b5b800, print_level=1, print_what=LOCATION, print_args=1, sal=...) at ../../src/gdb/stack.c:1172
#14 0x00000000005daf95 in print_frame_info (frame=0x1b5b800, print_level=1, print_what=LOCATION, print_args=1) at ../../src/gdb/stack.c:824
#15 0x00000000005dcdc5 in backtrace_command_1 (count_exp=0x0, show_locals=0, no_filters=0, from_tty=1) at ../../src/gdb/stack.c:1763
#16 0x00000000005dd1b6 in backtrace_command (arg=0x0, from_tty=1) at ../../src/gdb/stack.c:1860
#17 0x00000000004dc53f in do_cfunc (c=0x151bec0, args=0x0, from_tty=1) at ../../src/gdb/cli/cli-decode.c:113
#18 0x00000000004df5d4 in cmd_func (cmd=0x151bec0, args=0x0, from_tty=1) at ../../src/gdb/cli/cli-decode.c:1888
#19 0x00000000006e43d1 in execute_command (p=0x14623a2 "", from_tty=1) at ../../src/gdb/top.c:478
Odd that this bit of val_print_scalar_formatted:
/* A scalar object that does not have all bits available can't be
printed, because all bits contribute to its representation. */
if (!value_bits_valid (val, TARGET_CHAR_BIT * embedded_offset,
TARGET_CHAR_BIT * TYPE_LENGTH (type)))
val_print_optimized_out (stream);
didn't catch this. I then added this further.
--- c/gdb/value.c
+++ w/gdb/value.c
@@ -1078,6 +1078,8 @@ set_value_optimized_out (struct value *value, int val)
int
value_entirely_optimized_out (const struct value *value)
{
+ gdb_assert (value->optimized_out || !value->lazy);
+
if (!value->optimized_out)
return 0;
if (value->lval != lval_computed
@@ -1089,6 +1091,8 @@ value_entirely_optimized_out (const struct value *value)
int
value_bits_valid (const struct value *value, int offset, int length)
{
+ gdb_assert (value->optimized_out || !value->lazy);
+
if (!value->optimized_out)
return 1;
if (value->lval != lval_computed
And I see:
Breakpoint 2, 0x000000000040058f in breakpt ()
(gdb) PASS: gdb.dwarf2/dw2-op-out-param.exp: continue to breakpoint: Stop in breakpt for struct_param_two_reg_pieces
bt
#0 0x000000000040058f in breakpt ()
#1 0x00000000004005c2 in struct_param_two_reg_pieces (operand0=../../src/gdb/value.c:1081: internal-error: value_entirely_optimized_out: Assertion `value->optimized_out || !value->lazy' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n) FAIL: gdb.dwarf2/dw2-op-out-param.exp: Backtrace for test struct_param_two_reg_pieces (GDB internal error)
Resyncing due to internal error.
... and then I ran out of time. Gotta run an errand. :-)
--
Pedro Alves
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [1/3] [PATCH] value_optimized_out and value_fetch_lazy
2013-07-03 18:42 ` Pedro Alves
@ 2013-07-04 9:51 ` Andrew Burgess
0 siblings, 0 replies; 14+ messages in thread
From: Andrew Burgess @ 2013-07-04 9:51 UTC (permalink / raw)
To: gdb-patches; +Cc: Pedro Alves
On 03/07/2013 7:42 PM, Pedro Alves wrote:
> On 07/01/2013 07:06 PM, Andrew Burgess wrote:
>
>> 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.
>
> This is OK, but should mention the "user-regs.h" include
> move too.
Applied with the following changelog entry:
gdb/ChangeLog
2013-07-04 Andrew Burgess <aburgess@broadcom.com>
* valops.c: Don't include "user-regs.h".
(value_fetch_lazy): Moved to value.c.
* value.c: Include "user-regs.h".
(value_fetch_lazy): Moved from valops.c.
Thanks,
Andrew
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [2/3] [PATCH] value_optimized_out and value_fetch_lazy
2013-07-03 18:43 ` Pedro Alves
@ 2013-07-04 11:23 ` Andrew Burgess
0 siblings, 0 replies; 14+ messages in thread
From: Andrew Burgess @ 2013-07-04 11:23 UTC (permalink / raw)
To: gdb-patches; +Cc: Pedro Alves
On 03/07/2013 7:43 PM, Pedro Alves wrote:
> On 07/01/2013 07:06 PM, Andrew Burgess wrote:
>> Here's the improved patch, ok to apply?
>
> OK, with ...
>
>> gdb/ChangeLog
>>
>> 2013-07-01 Andrew Burgess <aburgess@broadcom.com>
>>
>> * stack.c (read_frame_arg): No longer need to fetch lazy values,
>> checking for optimized out will ensure lazy values are loaded.
>
> Write:
>
> * stack.c (read_frame_arg): No longer fetch lazy values.
>
>> * value.c (value_optimized_out): If the value is not already marked
>> optimized out, and is lazy then fetch it so we can know for sure
>> if the value is optimized out.
>
> Write:
>
> * value.c (value_optimized_out): If the value is not already marked
> optimized out, and is lazy then fetch it.
>
> and put the "so we can know for sure if the value is optimized out." comment
> in the sources.
>
>> (value_primitive_field): Move optimized out check later to later in
>> the function after we have loaded any lazy values.
>
> "later to later" sounds like a later too much. It'd be great to have a
> comment in the sources about this detail.
>
>> (value_fetch_lazy): Use optimized out flag directly rather than
>> calling optimized_out method to avoid triggering recursion.
>
> Write:
>
> (value_fetch_lazy): Use optimized out flag directly rather than
> calling optimized_out method.
>
> and put the "to avoid triggering recursion." comment in the sources.
>
Applied with the following ChangeLog:
gdb/ChangeLog
* stack.c (read_frame_arg): No longer fetch lazy values.
* value.c (value_optimized_out): If the value is not already
marked optimized out, and is lazy then fetch it.
(value_primitive_field): Move optimized out check to later in the
function, after we have loaded any lazy values.
(value_fetch_lazy): Use optimized out flag directly rather than
calling optimized_out method.
And these additional comments:
diff --git a/gdb/value.c b/gdb/value.c
index e3a60dd..abaf23b 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -1054,6 +1054,8 @@ value_contents_equal (struct value *val1, struct value *val2)
int
value_optimized_out (struct value *value)
{
+ /* We can only know if a value is optimized out once we have tried to
+ fetch it. */
if (!value->optimized_out && value->lazy)
value_fetch_lazy (value);
@@ -2677,6 +2679,9 @@ value_primitive_field (struct value *arg1, int offset,
if (VALUE_LVAL (arg1) == lval_register && value_lazy (arg1))
value_fetch_lazy (arg1);
+ /* The optimized_out flag is only set correctly once a lazy value is
+ loaded, having just loaded some lazy values we should check the
+ optimized out case now. */
if (arg1->optimized_out)
v = allocate_optimized_out_value (type);
else
@@ -2715,6 +2720,9 @@ value_primitive_field (struct value *arg1, int offset,
if (VALUE_LVAL (arg1) == lval_register && value_lazy (arg1))
value_fetch_lazy (arg1);
+ /* The optimized_out flag is only set correctly once a lazy value is
+ loaded, having just loaded some lazy values we should check for
+ the optimized out case now. */
if (arg1->optimized_out)
v = allocate_optimized_out_value (type);
else if (value_lazy (arg1))
@@ -3541,6 +3549,9 @@ value_fetch_lazy (struct value *val)
else if (VALUE_LVAL (val) == lval_computed
&& value_computed_funcs (val)->read != NULL)
value_computed_funcs (val)->read (val);
+ /* Don't call value_optimized_out on val, doing so would result in a
+ recursive call back to value_fetch_lazy, instead check the
+ optimized_out flag directly. */
else if (val->optimized_out)
/* Keep it optimized out. */;
else
Thanks,
Andrew
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [3/3] [PATCH] value_optimized_out and value_fetch_lazy
2013-07-03 19:57 ` Pedro Alves
@ 2013-07-04 16:41 ` Andrew Burgess
2013-07-04 17:08 ` Pedro Alves
0 siblings, 1 reply; 14+ messages in thread
From: Andrew Burgess @ 2013-07-04 16:41 UTC (permalink / raw)
To: gdb-patches; +Cc: Pedro Alves
On 03/07/2013 8:57 PM, Pedro Alves wrote:
> On 07/01/2013 07:06 PM, Andrew Burgess wrote:
>> 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?
>>
>> This tiny patch notices that when we mark a value as optimized out
>> we can also mark the value as no longer lazy.
>> This patch is not required, but felt like a good thing to me, not
>> sure if everyone will agree though.
>>
>> Should I apply?
>>
>> Thanks
>> Andrew
>>
>>
>>
>>
>> gdb/ChangeLog
>>
>> 2013-07-01 Andrew Burgess <aburgess@broadcom.com>
>>
>> * value.c (set_value_optimized_out): A value that is optimized out
>> is no longer lazy.
>
> Hmm.
>
> I tried going a step further. If the value is optimized
> out, then we don't really need its contents buffer.
>
> That is:
>
> gdb/value.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/gdb/value.c b/gdb/value.c
> index 970fb66..50f8889 100644
> --- a/gdb/value.c
> +++ b/gdb/value.c
> @@ -690,6 +690,8 @@ allocate_value_lazy (struct type *type)
> void
> allocate_value_contents (struct value *val)
> {
> + gdb_assert (!val->optimized_out);
> +
> if (!val->contents)
> val->contents = (gdb_byte *) xzalloc (TYPE_LENGTH (val->enclosing_type));
> }
> @@ -1064,6 +1066,13 @@ void
> set_value_optimized_out (struct value *value, int val)
> {
> value->optimized_out = val;
> +
> + if (val)
> + {
> + set_value_lazy (value, 0);
> + xfree (value->contents);
> + value->contents = NULL;
> + }
> }
>
> and then ran the testsuite. Sure enough, that catches a gdb crash.
> In gdb.dwarf2/dw2-op-out-param.exp:
>
> #0 0x000000000056bed0 in extract_signed_integer (addr=0x0, len=8, byte_order=BFD_ENDIAN_LITTLE) at ../../src/gdb/findvar.c:78
> #1 0x000000000057e761 in unpack_long (type=0x1793f80, valaddr=0x0) at ../../src/gdb/value.c:2453
> #2 0x000000000059baae in print_scalar_formatted (valaddr=0x0, type=0x1793f80, options=0x7fffdb2376c0, size=0, stream=0x1929a00) at ../../src/gdb/printcmd.c:404
> #3 0x000000000059763b in val_print_scalar_formatted (type=0x1793f80, valaddr=0x0, embedded_offset=0, val=0x19dfcd0, options=0x7fffdb2376c0, size=0, stream=0x1929a00)
> at ../../src/gdb/valprint.c:970
> #4 0x00000000006d1f2f in c_val_print (type=0x1793f80, valaddr=0x0, embedded_offset=0, address=9225056, stream=0x1929a00, recurse=3, original_value=0x19dfcd0, options=
> 0x7fffdb237860) at ../../src/gdb/c-valprint.c:407
> #5 0x0000000000596f4a in val_print (type=0x1793f80, valaddr=0x0, embedded_offset=0, address=9225056, stream=0x1929a00, recurse=3, val=0x19dfcd0, options=0x7fffdb237940,
> language=0x8d34a0) at ../../src/gdb/valprint.c:778
> #6 0x00000000006d3491 in cp_print_value_fields (type=0x1796110, real_type=0x1796110, valaddr=0x0, offset=0, address=9225056, stream=0x1929a00, recurse=2, val=0x19dfcd0,
> options=0x7fffdb237d40, dont_print_vb=0x0, dont_print_statmem=0) at ../../src/gdb/cp-valprint.c:363
> #7 0x00000000006d37ef in cp_print_value_fields_rtti (type=0x1796110, valaddr=0x0, offset=0, address=9225056, stream=0x1929a00, recurse=2, val=0x19dfcd0, options=
> 0x7fffdb237d40, dont_print_vb=0x0, dont_print_statmem=0) at ../../src/gdb/cp-valprint.c:458
> #8 0x00000000006d1e45 in c_val_print (type=0x1796110, valaddr=0x0, embedded_offset=0, address=9225056, stream=0x1929a00, recurse=2, original_value=0x19dfcd0, options=
> 0x7fffdb237d40) at ../../src/gdb/c-valprint.c:393
> #9 0x0000000000596f4a in val_print (type=0x1796110, valaddr=0x0, embedded_offset=0, address=9225056, stream=0x1929a00, recurse=2, val=0x19dfcd0, options=0x7fffdb237e60,
> language=0x8d34a0) at ../../src/gdb/valprint.c:778
> #10 0x0000000000597157 in common_val_print (val=0x19dfcd0, stream=0x1929a00, recurse=2, options=0x7fffdb237e60, language=0x8d34a0) at ../../src/gdb/valprint.c:840
> #11 0x00000000005d9ea6 in print_frame_arg (arg=0x7fffdb237f30) at ../../src/gdb/stack.c:284
> #12 0x00000000005daa49 in print_frame_args (func=0x1796af0, frame=0x1b5b800, num=-1, stream=0x168fd10) at ../../src/gdb/stack.c:645
> #13 0x00000000005db9df in print_frame (frame=0x1b5b800, print_level=1, print_what=LOCATION, print_args=1, sal=...) at ../../src/gdb/stack.c:1172
> #14 0x00000000005daf95 in print_frame_info (frame=0x1b5b800, print_level=1, print_what=LOCATION, print_args=1) at ../../src/gdb/stack.c:824
> #15 0x00000000005dcdc5 in backtrace_command_1 (count_exp=0x0, show_locals=0, no_filters=0, from_tty=1) at ../../src/gdb/stack.c:1763
> #16 0x00000000005dd1b6 in backtrace_command (arg=0x0, from_tty=1) at ../../src/gdb/stack.c:1860
> #17 0x00000000004dc53f in do_cfunc (c=0x151bec0, args=0x0, from_tty=1) at ../../src/gdb/cli/cli-decode.c:113
> #18 0x00000000004df5d4 in cmd_func (cmd=0x151bec0, args=0x0, from_tty=1) at ../../src/gdb/cli/cli-decode.c:1888
> #19 0x00000000006e43d1 in execute_command (p=0x14623a2 "", from_tty=1) at ../../src/gdb/top.c:478
>
> Odd that this bit of val_print_scalar_formatted:
>
> /* A scalar object that does not have all bits available can't be
> printed, because all bits contribute to its representation. */
> if (!value_bits_valid (val, TARGET_CHAR_BIT * embedded_offset,
> TARGET_CHAR_BIT * TYPE_LENGTH (type)))
> val_print_optimized_out (stream);
>
> didn't catch this. I then added this further.
>
> --- c/gdb/value.c
> +++ w/gdb/value.c
> @@ -1078,6 +1078,8 @@ set_value_optimized_out (struct value *value, int val)
> int
> value_entirely_optimized_out (const struct value *value)
> {
> + gdb_assert (value->optimized_out || !value->lazy);
> +
> if (!value->optimized_out)
> return 0;
> if (value->lval != lval_computed
> @@ -1089,6 +1091,8 @@ value_entirely_optimized_out (const struct value *value)
> int
> value_bits_valid (const struct value *value, int offset, int length)
> {
> + gdb_assert (value->optimized_out || !value->lazy);
> +
> if (!value->optimized_out)
> return 1;
> if (value->lval != lval_computed
>
> And I see:
>
> Breakpoint 2, 0x000000000040058f in breakpt ()
> (gdb) PASS: gdb.dwarf2/dw2-op-out-param.exp: continue to breakpoint: Stop in breakpt for struct_param_two_reg_pieces
> bt
> #0 0x000000000040058f in breakpt ()
> #1 0x00000000004005c2 in struct_param_two_reg_pieces (operand0=../../src/gdb/value.c:1081: internal-error: value_entirely_optimized_out: Assertion `value->optimized_out || !value->lazy' failed.
> A problem internal to GDB has been detected,
> further debugging may prove unreliable.
> Quit this debugging session? (y or n) FAIL: gdb.dwarf2/dw2-op-out-param.exp: Backtrace for test struct_param_two_reg_pieces (GDB internal error)
> Resyncing due to internal error.
>
> ... and then I ran out of time. Gotta run an errand. :-)
>
The issue here is that optimized_out does not mean "the value is
optimized out", but instead (in the general case) means this value is at
least partially optimized out.
The problem case are computed values, where if part of the value is
missing then the optimized_out flag is set, but we still rely on being
able to access the contents of the value, with the result that your
patch above, freeing the contents, causes problems.
I don't include the patch here, but as an experiment I made a quick hack
where all the calls to set_value_optimized_out inside dwarf2loc.c call a
new method set_value_partially_optimized_out, this new method sets the
optimized_out flag but does not free the contents buffer, all the tests
now pass. I know this is a hack, we create computed values in other
situations, but the case of computed value and partially optimized out
values only appear to be tested for the dwarf case.
So... I wonder, there appears to be some overlap between the value
unavailable data and the value optimized_out flag. I believe I
understand the difference:
* unavailable - data was not collected at a tracepoint.
* optimized_out - insufficient debug information, or the debug info
specifically states the value is gone.
However, at a higher level there does seem to be some overlap. Imagine
if we collapsed together the unavailable and optimized_out properties of
a value, when handling a value gdb would then walk the following path:
is-value-available? ----- yes -----> process-as-normal
|
'-- no --- > why-not? --- optimized_out ---> opt-out-case
|
'-------- unavailable ---> unavailable-case
The benefit however would be all the code we currently have for
propagating unavailable / optimized_out status between values would be
unified, we would gain the ability to support partially optimized_out
values, and from a quick look at the code I believe there are a couple
of places where we handle optimized_out but not unavailable, this would
be fixed.
What do people think?
Cheers,
Andrew
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [3/3] [PATCH] value_optimized_out and value_fetch_lazy
2013-07-04 16:41 ` Andrew Burgess
@ 2013-07-04 17:08 ` Pedro Alves
2013-07-05 9:31 ` Pedro Alves
0 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2013-07-04 17:08 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
On 07/04/2013 05:40 PM, Andrew Burgess wrote:
> On 03/07/2013 8:57 PM, Pedro Alves wrote:
>> On 07/01/2013 07:06 PM, Andrew Burgess wrote:
>>> 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?
>>>
>>> This tiny patch notices that when we mark a value as optimized out
>>> we can also mark the value as no longer lazy.
>>> This patch is not required, but felt like a good thing to me, not
>>> sure if everyone will agree though.
>>>
>>> Should I apply?
>>>
>>> Thanks
>>> Andrew
>>>
>>>
>>>
>>>
>>> gdb/ChangeLog
>>>
>>> 2013-07-01 Andrew Burgess <aburgess@broadcom.com>
>>>
>>> * value.c (set_value_optimized_out): A value that is optimized out
>>> is no longer lazy.
>>
>> Hmm.
>>
>> I tried going a step further. If the value is optimized
>> out, then we don't really need its contents buffer.
>>
>> That is:
>>
>> gdb/value.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/gdb/value.c b/gdb/value.c
>> index 970fb66..50f8889 100644
>> --- a/gdb/value.c
>> +++ b/gdb/value.c
>> @@ -690,6 +690,8 @@ allocate_value_lazy (struct type *type)
>> void
>> allocate_value_contents (struct value *val)
>> {
>> + gdb_assert (!val->optimized_out);
>> +
>> if (!val->contents)
>> val->contents = (gdb_byte *) xzalloc (TYPE_LENGTH (val->enclosing_type));
>> }
>> @@ -1064,6 +1066,13 @@ void
>> set_value_optimized_out (struct value *value, int val)
>> {
>> value->optimized_out = val;
>> +
>> + if (val)
>> + {
>> + set_value_lazy (value, 0);
>> + xfree (value->contents);
>> + value->contents = NULL;
>> + }
>> }
>>
>> and then ran the testsuite. Sure enough, that catches a gdb crash.
>> In gdb.dwarf2/dw2-op-out-param.exp:
>>
>> #0 0x000000000056bed0 in extract_signed_integer (addr=0x0, len=8, byte_order=BFD_ENDIAN_LITTLE) at ../../src/gdb/findvar.c:78
>> #1 0x000000000057e761 in unpack_long (type=0x1793f80, valaddr=0x0) at ../../src/gdb/value.c:2453
>> #2 0x000000000059baae in print_scalar_formatted (valaddr=0x0, type=0x1793f80, options=0x7fffdb2376c0, size=0, stream=0x1929a00) at ../../src/gdb/printcmd.c:404
>> #3 0x000000000059763b in val_print_scalar_formatted (type=0x1793f80, valaddr=0x0, embedded_offset=0, val=0x19dfcd0, options=0x7fffdb2376c0, size=0, stream=0x1929a00)
>> at ../../src/gdb/valprint.c:970
>> #4 0x00000000006d1f2f in c_val_print (type=0x1793f80, valaddr=0x0, embedded_offset=0, address=9225056, stream=0x1929a00, recurse=3, original_value=0x19dfcd0, options=
>> 0x7fffdb237860) at ../../src/gdb/c-valprint.c:407
>> #5 0x0000000000596f4a in val_print (type=0x1793f80, valaddr=0x0, embedded_offset=0, address=9225056, stream=0x1929a00, recurse=3, val=0x19dfcd0, options=0x7fffdb237940,
>> language=0x8d34a0) at ../../src/gdb/valprint.c:778
>> #6 0x00000000006d3491 in cp_print_value_fields (type=0x1796110, real_type=0x1796110, valaddr=0x0, offset=0, address=9225056, stream=0x1929a00, recurse=2, val=0x19dfcd0,
>> options=0x7fffdb237d40, dont_print_vb=0x0, dont_print_statmem=0) at ../../src/gdb/cp-valprint.c:363
>> #7 0x00000000006d37ef in cp_print_value_fields_rtti (type=0x1796110, valaddr=0x0, offset=0, address=9225056, stream=0x1929a00, recurse=2, val=0x19dfcd0, options=
>> 0x7fffdb237d40, dont_print_vb=0x0, dont_print_statmem=0) at ../../src/gdb/cp-valprint.c:458
>> #8 0x00000000006d1e45 in c_val_print (type=0x1796110, valaddr=0x0, embedded_offset=0, address=9225056, stream=0x1929a00, recurse=2, original_value=0x19dfcd0, options=
>> 0x7fffdb237d40) at ../../src/gdb/c-valprint.c:393
>> #9 0x0000000000596f4a in val_print (type=0x1796110, valaddr=0x0, embedded_offset=0, address=9225056, stream=0x1929a00, recurse=2, val=0x19dfcd0, options=0x7fffdb237e60,
>> language=0x8d34a0) at ../../src/gdb/valprint.c:778
>> #10 0x0000000000597157 in common_val_print (val=0x19dfcd0, stream=0x1929a00, recurse=2, options=0x7fffdb237e60, language=0x8d34a0) at ../../src/gdb/valprint.c:840
>> #11 0x00000000005d9ea6 in print_frame_arg (arg=0x7fffdb237f30) at ../../src/gdb/stack.c:284
>> #12 0x00000000005daa49 in print_frame_args (func=0x1796af0, frame=0x1b5b800, num=-1, stream=0x168fd10) at ../../src/gdb/stack.c:645
>> #13 0x00000000005db9df in print_frame (frame=0x1b5b800, print_level=1, print_what=LOCATION, print_args=1, sal=...) at ../../src/gdb/stack.c:1172
>> #14 0x00000000005daf95 in print_frame_info (frame=0x1b5b800, print_level=1, print_what=LOCATION, print_args=1) at ../../src/gdb/stack.c:824
>> #15 0x00000000005dcdc5 in backtrace_command_1 (count_exp=0x0, show_locals=0, no_filters=0, from_tty=1) at ../../src/gdb/stack.c:1763
>> #16 0x00000000005dd1b6 in backtrace_command (arg=0x0, from_tty=1) at ../../src/gdb/stack.c:1860
>> #17 0x00000000004dc53f in do_cfunc (c=0x151bec0, args=0x0, from_tty=1) at ../../src/gdb/cli/cli-decode.c:113
>> #18 0x00000000004df5d4 in cmd_func (cmd=0x151bec0, args=0x0, from_tty=1) at ../../src/gdb/cli/cli-decode.c:1888
>> #19 0x00000000006e43d1 in execute_command (p=0x14623a2 "", from_tty=1) at ../../src/gdb/top.c:478
>>
>> Odd that this bit of val_print_scalar_formatted:
>>
>> /* A scalar object that does not have all bits available can't be
>> printed, because all bits contribute to its representation. */
>> if (!value_bits_valid (val, TARGET_CHAR_BIT * embedded_offset,
>> TARGET_CHAR_BIT * TYPE_LENGTH (type)))
>> val_print_optimized_out (stream);
>>
>> didn't catch this. I then added this further.
>>
>> --- c/gdb/value.c
>> +++ w/gdb/value.c
>> @@ -1078,6 +1078,8 @@ set_value_optimized_out (struct value *value, int val)
>> int
>> value_entirely_optimized_out (const struct value *value)
>> {
>> + gdb_assert (value->optimized_out || !value->lazy);
>> +
>> if (!value->optimized_out)
>> return 0;
>> if (value->lval != lval_computed
>> @@ -1089,6 +1091,8 @@ value_entirely_optimized_out (const struct value *value)
>> int
>> value_bits_valid (const struct value *value, int offset, int length)
>> {
>> + gdb_assert (value->optimized_out || !value->lazy);
>> +
>> if (!value->optimized_out)
>> return 1;
>> if (value->lval != lval_computed
>>
>> And I see:
>>
>> Breakpoint 2, 0x000000000040058f in breakpt ()
>> (gdb) PASS: gdb.dwarf2/dw2-op-out-param.exp: continue to breakpoint: Stop in breakpt for struct_param_two_reg_pieces
>> bt
>> #0 0x000000000040058f in breakpt ()
>> #1 0x00000000004005c2 in struct_param_two_reg_pieces (operand0=../../src/gdb/value.c:1081: internal-error: value_entirely_optimized_out: Assertion `value->optimized_out || !value->lazy' failed.
>> A problem internal to GDB has been detected,
>> further debugging may prove unreliable.
>> Quit this debugging session? (y or n) FAIL: gdb.dwarf2/dw2-op-out-param.exp: Backtrace for test struct_param_two_reg_pieces (GDB internal error)
>> Resyncing due to internal error.
>>
>> ... and then I ran out of time. Gotta run an errand. :-)
>>
>
> The issue here is that optimized_out does not mean "the value is
> optimized out", but instead (in the general case) means this value is at
> least partially optimized out.
>
> The problem case are computed values, where if part of the value is
> missing then the optimized_out flag is set, but we still rely on being
> able to access the contents of the value, with the result that your
> patch above, freeing the contents, causes problems.
Right.
> I don't include the patch here, but as an experiment I made a quick hack
> where all the calls to set_value_optimized_out inside dwarf2loc.c call a
> new method set_value_partially_optimized_out, this new method sets the
> optimized_out flag but does not free the contents buffer, all the tests
> now pass. I know this is a hack, we create computed values in other
> situations, but the case of computed value and partially optimized out
> values only appear to be tested for the dwarf case.
Yep. I'm actually about to post a patch in this direction. I'm just
splitting it into smaller unrelated chunks.
--
Pedro Alves
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [3/3] [PATCH] value_optimized_out and value_fetch_lazy
2013-07-04 17:08 ` Pedro Alves
@ 2013-07-05 9:31 ` Pedro Alves
2013-07-05 10:56 ` Pedro Alves
0 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2013-07-05 9:31 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
On 07/04/2013 06:07 PM, Pedro Alves wrote:
> Yep. I'm actually about to post a patch in this direction. I'm just
> splitting it into smaller unrelated chunks.
Hmm, this is consuming more time than I expected, so I'm going to
stop looking at it for now, actually. This is last day before two
weeks vacation, and I'll better spend some of today doing some
reviews.
--
Pedro Alves
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [3/3] [PATCH] value_optimized_out and value_fetch_lazy
2013-07-05 9:31 ` Pedro Alves
@ 2013-07-05 10:56 ` Pedro Alves
0 siblings, 0 replies; 14+ messages in thread
From: Pedro Alves @ 2013-07-05 10:56 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess
On 07/05/2013 10:31 AM, Pedro Alves wrote:
> On 07/04/2013 06:07 PM, Pedro Alves wrote:
>
>> Yep. I'm actually about to post a patch in this direction. I'm just
>> splitting it into smaller unrelated chunks.
>
> Hmm, this is consuming more time than I expected, so I'm going to
> stop looking at it for now, actually. This is last day before two
> weeks vacation, and I'll better spend some of today doing some
> reviews.
BTW, I pushed what I had to:
git@github.com:palves/gdb.git value_optimized_out
https://github.com/palves/gdb/commits/value_optimized_out
--
Pedro Alves
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2013-07-05 10:56 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-10 12:46 [PATCH] value_optimized_out and value_fetch_lazy Andrew Burgess
2013-06-19 15:55 ` Pedro Alves
2013-07-01 18:06 ` [3/3] " Andrew Burgess
2013-07-03 19:57 ` Pedro Alves
2013-07-04 16:41 ` Andrew Burgess
2013-07-04 17:08 ` Pedro Alves
2013-07-05 9:31 ` Pedro Alves
2013-07-05 10:56 ` Pedro Alves
2013-07-01 18:06 ` [1/3] " Andrew Burgess
2013-07-03 18:42 ` Pedro Alves
2013-07-04 9:51 ` Andrew Burgess
2013-07-01 18:06 ` [2/3] " Andrew Burgess
2013-07-03 18:43 ` Pedro Alves
2013-07-04 11:23 ` Andrew Burgess
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox