From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x32e.google.com (mail-wm1-x32e.google.com [IPv6:2a00:1450:4864:20::32e]) by sourceware.org (Postfix) with ESMTPS id 47DA43854830 for ; Fri, 10 Feb 2023 14:19:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 47DA43854830 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-x32e.google.com with SMTP id az4-20020a05600c600400b003dff767a1f1so4179632wmb.2 for ; Fri, 10 Feb 2023 06:19:17 -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=Hst6/4djRAccq/lKDGdyRU5MS6x7pFWc/7ufDvYMbPw=; b=dyExQm73y4BDauASmA0fQ5Rl9mWQVyHHSMaAu27nUkzGWkFSJOcbCOhs01s5mXEQv8 4XGeJx6GhGRYMeuLeN/4NV/hzHEyMMm1Au49etrp7EvMVvyuPNYRPdMQCtgGdqYreIjh AwTXTpxMNJQjTwFdp/X3ph4qyaJ+5nnOEoXsfqq6wM1yiLW2oR2gfVpr0hXTM9KHNzkv x5XF6rhCpQpPHL3T02aARYkMoXhMaBPyHIkgFh80EQq/A5qmQSBrZ1g24QV50qMs6xxy 3ptiesO0pdWeDZxGOD1k8YAJWN3Jd1Di6vWdr8rS9dq9QbZ5LiMhwoc5D8cjurFmtgBj Pcaw== 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=Hst6/4djRAccq/lKDGdyRU5MS6x7pFWc/7ufDvYMbPw=; b=4t98N+pt56kh0SwWTwAg8oCE75ptDXeC5ihsajarFl9FyDT5IFN5MPgnIUKq3Xdeae esriCVhhXlEQC7ppqF6IuaqlJzTaxmLwLD2S946VkIhl2KnocHSrkoHeAn51/LfIdlXI pEN9wlJcPlqzoAjrWj0zBg1fwb6rYpacFL84OkyCdmKGAMdQDkMl4FJklFXLJICGrvjH 5YuqjkumLNpWFxYfOQkzVWLbsFqJ60z1DvU3s9LmY63CQcFGiJYsMGGfZEfVmorvX/tr JggCKNV86IFmcSLZm0De925mXR0QTMuz8sroH6sYdwS7c5w1dmHf4XFCEjnT2edKSbTT 49CA== X-Gm-Message-State: AO0yUKXcST7+geCvx4hU4zUWU6llAL8opyg/py8g5UE/raPCQKLUe3Ko IHmPj/23FuWz+yPrZ2aOwF2d0wm+FNnJ3J/S X-Google-Smtp-Source: AK7set+3qkW/eo9JcRiVFL7jE/bItsZQ/AK6j6alJ/M4gAohiknm/xzBfxM0GWi19ls412cenzjNsA== X-Received: by 2002:a05:600c:130f:b0:3e0:c5e:ad52 with SMTP id j15-20020a05600c130f00b003e00c5ead52mr13259075wmf.6.1676038756038; Fri, 10 Feb 2023 06:19:16 -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 iv12-20020a05600c548c00b003dc521f336esm5836731wmb.14.2023.02.10.06.19.14 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Fri, 10 Feb 2023 06:19:15 -0800 (PST) Date: Fri, 10 Feb 2023 14:19:14 +0000 (GMT) From: "Maciej W. Rozycki" To: gdb-patches@sourceware.org cc: Andrew Burgess , Tom Tromey , Richard Bunt Subject: [PATCH v4 2/6] 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 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,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: 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 affect 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 does not apply to values previously printed. 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, by suppressing the value size check in `value_copy' -- under an observation that if the original value has been already loaded (i.e. it's not lazy), then it must have previously passed said check -- making the last two commands succeed: (gdb) p/d $2 $8 = {0 } (gdb) p/d $2 [99] $9 = 0 (gdb) Expand the testsuite accordingly, covering both value history handling and the use of `value_copy' by `make_cv_value', used by Python code. --- Changes from v3: - Clarify the change description as to what the outside user may well infer; s/effect/affect/. Changes from v2: - Always suppress the size check in `value_copy' (keeping the grammatical fix to the function description). - Add Python coverage for `make_cv_value' vs `max-value-size'. New change in v2. --- gdb/testsuite/gdb.base/max-value-size.exp | 3 ++ gdb/testsuite/gdb.python/py-xmethods.exp | 17 +++++++++++++ gdb/testsuite/gdb.python/py-xmethods.py | 17 +++++++++++++ gdb/value.c | 38 ++++++++++++++++++------------ 4 files changed, 61 insertions(+), 14 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/testsuite/gdb.python/py-xmethods.exp =================================================================== --- src.orig/gdb/testsuite/gdb.python/py-xmethods.exp +++ src/gdb/testsuite/gdb.python/py-xmethods.exp @@ -160,3 +160,20 @@ gdb_test_no_output "enable xmethod progs gdb_test "pt e.method('a')" "type = void" gdb_test "pt e.method(10)" \ "NotImplementedError.*Error while fetching result type of an xmethod worker defined in Python." + +# Verify `max-value-size' obedience with `gdb.Value.const_value'. +gdb_test "p a1.getarray()" \ + "From Python .* = \\{0, 1, 2, 3, 4, 5, 6, 7, 8, 9\\}" \ + "after: a1.getarray" +gdb_test "p b1.getarray()" \ + "From Python .* = \\{0, 1, 2, 3, 4, 5, 6, 7, 8, 9\\}" \ + "after: b1.getarray" +gdb_test_no_output "python gdb.set_parameter('max-value-size', 16)" +gdb_test "p a1.getarray()" \ + "From Python .*value requires $decimal bytes,\ + which is more than max-value-size" \ + "after: a1.getarray (max-value-size)" +gdb_test "p b1.getarray()" \ + "From Python .*value requires $decimal bytes,\ + which is more than max-value-size" \ + "after: b1.getarray (max-value-size)" Index: src/gdb/testsuite/gdb.python/py-xmethods.py =================================================================== --- src.orig/gdb/testsuite/gdb.python/py-xmethods.py +++ src/gdb/testsuite/gdb.python/py-xmethods.py @@ -39,6 +39,11 @@ from gdb.xmethod import SimpleXMethodMat return obj["a"] +def A_getarray(obj): + print("From Python :") + return obj["array"] + + def A_getarrayind(obj, index): print("From Python :") return obj["array"][index] @@ -48,12 +53,18 @@ from gdb.xmethod import SimpleXMethodMat return obj["array"][index].reference_value() +def B_getarray(obj): + print("From Python :") + return obj["array"].const_value() + + def B_indexoper(obj, index): return obj["array"][index].const_value().reference_value() type_A = gdb.parse_and_eval("(dop::A *) 0").type.target() type_B = gdb.parse_and_eval("(dop::B *) 0").type.target() +type_array = gdb.parse_and_eval("(int[10] *) 0").type.target() type_int = gdb.parse_and_eval("(int *) 0").type.target() @@ -211,12 +222,18 @@ global_dm_list = [ SimpleXMethodMatcher(r"plus_plus_A", r"^dop::A$", r"operator\+\+", plus_plus_A), SimpleXMethodMatcher(r"A_geta", r"^dop::A$", r"^geta$", A_geta), SimpleXMethodMatcher( + r"A_getarray", r"^dop::A$", r"^getarray$", A_getarray, type_array + ), + SimpleXMethodMatcher( r"A_getarrayind", r"^dop::A$", r"^getarrayind$", A_getarrayind, type_int ), SimpleXMethodMatcher( r"A_indexoper", r"^dop::A$", r"operator\[\]", A_indexoper, type_int ), SimpleXMethodMatcher( + r"B_getarray", r"^dop::B$", r"^getarray$", B_getarray, type_array + ), + SimpleXMethodMatcher( r"B_indexoper", r"^dop::B$", r"operator\[\]", B_indexoper, type_int ), ] 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,9 +1763,8 @@ 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. */ struct value * value_copy (const value *arg) @@ -1765,7 +1775,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, false); val->type = arg->type; VALUE_LVAL (val) = arg->lval; val->location = arg->location; @@ -4162,7 +4172,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. */