From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x32a.google.com (mail-wm1-x32a.google.com [IPv6:2a00:1450:4864:20::32a]) by sourceware.org (Postfix) with ESMTPS id 338DB385843A for ; Fri, 10 Feb 2023 14:19:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 338DB385843A 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-wm1-x32a.google.com with SMTP id r18so3918795wmq.5 for ; Fri, 10 Feb 2023 06:19:35 -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=IAM0viGL1SQNp7zrHCCBrKA76ttaqscxgUSTs+ni99c=; b=OwCid4MMKvQBVsJpyZ/BvjCijnU2D2A+IR1no4Ik01QgdEs1f12DRdAv9II7eHNWFs jy6kcV6lVel9wmxkUyZ6lf4GTbOtzzRB/5aP/431yaMtYeY/qT1KIx/59gx9VR937qyZ c9P7uLBOMSe3ri352KCJ+LSgnNS+ybCC1m0bV/ZggXYIlteD9mGVU/TYYLrv3skE9Ym9 mjhvcDTwjMrJoU9wyilVh66YL7oyqXSsvHxJ8c5dmsFboFl3b8J7J1lpzzsSbgoG1EaO rTvpFNqoedVMz3wt4U6TA3mVXfRVNjhK+a8Ss8IVrcfsYhDZCvQbNLjGx/qUB8juC3YE 4jVQ== 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=IAM0viGL1SQNp7zrHCCBrKA76ttaqscxgUSTs+ni99c=; b=1Zu1kyiz7yKEI/hLbIR8OSlrcwJM+Bp/N12uLaF+k5wpcOEw4qGKRWByjQ00379bkw 6CnCOrg3PuwJv4NxPjqfdo4MtH4L1IN9UA8D3/qhmlsjzeHTU7lMh0HS/W9Lb497kuQC emTYLNaQoELt+1nrLwPYJ6vzXqYGr/VMfyclqpop74NX+nT898eNC1hRWV8bxHssImuD Em7/eASzHanxVp6HmXMlR8j7oWan0+4fWHF9NfFg+Cb1G0St9tGZl9/WhzzUqxrh7weo 0o377oRwUlW+fszA8JHnl894DOZG4291+C9TKIvxnF5ifinMWsx4JPhvJYQxBuPAULSn jlLA== X-Gm-Message-State: AO0yUKXBuYQInfg7CMp/EFD0xlVGB/iCCMTH7lrKpZ2PfahhO4/VP5Cj z0FFjRu268BVyLhdVrgBZepa82kE2+Pnd1yu X-Google-Smtp-Source: AK7set/b00vW387nHL0dWuZ6p+a0lcXQNSTB7sX42u063N1hlWI16qqZlpXeSAZQnX5p49EwkPStpg== X-Received: by 2002:a05:600c:91e:b0:3dc:5362:134a with SMTP id m30-20020a05600c091e00b003dc5362134amr12502005wmp.9.1676038773954; Fri, 10 Feb 2023 06:19:33 -0800 (PST) Received: from tpp.orcam.me.uk (tpp.orcam.me.uk. [2001:8b0:154:0:ea6a:64ff:fe24:f2fc]) by smtp.gmail.com with ESMTPSA id z5-20020a05600c0a0500b003dc43a10fa5sm5742765wmp.13.2023.02.10.06.19.32 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Fri, 10 Feb 2023 06:19:33 -0800 (PST) Date: Fri, 10 Feb 2023 14:19:32 +0000 (GMT) From: "Maciej W. Rozycki" To: gdb-patches@sourceware.org cc: Andrew Burgess , Tom Tromey , Richard Bunt Subject: [PATCH v4 4/6] 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 Content-Type: text/plain; charset=US-ASCII 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 List-Id: 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). Mark values recorded in history as such then and verify accesses to be in-range for them: (gdb) print one_hundred[-1] $1 = (gdb) print one_hundred[100] $2 = Add a suitable test case, which also covers integer overflows in data location calculation. --- Changes from v3: - Do not mark ranges outside values put in the value history unavailable. Instead annotate values as placed in the value history and verify ranges explicitly at access time for them. Remove all previous additions for range handling. Changes from v2: - Correct the copyright year in new files. New change in v2. --- 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 | 17 ++++ 4 files changed, 133 insertions(+), 1 deletion(-) gdb-value-history-unavailable.diff 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) 2023 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) 2023 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 @@ -185,6 +185,7 @@ struct value initialized (1), stack (0), is_zero (false), + in_history (false), type (type_), enclosing_type (type_) { @@ -239,6 +240,9 @@ struct value otherwise. */ bool is_zero : 1; + /* True if this a value recorded in value history; false otherwise. */ + bool in_history : 1; + /* Location of value (if lval). */ union { @@ -385,7 +389,13 @@ value_bits_available (const struct value { gdb_assert (!value->lazy); - return !ranges_contain (value->unavailable, offset, length); + /* 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 val_len = TARGET_CHAR_BIT * value_enclosing_type (value)->length (); + return !((value->in_history + && (offset < 0 || offset + length > val_len)) + || ranges_contain (value->unavailable, offset, length)); } int @@ -1797,6 +1807,7 @@ value_copy (const value *arg) val->modifiable = arg->modifiable; val->stack = arg->stack; val->is_zero = arg->is_zero; + val->in_history = arg->in_history; val->initialized = arg->initialized; val->unavailable = arg->unavailable; val->optimized_out = arg->optimized_out; @@ -1950,6 +1961,10 @@ record_latest_value (struct value *val) a value on the value history never changes. */ if (value_lazy (val)) value_fetch_lazy (val); + + /* Mark the value as recorded in the history for the availability check. */ + val->in_history = true; + /* 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... */