From patchwork Fri Aug 22 16:20:38 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 2517 Received: (qmail 12940 invoked by alias); 22 Aug 2014 16:20:45 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 12927 invoked by uid 89); 22 Aug 2014 16:20:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 22 Aug 2014 16:20:42 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s7MGKfQd024942 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Fri, 22 Aug 2014 12:20:41 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s7MGKcX3005263; Fri, 22 Aug 2014 12:20:39 -0400 Message-ID: <53F76DD6.2090406@redhat.com> Date: Fri, 22 Aug 2014 17:20:38 +0100 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.7.0 MIME-Version: 1.0 To: Jan Kratochvil CC: gdb-patches@sourceware.org Subject: Re: Regression for i686 gdb.dwarf2/pieces-optimized-out.exp [Re: [PATCH v2] Handle partially optimized out values similarly to unavailable values] References: <20140709103312.GA27884@host2.jankratochvil.net> <53BD2CE0.1000308@redhat.com> <20140709153121.GA7989@host2.jankratochvil.net> <53C41D5D.9030109@redhat.com> <20140716215838.GA29855@host2.jankratochvil.net> <53D0F72D.9010701@redhat.com> <20140815201332.GA2806@host2.jankratochvil.net> <53F3DF83.3020006@redhat.com> <20140821195728.GA22340@host2.jankratochvil.net> In-Reply-To: <20140821195728.GA22340@host2.jankratochvil.net> On 08/21/2014 08:57 PM, Jan Kratochvil wrote: > On Wed, 20 Aug 2014 01:36:35 +0200, Pedro Alves wrote: >> Great, I pushed this in then. > > -PASS: gdb.dwarf2/pieces-optimized-out.exp: print s > +FAIL: gdb.dwarf2/pieces-optimized-out.exp: print s > > for i686 and x86_64-m32 configurations on all known platforms: > > 9a0dc9e3699018b15980bb6a39eb33dea8fefa34 is the first bad commit > commit 9a0dc9e3699018b15980bb6a39eb33dea8fefa34 > Author: Pedro Alves > Date: Wed Aug 20 00:07:40 2014 +0100 > > Handle partially optimized out values similarly to unavailable values > Thanks Jan. So it didn't take that long to get back to handling partially optimized out bitfields after all... I think I found an easy way. We don't currently print partially optimized out scalars, though we could print the valid/invalid bits with p/t, for example, and I think the code ends up clearer anyway. ---------- Subject: [PATCH] Regression for i686 gdb.dwarf2/pieces-optimized-out.exp Git 9a0dc9e3 regressed gdb.dwarf2/pieces-optimized-out.exp, visible on i686 (the test doesn't run on x86_64): (gdb) p s -$1 = {a = 5, b = , c = , d = } +$1 = {a = 5, b = , c = 0, d = 0} -(gdb) PASS: gdb.dwarf2/pieces-optimized-out.exp: print s +(gdb) FAIL: gdb.dwarf2/pieces-optimized-out.exp: print s The regression was caused by this removal in cp-valprint.c: @@ -293,12 +293,6 @@ cp_print_value_fields (struct type *type, struct type *real_type, { fputs_filtered (_(""), stream); } - else if (!value_bits_valid (val, - TYPE_FIELD_BITPOS (type, i), - TYPE_FIELD_BITSIZE (type, i))) - { - val_print_optimized_out (val, stream); - } else { struct value_print_options opts = *options; The idea was that we'd just fallback to calling value_field_bitfield, which handles unavailable values (in unpack_value_bits_as_long_1) so should be able to handle optimized out values too. Alas, it doesn't. This is currently a bit too messy. Instead of teaching unpack_value_bits_as_long_1 about optimized out bits, let's bite the bullet and teach the value code to handle partially optimized out bitfield, by having it unpack a bitfield and then propagate the range metadata. Turns out the resulting code looks simpler and clearer. Tested on x86_64 Fedora 20, -m64/-m32. gdb/ChangeLog: 2014-08-22 Pedro Alves * value.c (value_ranges_copy_adjusted): New function, factored out from ... (value_contents_copy_raw): ... here. (unpack_value_bits_as_long_1): Rename back to ... (unpack_bits_as_long): ... this. Remove 'original_value' and 'result' parameters. Change return type to LONGEST. (unpack_value_bits_as_long): Delete. (unpack_value_field_as_long_1): Delete. (unpack_value_field_as_long, unpack_field_as_long): Reimplement. (unpack_value_bitfield): New function. (value_field_bitfield): Reimplement using unpack_value_bitfield. (value_fetch_lazy): Use unpack_value_bitfield. * value.h (unpack_value_bits_as_long): Delete declaration. --- gdb/value.c | 227 +++++++++++++++++++++++++++++------------------------------- gdb/value.h | 7 -- 2 files changed, 109 insertions(+), 125 deletions(-) diff --git a/gdb/value.c b/gdb/value.c index 077d234..6620f96 100644 --- a/gdb/value.c +++ b/gdb/value.c @@ -1219,6 +1219,22 @@ ranges_copy_adjusted (VEC (range_s) **dst_range, int dst_bit_offset, } } +/* Copy the ranges metadata in SRC that overlaps [SRC_BIT_OFFSET, + SRC_BIT_OFFSET+BIT_LENGTH) into DST, adjusted. */ + +static void +value_ranges_copy_adjusted (struct value *dst, int dst_bit_offset, + const struct value *src, int src_bit_offset, + int bit_length) +{ + ranges_copy_adjusted (&dst->unavailable, dst_bit_offset, + src->unavailable, src_bit_offset, + bit_length); + ranges_copy_adjusted (&dst->optimized_out, dst_bit_offset, + src->optimized_out, src_bit_offset, + bit_length); +} + /* Copy LENGTH bytes of SRC value's (all) contents (value_contents_all) starting at SRC_OFFSET, into DST value's (all) contents, starting at DST_OFFSET. If unavailable contents are @@ -1261,13 +1277,9 @@ value_contents_copy_raw (struct value *dst, int dst_offset, dst_bit_offset = dst_offset * TARGET_CHAR_BIT; bit_length = length * TARGET_CHAR_BIT; - ranges_copy_adjusted (&dst->unavailable, dst_bit_offset, - src->unavailable, src_bit_offset, - bit_length); - - ranges_copy_adjusted (&dst->optimized_out, dst_bit_offset, - src->optimized_out, src_bit_offset, - bit_length); + value_ranges_copy_adjusted (dst, dst_bit_offset, + src, src_bit_offset, + bit_length); } /* Copy LENGTH bytes of SRC value's (all) contents @@ -3105,16 +3117,24 @@ value_fn_field (struct value **arg1p, struct fn_field *f, -/* Helper function for both unpack_value_bits_as_long and - unpack_bits_as_long. See those functions for more details on the - interface; the only difference is that this function accepts either - a NULL or a non-NULL ORIGINAL_VALUE. */ +/* Unpack a bitfield of the specified FIELD_TYPE, from the object at + VALADDR, and store the result in *RESULT. + The bitfield starts at BITPOS bits and contains BITSIZE bits. -static int -unpack_value_bits_as_long_1 (struct type *field_type, const gdb_byte *valaddr, - int embedded_offset, int bitpos, int bitsize, - const struct value *original_value, - LONGEST *result) + Extracting bits depends on endianness of the machine. Compute the + number of least significant bits to discard. For big endian machines, + we compute the total number of bits in the anonymous object, subtract + off the bit count from the MSB of the object to the MSB of the + bitfield, then the size of the bitfield, which leaves the LSB discard + count. For little endian machines, the discard count is simply the + number of bits from the LSB of the anonymous object to the LSB of the + bitfield. + + If the field is signed, we also do sign extension. */ + +static LONGEST +unpack_bits_as_long (struct type *field_type, const gdb_byte *valaddr, + int bitpos, int bitsize) { enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (field_type)); ULONGEST val; @@ -3133,12 +3153,7 @@ unpack_value_bits_as_long_1 (struct type *field_type, const gdb_byte *valaddr, read_offset = bitpos / 8; - if (original_value != NULL - && !value_bits_available (original_value, embedded_offset + bitpos, - bitsize)) - return 0; - - val = extract_unsigned_integer (valaddr + embedded_offset + read_offset, + val = extract_unsigned_integer (valaddr + read_offset, bytes_read, byte_order); /* Extract bits. See comment above. */ @@ -3165,60 +3180,7 @@ unpack_value_bits_as_long_1 (struct type *field_type, const gdb_byte *valaddr, } } - *result = val; - return 1; -} - -/* Unpack a bitfield of the specified FIELD_TYPE, from the object at - VALADDR + EMBEDDED_OFFSET, and store the result in *RESULT. - VALADDR points to the contents of ORIGINAL_VALUE, which must not be - NULL. The bitfield starts at BITPOS bits and contains BITSIZE - bits. - - Returns false if the value contents are unavailable, otherwise - returns true, indicating a valid value has been stored in *RESULT. - - Extracting bits depends on endianness of the machine. Compute the - number of least significant bits to discard. For big endian machines, - we compute the total number of bits in the anonymous object, subtract - off the bit count from the MSB of the object to the MSB of the - bitfield, then the size of the bitfield, which leaves the LSB discard - count. For little endian machines, the discard count is simply the - number of bits from the LSB of the anonymous object to the LSB of the - bitfield. - - If the field is signed, we also do sign extension. */ - -int -unpack_value_bits_as_long (struct type *field_type, const gdb_byte *valaddr, - int embedded_offset, int bitpos, int bitsize, - const struct value *original_value, - LONGEST *result) -{ - gdb_assert (original_value != NULL); - - return unpack_value_bits_as_long_1 (field_type, valaddr, embedded_offset, - bitpos, bitsize, original_value, result); - -} - -/* Unpack a field FIELDNO of the specified TYPE, from the object at - VALADDR + EMBEDDED_OFFSET. VALADDR points to the contents of - ORIGINAL_VALUE. See unpack_value_bits_as_long for more - details. */ - -static int -unpack_value_field_as_long_1 (struct type *type, const gdb_byte *valaddr, - int embedded_offset, int fieldno, - const struct value *val, LONGEST *result) -{ - int bitpos = TYPE_FIELD_BITPOS (type, fieldno); - int bitsize = TYPE_FIELD_BITSIZE (type, fieldno); - struct type *field_type = TYPE_FIELD_TYPE (type, fieldno); - - return unpack_value_bits_as_long_1 (field_type, valaddr, embedded_offset, - bitpos, bitsize, val, - result); + return val; } /* Unpack a field FIELDNO of the specified TYPE, from the object at @@ -3231,51 +3193,95 @@ unpack_value_field_as_long (struct type *type, const gdb_byte *valaddr, int embedded_offset, int fieldno, const struct value *val, LONGEST *result) { + int bitpos = TYPE_FIELD_BITPOS (type, fieldno); + int bitsize = TYPE_FIELD_BITSIZE (type, fieldno); + struct type *field_type = TYPE_FIELD_TYPE (type, fieldno); + int bit_offset; + gdb_assert (val != NULL); - return unpack_value_field_as_long_1 (type, valaddr, embedded_offset, - fieldno, val, result); + bit_offset = embedded_offset * TARGET_CHAR_BIT + bitpos; + if (value_bits_any_optimized_out (val, bit_offset, bitsize) + || !value_bits_available (val, bit_offset, bitsize)) + return 0; + + *result = unpack_bits_as_long (field_type, valaddr + embedded_offset, + bitpos, bitsize); + return 1; } /* Unpack a field FIELDNO of the specified TYPE, from the anonymous - object at VALADDR. See unpack_value_bits_as_long for more details. - This function differs from unpack_value_field_as_long in that it - operates without a struct value object. */ + object at VALADDR. See unpack_bits_as_long for more details. */ LONGEST unpack_field_as_long (struct type *type, const gdb_byte *valaddr, int fieldno) { - LONGEST result; + int bitpos = TYPE_FIELD_BITPOS (type, fieldno); + int bitsize = TYPE_FIELD_BITSIZE (type, fieldno); + struct type *field_type = TYPE_FIELD_TYPE (type, fieldno); - unpack_value_field_as_long_1 (type, valaddr, 0, fieldno, NULL, &result); - return result; + return unpack_bits_as_long (field_type, valaddr, bitpos, bitsize); +} + +/* Unpack a bitfield of BITSIZE bits found at BITPOS in the object at + VALADDR + EMBEDDEDOFFSET that has the type of DEST_VAL and store + the contents in DEST_VAL, zero or sign extending if the type of + DEST_VAL is wider than BITSIZE. VALADDR points to the contents of + VAL. If the VAL's contents required to extract the bitfield from + are unavailable/optimized out, DEST_VAL is correspondingly + marked unavailable/optimized out. */ + +static void +unpack_value_bitfield (struct value *dest_val, + int bitpos, int bitsize, + const gdb_byte *valaddr, int embedded_offset, + const struct value *val) +{ + enum bfd_endian byte_order; + int src_bit_offset; + int dst_bit_offset; + LONGEST num; + struct type *field_type = value_type (dest_val); + + /* First, unpack and sign extend the bitfield as if it was wholly + available. Invalid/unavailable bits are read as zero, but that's + OK, as they'll end up marked below. */ + byte_order = gdbarch_byte_order (get_type_arch (field_type)); + num = unpack_bits_as_long (field_type, valaddr + embedded_offset, + bitpos, bitsize); + store_signed_integer (value_contents_raw (dest_val), + TYPE_LENGTH (field_type), byte_order, num); + + /* Now copy the optimized out / unavailability ranges to the right + bits. */ + src_bit_offset = embedded_offset * TARGET_CHAR_BIT + bitpos; + if (byte_order == BFD_ENDIAN_BIG) + dst_bit_offset = TYPE_LENGTH (field_type) * TARGET_CHAR_BIT - bitsize; + else + dst_bit_offset = 0; + value_ranges_copy_adjusted (dest_val, dst_bit_offset, + val, src_bit_offset, bitsize); } /* Return a new value with type TYPE, which is FIELDNO field of the object at VALADDR + EMBEDDEDOFFSET. VALADDR points to the contents of VAL. If the VAL's contents required to extract the bitfield - from are unavailable, the new value is correspondingly marked as - unavailable. */ + from are unavailable/optimized out, the new value is + correspondingly marked unavailable/optimized out. */ struct value * value_field_bitfield (struct type *type, int fieldno, const gdb_byte *valaddr, int embedded_offset, const struct value *val) { - LONGEST l; + int bitpos = TYPE_FIELD_BITPOS (type, fieldno); + int bitsize = TYPE_FIELD_BITSIZE (type, fieldno); + struct value *res_val = allocate_value (TYPE_FIELD_TYPE (type, fieldno)); - if (!unpack_value_field_as_long (type, valaddr, embedded_offset, fieldno, - val, &l)) - { - struct type *field_type = TYPE_FIELD_TYPE (type, fieldno); - struct value *retval = allocate_value (field_type); - mark_value_bytes_unavailable (retval, 0, TYPE_LENGTH (field_type)); - return retval; - } - else - { - return value_from_longest (TYPE_FIELD_TYPE (type, fieldno), l); - } + unpack_value_bitfield (res_val, bitpos, bitsize, + valaddr, embedded_offset, val); + + return res_val; } /* Modify the value of a bitfield. ADDR points to a block of memory in @@ -3757,30 +3763,15 @@ value_fetch_lazy (struct value *val) 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_lazy (parent)) value_fetch_lazy (parent); - if (value_bits_any_optimized_out (parent, - TARGET_CHAR_BIT * offset + value_bitpos (val), - value_bitsize (val))) - mark_value_bytes_optimized_out (val, value_embedded_offset (val), - TYPE_LENGTH (type)); - else 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); + unpack_value_bitfield (val, + value_bitpos (val), value_bitsize (val), + value_contents_for_printing (parent), + value_offset (val), parent); } else if (VALUE_LVAL (val) == lval_memory) { diff --git a/gdb/value.h b/gdb/value.h index 5d4949c..4cdbf21 100644 --- a/gdb/value.h +++ b/gdb/value.h @@ -606,13 +606,6 @@ extern DOUBLEST unpack_double (struct type *type, const gdb_byte *valaddr, int *invp); extern CORE_ADDR unpack_pointer (struct type *type, const gdb_byte *valaddr); -extern int unpack_value_bits_as_long (struct type *field_type, - const gdb_byte *valaddr, - int embedded_offset, int bitpos, - int bitsize, - const struct value *original_value, - LONGEST *result); - extern LONGEST unpack_field_as_long (struct type *type, const gdb_byte *valaddr, int fieldno);