From patchwork Thu Jan 12 09:02:01 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Maciej W. Rozycki" X-Patchwork-Id: 63090 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 7FA7D3854386 for ; Thu, 12 Jan 2023 09:02:26 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from mail-lf1-x12f.google.com (mail-lf1-x12f.google.com [IPv6:2a00:1450:4864:20::12f]) by sourceware.org (Postfix) with ESMTPS id A615D38493DC for ; Thu, 12 Jan 2023 09:02:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A615D38493DC Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=embecosm.com Received: by mail-lf1-x12f.google.com with SMTP id f34so27437617lfv.10 for ; Thu, 12 Jan 2023 01:02:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=mime-version:user-agent:references:message-id:in-reply-to:subject :cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=JqJi2Q6s3apIV8cecU7Kfx2aPe4GZwQFHcvp0tcyULc=; b=WeDPF7bfKaJDz4uRebr6TU+6cu1eaVajfzrMASDdYzZzgovoJsTpJS0SHkSc2hHQ2F f0vtPg129O/4UfWYJ6iUS7o30dT7MfcvIRng86pY6TKNX4q/ELa+hS+E5/L+jARQIa3y IbIDQfK17qmeAgqS0MFMk81/Kd5t1RXksSlbTd9wN/JF671mGxu14+ryK2G5TxR0My9/ VQqDA4XFotAYOcjnbnsHg++2SqG4wydt9qa2RlcrLM+NbGCoD7L6pTjyV2pMf851F5zi 0IkZ0ILBsHyNmXDPlEH+pjQk3jl39uTzKX4ZMiaNC1P1DsovEcCGWjeXc6vxdAk/EEX2 Cq5g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:user-agent:references:message-id:in-reply-to:subject :cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=JqJi2Q6s3apIV8cecU7Kfx2aPe4GZwQFHcvp0tcyULc=; b=UEENJD91oQawXVOS4lW/aPjb2gpFqRuGsGUasj9uePIIG63BuKkb8IGd5CKCzgGx4Q ahD9Idhm3rwm1i4JJ8/B31WTq75J1ToS4k3Heb1g9ZOCZ07H7leULoS23TST1MLXcyDo wMDPp+FJwS0rVCAvVlht0Q9pkqUyDhx4I0IEMwYlsnDMGloodVOmrFLgCPpHpZuotiL4 jUr9C/BOa6Pf2tQqF6c1dT7nNX5MzlHwkhT7K10ZKvR1qmgIwsDBErrDFq6qx6UP4Tiw 6O6e1phx5DfQJnp5UHzB5mcZJdoWv7tP52WCUWRLmBabY9sDdjS00CgMhynfcjsF0bo/ yEkQ== X-Gm-Message-State: AFqh2kpihIEOqxG7GD4wV/JsSfSZNQjqqKdzySrpvhYCmJtgkfggRIq3 e6juk3MP8U7GHzd8aw6LQ51qqEmmcUNQSqrH X-Google-Smtp-Source: AMrXdXsbhu4O/MggdvpN3LyHzJRK1SfG7xqdLRHaiouIy5e8bBc5ZOoXOuVgjkV+eTP8pzgwK0X1OQ== X-Received: by 2002:a05:6512:1049:b0:4a4:68b8:c2ec with SMTP id c9-20020a056512104900b004a468b8c2ecmr24765489lfb.67.1673514124240; Thu, 12 Jan 2023 01:02:04 -0800 (PST) Received: from [192.168.219.3] ([78.8.192.131]) by smtp.gmail.com with ESMTPSA id a18-20020a056512201200b004cc59b46f74sm3157472lfb.106.2023.01.12.01.02.02 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 12 Jan 2023 01:02:03 -0800 (PST) Date: Thu, 12 Jan 2023 09:02:01 +0000 (GMT) From: "Maciej W. Rozycki" To: gdb-patches@sourceware.org cc: Andrew Burgess , Tom Tromey , Richard Bunt Subject: [PATCH v2 3/5] GDB: Only make data actually retrieved into value history available In-Reply-To: Message-ID: References: User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, KAM_ASCII_DIVIDERS, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=no 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: , Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" While it makes sense to allow accessing out-of-bounds elements in the debuggee and see whatever there might happen to be there in memory (we are a debugger and not a programming rules enforcement facility and we want to make people's life easier in chasing bugs), e.g.: (gdb) print one_hundred[-1] $1 = 0 (gdb) print one_hundred[100] $2 = 0 (gdb) we shouldn't really pretend that we have any meaningful data around values recorded in history (what these commands really retrieve are current debuggee memory contents outside the original data accessed, really confusing in my opinion). Add code to mark data around values recorded in history unavailable then: (gdb) print one_hundred[-1] $1 = (gdb) print one_hundred[100] $2 = Adjust `value_entirely_available' accordingly; also zero-length types need special handling in `value_entirely_covered_by_range_vector'. Add a suitable test case, which also covers integer overflows in data location calculation. --- Hi, While at it I noticed that while values recorded in the history are just as one would expect immutable, one can poke at the original data object data in history has originated from by writing to the value in the history, e.g.: (gdb) print -elements 3 -- one_hundred $1 = {0, 1, 2...} (gdb) print -elements 3 -- $1 $2 = {0, 1, 2...} (gdb) set $1[0] = -1 (gdb) print -elements 3 -- $1 $3 = {0, 1, 2...} (gdb) print -elements 3 -- one_hundred $4 = {-1, 1, 2...} (gdb) This is undocumented and I find it surprising to say the least. Shall I file a bug for this phenomenon? LONG_MIN is not used, but I have thought we can have it anyway for consistency and so that people do not try to invent local variants. Maciej --- gdb/defs.h | 8 ++ gdb/testsuite/gdb.base/value-history-unavailable.c | 29 +++++++ gdb/testsuite/gdb.base/value-history-unavailable.exp | 73 +++++++++++++++++++ gdb/valarith.c | 15 +++ gdb/value.c | 24 ++++-- 5 files changed, 144 insertions(+), 5 deletions(-) gdb-value-history-unavailable.diff Index: src/gdb/defs.h =================================================================== --- src.orig/gdb/defs.h +++ src/gdb/defs.h @@ -469,6 +469,10 @@ enum val_prettyformat #define LONG_MAX ((long)(ULONG_MAX >> 1)) /* 0x7FFFFFFF for 32-bits */ #endif +#if !defined (LONG_MIN) /* 0x80000000 for 32-bits */ +#define LONG_MIN ((long) (~(long) 0 ^ LONG_MAX)) +#endif + #if !defined (ULONGEST_MAX) #define ULONGEST_MAX (~(ULONGEST)0) /* 0xFFFFFFFFFFFFFFFF for 64-bits */ #endif @@ -477,6 +481,10 @@ enum val_prettyformat #define LONGEST_MAX ((LONGEST)(ULONGEST_MAX >> 1)) #endif +#if !defined (LONGEST_MIN) /* 0x8000000000000000 for 64-bits */ +#define LONGEST_MIN ((LONGEST) (~(LONGEST) 0 ^ LONGEST_MAX)) +#endif + /* * Convert a LONGEST to an int. This is used in contexts (e.g. number of arguments to a function, number in a value history, register number, etc.) where the value must not be larger than can fit in an int. */ Index: src/gdb/testsuite/gdb.base/value-history-unavailable.c =================================================================== --- /dev/null +++ src/gdb/testsuite/gdb.base/value-history-unavailable.c @@ -0,0 +1,29 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright (C) 2022 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 . */ + +struct +{ + unsigned char x[1024]; + unsigned char a[1024]; + unsigned char y[1024]; +} a = { {-1} }; + +int +main () +{ + return 0; +} Index: src/gdb/testsuite/gdb.base/value-history-unavailable.exp =================================================================== --- /dev/null +++ src/gdb/testsuite/gdb.base/value-history-unavailable.exp @@ -0,0 +1,73 @@ +# Copyright (C) 2022 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 . + +# Test GDB's value availability ranges. + +standard_testfile + +if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } { + return -1 +} + +if ![runto_main] then { + perror "couldn't run to breakpoint" + continue +} + +set target_char_mask [get_valueof "/u" "a.x\[0]" "255" "get target char mask"] +set target_char_bit 0 +for {set i $target_char_mask} {$i > 0} {set i [expr $i >> 1]} { + incr target_char_bit +} +set target_char_rank -1 +for {set i $target_char_bit} {$i > 0} {set i [expr $i >> 1]} { + incr target_char_rank +} + +# Verify accesses to original inferior data. +gdb_test "print a.a" "\\\$2 = '\\\\000' " +gdb_test "print a.a\[-1\]" "\\\$3 = 0 '\\\\000'" +gdb_test "print a.a\[1024\]" "\\\$4 = 0 '\\\\000'" + +# Verify in-range value history accesses. +gdb_test "print \$2" "\\\$5 = '\\\\000' " +gdb_test "print \$2\[0\]" "\\\$6 = 0 '\\\\000'" +gdb_test "print \$2\[1023\]" "\\\$7 = 0 '\\\\000'" + +# Values outside the array recorded will have not been retrieved. +gdb_test "print \$2\[-1\]" "\\\$8 = " +gdb_test "print \$2\[1024\]" "\\\$9 = " +gdb_test "print \$2\[-1LL << 63 - $target_char_rank\]" \ + "\\\$10 = " +gdb_test "print \$2\[(1LL << 63 - $target_char_rank) - 1\]" \ + "\\\$11 = " + +# Accesses through pointers in history go straight to the inferior though. +gdb_test "print \$2\[0\]@1" "\\\$12 = \"\"" +gdb_test "print \$2\[-1\]@1" "\\\$13 = \"\"" +gdb_test "print \$2\[1024\]@1" "\\\$14 = \"\"" + +# Verify out-of-range value history accesses. +gdb_test "print \$2\[(-1LL << 63 - $target_char_rank) - 1\]" \ + "Integer overflow in data location calculation" +gdb_test "print \$2\[(1LL << 63 - $target_char_rank)\]" \ + "Integer overflow in data location calculation" +gdb_test "print \$2\[-1LL << 63\]" \ + "Integer overflow in data location calculation" +gdb_test "print \$2\[(1ULL << 63) - 1\]" \ + "Integer overflow in data location calculation" + +# Sanity-check a copy of an unavailable value. +gdb_test "print \$11" "\\\$15 = " Index: src/gdb/valarith.c =================================================================== --- src.orig/gdb/valarith.c +++ src/gdb/valarith.c @@ -182,6 +182,21 @@ value_subscript (struct value *array, LO } index -= *lowerbound; + + /* Do not try to dereference a pointer to an unavailable value. + Instead mock up a new one and give it the original address. */ + struct type *elt_type = check_typedef (tarray->target_type ()); + LONGEST elt_size = type_length_units (elt_type); + if (!value_lazy (array) + && !value_bytes_available (array, elt_size * index, elt_size)) + { + struct value *val = allocate_value (elt_type); + mark_value_bytes_unavailable (val, 0, elt_size); + VALUE_LVAL (val) = lval_memory; + set_value_address (val, value_address (array) + elt_size * index); + return val; + } + array = value_coerce_array (array); } Index: src/gdb/value.c =================================================================== --- src.orig/gdb/value.c +++ src/gdb/value.c @@ -421,7 +421,9 @@ value_entirely_available (struct value * if (value->lazy) value_fetch_lazy (value); - if (value->unavailable.empty ()) + if (value->unavailable.empty () + || value_bytes_available (value, 0, + value_enclosing_type (value)->length ())) return 1; return 0; } @@ -441,11 +443,12 @@ value_entirely_covered_by_range_vector ( if (ranges.size () == 1) { + ULONGEST length = value_enclosing_type (value)->length (); const struct range &t = ranges[0]; - if (t.offset == 0 - && t.length == (TARGET_CHAR_BIT - * value_enclosing_type (value)->length ())) + if (length == 0) + return t.offset == 0 && t.length == 0; + if (t.offset <= 0 && t.offset + t.length >= TARGET_CHAR_BIT * length) return 1; } @@ -1277,7 +1280,9 @@ require_not_optimized_out (const struct static void require_available (const struct value *value) { - if (!value->unavailable.empty ()) + if (!value->unavailable.empty () + && !value_bytes_available (value, 0, + value_enclosing_type (value)->length ())) throw_error (NOT_AVAILABLE_ERROR, _("value is not available")); } @@ -1962,6 +1967,15 @@ record_latest_value (struct value *val) a value on the value history never changes. */ if (value_lazy (val)) value_fetch_lazy (val); + + /* Don't pretend we have anything available there in the history beyond + the boundaries of the value recorded. It's not like inferior memory + where there is actual stuff underneath. */ + ULONGEST length = value_enclosing_type (val)->length (); + mark_value_bits_unavailable (val, LONGEST_MIN, 0 ^ LONGEST_MIN); + mark_value_bits_unavailable (val, length * TARGET_CHAR_BIT, + LONGEST_MAX - length * TARGET_CHAR_BIT); + /* We preserve VALUE_LVAL so that the user can find out where it was fetched from. This is a bit dubious, because then *&$1 does not just return $1 but the current contents of that location. c'est la vie... */