From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io1-xd35.google.com (mail-io1-xd35.google.com [IPv6:2607:f8b0:4864:20::d35]) by sourceware.org (Postfix) with ESMTPS id 198AF38A7C11 for ; Thu, 6 Oct 2022 20:06:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 198AF38A7C11 Received: by mail-io1-xd35.google.com with SMTP id i134so2140281ioa.8 for ; Thu, 06 Oct 2022 13:06:23 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=WOAer5ToLSgLmi0ZBnSsbWIgzekjBxz+UM4F4EfLyWk=; b=iaem5fc84nreSCvTJ3JlmiEebgb+kjGuBq/oFf7QHRKsl4J7I/GzRWTp66ePjstRHe a6p9mqBznrdKyks4I57wSfxzYa5I82+YTuDv33+IFjJMzKVBkhdkBXZ43v9gfoD0qsBf /YZSCvteyDQFyMSX2fwQpBf3qXAyE4xK4YL3lMsK41cB6/StXhen2CJNoXgjb8LFzcjF 4xh2LJho6onDQEySVBze8FmhRXogCsMGKMqtzlcuU0JUN8oRN4cLD+vbB/Ujmpkn1OWA GDtI+utgjyb5R5e7F4LGiwWhU4uwebJGcUGkYE0bR+xVQ7B9nQaoza3OaFkJNuMO09dd Wv0A== X-Gm-Message-State: ACrzQf0/ukMDe2bopUAN23VWlEh5Fgkl6AiMNZhidiGjHn/je+rgBLjO kntehNY6aXe0AcKqmll+ZYv6oGXF4d6G8Q== X-Google-Smtp-Source: AMsMyM7DGQHhM/zBuiLrYOeYfxL7510jGlgiGABfHom990qNzHfZYCE7DYMnFtGwGVylRArcVpHKeQ== X-Received: by 2002:a5e:9e01:0:b0:6a4:f730:624c with SMTP id i1-20020a5e9e01000000b006a4f730624cmr679637ioq.107.1665086782209; Thu, 06 Oct 2022 13:06:22 -0700 (PDT) Received: from localhost.localdomain (71-211-160-49.hlrn.qwest.net. [71.211.160.49]) by smtp.gmail.com with ESMTPSA id bp12-20020a056638440c00b0035a468b7fbesm122847jab.71.2022.10.06.13.06.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 06 Oct 2022 13:06:21 -0700 (PDT) From: Tom Tromey To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [PATCH 2/2] Fix crash in value_print_array_elements Date: Thu, 6 Oct 2022 14:06:10 -0600 Message-Id: <20221006200610.3678399-3-tromey@adacore.com> X-Mailer: git-send-email 2.34.3 In-Reply-To: <20221006200610.3678399-1-tromey@adacore.com> References: <20221006200610.3678399-1-tromey@adacore.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-11.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 06 Oct 2022 20:06:25 -0000 A user noticed that gdb would crash when printing a packed array after doing "set lang c". Packed arrays don't exist in C, but it's occasionally useful to print things in C mode when working in a non-C language -- this lets you see under the hood a little bit. The bug here is that generic value printing does not handle packed arrays at all. This patch fixes the bug by introducing a new function to extract a value from a bit offset and width. The new function includes a hack to avoid problems with some existing test cases when using -fgnat-encodings=all. Cleaning up this code looked difficult, and since "all" is effectively deprecated, I thought it made sense to simply work around the problems. --- gdb/testsuite/gdb.ada/packed_array.exp | 13 +++++ gdb/valprint.c | 20 ++++--- gdb/value.c | 79 ++++++++++++++++++++++++++ gdb/value.h | 21 +++++++ 4 files changed, 126 insertions(+), 7 deletions(-) diff --git a/gdb/testsuite/gdb.ada/packed_array.exp b/gdb/testsuite/gdb.ada/packed_array.exp index df34f31348a..e73298ec84c 100644 --- a/gdb/testsuite/gdb.ada/packed_array.exp +++ b/gdb/testsuite/gdb.ada/packed_array.exp @@ -63,4 +63,17 @@ foreach_with_prefix scenario {all minimal} { [string_to_regexp " = ($line, $line, $line, $line)"] gdb_test "print o2_var" \ [string_to_regexp " = ($line, $line, $line, $line)"] + + # This is a regression test for a crash with + # -fgnat-encodings=minimal, and with 'all' the output is unusual, + # so restrict the test. + if {$scenario == "minimal"} { + set line "{true, false, true, false, true}" + + gdb_test "set lang c" \ + "Warning: the current language does not match this frame." + gdb_test "print o2_var" \ + [string_to_regexp " = {$line, $line, $line, $line}"] \ + "print o2_var in c mode" + } } diff --git a/gdb/valprint.c b/gdb/valprint.c index 91a59419c4e..266a76dbdc4 100644 --- a/gdb/valprint.c +++ b/gdb/valprint.c @@ -1928,7 +1928,6 @@ value_print_array_elements (struct value *val, struct ui_file *stream, unsigned int things_printed = 0; unsigned len; struct type *elttype, *index_type; - unsigned eltlen; /* Position of the array element we are examining to see whether it is repeated. */ unsigned int rep1; @@ -1939,7 +1938,9 @@ value_print_array_elements (struct value *val, struct ui_file *stream, struct type *type = check_typedef (value_type (val)); elttype = type->target_type (); - eltlen = type_length_units (check_typedef (elttype)); + unsigned bit_stride = type->bit_stride (); + if (bit_stride == 0) + bit_stride = 8 * check_typedef (elttype)->length (); index_type = type->index_type (); if (index_type->code () == TYPE_CODE_RANGE) index_type = index_type->target_type (); @@ -1988,23 +1989,28 @@ value_print_array_elements (struct value *val, struct ui_file *stream, maybe_print_array_index (index_type, i + low_bound, stream, options); + struct value *element = value_from_component_bitsize (val, elttype, + bit_stride * i, + bit_stride); rep1 = i + 1; reps = 1; /* Only check for reps if repeat_count_threshold is not set to UINT_MAX (unlimited). */ if (options->repeat_count_threshold < UINT_MAX) { - while (rep1 < len - && value_contents_eq (val, i * eltlen, - val, rep1 * eltlen, - eltlen)) + while (rep1 < len) { + struct value *rep_elt + = value_from_component_bitsize (val, elttype, + rep1 * bit_stride, + bit_stride); + if (!value_contents_eq (element, rep_elt)) + break; ++reps; ++rep1; } } - struct value *element = value_from_component (val, elttype, eltlen * i); common_val_print (element, stream, recurse + 1, options, current_language); diff --git a/gdb/value.c b/gdb/value.c index 8ed941f3749..53b35af5620 100644 --- a/gdb/value.c +++ b/gdb/value.c @@ -916,6 +916,17 @@ value_contents_eq (const struct value *val1, LONGEST offset1, length * TARGET_CHAR_BIT); } +/* See value.h. */ + +bool +value_contents_eq (const struct value *val1, const struct value *val2) +{ + ULONGEST len1 = check_typedef (value_enclosing_type (val1))->length (); + ULONGEST len2 = check_typedef (value_enclosing_type (val2))->length (); + if (len1 != len2) + return false; + return value_contents_eq (val1, 0, val2, 0, len1); +} /* The value-history records all the values printed by print commands during this session. */ @@ -1368,6 +1379,43 @@ value_contents_copy_raw (struct value *dst, LONGEST dst_offset, bit_length); } +/* A helper for value_from_component_bitsize that copies bits from SRC + to DEST. */ + +static void +value_contents_copy_raw_bitwise (struct value *dst, LONGEST dst_bit_offset, + struct value *src, LONGEST src_bit_offset, + LONGEST bit_length) +{ + /* A lazy DST would make that this copy operation useless, since as + soon as DST's contents were un-lazied (by a later value_contents + call, say), the contents would be overwritten. A lazy SRC would + mean we'd be copying garbage. */ + gdb_assert (!dst->lazy && !src->lazy); + + /* The overwritten DST range gets unavailability ORed in, not + replaced. Make sure to remember to implement replacing if it + turns out actually necessary. */ + LONGEST dst_offset = dst_bit_offset / TARGET_CHAR_BIT; + LONGEST length = bit_length / TARGET_CHAR_BIT; + gdb_assert (value_bytes_available (dst, dst_offset, length)); + gdb_assert (!value_bits_any_optimized_out (dst, dst_bit_offset, + bit_length)); + + /* Copy the data. */ + gdb::array_view dst_contents = value_contents_all_raw (dst); + gdb::array_view src_contents = value_contents_all_raw (src); + copy_bitwise (dst_contents.data (), dst_bit_offset, + src_contents.data (), src_bit_offset, + bit_length, + type_byte_order (value_type (src)) == BFD_ENDIAN_BIG); + + /* Copy the meta-data. */ + value_ranges_copy_adjusted (dst, dst_bit_offset, + src, src_bit_offset, + bit_length); +} + /* Copy LENGTH bytes of SRC value's (all) contents (value_contents_all) starting at SRC_OFFSET byte, into DST value's (all) contents, starting at DST_OFFSET. If unavailable contents @@ -3774,6 +3822,37 @@ value_from_component (struct value *whole, struct type *type, LONGEST offset) return v; } +/* See value.h. */ + +struct value * +value_from_component_bitsize (struct value *whole, struct type *type, + LONGEST bit_offset, LONGEST bit_length) +{ + gdb_assert (!value_lazy (whole)); + + /* Preserve lvalue-ness if possible. This is needed to avoid + array-printing failures (including crashes) when printing Ada + arrays in programs compiled with -fgnat-encodings=all. */ + if ((bit_offset % TARGET_CHAR_BIT) == 0 + && (bit_length % TARGET_CHAR_BIT) == 0 + && bit_length == TARGET_CHAR_BIT * type->length ()) + return value_from_component (whole, type, bit_offset / TARGET_CHAR_BIT); + + struct value *v = allocate_value (type); + + LONGEST dst_offset = TARGET_CHAR_BIT * value_embedded_offset (v); + if (is_scalar_type (type) && type_byte_order (type) == BFD_ENDIAN_BIG) + dst_offset += TARGET_CHAR_BIT * type->length () - bit_length; + + value_contents_copy_raw_bitwise (v, dst_offset, + whole, + TARGET_CHAR_BIT + * value_embedded_offset (whole) + + bit_offset, + bit_length); + return v; +} + struct value * coerce_ref_if_computed (const struct value *arg) { diff --git a/gdb/value.h b/gdb/value.h index 52752df1f4c..57a7724388b 100644 --- a/gdb/value.h +++ b/gdb/value.h @@ -599,6 +599,12 @@ extern bool value_contents_eq (const struct value *val1, LONGEST offset1, const struct value *val2, LONGEST offset2, LONGEST length); +/* An overload of value_contents_eq that compares the entirety of both + values. */ + +extern bool value_contents_eq (const struct value *val1, + const struct value *val2); + /* Read LENGTH addressable memory units starting at MEMADDR into BUFFER, which is (or will be copied to) VAL's contents buffer offset by BIT_OFFSET bits. Marks value contents ranges as unavailable if @@ -687,6 +693,21 @@ extern struct value *value_from_history_ref (const char *, const char **); extern struct value *value_from_component (struct value *, struct type *, LONGEST); + +/* Create a new value by extracting it from WHOLE. TYPE is the type + of the new value. BIT_OFFSET and BIT_LENGTH describe the offset + and field width of the value to extract from WHOLE -- BIT_LENGTH + may differ from TYPE's length in the case where WHOLE's type is + packed. + + When the value does come from a non-byte-aligned offset or field + width, it will be marked non_lval. */ + +extern struct value *value_from_component_bitsize (struct value *whole, + struct type *type, + LONGEST bit_offset, + LONGEST bit_length); + extern struct value *value_at (struct type *type, CORE_ADDR addr); extern struct value *value_at_lazy (struct type *type, CORE_ADDR addr); -- 2.34.3