From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18144 invoked by alias); 10 Feb 2011 18:46:59 -0000 Received: (qmail 18133 invoked by uid 22791); 10 Feb 2011 18:46:57 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 10 Feb 2011 18:46:52 +0000 Received: (qmail 4550 invoked from network); 10 Feb 2011 18:46:50 -0000 Received: from unknown (HELO scottsdale.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 10 Feb 2011 18:46:50 -0000 From: Pedro Alves To: Tom Tromey Subject: Re: [unavailable values part 1, 01/17] base support for unavailable value contents Date: Thu, 10 Feb 2011 18:46:00 -0000 User-Agent: KMail/1.13.5 (Linux/2.6.35-25-generic; KDE/4.5.1; x86_64; ; ) Cc: gdb-patches@sourceware.org References: <201102071427.55970.pedro@codesourcery.com> In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201102101846.38823.pedro@codesourcery.com> X-IsSubscribed: yes 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 X-SW-Source: 2011-02/txt/msg00197.txt.bz2 On Thursday 10 February 2011 15:47:29, Tom Tromey wrote: > >>>>> "Pedro" == Pedro Alves writes: > > Pedro> Python pretty-printing does not yet know what to do > Pedro> to unavailable values, so this disables it. Does not mean > Pedro> someone can't add it later. > > Could you file a bug report for this? Sure, will do once this is in. > I think it may be fine to just leave out this change, but I am not > positive. Not sure either. That'll mean the printer will be working with / showing bogus value contents (usually 0, but not garanteed) and have no way to know it and let the user know (as today). Supposedly, someone else on the frontend side thought this was a good idea, since I had the written requirement that pretty printed varobjs shall only be supported if the entire object correspond to is collected. If we disable pretty-printing, we'll at least print using the raw format, showing the s. That said, I don't insist on this. It's just "temporary" until someone implements the python pretty-printer support. Just let me know what you prefer. > Pedro> +/* Returns true if RANGES contains any range that overlaps [OFFSET, > Pedro> + OFFSET+LENGTH). */ > Pedro> + > Pedro> +static int > Pedro> +ranges_contain_p (VEC(range_s) *ranges, int offset, int length) > Pedro> +{ > Pedro> + int i; > Pedro> + range_s *r; > Pedro> + > Pedro> + for (i = 0; VEC_iterate (range_s, ranges, i, r); i++) > Pedro> + if (ranges_overlap (r->offset, r->length, > Pedro> + offset, length)) > Pedro> + return 1; > Pedro> + > Pedro> + return 0; > Pedro> +} > Pedro> + > > It seems to me that since the ranges are sorted by starting address, and > coalesced overlap, then you could use a binary search here, aka > VEC_lower_bound. It may not be worth doing though. Yeah, I don't expect to have more than a handful of ranges to work with. But in any case, I've done that change. Below's my current patch. WDYT? > > Pedro> +static VEC(range_s) * > Pedro> +ranges_copy (VEC(range_s) *ranges) > Pedro> +{ > Pedro> + int i; > Pedro> + range_s *r; > Pedro> + VEC(range_s) *copy = NULL; > Pedro> + > Pedro> + for (i = 0; VEC_iterate (range_s, ranges, i, r); i++) > Pedro> + VEC_safe_push (range_s, copy, r); > > It is slightly more memory-efficient to grow the copy vector to the > right size and then quick_push the elements. Indeed, and there's an even better way that I missed completely: "VEC_copy". :-) Thanks for pointing this out. > > Pedro> + /* Insert the range sorted. If there's overlap or the new range > Pedro> + would be contiguous with an existing range, merge. */ > > This could also use a binary search. Again, maybe not worth the effort. Also done. I've added some "pictures" in the comments, but I'm not sure if that's overboard. The new tests still pass cleanly with the series fully applied on top of this. -- Pedro Alves Index: src/gdb/value.h =================================================================== --- src.orig/gdb/value.h 2011-02-10 18:21:33.747075995 +0000 +++ src/gdb/value.h 2011-02-10 18:42:12.037075997 +0000 @@ -360,6 +360,20 @@ extern int value_bits_valid (const struc extern int value_bits_synthetic_pointer (const struct value *value, int offset, int length); +/* Given a value, determine whether the contents bytes starting at + OFFSET and extending for LENGTH bytes are available. This returns + nonzero if all bytes in the given range are available, zero if any + byte is unavailable. */ + +extern int value_bytes_available (const struct value *value, + int offset, int length); + +/* Mark VALUE's content bytes starting at OFFSET and extending for + LENGTH bytes as unavailable. */ + +extern void mark_value_bytes_unavailable (struct value *value, + int offset, int length); + #include "symtab.h" Index: src/gdb/value.c =================================================================== --- src.orig/gdb/value.c 2011-02-10 18:21:33.747075995 +0000 +++ src/gdb/value.c 2011-02-10 18:42:13.497075998 +0000 @@ -63,6 +63,110 @@ struct internal_function void *cookie; }; +/* Defines an [OFFSET, OFFSET + LENGTH) range. */ + +struct range +{ + /* Lowest offset in the range. */ + int offset; + + /* Length of the range. */ + int length; +}; + +typedef struct range range_s; + +DEF_VEC_O(range_s); + +/* Returns true if the ranges defined by [offset1, offset1+len1) and + [offset2, offset2+len2) overlap. */ + +static int +ranges_overlap (int offset1, int len1, + int offset2, int len2) +{ + ULONGEST h, l; + + l = max (offset1, offset2); + h = min (offset1 + len1, offset2 + len2); + return (l < h); +} + +/* Returns true if the first argument is strictly less than the + second, useful for VEC_lower_bound. We keep ranges sorted by + offset and coalesce overlapping and contiguous ranges, so this just + compares the starting offset. */ + +static int +range_lessthan (const range_s *r1, const range_s *r2) +{ + return r1->offset < r2->offset; +} + +/* Returns true if RANGES contains any range that overlaps [OFFSET, + OFFSET+LENGTH). */ + +static int +ranges_contain_p (VEC(range_s) *ranges, int offset, int length) +{ + range_s what; + int i; + + what.offset = offset; + what.length = length; + + /* We keep ranges sorted by offset and coalesce overlapping and + contiguous ranges, so to check if a range list contains a given + range, we can do a binary search for the position the given range + would be inserted if we only considered the starting OFFSET of + ranges. We call that position I. Since we also have LENGTH to + care for (this is a range afterall), we need to check if the + _previous_ range overlaps the I range. E.g., + + R + |---| + |---| |---| |------| ... |--| + 0 1 2 N + + I=1 + + In the case above, the binary search would return `I=1', meaning, + this OFFSET should be inserted at position 1, and the current + position 1 should be pushed further (and before 2). But, `0' + overlaps with R. + + Then we need to check if the I range overlaps the I range itself. + E.g., + + R + |---| + |---| |---| |-------| ... |--| + 0 1 2 N + + I=1 + */ + + i = VEC_lower_bound (range_s, ranges, &what, range_lessthan); + + if (i > 0) + { + struct range *bef = VEC_index (range_s, ranges, i - 1); + + if (ranges_overlap (bef->offset, bef->length, offset, length)) + return 1; + } + + if (i < VEC_length (range_s, ranges)) + { + struct range *r = VEC_index (range_s, ranges, i); + + if (ranges_overlap (r->offset, r->length, offset, length)) + return 1; + } + + return 0; +} + static struct cmd_list_element *functionlist; struct value @@ -206,6 +310,11 @@ struct value valid if lazy is nonzero. */ gdb_byte *contents; + /* Unavailable ranges in CONTENTS. We mark unavailable ranges, + rather than available, since the common and default case is for a + value to be available. This is filled in at value read time. */ + VEC(range_s) *unavailable; + /* The number of references to this value. When a value is created, the value chain holds a reference, so REFERENCE_COUNT is 1. If release_value is called, this value is removed from the chain but @@ -214,6 +323,179 @@ struct value int reference_count; }; +int +value_bytes_available (const struct value *value, int offset, int length) +{ + gdb_assert (!value->lazy); + + return !ranges_contain_p (value->unavailable, offset, length); +} + +void +mark_value_bytes_unavailable (struct value *value, int offset, int length) +{ + range_s newr; + int i; + + /* Insert the range sorted. If there's overlap or the new range + would be contiguous with an existing range, merge. */ + + newr.offset = offset; + newr.length = length; + + /* Do a binary search for the position the given range would be + inserted if we only considered the starting OFFSET of ranges. + Call that position I. Since we also have LENGTH to care for + (this is a range afterall), we need to check if the _previous_ + range overlaps the I range. E.g., calling R the new range: + + #1 - overlaps with previous + + R + |-...-| + |---| |---| |------| ... |--| + 0 1 2 N + + I=1 + + In the case #1 above, the binary search would return `I=1', + meaning, this OFFSET should be inserted at position 1, and the + current position 1 should be pushed further (and become 2). But, + note that `0' overlaps with R, so we want to merge them. + + A similar consideration needs to be taken if the new range would + be contiguous with the previous range: + + #2 - contiguous with previous + + R + |-...-| + |--| |---| |------| ... |--| + 0 1 2 N + + I=1 + + If there's no overlap with the previous range, as in: + + #3 - not overlapping and not contiguous + + R + |-...-| + |--| |---| |------| ... |--| + 0 1 2 N + + I=0 + + or if I is 0: + + #4 - R is the range with lowest offset + + R + |-...-| + |--| |---| |------| ... |--| + 0 1 2 N + + I=0 + + ... we just push the new range to the head. + + All the 4 cases above need to consider that the new range may + also overlap several of the ranges that follow, or that R may be + contiguous with the following range, and merge. E.g., + + #5 - overlapping following ranges + + R + |------------------------| + |--| |---| |------| ... |--| + 0 1 2 N + + I=0 + + or: + + R + |-------| + |--| |---| |------| ... |--| + 0 1 2 N + + I=1 + + */ + + i = VEC_lower_bound (range_s, value->unavailable, &newr, range_lessthan); + if (i > 0) + { + struct range *bef = VEC_index (range_s, value->unavailable, i - i); + + if (ranges_overlap (bef->offset, bef->length, offset, length)) + { + /* #1 */ + ULONGEST l = min (bef->offset, offset); + ULONGEST h = max (bef->offset + bef->length, offset + length); + + bef->offset = l; + bef->length = h - l; + i--; + } + else if (offset == bef->offset + bef->length) + { + /* #2 */ + bef->length += length; + i--; + } + else + { + /* #3 */ + VEC_safe_insert (range_s, value->unavailable, i, &newr); + } + } + else + { + /* #4 */ + VEC_safe_insert (range_s, value->unavailable, i, &newr); + } + + /* Check whether the ranges following the one we've just added or + touched can be folded in (#5 above). */ + if (i + 1 < VEC_length (range_s, value->unavailable)) + { + struct range *t; + struct range *r; + int removed = 0; + int next = i + 1; + + /* Get the range we just touched. */ + t = VEC_index (range_s, value->unavailable, i); + removed = 0; + + i = next; + for (; VEC_iterate (range_s, value->unavailable, i, r); i++) + if (r->offset <= t->offset + t->length) + { + ULONGEST l, h; + + l = min (t->offset, r->offset); + h = max (t->offset + t->length, r->offset + r->length); + + t->offset = l; + t->length = h - l; + + removed++; + } + else + { + /* If we couldn't merge this one, we won't be able to + merge following ones either, since the ranges are + always sorted by OFFSET. */ + break; + } + + if (removed != 0) + VEC_block_remove (range_s, value->unavailable, next, removed); + } +} + /* Prototypes for local functions. */ static void show_values (char *, int); @@ -420,12 +702,19 @@ value_enclosing_type (struct value *valu } static void -require_not_optimized_out (struct value *value) +require_not_optimized_out (const struct value *value) { if (value->optimized_out) error (_("value has been optimized out")); } +static void +require_available (const struct value *value) +{ + if (!VEC_empty (range_s, value->unavailable)) + error (_("value is not available")); +} + const gdb_byte * value_contents_for_printing (struct value *value) { @@ -446,6 +735,7 @@ value_contents_all (struct value *value) { const gdb_byte *result = value_contents_for_printing (value); require_not_optimized_out (value); + require_available (value); return result; } @@ -478,6 +768,7 @@ value_contents (struct value *value) { const gdb_byte *result = value_contents_writeable (value); require_not_optimized_out (value); + require_available (value); return result; } @@ -703,6 +994,7 @@ value_free (struct value *val) } xfree (val->contents); + VEC_free (range_s, val->unavailable); } xfree (val); } @@ -833,6 +1125,7 @@ value_copy (struct value *arg) TYPE_LENGTH (value_enclosing_type (arg))); } + val->unavailable = VEC_copy (range_s, arg->unavailable); val->parent = arg->parent; if (val->parent) value_incref (val->parent); Index: src/gdb/valprint.h =================================================================== --- src.orig/gdb/valprint.h 2011-02-10 18:21:33.747075995 +0000 +++ src/gdb/valprint.h 2011-02-10 18:42:12.037075997 +0000 @@ -154,4 +154,6 @@ int read_string (CORE_ADDR addr, int len extern void val_print_optimized_out (struct ui_file *stream); +extern void val_print_unavailable (struct ui_file *stream); + #endif Index: src/gdb/valprint.c =================================================================== --- src.orig/gdb/valprint.c 2011-02-10 18:21:33.747075995 +0000 +++ src/gdb/valprint.c 2011-02-10 18:42:12.037075997 +0000 @@ -260,7 +260,7 @@ scalar_type_p (struct type *type) static int valprint_check_validity (struct ui_file *stream, struct type *type, - int offset, + int embedded_offset, const struct value *val) { CHECK_TYPEDEF (type); @@ -269,19 +269,25 @@ valprint_check_validity (struct ui_file && TYPE_CODE (type) != TYPE_CODE_STRUCT && TYPE_CODE (type) != TYPE_CODE_ARRAY) { - if (! value_bits_valid (val, TARGET_CHAR_BIT * offset, - TARGET_CHAR_BIT * TYPE_LENGTH (type))) + if (!value_bits_valid (val, TARGET_CHAR_BIT * embedded_offset, + TARGET_CHAR_BIT * TYPE_LENGTH (type))) { val_print_optimized_out (stream); return 0; } - if (value_bits_synthetic_pointer (val, TARGET_CHAR_BIT * offset, + if (value_bits_synthetic_pointer (val, TARGET_CHAR_BIT * embedded_offset, TARGET_CHAR_BIT * TYPE_LENGTH (type))) { fputs_filtered (_(""), stream); return 0; } + + if (!value_bytes_available (val, embedded_offset, TYPE_LENGTH (type))) + { + val_print_unavailable (stream); + return 0; + } } return 1; @@ -293,6 +299,12 @@ val_print_optimized_out (struct ui_file fprintf_filtered (stream, _("")); } +void +val_print_unavailable (struct ui_file *stream) +{ + fprintf_filtered (stream, _("")); +} + /* Print using the given LANGUAGE the data of type TYPE located at VALADDR + EMBEDDED_OFFSET (within GDB), which came from the inferior at address ADDRESS + EMBEDDED_OFFSET, onto stdio stream @@ -560,6 +572,8 @@ val_print_scalar_formatted (struct type if (!value_bits_valid (val, TARGET_CHAR_BIT * embedded_offset, TARGET_CHAR_BIT * TYPE_LENGTH (type))) val_print_optimized_out (stream); + else if (!value_bytes_available (val, embedded_offset, TYPE_LENGTH (type))) + val_print_unavailable (stream); else print_scalar_formatted (valaddr + embedded_offset, type, options, size, stream); Index: src/gdb/python/py-prettyprint.c =================================================================== --- src.orig/gdb/python/py-prettyprint.c 2011-02-10 18:21:33.747075995 +0000 +++ src/gdb/python/py-prettyprint.c 2011-02-10 18:42:12.037075997 +0000 @@ -690,6 +690,11 @@ apply_val_pretty_printer (struct type *t struct cleanup *cleanups; int result = 0; enum string_repr_result print_result; + + /* No pretty-printer support for unavailable values. */ + if (!value_bytes_available (val, embedded_offset, TYPE_LENGTH (type))) + return 0; + cleanups = ensure_python_env (gdbarch, language); /* Instantiate the printer. */