From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5577 invoked by alias); 1 Jul 2013 18:06:30 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 5566 invoked by uid 89); 1 Jul 2013 18:06:30 -0000 X-Spam-SWARE-Status: No, score=-3.9 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD autolearn=ham version=3.3.1 Received: from mms3.broadcom.com (HELO mms3.broadcom.com) (216.31.210.19) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Mon, 01 Jul 2013 18:06:29 +0000 Received: from [10.9.208.57] by mms3.broadcom.com with ESMTP (Broadcom SMTP Relay (Email Firewall v6.5)); Mon, 01 Jul 2013 10:56:58 -0700 X-Server-Uuid: B86B6450-0931-4310-942E-F00ED04CA7AF Received: from IRVEXCHSMTP2.corp.ad.broadcom.com (10.9.207.52) by IRVEXCHCAS08.corp.ad.broadcom.com (10.9.208.57) with Microsoft SMTP Server (TLS) id 14.1.438.0; Mon, 1 Jul 2013 11:06:19 -0700 Received: from mail-irva-13.broadcom.com (10.10.10.20) by IRVEXCHSMTP2.corp.ad.broadcom.com (10.9.207.52) with Microsoft SMTP Server id 14.1.438.0; Mon, 1 Jul 2013 11:06:19 -0700 Received: from [10.177.73.66] (unknown [10.177.73.66]) by mail-irva-13.broadcom.com (Postfix) with ESMTP id 2153EF2D72; Mon, 1 Jul 2013 11:06:17 -0700 (PDT) Message-ID: <51D1C519.3090001@broadcom.com> Date: Mon, 01 Jul 2013 18:06:00 -0000 From: "Andrew Burgess" User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:17.0) Gecko/20130620 Thunderbird/17.0.7 MIME-Version: 1.0 To: "gdb-patches@sourceware.org" cc: "Pedro Alves" Subject: Re: [1/3] [PATCH] value_optimized_out and value_fetch_lazy References: <51B5A95F.7090400@broadcom.com> <51C1D347.3020906@redhat.com> In-Reply-To: <51C1D347.3020906@redhat.com> Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-07/txt/msg00056.txt.bz2 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 * 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 #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 #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) {