From patchwork Thu Jan 12 09:01:39 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: 63088 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 CE29638543BB for ; Thu, 12 Jan 2023 09:02:02 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from mail-lf1-x136.google.com (mail-lf1-x136.google.com [IPv6:2a00:1450:4864:20::136]) by sourceware.org (Postfix) with ESMTPS id 4095F385B532 for ; Thu, 12 Jan 2023 09:01:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4095F385B532 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-x136.google.com with SMTP id b3so27482676lfv.2 for ; Thu, 12 Jan 2023 01:01:43 -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=xvW8x6JNTBOCRxzkPgBxjFeAAOuKjXePWRMzc4zmtbs=; b=ZejxlyuYiHyEmkIFXPqeYNFUezmzbJI/X21O9Hf3WrgZpo16tm+ijHRjUn9TrlG7+l 2tfnxyRvXzLSNzug8K7d9HO8AP826sN8R0JYprIE0jmmyC7KmQ7Ann+ZcIV6PPjZAUgY JCEb3c1vpntluk8W/wM3yJI2YRHriDFtAaZE6gQCoDcm7yWaM/Rgu8z6K8Xpvq6FtUA6 lVjCxTeFvnmZmKF70DmaBYnD5DYO9DFZxnnMB1Q+L+h5mNhO/3O+SrWlFzvB3f/iQZJc nGmVZG++ZFe3tiEzH55bBPU0orsE+imKffpdUEtbpDmn6I0a4EsktdokK/y0dzqnt9+f ZDfQ== 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=xvW8x6JNTBOCRxzkPgBxjFeAAOuKjXePWRMzc4zmtbs=; b=aHNoThHp+CPzsMwdjweMIS2UnsuX/rjZllLbeUibouSqIVOpvDJZRnD4RNYibdJ8dc rHzset0dEEEYwBs3M3TsKhGQA6+6KOgwVV5IKaYpp2cdfGpkul5mup/arYBO8HrA6eMV 4m1B5+FvyPiDRYMZN81aUcbCasDyIq0NR4u+B7TprQC47CkDy5Hi8ieg/o/u+0gJ+8n8 3+7NUtI9jMipOpvFntZH/7+d8PLWIMMuOE4JOkc4YsC+Oz+xFnZj2a6W8Jd9KBWn3Uj7 gqpuaAEma/yh3Q9anM79yBa65jOHxRmUQCXdJJmty62YpELy3cRp5KfSGXWFYulAVGN1 m6Lg== X-Gm-Message-State: AFqh2kpFjFZPlFeAD/DrBdQE00XjgF8jeYNujADdXqp0qa8B3peafq+a XpPoDo2eHJIEhF2wXuKVuZS2zZPv0VHGFIYd X-Google-Smtp-Source: AMrXdXsOJVbwkDpSQioYtQhq1gHZNS+RKW+ng8bwk22fjlZ4JVHT32LwbBjLO1mkr1nPyA4j4rFRHw== X-Received: by 2002:a05:6512:3e05:b0:4b9:a91c:b0c9 with SMTP id i5-20020a0565123e0500b004b9a91cb0c9mr23614183lfv.7.1673514101857; Thu, 12 Jan 2023 01:01:41 -0800 (PST) Received: from [192.168.219.3] ([78.8.192.131]) by smtp.gmail.com with ESMTPSA id w29-20020a0565120b1d00b004b6e9530900sm3142620lfu.110.2023.01.12.01.01.40 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 12 Jan 2023 01:01:41 -0800 (PST) Date: Thu, 12 Jan 2023 09:01:39 +0000 (GMT) From: "Maciej W. Rozycki" To: gdb-patches@sourceware.org cc: Andrew Burgess , Tom Tromey , Richard Bunt Subject: [PATCH v2 1/5] GDB: Ignore `max-value-size' setting with value history accesses 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, 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" We have an inconsistency in value history accesses where array element accesses cause an error for entries exceeding the currently selected `max-value-size' setting even where such accesses successfully complete for elements located in the inferior, e.g.: (gdb) p/d one $1 = 0 (gdb) p/d one_hundred $2 = {0 } (gdb) p/d one_hundred[99] $3 = 0 (gdb) set max-value-size 25 (gdb) p/d one_hundred value requires 100 bytes, which is more than max-value-size (gdb) p/d one_hundred[99] $7 = 0 (gdb) p/d $2 value requires 100 bytes, which is more than max-value-size (gdb) p/d $2[99] value requires 100 bytes, which is more than max-value-size (gdb) According to our documentation the `max-value-size' setting is a safety guard against allocating an overly large amount of memory. Moreover a statement in documentation says, concerning this setting, that: "Setting this variable does not effect values that have already been allocated within GDB, only future allocations." While in the implementer-speak the sentence may be unambiguous I think the outside user may well infer that the setting only applies to values that need to be retrieved from the debuggee. Therefore rather than just fixing this inconsistency it seems reasonable to lift the setting for value history accesses, under an implication that by having been retrieved from the debuggee they have already passed the safety check. Do it then, making the last two commands succeed: (gdb) p/d $2 $8 = {0 } (gdb) p/d $2 [99] $9 = 0 (gdb) Expand the testsuite accordingly. --- gdb/testsuite/gdb.base/max-value-size.exp | 3 + gdb/value.c | 56 ++++++++++++++++++++---------- 2 files changed, 42 insertions(+), 17 deletions(-) gdb-value-history-size.diff Index: src/gdb/testsuite/gdb.base/max-value-size.exp =================================================================== --- src.orig/gdb/testsuite/gdb.base/max-value-size.exp +++ src/gdb/testsuite/gdb.base/max-value-size.exp @@ -53,6 +53,9 @@ proc do_value_printing { max_value_size gdb_test "p/d one_hundred" " = \\{0 \\}" } gdb_test "p/d one_hundred \[99\]" " = 0" + # Verify that accessing value history is undisturbed. + gdb_test "p/d \$2" " = \\{0 \\}" + gdb_test "p/d \$2 \[99\]" " = 0" } } Index: src/gdb/value.c =================================================================== --- src.orig/gdb/value.c +++ src/gdb/value.c @@ -1034,31 +1034,42 @@ check_type_length_before_alloc (const st } } -/* Allocate the contents of VAL if it has not been allocated yet. */ +/* Allocate the contents of VAL if it has not been allocated yet. + If CHECK_SIZE is true, then apply the usual max-value-size checks. */ static void -allocate_value_contents (struct value *val) +allocate_value_contents (struct value *val, bool check_size) { if (!val->contents) { - check_type_length_before_alloc (val->enclosing_type); + if (check_size) + check_type_length_before_alloc (val->enclosing_type); val->contents.reset ((gdb_byte *) xzalloc (val->enclosing_type->length ())); } } -/* Allocate a value and its contents for type TYPE. */ +/* Allocate a value and its contents for type TYPE. If CHECK_SIZE is true, + then apply the usual max-value-size checks. */ -struct value * -allocate_value (struct type *type) +static struct value * +allocate_value (struct type *type, bool check_size) { struct value *val = allocate_value_lazy (type); - allocate_value_contents (val); + allocate_value_contents (val, check_size); val->lazy = 0; return val; } +/* Allocate a value and its contents for type TYPE. */ + +struct value * +allocate_value (struct type *type) +{ + return allocate_value (type, true); +} + /* Allocate a value that has the correct length for COUNT repetitions of type TYPE. */ @@ -1169,7 +1180,7 @@ value_contents_raw (struct value *value) struct gdbarch *arch = get_value_arch (value); int unit_size = gdbarch_addressable_memory_unit_size (arch); - allocate_value_contents (value); + allocate_value_contents (value, true); ULONGEST length = value_type (value)->length (); return gdb::make_array_view @@ -1179,7 +1190,7 @@ value_contents_raw (struct value *value) gdb::array_view value_contents_all_raw (struct value *value) { - allocate_value_contents (value); + allocate_value_contents (value, true); ULONGEST length = value_enclosing_type (value)->length (); return gdb::make_array_view (value->contents.get (), length); @@ -1752,12 +1763,14 @@ value_release_to_mark (const struct valu return result; } -/* Return a copy of the value ARG. - It contains the same contents, for same memory address, - but it's a different block of storage. */ +/* Return a copy of the value ARG. It contains the same contents, + for the same memory address, but it's a different block of storage. + If CHECK_SIZE is true, then throw an exception whenever the size + of memory allocated for the contents of the value would exceed + max-value-size. */ -struct value * -value_copy (const value *arg) +static struct value * +value_copy (const value *arg, bool check_size) { struct type *encl_type = value_enclosing_type (arg); struct value *val; @@ -1765,7 +1778,7 @@ value_copy (const value *arg) if (value_lazy (arg)) val = allocate_value_lazy (encl_type); else - val = allocate_value (encl_type); + val = allocate_value (encl_type, check_size); val->type = arg->type; VALUE_LVAL (val) = arg->lval; val->location = arg->location; @@ -1802,6 +1815,15 @@ value_copy (const value *arg) return val; } +/* Return a copy of the value ARG. It contains the same contents, + for the same memory address, but it's a different block of storage. */ + +struct value * +value_copy (const value *arg) +{ + return value_copy (arg, true); +} + /* Return a "const" and/or "volatile" qualified version of the value V. If CNST is true, then the returned value will be qualified with "const". @@ -1965,7 +1987,7 @@ access_value_history (int num) absnum--; - return value_copy (value_history[absnum].get ()); + return value_copy (value_history[absnum].get (), false); } /* See value.h. */ @@ -4162,7 +4184,7 @@ void value_fetch_lazy (struct value *val) { gdb_assert (value_lazy (val)); - allocate_value_contents (val); + allocate_value_contents (val, true); /* A value is either lazy, or fully fetched. The availability/validity is only established as we try to fetch a value. */