Message ID | 20230311114934.3632834-1-grees@undo.io |
---|---|
State | New |
Headers |
Return-Path: <gdb-patches-bounces+patchwork=sourceware.org@sourceware.org> 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 7D73C3858401 for <patchwork@sourceware.org>; Sat, 11 Mar 2023 11:50:05 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7D73C3858401 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1678535405; bh=8g8N6EBKGdM+djhMDLkJqb1DswAD5IBByLsuY6OTsuY=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=qZVbPKk6Sz0QZ+v8/vQW15OgTClwil/goIBBWs1VL7+Xnn5Ocd/zjsvmTyz955gup LAvywGu5fu3uRYjubUBhf59cHl8nd2/jV9le7mvHQx1+PVqMf/HeoZPOzjLkNLKwPT gqlo6QiICTBCi/O3qwDaJkRkucDg5FzMDH4SyksU= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from mail-wr1-x431.google.com (mail-wr1-x431.google.com [IPv6:2a00:1450:4864:20::431]) by sourceware.org (Postfix) with ESMTPS id 7564E3858D28 for <gdb-patches@sourceware.org>; Sat, 11 Mar 2023 11:49:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7564E3858D28 Received: by mail-wr1-x431.google.com with SMTP id r18so7368951wrx.1 for <gdb-patches@sourceware.org>; Sat, 11 Mar 2023 03:49:38 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1678535377; 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=8g8N6EBKGdM+djhMDLkJqb1DswAD5IBByLsuY6OTsuY=; b=5IX/kbC3TyfSurk+F1ffWNnhhxNxmSd3OSz7QRjosPI7zhC3Aqj5A5Ki88Hfuu0YZu VXf/YMZL6HXHR6AQuf0O0PRul7t43FmPXJTloi2tfWzedRui91CFYNiagqHrww63/x/d LC/8E7V9GNa/NmDP+6MqDkZbk7F9Pz79aD6GhWHiEkEDleg72oZCsg7erZQ2YitZMjJu C8pmN5wgOdVqH/BiigM+bpvmg+cCzD/sd9uqZtVMxAJq0KkzZP5LnxMHHW2haG0BUTBl byeP8eNERbG43ylGcMbR+8Gh4blrl9rEWyBn18yhdSS9gcOrZxjfg8DjT1BmoQ5CiJof 846g== X-Gm-Message-State: AO0yUKWxbh1r9EVAzLmCTdZ5ef/qaRjU9GC/HV1/UrKRuHKk9gz9oYlo 7cmM6gjsEMigHr3uY8xGoRLowDmdtgMz8wkV35+jNhIkovoqpm2SHDWIdNoid1q8HgyfGLb8QKV r8JHAk8wNK1g461pW/Bx7Mt9bt6itmfioGZJA5igMOjM4Utu1t5/Q/ggK8Uv7ODYGr1JtTWfSBc o= X-Google-Smtp-Source: AK7set+ueCVyiKbFziGRfRgMPOwwqeYU8VAEQycZTJt4tvtNUcgYNrYei6ZsXircjM7pCBv5wwgf2Q== X-Received: by 2002:a5d:664c:0:b0:2c6:67eb:a9d7 with SMTP id f12-20020a5d664c000000b002c667eba9d7mr17505561wrw.27.1678535376913; Sat, 11 Mar 2023 03:49:36 -0800 (PST) Received: from tilsit-thinkpad.undoers.io (cpc91202-cmbg18-2-0-cust74.5-4.cable.virginm.net. [81.100.88.75]) by smtp.gmail.com with ESMTPSA id f11-20020adff44b000000b002c6e8cb612fsm2280955wrp.92.2023.03.11.03.49.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 11 Mar 2023 03:49:36 -0800 (PST) To: gdb-patches@sourceware.org Cc: Gareth Rees <grees@undo.io> Subject: [PATCH v6] [gdb/mi] Don't treat references to compound values as "simple". Date: Sat, 11 Mar 2023 11:49:34 +0000 Message-Id: <20230311114934.3632834-1-grees@undo.io> X-Mailer: git-send-email 2.26.0 In-Reply-To: <20221020174702.514681-1-grees@undo.io> References: <20221020174702.514681-1-grees@undo.io> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, 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 <gdb-patches.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/gdb-patches>, <mailto:gdb-patches-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/gdb-patches>, <mailto:gdb-patches-request@sourceware.org?subject=subscribe> From: Gareth Rees via Gdb-patches <gdb-patches@sourceware.org> Reply-To: Gareth Rees <grees@undo.io> Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" <gdb-patches-bounces+patchwork=sourceware.org@sourceware.org> |
Series |
[v6,gdb/mi] Don't treat references to compound values as "simple".
|
|
Commit Message
Gareth Rees
March 11, 2023, 11:49 a.m. UTC
SUMMARY The '--simple-values' argument to '-stack-list-arguments' and similar GDB/MI commands does not take reference types into account, so that references to arbitrarily large structures are considered "simple" and printed. This means that the '--simple-values' argument cannot be used by IDEs when tracing the stack due to the time taken to print large structures passed by reference. DETAILS Various GDB/MI commands ('-stack-list-arguments', '-stack-list-locals', '-stack-list-variables' and so on) take a PRINT-VALUES argument which may be '--no-values' (0), '--all-values' (1) or '--simple-values' (2). In the '--simple-values' case, the command is supposed to print the name, type, and value of variables with simple types, and print only the name and type of variables with compound types. The '--simple-values' argument ought to be suitable for IDEs that need to update their user interface with the program's call stack every time the program stops. However, it does not take C++ reference types into account, and this makes the argument unsuitable for this purpose. For example, consider the following C++ program: struct s { int v[10]; }; int sum(const struct s &s) { int total = 0; for (int i = 0; i < 10; ++i) total += s.v[i]; return total; } int main(void) { struct s s = { { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 } }; return sum(s); } If we start GDB in MI mode and continue to 'sum', the behaviour of '-stack-list-arguments' is as follows: (gdb) -stack-list-arguments --simple-values ^done,stack-args=[frame={level="0",args=[{name="s",type="const s &",value="@0x7fffffffe310: {v = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10}}"}]},frame={level="1",args=[]}] Note that the value of the argument 's' was printed, even though 's' is a reference to a structure, which is not a simple value. See https://github.com/microsoft/MIEngine/pull/673 for a case where this behaviour caused Microsoft to avoid the use of '--simple-values' in their MIEngine debug adapter, because it caused Visual Studio Code to take too long to refresh the call stack in the user interface. SOLUTIONS There are two ways we could fix this problem, depending on whether we consider the current behaviour to be a bug. 1. If the current behaviour is a bug, then we can update the behaviour of '--simple-values' so that it takes reference types into account: that is, a value is simple if it is neither an array, struct, or union, nor a reference to an array, struct or union. In this case we must add a feature to the '-list-features' command so that IDEs can detect that it is safe to use the '--simple-values' argument when refreshing the call stack. 2. If the current behaviour is not a bug, then we can add a new option for the PRINT-VALUES argument, for example, '--scalar-values' (3), that would be suitable for use by IDEs. In this case we must add a feature to the '-list-features' command so that IDEs can detect that the '--scalar-values' argument is available for use when refreshing the call stack. PATCH This patch implements solution (1) as I think the current behaviour of not printing structures, but printing references to structures, is contrary to reasonable expectation. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29554 --- gdb/NEWS | 8 +++ gdb/doc/gdb.texinfo | 7 +++ gdb/mi/mi-cmd-stack.c | 9 +-- gdb/mi/mi-cmd-var.c | 27 ++++++--- gdb/mi/mi-cmds.h | 5 ++ gdb/mi/mi-main.c | 7 +-- gdb/python/py-framefilter.c | 6 +- gdb/testsuite/gdb.mi/mi-stack.exp | 16 ++++- gdb/testsuite/gdb.mi/print-simple-values.cc | 62 ++++++++++++++++++++ gdb/testsuite/gdb.mi/print-simple-values.exp | 53 +++++++++++++++++ 10 files changed, 175 insertions(+), 25 deletions(-) create mode 100644 gdb/testsuite/gdb.mi/print-simple-values.cc create mode 100644 gdb/testsuite/gdb.mi/print-simple-values.exp
Comments
This is a patch modifying the behaviour of the --simple-values option but as discussed previously, if GDB maintainers agree that adding a new option is the best approach, I would be happy to submit a patch for that solution instead. On Sat, 11 Mar 2023 at 11:49, Gareth Rees <grees@undo.io> wrote: > > SUMMARY > > The '--simple-values' argument to '-stack-list-arguments' and similar > GDB/MI commands does not take reference types into account, so that > references to arbitrarily large structures are considered "simple" and > printed. This means that the '--simple-values' argument cannot be used > by IDEs when tracing the stack due to the time taken to print large > structures passed by reference. > > DETAILS > > Various GDB/MI commands ('-stack-list-arguments', '-stack-list-locals', > '-stack-list-variables' and so on) take a PRINT-VALUES argument which > may be '--no-values' (0), '--all-values' (1) or '--simple-values' (2). > In the '--simple-values' case, the command is supposed to print the > name, type, and value of variables with simple types, and print only the > name and type of variables with compound types. > > The '--simple-values' argument ought to be suitable for IDEs that need > to update their user interface with the program's call stack every time > the program stops. However, it does not take C++ reference types into > account, and this makes the argument unsuitable for this purpose. > > For example, consider the following C++ program: > > struct s { > int v[10]; > }; > > int > sum(const struct s &s) > { > int total = 0; > for (int i = 0; i < 10; ++i) total += s.v[i]; > return total; > } > > int > main(void) > { > struct s s = { { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 } }; > return sum(s); > } > > If we start GDB in MI mode and continue to 'sum', the behaviour of > '-stack-list-arguments' is as follows: > > (gdb) > -stack-list-arguments --simple-values > ^done,stack-args=[frame={level="0",args=[{name="s",type="const s &",value="@0x7fffffffe310: {v = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10}}"}]},frame={level="1",args=[]}] > > Note that the value of the argument 's' was printed, even though 's' is > a reference to a structure, which is not a simple value. > > See https://github.com/microsoft/MIEngine/pull/673 for a case where this > behaviour caused Microsoft to avoid the use of '--simple-values' in > their MIEngine debug adapter, because it caused Visual Studio Code to > take too long to refresh the call stack in the user interface. > > SOLUTIONS > > There are two ways we could fix this problem, depending on whether we > consider the current behaviour to be a bug. > > 1. If the current behaviour is a bug, then we can update the behaviour > of '--simple-values' so that it takes reference types into account: > that is, a value is simple if it is neither an array, struct, or > union, nor a reference to an array, struct or union. > > In this case we must add a feature to the '-list-features' command so > that IDEs can detect that it is safe to use the '--simple-values' > argument when refreshing the call stack. > > 2. If the current behaviour is not a bug, then we can add a new option > for the PRINT-VALUES argument, for example, '--scalar-values' (3), > that would be suitable for use by IDEs. > > In this case we must add a feature to the '-list-features' command > so that IDEs can detect that the '--scalar-values' argument is > available for use when refreshing the call stack. > > PATCH > > This patch implements solution (1) as I think the current behaviour of > not printing structures, but printing references to structures, is > contrary to reasonable expectation. > > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29554 > --- > gdb/NEWS | 8 +++ > gdb/doc/gdb.texinfo | 7 +++ > gdb/mi/mi-cmd-stack.c | 9 +-- > gdb/mi/mi-cmd-var.c | 27 ++++++--- > gdb/mi/mi-cmds.h | 5 ++ > gdb/mi/mi-main.c | 7 +-- > gdb/python/py-framefilter.c | 6 +- > gdb/testsuite/gdb.mi/mi-stack.exp | 16 ++++- > gdb/testsuite/gdb.mi/print-simple-values.cc | 62 ++++++++++++++++++++ > gdb/testsuite/gdb.mi/print-simple-values.exp | 53 +++++++++++++++++ > 10 files changed, 175 insertions(+), 25 deletions(-) > create mode 100644 gdb/testsuite/gdb.mi/print-simple-values.cc > create mode 100644 gdb/testsuite/gdb.mi/print-simple-values.exp > > diff --git a/gdb/NEWS b/gdb/NEWS > index cc262f1f8a6..1971b76a736 100644 > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -104,6 +104,14 @@ show always-read-ctf > without a thread restriction. The same is also true for the 'task' > field of an Ada task-specific breakpoint. > > +** The '--simple-values' argument to the '-stack-list-arguments', > + '-stack-list-locals', '-stack-list-variables', and '-var-list-children' > + commands now takes reference types into account: that is, a value is now > + considered simple if it is neither an array, structure, or union, nor a > + reference to an array, structure, or union. (Previously all references were > + considered simple.) Support for this feature can be verified by using the > + '-list-features' command, which should contain "simple-values-ref-types". > + > *** Changes in GDB 13 > > * MI version 1 is deprecated, and will be removed in GDB 14. > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index 954f1481dae..dff7ba374bd 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -37948,6 +37948,13 @@ option (@pxref{GDB/MI Program Execution}). > @item data-disassemble-a-option > Indicates that the @code{-data-disassemble} command supports the @option{-a} > option (@pxref{GDB/MI Data Manipulation}). > +@item simple-values-ref-types > +Indicates that the @code{--simple-values} argument to the > +@code{-stack-list-arguments}, @code{-stack-list-locals}, > +@code{-stack-list-variables}, and @code{-var-list-children} commands > +takes reference types into account: that is, a value is considered > +simple if it is neither an array, structure, or union, nor a reference > +to an array, structure, or union. > @end ftable > > @findex -list-target-features > diff --git a/gdb/mi/mi-cmd-stack.c b/gdb/mi/mi-cmd-stack.c > index 4c4662ab5d7..406bf85ceec 100644 > --- a/gdb/mi/mi-cmd-stack.c > +++ b/gdb/mi/mi-cmd-stack.c > @@ -645,13 +645,8 @@ list_args_or_locals (const frame_print_options &fp_opts, > switch (values) > { > case PRINT_SIMPLE_VALUES: > - { > - struct type *type = check_typedef (sym2->type ()); > - if (type->code () == TYPE_CODE_ARRAY > - || type->code () == TYPE_CODE_STRUCT > - || type->code () == TYPE_CODE_UNION) > - break; > - } > + if (!mi_simple_type_p (sym2->type ())) > + break; > /* FALLTHROUGH */ > > case PRINT_ALL_VALUES: > diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c > index e026c239b87..36eea4619a7 100644 > --- a/gdb/mi/mi-cmd-var.c > +++ b/gdb/mi/mi-cmd-var.c > @@ -335,14 +335,27 @@ mi_print_value_p (struct varobj *var, enum print_values print_values) > if (type == NULL) > return 1; > else > - { > - type = check_typedef (type); > + return mi_simple_type_p (type); > +} > > - /* For PRINT_SIMPLE_VALUES, only print the value if it has a type > - and that type is not a compound type. */ > - return (type->code () != TYPE_CODE_ARRAY > - && type->code () != TYPE_CODE_STRUCT > - && type->code () != TYPE_CODE_UNION); > +/* See mi-cmds.h. */ > + > +bool > +mi_simple_type_p (struct type *type) > +{ > + type = check_typedef (type); > + > + if (TYPE_IS_REFERENCE (type)) > + type = check_typedef (type->target_type ()); > + > + switch (type->code ()) > + { > + case TYPE_CODE_ARRAY: > + case TYPE_CODE_STRUCT: > + case TYPE_CODE_UNION: > + return false; > + default: > + return true; > } > } > > diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h > index 490f50484d9..d867c298df9 100644 > --- a/gdb/mi/mi-cmds.h > +++ b/gdb/mi/mi-cmds.h > @@ -226,4 +226,9 @@ using remove_mi_cmd_entries_ftype > = gdb::function_view<bool (mi_command *)>; > extern void remove_mi_cmd_entries (remove_mi_cmd_entries_ftype callback); > > +/* Return true if type is a simple type: that is, neither an array, structure, > + or union, nor a reference to an array, structure, or union. */ > + > +extern bool mi_simple_type_p (struct type *type); > + > #endif /* MI_MI_CMDS_H */ > diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c > index 0013e5dfafd..cedad60d0b8 100644 > --- a/gdb/mi/mi-main.c > +++ b/gdb/mi/mi-main.c > @@ -1656,6 +1656,7 @@ mi_cmd_list_features (const char *command, char **argv, int argc) > uiout->field_string (NULL, "undefined-command-error-code"); > uiout->field_string (NULL, "exec-run-start-option"); > uiout->field_string (NULL, "data-disassemble-a-option"); > + uiout->field_string (NULL, "simple-values-ref-types"); > > if (ext_lang_initialized_p (get_ext_lang_defn (EXT_LANG_PYTHON))) > uiout->field_string (NULL, "python"); > @@ -2458,7 +2459,6 @@ static void > print_variable_or_computed (const char *expression, enum print_values values) > { > struct value *val; > - struct type *type; > struct ui_out *uiout = current_uiout; > > string_file stb; > @@ -2478,12 +2478,9 @@ print_variable_or_computed (const char *expression, enum print_values values) > switch (values) > { > case PRINT_SIMPLE_VALUES: > - type = check_typedef (val->type ()); > type_print (val->type (), "", &stb, -1); > uiout->field_stream ("type", stb); > - if (type->code () != TYPE_CODE_ARRAY > - && type->code () != TYPE_CODE_STRUCT > - && type->code () != TYPE_CODE_UNION) > + if (mi_simple_type_p (val->type ())) > { > struct value_print_options opts; > > diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c > index 0e8b2409636..e555dc3d879 100644 > --- a/gdb/python/py-framefilter.c > +++ b/gdb/python/py-framefilter.c > @@ -235,14 +235,10 @@ py_print_value (struct ui_out *out, struct value *val, > if (args_type == MI_PRINT_SIMPLE_VALUES > || args_type == MI_PRINT_ALL_VALUES) > { > - struct type *type = check_typedef (val->type ()); > - > if (args_type == MI_PRINT_ALL_VALUES) > should_print = 1; > else if (args_type == MI_PRINT_SIMPLE_VALUES > - && type->code () != TYPE_CODE_ARRAY > - && type->code () != TYPE_CODE_STRUCT > - && type->code () != TYPE_CODE_UNION) > + && mi_simple_type_p (val->type ())) > should_print = 1; > } > else if (args_type != NO_VALUES) > diff --git a/gdb/testsuite/gdb.mi/mi-stack.exp b/gdb/testsuite/gdb.mi/mi-stack.exp > index 633e090c13a..777a425894f 100644 > --- a/gdb/testsuite/gdb.mi/mi-stack.exp > +++ b/gdb/testsuite/gdb.mi/mi-stack.exp > @@ -87,6 +87,9 @@ proc test_stack_args_listing {} { > # -stack-list-arguments 1 1 1 > # -stack-list-arguments 1 1 3 > # -stack-list-arguments > + # -stack-list-arguments 1 1 300 > + # -stack-list-arguments 2 1 1 > + # -stack-list-arguments --simple-values 1 1 > > mi_gdb_test "231-stack-list-arguments 0" \ > "231\\^done,stack-args=\\\[frame=\{level=\"0\",args=\\\[\\\]\},frame=\{level=\"1\",args=\\\[name=\"strarg\"\\\]\},frame=\{level=\"2\",args=\\\[name=\"intarg\",name=\"strarg\"\\\]\},frame=\{level=\"3\",args=\\\[name=\"intarg\",name=\"strarg\",name=\"fltarg\"\\\]\},frame=\{level=\"4\",args=\\\[\\\]\}\\\]" \ > @@ -119,6 +122,14 @@ proc test_stack_args_listing {} { > mi_gdb_test "235-stack-list-arguments 1 1 300" \ > "235\\^done,stack-args=\\\[frame=\{level=\"1\",args=\\\[\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\}\\\]\},frame=\{level=\"2\",args=\\\[\{name=\"intarg\",value=\"2\"\},\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\}\\\]\},frame=\{level=\"3\",args=\\\[\{name=\"intarg\",value=\"2\"\},\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\},\{name=\"fltarg\",value=\"3.5\"\}\\\]\},frame=\{level=\"4\",args=\\\[\\\]\}\\\]" \ > "stack args listing 1 1 300" > + > + mi_gdb_test "236-stack-list-arguments 2 1 1" \ > + "236\\^done,stack-args=\\\[frame=\\{level=\"1\",args=\\\[\\{name=\"strarg\",type=\"char \\*\",value=\"$hex \\\\\"A string argument.\\\\\"\"\\}\\\]\\}\\\]" \ > + "stack args listing 2 1 1" > + > + mi_gdb_test "237-stack-list-arguments --simple-values 1 1" \ > + "237\\^done,stack-args=\\\[frame=\\{level=\"1\",args=\\\[\\{name=\"strarg\",type=\"char \\*\",value=\"$hex \\\\\"A string argument.\\\\\"\"\\}\\\]\\}\\\]" \ > + "stack args listing --simple-values 1 1" > } > > proc test_stack_info_depth {} { > @@ -157,7 +168,6 @@ proc test_stack_locals_listing {} { > # -stack-list-locals 0 (--no-values) > # -stack-list-locals 1 (--all-values) > # -stack-list-locals 2 (--simple-values) > - # -stack-list-arguments > > mi_gdb_test "232-stack-list-locals 0" \ > "232\\^done,locals=\\\[name=\"A\",name=\"B\",name=\"C\",name=\"D\"\\\]" \ > @@ -173,6 +183,10 @@ proc test_stack_locals_listing {} { > "232\\^done,locals=\\\[\{name=\"A\",value=\"1\"\},\{name=\"B\",value=\"2\"\},\{name=\"C\",value=\"3\"\},\{name=\"D\",value=\"\\{0, 1, 2\\}\"\}\\\]" \ > "stack locals listing of names and values" > > + mi_gdb_test "232-stack-list-locals 2" \ > + "232\\^done,locals=\\\[\{name=\"A\",type=\"int\",value=\"1\"\},\{name=\"B\",type=\"int\",value=\"2\"\},\{name=\"C\",type=\"int\",value=\"3\"\},\{name=\"D\",type=\"int \\\[3\\\]\"\}\\\]" \ > + "stack locals listing 2" > + > mi_gdb_test "232-stack-list-locals --simple-values" \ > "232\\^done,locals=\\\[\{name=\"A\",type=\"int\",value=\"1\"\},\{name=\"B\",type=\"int\",value=\"2\"\},\{name=\"C\",type=\"int\",value=\"3\"\},\{name=\"D\",type=\"int \\\[3\\\]\"\}\\\]" \ > "stack locals listing, simple types: names and values, complex type: names and types" > diff --git a/gdb/testsuite/gdb.mi/print-simple-values.cc b/gdb/testsuite/gdb.mi/print-simple-values.cc > new file mode 100644 > index 00000000000..1aad724efb9 > --- /dev/null > +++ b/gdb/testsuite/gdb.mi/print-simple-values.cc > @@ -0,0 +1,62 @@ > +/* This test case is part of GDB, the GNU debugger. > + > + Copyright 2022-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 <http://www.gnu.org/licenses/>. */ > + > +/* Test program for PRINT_SIMPLE_VALUES. > + > + In the function f: > + > + * The arguments i, ir, and irr are ints or references to ints, which > + must be printed by PRINT_SIMPLE_VALUES. > + > + * The arguments a, s, and u are non-scalar values, which must not be > + printed by PRINT_SIMPLE_VALUES. > + > + * The arguments ar, arr, sr, srr, ur, and urr are references to > + non-scalar values, which must not be printed by > + PRINT_SIMPLE_VALUES. */ > + > +struct s > +{ > + int v; > +}; > + > +union u > +{ > + int v; > +}; > + > +int > +f (int i, int &ir, int &&irr, > + int a[1], int (&ar)[1], int (&&arr)[1], > + struct s s, struct s &sr, struct s &&srr, > + union u u, union u &ur, union u &&urr) > +{ > + return (i + ir + irr > + + a[0] + ar[0] + arr[0] > + + s.v + sr.v + srr.v > + + u.v + ur.v + urr.v); > +} > + > +int > +main (void) > +{ > + int i = 1, j = 2; > + int a[1] = { 4 }, b[1] = { 5 }; > + struct s s = { 7 }, t = { 8 }; > + union u u = { 10 }, v = { 11 }; > + return f (i, j, 3, a, b, { 6 }, s, t, { 9 }, u, v, { 12 }); > +} > diff --git a/gdb/testsuite/gdb.mi/print-simple-values.exp b/gdb/testsuite/gdb.mi/print-simple-values.exp > new file mode 100644 > index 00000000000..9436645df84 > --- /dev/null > +++ b/gdb/testsuite/gdb.mi/print-simple-values.exp > @@ -0,0 +1,53 @@ > +# Copyright 2022-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 <http://www.gnu.org/licenses/>. > + > +# Test of PRINT_SIMPLE_VALUES. > +# > +# Test that PRINT_SIMPLE_VALUES distinguishes simple and compound types, > +# including C++ reference and rvalue reference types. > + > +require allow_cplus_tests > + > +load_lib mi-support.exp > +set MIFLAGS "-i=mi" > + > +standard_testfile .cc > + > +if [build_executable "failed to prepare" $testfile $srcfile {debug c++}] { > + return -1 > +} > + > +if [mi_clean_restart $binfile] { > + return > +} > + > +mi_runto_main > + > +mi_gdb_test "-break-insert f" "\\^done.*" "set breakpoint on f" > + > +mi_send_resuming_command "exec-continue" "exec-continue to breakpoint on f" > + > +mi_expect_stop "breakpoint-hit" "f" ".*" ".*" ".*" {.* disp="keep"} \ > + "run until breakpoint on f" > + > +mi_gdb_test "-stack-list-arguments 2" \ > + "\\^done,stack-args=\\\[frame=\\{level=\"0\",args=\\\[\\{name=\"i\",type=\"int\",value=\"1\"\\},\\{name=\"ir\",type=\"int &\",value=\"@$hex: 2\"\\},\\{name=\"irr\",type=\"int &&\",value=\"@$hex: 3\"\\},\\{name=\"a\",type=\"int \\*\",value=\"$hex\"\\},\\{name=\"ar\",type=\"int \\(&\\)\\\[1\\\]\"\\},\\{name=\"arr\",type=\"int \\(&&\\)\\\[1\\\]\"\\},\\{name=\"s\",type=\"s\"\\},\\{name=\"sr\",type=\"s &\"\\},\\{name=\"srr\",type=\"s &&\"\\},\\{name=\"u\",type=\"u\"\\},\\{name=\"ur\",type=\"u &\"\\},\\{name=\"urr\",type=\"u &&\"\\}\\\]\\},frame=\\{level=\"1\",args=\\\[\\\]\\}\\\]" \ > + "stack arguments listing 2" > + > +mi_gdb_test "-stack-list-arguments --simple-values" \ > + "\\^done,stack-args=\\\[frame=\\{level=\"0\",args=\\\[\\{name=\"i\",type=\"int\",value=\"1\"\\},\\{name=\"ir\",type=\"int &\",value=\"@$hex: 2\"\\},\\{name=\"irr\",type=\"int &&\",value=\"@$hex: 3\"\\},\\{name=\"a\",type=\"int \\*\",value=\"$hex\"\\},\\{name=\"ar\",type=\"int \\(&\\)\\\[1\\\]\"\\},\\{name=\"arr\",type=\"int \\(&&\\)\\\[1\\\]\"\\},\\{name=\"s\",type=\"s\"\\},\\{name=\"sr\",type=\"s &\"\\},\\{name=\"srr\",type=\"s &&\"\\},\\{name=\"u\",type=\"u\"\\},\\{name=\"ur\",type=\"u &\"\\},\\{name=\"urr\",type=\"u &&\"\\}\\\]\\},frame=\\{level=\"1\",args=\\\[\\\]\\}\\\]" \ > + "stack arguments listing --simple-values" > + > +mi_gdb_exit > -- > 2.26.0 >
Second ping. On Tue, 21 Mar 2023 at 09:50, Gareth Rees <grees@undo.io> wrote: > > This is a patch modifying the behaviour of the --simple-values option > but as discussed previously, if GDB maintainers agree that adding a > new option is the best approach, I would be happy to submit a patch > for that solution instead. > > On Sat, 11 Mar 2023 at 11:49, Gareth Rees <grees@undo.io> wrote: > > > > SUMMARY > > > > The '--simple-values' argument to '-stack-list-arguments' and similar > > GDB/MI commands does not take reference types into account, so that > > references to arbitrarily large structures are considered "simple" and > > printed. This means that the '--simple-values' argument cannot be used > > by IDEs when tracing the stack due to the time taken to print large > > structures passed by reference. > > > > DETAILS > > > > Various GDB/MI commands ('-stack-list-arguments', '-stack-list-locals', > > '-stack-list-variables' and so on) take a PRINT-VALUES argument which > > may be '--no-values' (0), '--all-values' (1) or '--simple-values' (2). > > In the '--simple-values' case, the command is supposed to print the > > name, type, and value of variables with simple types, and print only the > > name and type of variables with compound types. > > > > The '--simple-values' argument ought to be suitable for IDEs that need > > to update their user interface with the program's call stack every time > > the program stops. However, it does not take C++ reference types into > > account, and this makes the argument unsuitable for this purpose. > > > > For example, consider the following C++ program: > > > > struct s { > > int v[10]; > > }; > > > > int > > sum(const struct s &s) > > { > > int total = 0; > > for (int i = 0; i < 10; ++i) total += s.v[i]; > > return total; > > } > > > > int > > main(void) > > { > > struct s s = { { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 } }; > > return sum(s); > > } > > > > If we start GDB in MI mode and continue to 'sum', the behaviour of > > '-stack-list-arguments' is as follows: > > > > (gdb) > > -stack-list-arguments --simple-values > > ^done,stack-args=[frame={level="0",args=[{name="s",type="const s &",value="@0x7fffffffe310: {v = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10}}"}]},frame={level="1",args=[]}] > > > > Note that the value of the argument 's' was printed, even though 's' is > > a reference to a structure, which is not a simple value. > > > > See https://github.com/microsoft/MIEngine/pull/673 for a case where this > > behaviour caused Microsoft to avoid the use of '--simple-values' in > > their MIEngine debug adapter, because it caused Visual Studio Code to > > take too long to refresh the call stack in the user interface. > > > > SOLUTIONS > > > > There are two ways we could fix this problem, depending on whether we > > consider the current behaviour to be a bug. > > > > 1. If the current behaviour is a bug, then we can update the behaviour > > of '--simple-values' so that it takes reference types into account: > > that is, a value is simple if it is neither an array, struct, or > > union, nor a reference to an array, struct or union. > > > > In this case we must add a feature to the '-list-features' command so > > that IDEs can detect that it is safe to use the '--simple-values' > > argument when refreshing the call stack. > > > > 2. If the current behaviour is not a bug, then we can add a new option > > for the PRINT-VALUES argument, for example, '--scalar-values' (3), > > that would be suitable for use by IDEs. > > > > In this case we must add a feature to the '-list-features' command > > so that IDEs can detect that the '--scalar-values' argument is > > available for use when refreshing the call stack. > > > > PATCH > > > > This patch implements solution (1) as I think the current behaviour of > > not printing structures, but printing references to structures, is > > contrary to reasonable expectation. > > > > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29554 > > --- > > gdb/NEWS | 8 +++ > > gdb/doc/gdb.texinfo | 7 +++ > > gdb/mi/mi-cmd-stack.c | 9 +-- > > gdb/mi/mi-cmd-var.c | 27 ++++++--- > > gdb/mi/mi-cmds.h | 5 ++ > > gdb/mi/mi-main.c | 7 +-- > > gdb/python/py-framefilter.c | 6 +- > > gdb/testsuite/gdb.mi/mi-stack.exp | 16 ++++- > > gdb/testsuite/gdb.mi/print-simple-values.cc | 62 ++++++++++++++++++++ > > gdb/testsuite/gdb.mi/print-simple-values.exp | 53 +++++++++++++++++ > > 10 files changed, 175 insertions(+), 25 deletions(-) > > create mode 100644 gdb/testsuite/gdb.mi/print-simple-values.cc > > create mode 100644 gdb/testsuite/gdb.mi/print-simple-values.exp > > > > diff --git a/gdb/NEWS b/gdb/NEWS > > index cc262f1f8a6..1971b76a736 100644 > > --- a/gdb/NEWS > > +++ b/gdb/NEWS > > @@ -104,6 +104,14 @@ show always-read-ctf > > without a thread restriction. The same is also true for the 'task' > > field of an Ada task-specific breakpoint. > > > > +** The '--simple-values' argument to the '-stack-list-arguments', > > + '-stack-list-locals', '-stack-list-variables', and '-var-list-children' > > + commands now takes reference types into account: that is, a value is now > > + considered simple if it is neither an array, structure, or union, nor a > > + reference to an array, structure, or union. (Previously all references were > > + considered simple.) Support for this feature can be verified by using the > > + '-list-features' command, which should contain "simple-values-ref-types". > > + > > *** Changes in GDB 13 > > > > * MI version 1 is deprecated, and will be removed in GDB 14. > > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > > index 954f1481dae..dff7ba374bd 100644 > > --- a/gdb/doc/gdb.texinfo > > +++ b/gdb/doc/gdb.texinfo > > @@ -37948,6 +37948,13 @@ option (@pxref{GDB/MI Program Execution}). > > @item data-disassemble-a-option > > Indicates that the @code{-data-disassemble} command supports the @option{-a} > > option (@pxref{GDB/MI Data Manipulation}). > > +@item simple-values-ref-types > > +Indicates that the @code{--simple-values} argument to the > > +@code{-stack-list-arguments}, @code{-stack-list-locals}, > > +@code{-stack-list-variables}, and @code{-var-list-children} commands > > +takes reference types into account: that is, a value is considered > > +simple if it is neither an array, structure, or union, nor a reference > > +to an array, structure, or union. > > @end ftable > > > > @findex -list-target-features > > diff --git a/gdb/mi/mi-cmd-stack.c b/gdb/mi/mi-cmd-stack.c > > index 4c4662ab5d7..406bf85ceec 100644 > > --- a/gdb/mi/mi-cmd-stack.c > > +++ b/gdb/mi/mi-cmd-stack.c > > @@ -645,13 +645,8 @@ list_args_or_locals (const frame_print_options &fp_opts, > > switch (values) > > { > > case PRINT_SIMPLE_VALUES: > > - { > > - struct type *type = check_typedef (sym2->type ()); > > - if (type->code () == TYPE_CODE_ARRAY > > - || type->code () == TYPE_CODE_STRUCT > > - || type->code () == TYPE_CODE_UNION) > > - break; > > - } > > + if (!mi_simple_type_p (sym2->type ())) > > + break; > > /* FALLTHROUGH */ > > > > case PRINT_ALL_VALUES: > > diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c > > index e026c239b87..36eea4619a7 100644 > > --- a/gdb/mi/mi-cmd-var.c > > +++ b/gdb/mi/mi-cmd-var.c > > @@ -335,14 +335,27 @@ mi_print_value_p (struct varobj *var, enum print_values print_values) > > if (type == NULL) > > return 1; > > else > > - { > > - type = check_typedef (type); > > + return mi_simple_type_p (type); > > +} > > > > - /* For PRINT_SIMPLE_VALUES, only print the value if it has a type > > - and that type is not a compound type. */ > > - return (type->code () != TYPE_CODE_ARRAY > > - && type->code () != TYPE_CODE_STRUCT > > - && type->code () != TYPE_CODE_UNION); > > +/* See mi-cmds.h. */ > > + > > +bool > > +mi_simple_type_p (struct type *type) > > +{ > > + type = check_typedef (type); > > + > > + if (TYPE_IS_REFERENCE (type)) > > + type = check_typedef (type->target_type ()); > > + > > + switch (type->code ()) > > + { > > + case TYPE_CODE_ARRAY: > > + case TYPE_CODE_STRUCT: > > + case TYPE_CODE_UNION: > > + return false; > > + default: > > + return true; > > } > > } > > > > diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h > > index 490f50484d9..d867c298df9 100644 > > --- a/gdb/mi/mi-cmds.h > > +++ b/gdb/mi/mi-cmds.h > > @@ -226,4 +226,9 @@ using remove_mi_cmd_entries_ftype > > = gdb::function_view<bool (mi_command *)>; > > extern void remove_mi_cmd_entries (remove_mi_cmd_entries_ftype callback); > > > > +/* Return true if type is a simple type: that is, neither an array, structure, > > + or union, nor a reference to an array, structure, or union. */ > > + > > +extern bool mi_simple_type_p (struct type *type); > > + > > #endif /* MI_MI_CMDS_H */ > > diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c > > index 0013e5dfafd..cedad60d0b8 100644 > > --- a/gdb/mi/mi-main.c > > +++ b/gdb/mi/mi-main.c > > @@ -1656,6 +1656,7 @@ mi_cmd_list_features (const char *command, char **argv, int argc) > > uiout->field_string (NULL, "undefined-command-error-code"); > > uiout->field_string (NULL, "exec-run-start-option"); > > uiout->field_string (NULL, "data-disassemble-a-option"); > > + uiout->field_string (NULL, "simple-values-ref-types"); > > > > if (ext_lang_initialized_p (get_ext_lang_defn (EXT_LANG_PYTHON))) > > uiout->field_string (NULL, "python"); > > @@ -2458,7 +2459,6 @@ static void > > print_variable_or_computed (const char *expression, enum print_values values) > > { > > struct value *val; > > - struct type *type; > > struct ui_out *uiout = current_uiout; > > > > string_file stb; > > @@ -2478,12 +2478,9 @@ print_variable_or_computed (const char *expression, enum print_values values) > > switch (values) > > { > > case PRINT_SIMPLE_VALUES: > > - type = check_typedef (val->type ()); > > type_print (val->type (), "", &stb, -1); > > uiout->field_stream ("type", stb); > > - if (type->code () != TYPE_CODE_ARRAY > > - && type->code () != TYPE_CODE_STRUCT > > - && type->code () != TYPE_CODE_UNION) > > + if (mi_simple_type_p (val->type ())) > > { > > struct value_print_options opts; > > > > diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c > > index 0e8b2409636..e555dc3d879 100644 > > --- a/gdb/python/py-framefilter.c > > +++ b/gdb/python/py-framefilter.c > > @@ -235,14 +235,10 @@ py_print_value (struct ui_out *out, struct value *val, > > if (args_type == MI_PRINT_SIMPLE_VALUES > > || args_type == MI_PRINT_ALL_VALUES) > > { > > - struct type *type = check_typedef (val->type ()); > > - > > if (args_type == MI_PRINT_ALL_VALUES) > > should_print = 1; > > else if (args_type == MI_PRINT_SIMPLE_VALUES > > - && type->code () != TYPE_CODE_ARRAY > > - && type->code () != TYPE_CODE_STRUCT > > - && type->code () != TYPE_CODE_UNION) > > + && mi_simple_type_p (val->type ())) > > should_print = 1; > > } > > else if (args_type != NO_VALUES) > > diff --git a/gdb/testsuite/gdb.mi/mi-stack.exp b/gdb/testsuite/gdb.mi/mi-stack.exp > > index 633e090c13a..777a425894f 100644 > > --- a/gdb/testsuite/gdb.mi/mi-stack.exp > > +++ b/gdb/testsuite/gdb.mi/mi-stack.exp > > @@ -87,6 +87,9 @@ proc test_stack_args_listing {} { > > # -stack-list-arguments 1 1 1 > > # -stack-list-arguments 1 1 3 > > # -stack-list-arguments > > + # -stack-list-arguments 1 1 300 > > + # -stack-list-arguments 2 1 1 > > + # -stack-list-arguments --simple-values 1 1 > > > > mi_gdb_test "231-stack-list-arguments 0" \ > > "231\\^done,stack-args=\\\[frame=\{level=\"0\",args=\\\[\\\]\},frame=\{level=\"1\",args=\\\[name=\"strarg\"\\\]\},frame=\{level=\"2\",args=\\\[name=\"intarg\",name=\"strarg\"\\\]\},frame=\{level=\"3\",args=\\\[name=\"intarg\",name=\"strarg\",name=\"fltarg\"\\\]\},frame=\{level=\"4\",args=\\\[\\\]\}\\\]" \ > > @@ -119,6 +122,14 @@ proc test_stack_args_listing {} { > > mi_gdb_test "235-stack-list-arguments 1 1 300" \ > > "235\\^done,stack-args=\\\[frame=\{level=\"1\",args=\\\[\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\}\\\]\},frame=\{level=\"2\",args=\\\[\{name=\"intarg\",value=\"2\"\},\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\}\\\]\},frame=\{level=\"3\",args=\\\[\{name=\"intarg\",value=\"2\"\},\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\},\{name=\"fltarg\",value=\"3.5\"\}\\\]\},frame=\{level=\"4\",args=\\\[\\\]\}\\\]" \ > > "stack args listing 1 1 300" > > + > > + mi_gdb_test "236-stack-list-arguments 2 1 1" \ > > + "236\\^done,stack-args=\\\[frame=\\{level=\"1\",args=\\\[\\{name=\"strarg\",type=\"char \\*\",value=\"$hex \\\\\"A string argument.\\\\\"\"\\}\\\]\\}\\\]" \ > > + "stack args listing 2 1 1" > > + > > + mi_gdb_test "237-stack-list-arguments --simple-values 1 1" \ > > + "237\\^done,stack-args=\\\[frame=\\{level=\"1\",args=\\\[\\{name=\"strarg\",type=\"char \\*\",value=\"$hex \\\\\"A string argument.\\\\\"\"\\}\\\]\\}\\\]" \ > > + "stack args listing --simple-values 1 1" > > } > > > > proc test_stack_info_depth {} { > > @@ -157,7 +168,6 @@ proc test_stack_locals_listing {} { > > # -stack-list-locals 0 (--no-values) > > # -stack-list-locals 1 (--all-values) > > # -stack-list-locals 2 (--simple-values) > > - # -stack-list-arguments > > > > mi_gdb_test "232-stack-list-locals 0" \ > > "232\\^done,locals=\\\[name=\"A\",name=\"B\",name=\"C\",name=\"D\"\\\]" \ > > @@ -173,6 +183,10 @@ proc test_stack_locals_listing {} { > > "232\\^done,locals=\\\[\{name=\"A\",value=\"1\"\},\{name=\"B\",value=\"2\"\},\{name=\"C\",value=\"3\"\},\{name=\"D\",value=\"\\{0, 1, 2\\}\"\}\\\]" \ > > "stack locals listing of names and values" > > > > + mi_gdb_test "232-stack-list-locals 2" \ > > + "232\\^done,locals=\\\[\{name=\"A\",type=\"int\",value=\"1\"\},\{name=\"B\",type=\"int\",value=\"2\"\},\{name=\"C\",type=\"int\",value=\"3\"\},\{name=\"D\",type=\"int \\\[3\\\]\"\}\\\]" \ > > + "stack locals listing 2" > > + > > mi_gdb_test "232-stack-list-locals --simple-values" \ > > "232\\^done,locals=\\\[\{name=\"A\",type=\"int\",value=\"1\"\},\{name=\"B\",type=\"int\",value=\"2\"\},\{name=\"C\",type=\"int\",value=\"3\"\},\{name=\"D\",type=\"int \\\[3\\\]\"\}\\\]" \ > > "stack locals listing, simple types: names and values, complex type: names and types" > > diff --git a/gdb/testsuite/gdb.mi/print-simple-values.cc b/gdb/testsuite/gdb.mi/print-simple-values.cc > > new file mode 100644 > > index 00000000000..1aad724efb9 > > --- /dev/null > > +++ b/gdb/testsuite/gdb.mi/print-simple-values.cc > > @@ -0,0 +1,62 @@ > > +/* This test case is part of GDB, the GNU debugger. > > + > > + Copyright 2022-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 <http://www.gnu.org/licenses/>. */ > > + > > +/* Test program for PRINT_SIMPLE_VALUES. > > + > > + In the function f: > > + > > + * The arguments i, ir, and irr are ints or references to ints, which > > + must be printed by PRINT_SIMPLE_VALUES. > > + > > + * The arguments a, s, and u are non-scalar values, which must not be > > + printed by PRINT_SIMPLE_VALUES. > > + > > + * The arguments ar, arr, sr, srr, ur, and urr are references to > > + non-scalar values, which must not be printed by > > + PRINT_SIMPLE_VALUES. */ > > + > > +struct s > > +{ > > + int v; > > +}; > > + > > +union u > > +{ > > + int v; > > +}; > > + > > +int > > +f (int i, int &ir, int &&irr, > > + int a[1], int (&ar)[1], int (&&arr)[1], > > + struct s s, struct s &sr, struct s &&srr, > > + union u u, union u &ur, union u &&urr) > > +{ > > + return (i + ir + irr > > + + a[0] + ar[0] + arr[0] > > + + s.v + sr.v + srr.v > > + + u.v + ur.v + urr.v); > > +} > > + > > +int > > +main (void) > > +{ > > + int i = 1, j = 2; > > + int a[1] = { 4 }, b[1] = { 5 }; > > + struct s s = { 7 }, t = { 8 }; > > + union u u = { 10 }, v = { 11 }; > > + return f (i, j, 3, a, b, { 6 }, s, t, { 9 }, u, v, { 12 }); > > +} > > diff --git a/gdb/testsuite/gdb.mi/print-simple-values.exp b/gdb/testsuite/gdb.mi/print-simple-values.exp > > new file mode 100644 > > index 00000000000..9436645df84 > > --- /dev/null > > +++ b/gdb/testsuite/gdb.mi/print-simple-values.exp > > @@ -0,0 +1,53 @@ > > +# Copyright 2022-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 <http://www.gnu.org/licenses/>. > > + > > +# Test of PRINT_SIMPLE_VALUES. > > +# > > +# Test that PRINT_SIMPLE_VALUES distinguishes simple and compound types, > > +# including C++ reference and rvalue reference types. > > + > > +require allow_cplus_tests > > + > > +load_lib mi-support.exp > > +set MIFLAGS "-i=mi" > > + > > +standard_testfile .cc > > + > > +if [build_executable "failed to prepare" $testfile $srcfile {debug c++}] { > > + return -1 > > +} > > + > > +if [mi_clean_restart $binfile] { > > + return > > +} > > + > > +mi_runto_main > > + > > +mi_gdb_test "-break-insert f" "\\^done.*" "set breakpoint on f" > > + > > +mi_send_resuming_command "exec-continue" "exec-continue to breakpoint on f" > > + > > +mi_expect_stop "breakpoint-hit" "f" ".*" ".*" ".*" {.* disp="keep"} \ > > + "run until breakpoint on f" > > + > > +mi_gdb_test "-stack-list-arguments 2" \ > > + "\\^done,stack-args=\\\[frame=\\{level=\"0\",args=\\\[\\{name=\"i\",type=\"int\",value=\"1\"\\},\\{name=\"ir\",type=\"int &\",value=\"@$hex: 2\"\\},\\{name=\"irr\",type=\"int &&\",value=\"@$hex: 3\"\\},\\{name=\"a\",type=\"int \\*\",value=\"$hex\"\\},\\{name=\"ar\",type=\"int \\(&\\)\\\[1\\\]\"\\},\\{name=\"arr\",type=\"int \\(&&\\)\\\[1\\\]\"\\},\\{name=\"s\",type=\"s\"\\},\\{name=\"sr\",type=\"s &\"\\},\\{name=\"srr\",type=\"s &&\"\\},\\{name=\"u\",type=\"u\"\\},\\{name=\"ur\",type=\"u &\"\\},\\{name=\"urr\",type=\"u &&\"\\}\\\]\\},frame=\\{level=\"1\",args=\\\[\\\]\\}\\\]" \ > > + "stack arguments listing 2" > > + > > +mi_gdb_test "-stack-list-arguments --simple-values" \ > > + "\\^done,stack-args=\\\[frame=\\{level=\"0\",args=\\\[\\{name=\"i\",type=\"int\",value=\"1\"\\},\\{name=\"ir\",type=\"int &\",value=\"@$hex: 2\"\\},\\{name=\"irr\",type=\"int &&\",value=\"@$hex: 3\"\\},\\{name=\"a\",type=\"int \\*\",value=\"$hex\"\\},\\{name=\"ar\",type=\"int \\(&\\)\\\[1\\\]\"\\},\\{name=\"arr\",type=\"int \\(&&\\)\\\[1\\\]\"\\},\\{name=\"s\",type=\"s\"\\},\\{name=\"sr\",type=\"s &\"\\},\\{name=\"srr\",type=\"s &&\"\\},\\{name=\"u\",type=\"u\"\\},\\{name=\"ur\",type=\"u &\"\\},\\{name=\"urr\",type=\"u &&\"\\}\\\]\\},frame=\\{level=\"1\",args=\\\[\\\]\\}\\\]" \ > > + "stack arguments listing --simple-values" > > + > > +mi_gdb_exit > > -- > > 2.26.0 > >
>>>>> "Gareth" == Gareth Rees via Gdb-patches <gdb-patches@sourceware.org> writes:
Gareth> The '--simple-values' argument to '-stack-list-arguments' and similar
Gareth> GDB/MI commands does not take reference types into account, so that
Gareth> references to arbitrarily large structures are considered "simple" and
Gareth> printed. This means that the '--simple-values' argument cannot be used
Gareth> by IDEs when tracing the stack due to the time taken to print large
Gareth> structures passed by reference.
Hi. Thank you for the patch.
This looks good to me. I haven't been following the progress of this
patch -- have the documentation changes been reviewed? If so, please
check it in.
Tom
Tom Tromey wrote: > This looks good to me. I haven't been following the progress of this > patch -- have the documentation changes been reviewed? Thank you for looking at this. The documentation changes were reviewed by Eli Zaretskii in message <834jxif0yp.fsf@gnu.org> on 8 September 2022. > If so, please check it in. I'm not sure how to do this. I'm not a GDB maintainer, so I am unable to commit the patch.
Third ping. On Tue, 21 Mar 2023 at 09:50, Gareth Rees <grees@undo.io> wrote: > > This is a patch modifying the behaviour of the --simple-values option > but as discussed previously, if GDB maintainers agree that adding a > new option is the best approach, I would be happy to submit a patch > for that solution instead. > > On Sat, 11 Mar 2023 at 11:49, Gareth Rees <grees@undo.io> wrote: > > > > SUMMARY > > > > The '--simple-values' argument to '-stack-list-arguments' and similar > > GDB/MI commands does not take reference types into account, so that > > references to arbitrarily large structures are considered "simple" and > > printed. This means that the '--simple-values' argument cannot be used > > by IDEs when tracing the stack due to the time taken to print large > > structures passed by reference. > > > > DETAILS > > > > Various GDB/MI commands ('-stack-list-arguments', '-stack-list-locals', > > '-stack-list-variables' and so on) take a PRINT-VALUES argument which > > may be '--no-values' (0), '--all-values' (1) or '--simple-values' (2). > > In the '--simple-values' case, the command is supposed to print the > > name, type, and value of variables with simple types, and print only the > > name and type of variables with compound types. > > > > The '--simple-values' argument ought to be suitable for IDEs that need > > to update their user interface with the program's call stack every time > > the program stops. However, it does not take C++ reference types into > > account, and this makes the argument unsuitable for this purpose. > > > > For example, consider the following C++ program: > > > > struct s { > > int v[10]; > > }; > > > > int > > sum(const struct s &s) > > { > > int total = 0; > > for (int i = 0; i < 10; ++i) total += s.v[i]; > > return total; > > } > > > > int > > main(void) > > { > > struct s s = { { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 } }; > > return sum(s); > > } > > > > If we start GDB in MI mode and continue to 'sum', the behaviour of > > '-stack-list-arguments' is as follows: > > > > (gdb) > > -stack-list-arguments --simple-values > > ^done,stack-args=[frame={level="0",args=[{name="s",type="const s &",value="@0x7fffffffe310: {v = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10}}"}]},frame={level="1",args=[]}] > > > > Note that the value of the argument 's' was printed, even though 's' is > > a reference to a structure, which is not a simple value. > > > > See https://github.com/microsoft/MIEngine/pull/673 for a case where this > > behaviour caused Microsoft to avoid the use of '--simple-values' in > > their MIEngine debug adapter, because it caused Visual Studio Code to > > take too long to refresh the call stack in the user interface. > > > > SOLUTIONS > > > > There are two ways we could fix this problem, depending on whether we > > consider the current behaviour to be a bug. > > > > 1. If the current behaviour is a bug, then we can update the behaviour > > of '--simple-values' so that it takes reference types into account: > > that is, a value is simple if it is neither an array, struct, or > > union, nor a reference to an array, struct or union. > > > > In this case we must add a feature to the '-list-features' command so > > that IDEs can detect that it is safe to use the '--simple-values' > > argument when refreshing the call stack. > > > > 2. If the current behaviour is not a bug, then we can add a new option > > for the PRINT-VALUES argument, for example, '--scalar-values' (3), > > that would be suitable for use by IDEs. > > > > In this case we must add a feature to the '-list-features' command > > so that IDEs can detect that the '--scalar-values' argument is > > available for use when refreshing the call stack. > > > > PATCH > > > > This patch implements solution (1) as I think the current behaviour of > > not printing structures, but printing references to structures, is > > contrary to reasonable expectation. > > > > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29554 > > --- > > gdb/NEWS | 8 +++ > > gdb/doc/gdb.texinfo | 7 +++ > > gdb/mi/mi-cmd-stack.c | 9 +-- > > gdb/mi/mi-cmd-var.c | 27 ++++++--- > > gdb/mi/mi-cmds.h | 5 ++ > > gdb/mi/mi-main.c | 7 +-- > > gdb/python/py-framefilter.c | 6 +- > > gdb/testsuite/gdb.mi/mi-stack.exp | 16 ++++- > > gdb/testsuite/gdb.mi/print-simple-values.cc | 62 ++++++++++++++++++++ > > gdb/testsuite/gdb.mi/print-simple-values.exp | 53 +++++++++++++++++ > > 10 files changed, 175 insertions(+), 25 deletions(-) > > create mode 100644 gdb/testsuite/gdb.mi/print-simple-values.cc > > create mode 100644 gdb/testsuite/gdb.mi/print-simple-values.exp > > > > diff --git a/gdb/NEWS b/gdb/NEWS > > index cc262f1f8a6..1971b76a736 100644 > > --- a/gdb/NEWS > > +++ b/gdb/NEWS > > @@ -104,6 +104,14 @@ show always-read-ctf > > without a thread restriction. The same is also true for the 'task' > > field of an Ada task-specific breakpoint. > > > > +** The '--simple-values' argument to the '-stack-list-arguments', > > + '-stack-list-locals', '-stack-list-variables', and '-var-list-children' > > + commands now takes reference types into account: that is, a value is now > > + considered simple if it is neither an array, structure, or union, nor a > > + reference to an array, structure, or union. (Previously all references were > > + considered simple.) Support for this feature can be verified by using the > > + '-list-features' command, which should contain "simple-values-ref-types". > > + > > *** Changes in GDB 13 > > > > * MI version 1 is deprecated, and will be removed in GDB 14. > > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > > index 954f1481dae..dff7ba374bd 100644 > > --- a/gdb/doc/gdb.texinfo > > +++ b/gdb/doc/gdb.texinfo > > @@ -37948,6 +37948,13 @@ option (@pxref{GDB/MI Program Execution}). > > @item data-disassemble-a-option > > Indicates that the @code{-data-disassemble} command supports the @option{-a} > > option (@pxref{GDB/MI Data Manipulation}). > > +@item simple-values-ref-types > > +Indicates that the @code{--simple-values} argument to the > > +@code{-stack-list-arguments}, @code{-stack-list-locals}, > > +@code{-stack-list-variables}, and @code{-var-list-children} commands > > +takes reference types into account: that is, a value is considered > > +simple if it is neither an array, structure, or union, nor a reference > > +to an array, structure, or union. > > @end ftable > > > > @findex -list-target-features > > diff --git a/gdb/mi/mi-cmd-stack.c b/gdb/mi/mi-cmd-stack.c > > index 4c4662ab5d7..406bf85ceec 100644 > > --- a/gdb/mi/mi-cmd-stack.c > > +++ b/gdb/mi/mi-cmd-stack.c > > @@ -645,13 +645,8 @@ list_args_or_locals (const frame_print_options &fp_opts, > > switch (values) > > { > > case PRINT_SIMPLE_VALUES: > > - { > > - struct type *type = check_typedef (sym2->type ()); > > - if (type->code () == TYPE_CODE_ARRAY > > - || type->code () == TYPE_CODE_STRUCT > > - || type->code () == TYPE_CODE_UNION) > > - break; > > - } > > + if (!mi_simple_type_p (sym2->type ())) > > + break; > > /* FALLTHROUGH */ > > > > case PRINT_ALL_VALUES: > > diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c > > index e026c239b87..36eea4619a7 100644 > > --- a/gdb/mi/mi-cmd-var.c > > +++ b/gdb/mi/mi-cmd-var.c > > @@ -335,14 +335,27 @@ mi_print_value_p (struct varobj *var, enum print_values print_values) > > if (type == NULL) > > return 1; > > else > > - { > > - type = check_typedef (type); > > + return mi_simple_type_p (type); > > +} > > > > - /* For PRINT_SIMPLE_VALUES, only print the value if it has a type > > - and that type is not a compound type. */ > > - return (type->code () != TYPE_CODE_ARRAY > > - && type->code () != TYPE_CODE_STRUCT > > - && type->code () != TYPE_CODE_UNION); > > +/* See mi-cmds.h. */ > > + > > +bool > > +mi_simple_type_p (struct type *type) > > +{ > > + type = check_typedef (type); > > + > > + if (TYPE_IS_REFERENCE (type)) > > + type = check_typedef (type->target_type ()); > > + > > + switch (type->code ()) > > + { > > + case TYPE_CODE_ARRAY: > > + case TYPE_CODE_STRUCT: > > + case TYPE_CODE_UNION: > > + return false; > > + default: > > + return true; > > } > > } > > > > diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h > > index 490f50484d9..d867c298df9 100644 > > --- a/gdb/mi/mi-cmds.h > > +++ b/gdb/mi/mi-cmds.h > > @@ -226,4 +226,9 @@ using remove_mi_cmd_entries_ftype > > = gdb::function_view<bool (mi_command *)>; > > extern void remove_mi_cmd_entries (remove_mi_cmd_entries_ftype callback); > > > > +/* Return true if type is a simple type: that is, neither an array, structure, > > + or union, nor a reference to an array, structure, or union. */ > > + > > +extern bool mi_simple_type_p (struct type *type); > > + > > #endif /* MI_MI_CMDS_H */ > > diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c > > index 0013e5dfafd..cedad60d0b8 100644 > > --- a/gdb/mi/mi-main.c > > +++ b/gdb/mi/mi-main.c > > @@ -1656,6 +1656,7 @@ mi_cmd_list_features (const char *command, char **argv, int argc) > > uiout->field_string (NULL, "undefined-command-error-code"); > > uiout->field_string (NULL, "exec-run-start-option"); > > uiout->field_string (NULL, "data-disassemble-a-option"); > > + uiout->field_string (NULL, "simple-values-ref-types"); > > > > if (ext_lang_initialized_p (get_ext_lang_defn (EXT_LANG_PYTHON))) > > uiout->field_string (NULL, "python"); > > @@ -2458,7 +2459,6 @@ static void > > print_variable_or_computed (const char *expression, enum print_values values) > > { > > struct value *val; > > - struct type *type; > > struct ui_out *uiout = current_uiout; > > > > string_file stb; > > @@ -2478,12 +2478,9 @@ print_variable_or_computed (const char *expression, enum print_values values) > > switch (values) > > { > > case PRINT_SIMPLE_VALUES: > > - type = check_typedef (val->type ()); > > type_print (val->type (), "", &stb, -1); > > uiout->field_stream ("type", stb); > > - if (type->code () != TYPE_CODE_ARRAY > > - && type->code () != TYPE_CODE_STRUCT > > - && type->code () != TYPE_CODE_UNION) > > + if (mi_simple_type_p (val->type ())) > > { > > struct value_print_options opts; > > > > diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c > > index 0e8b2409636..e555dc3d879 100644 > > --- a/gdb/python/py-framefilter.c > > +++ b/gdb/python/py-framefilter.c > > @@ -235,14 +235,10 @@ py_print_value (struct ui_out *out, struct value *val, > > if (args_type == MI_PRINT_SIMPLE_VALUES > > || args_type == MI_PRINT_ALL_VALUES) > > { > > - struct type *type = check_typedef (val->type ()); > > - > > if (args_type == MI_PRINT_ALL_VALUES) > > should_print = 1; > > else if (args_type == MI_PRINT_SIMPLE_VALUES > > - && type->code () != TYPE_CODE_ARRAY > > - && type->code () != TYPE_CODE_STRUCT > > - && type->code () != TYPE_CODE_UNION) > > + && mi_simple_type_p (val->type ())) > > should_print = 1; > > } > > else if (args_type != NO_VALUES) > > diff --git a/gdb/testsuite/gdb.mi/mi-stack.exp b/gdb/testsuite/gdb.mi/mi-stack.exp > > index 633e090c13a..777a425894f 100644 > > --- a/gdb/testsuite/gdb.mi/mi-stack.exp > > +++ b/gdb/testsuite/gdb.mi/mi-stack.exp > > @@ -87,6 +87,9 @@ proc test_stack_args_listing {} { > > # -stack-list-arguments 1 1 1 > > # -stack-list-arguments 1 1 3 > > # -stack-list-arguments > > + # -stack-list-arguments 1 1 300 > > + # -stack-list-arguments 2 1 1 > > + # -stack-list-arguments --simple-values 1 1 > > > > mi_gdb_test "231-stack-list-arguments 0" \ > > "231\\^done,stack-args=\\\[frame=\{level=\"0\",args=\\\[\\\]\},frame=\{level=\"1\",args=\\\[name=\"strarg\"\\\]\},frame=\{level=\"2\",args=\\\[name=\"intarg\",name=\"strarg\"\\\]\},frame=\{level=\"3\",args=\\\[name=\"intarg\",name=\"strarg\",name=\"fltarg\"\\\]\},frame=\{level=\"4\",args=\\\[\\\]\}\\\]" \ > > @@ -119,6 +122,14 @@ proc test_stack_args_listing {} { > > mi_gdb_test "235-stack-list-arguments 1 1 300" \ > > "235\\^done,stack-args=\\\[frame=\{level=\"1\",args=\\\[\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\}\\\]\},frame=\{level=\"2\",args=\\\[\{name=\"intarg\",value=\"2\"\},\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\}\\\]\},frame=\{level=\"3\",args=\\\[\{name=\"intarg\",value=\"2\"\},\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\},\{name=\"fltarg\",value=\"3.5\"\}\\\]\},frame=\{level=\"4\",args=\\\[\\\]\}\\\]" \ > > "stack args listing 1 1 300" > > + > > + mi_gdb_test "236-stack-list-arguments 2 1 1" \ > > + "236\\^done,stack-args=\\\[frame=\\{level=\"1\",args=\\\[\\{name=\"strarg\",type=\"char \\*\",value=\"$hex \\\\\"A string argument.\\\\\"\"\\}\\\]\\}\\\]" \ > > + "stack args listing 2 1 1" > > + > > + mi_gdb_test "237-stack-list-arguments --simple-values 1 1" \ > > + "237\\^done,stack-args=\\\[frame=\\{level=\"1\",args=\\\[\\{name=\"strarg\",type=\"char \\*\",value=\"$hex \\\\\"A string argument.\\\\\"\"\\}\\\]\\}\\\]" \ > > + "stack args listing --simple-values 1 1" > > } > > > > proc test_stack_info_depth {} { > > @@ -157,7 +168,6 @@ proc test_stack_locals_listing {} { > > # -stack-list-locals 0 (--no-values) > > # -stack-list-locals 1 (--all-values) > > # -stack-list-locals 2 (--simple-values) > > - # -stack-list-arguments > > > > mi_gdb_test "232-stack-list-locals 0" \ > > "232\\^done,locals=\\\[name=\"A\",name=\"B\",name=\"C\",name=\"D\"\\\]" \ > > @@ -173,6 +183,10 @@ proc test_stack_locals_listing {} { > > "232\\^done,locals=\\\[\{name=\"A\",value=\"1\"\},\{name=\"B\",value=\"2\"\},\{name=\"C\",value=\"3\"\},\{name=\"D\",value=\"\\{0, 1, 2\\}\"\}\\\]" \ > > "stack locals listing of names and values" > > > > + mi_gdb_test "232-stack-list-locals 2" \ > > + "232\\^done,locals=\\\[\{name=\"A\",type=\"int\",value=\"1\"\},\{name=\"B\",type=\"int\",value=\"2\"\},\{name=\"C\",type=\"int\",value=\"3\"\},\{name=\"D\",type=\"int \\\[3\\\]\"\}\\\]" \ > > + "stack locals listing 2" > > + > > mi_gdb_test "232-stack-list-locals --simple-values" \ > > "232\\^done,locals=\\\[\{name=\"A\",type=\"int\",value=\"1\"\},\{name=\"B\",type=\"int\",value=\"2\"\},\{name=\"C\",type=\"int\",value=\"3\"\},\{name=\"D\",type=\"int \\\[3\\\]\"\}\\\]" \ > > "stack locals listing, simple types: names and values, complex type: names and types" > > diff --git a/gdb/testsuite/gdb.mi/print-simple-values.cc b/gdb/testsuite/gdb.mi/print-simple-values.cc > > new file mode 100644 > > index 00000000000..1aad724efb9 > > --- /dev/null > > +++ b/gdb/testsuite/gdb.mi/print-simple-values.cc > > @@ -0,0 +1,62 @@ > > +/* This test case is part of GDB, the GNU debugger. > > + > > + Copyright 2022-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 <http://www.gnu.org/licenses/>. */ > > + > > +/* Test program for PRINT_SIMPLE_VALUES. > > + > > + In the function f: > > + > > + * The arguments i, ir, and irr are ints or references to ints, which > > + must be printed by PRINT_SIMPLE_VALUES. > > + > > + * The arguments a, s, and u are non-scalar values, which must not be > > + printed by PRINT_SIMPLE_VALUES. > > + > > + * The arguments ar, arr, sr, srr, ur, and urr are references to > > + non-scalar values, which must not be printed by > > + PRINT_SIMPLE_VALUES. */ > > + > > +struct s > > +{ > > + int v; > > +}; > > + > > +union u > > +{ > > + int v; > > +}; > > + > > +int > > +f (int i, int &ir, int &&irr, > > + int a[1], int (&ar)[1], int (&&arr)[1], > > + struct s s, struct s &sr, struct s &&srr, > > + union u u, union u &ur, union u &&urr) > > +{ > > + return (i + ir + irr > > + + a[0] + ar[0] + arr[0] > > + + s.v + sr.v + srr.v > > + + u.v + ur.v + urr.v); > > +} > > + > > +int > > +main (void) > > +{ > > + int i = 1, j = 2; > > + int a[1] = { 4 }, b[1] = { 5 }; > > + struct s s = { 7 }, t = { 8 }; > > + union u u = { 10 }, v = { 11 }; > > + return f (i, j, 3, a, b, { 6 }, s, t, { 9 }, u, v, { 12 }); > > +} > > diff --git a/gdb/testsuite/gdb.mi/print-simple-values.exp b/gdb/testsuite/gdb.mi/print-simple-values.exp > > new file mode 100644 > > index 00000000000..9436645df84 > > --- /dev/null > > +++ b/gdb/testsuite/gdb.mi/print-simple-values.exp > > @@ -0,0 +1,53 @@ > > +# Copyright 2022-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 <http://www.gnu.org/licenses/>. > > + > > +# Test of PRINT_SIMPLE_VALUES. > > +# > > +# Test that PRINT_SIMPLE_VALUES distinguishes simple and compound types, > > +# including C++ reference and rvalue reference types. > > + > > +require allow_cplus_tests > > + > > +load_lib mi-support.exp > > +set MIFLAGS "-i=mi" > > + > > +standard_testfile .cc > > + > > +if [build_executable "failed to prepare" $testfile $srcfile {debug c++}] { > > + return -1 > > +} > > + > > +if [mi_clean_restart $binfile] { > > + return > > +} > > + > > +mi_runto_main > > + > > +mi_gdb_test "-break-insert f" "\\^done.*" "set breakpoint on f" > > + > > +mi_send_resuming_command "exec-continue" "exec-continue to breakpoint on f" > > + > > +mi_expect_stop "breakpoint-hit" "f" ".*" ".*" ".*" {.* disp="keep"} \ > > + "run until breakpoint on f" > > + > > +mi_gdb_test "-stack-list-arguments 2" \ > > + "\\^done,stack-args=\\\[frame=\\{level=\"0\",args=\\\[\\{name=\"i\",type=\"int\",value=\"1\"\\},\\{name=\"ir\",type=\"int &\",value=\"@$hex: 2\"\\},\\{name=\"irr\",type=\"int &&\",value=\"@$hex: 3\"\\},\\{name=\"a\",type=\"int \\*\",value=\"$hex\"\\},\\{name=\"ar\",type=\"int \\(&\\)\\\[1\\\]\"\\},\\{name=\"arr\",type=\"int \\(&&\\)\\\[1\\\]\"\\},\\{name=\"s\",type=\"s\"\\},\\{name=\"sr\",type=\"s &\"\\},\\{name=\"srr\",type=\"s &&\"\\},\\{name=\"u\",type=\"u\"\\},\\{name=\"ur\",type=\"u &\"\\},\\{name=\"urr\",type=\"u &&\"\\}\\\]\\},frame=\\{level=\"1\",args=\\\[\\\]\\}\\\]" \ > > + "stack arguments listing 2" > > + > > +mi_gdb_test "-stack-list-arguments --simple-values" \ > > + "\\^done,stack-args=\\\[frame=\\{level=\"0\",args=\\\[\\{name=\"i\",type=\"int\",value=\"1\"\\},\\{name=\"ir\",type=\"int &\",value=\"@$hex: 2\"\\},\\{name=\"irr\",type=\"int &&\",value=\"@$hex: 3\"\\},\\{name=\"a\",type=\"int \\*\",value=\"$hex\"\\},\\{name=\"ar\",type=\"int \\(&\\)\\\[1\\\]\"\\},\\{name=\"arr\",type=\"int \\(&&\\)\\\[1\\\]\"\\},\\{name=\"s\",type=\"s\"\\},\\{name=\"sr\",type=\"s &\"\\},\\{name=\"srr\",type=\"s &&\"\\},\\{name=\"u\",type=\"u\"\\},\\{name=\"ur\",type=\"u &\"\\},\\{name=\"urr\",type=\"u &&\"\\}\\\]\\},frame=\\{level=\"1\",args=\\\[\\\]\\}\\\]" \ > > + "stack arguments listing --simple-values" > > + > > +mi_gdb_exit > > -- > > 2.26.0 > >
Tom Tromey wrote: > > >>>>> "Gareth" == Gareth Rees via Gdb-patches <gdb-patches@sourceware.org> writes: > > Gareth> The '--simple-values' argument to '-stack-list-arguments' and similar > Gareth> GDB/MI commands does not take reference types into account, so that > Gareth> references to arbitrarily large structures are considered "simple" and > Gareth> printed. This means that the '--simple-values' argument cannot be used > Gareth> by IDEs when tracing the stack due to the time taken to print large > Gareth> structures passed by reference. > > This looks good to me. I haven't been following the progress of this > patch -- have the documentation changes been reviewed? If so, please > check it in. Following up again. I don't have commit rights, so I can't check it in myself. Could you check it in for me? But before you do, I just want to check that you're happy with the solution I've picked here. There are two ways to fix the problem of '--simple-values' printing references to compound values: First, if it's acceptable to modify the behaviour of the '--simple-values' option, then we can suppress the printing of references to compound types, as in this patch. This solution was preferred by Andrew Burgess [1] [2] [3] out of concern about the maintenance cost of accumulating backwards-compatible but arguably broken features. I added an argument that the original behaviour was a bug, and that it remains hard to use correctly [4]. Second, if we have to preserve the behaviour of the '--simple-values' option, then we need to add a new option. This solution was preferred by Eli Zaretskii [5] [6] out of concern about backwards compatibility. I've proposed a patch that applies the first solution, but I could work with either. So I need someone to break the tie between Andrew and Eli. Take a look at the cited messages and let me know if you're happy, and if you are, then commit the patch. [1] https://sourceware.org/pipermail/gdb-patches/2022-September/191747.html [2] https://sourceware.org/pipermail/gdb-patches/2023-March/197736.html [3] https://sourceware.org/pipermail/gdb-patches/2023-March/197894.html [4] https://sourceware.org/pipermail/gdb-patches/2023-March/197799.html [5] https://sourceware.org/pipermail/gdb-patches/2022-September/191754.html [6] https://sourceware.org/pipermail/gdb-patches/2023-March/197798.html
Fourth ping. On Tue, 21 Mar 2023 at 09:50, Gareth Rees <grees@undo.io> wrote: > > This is a patch modifying the behaviour of the --simple-values option > but as discussed previously, if GDB maintainers agree that adding a > new option is the best approach, I would be happy to submit a patch > for that solution instead. > > On Sat, 11 Mar 2023 at 11:49, Gareth Rees <grees@undo.io> wrote: > > > > SUMMARY > > > > The '--simple-values' argument to '-stack-list-arguments' and similar > > GDB/MI commands does not take reference types into account, so that > > references to arbitrarily large structures are considered "simple" and > > printed. This means that the '--simple-values' argument cannot be used > > by IDEs when tracing the stack due to the time taken to print large > > structures passed by reference. > > > > DETAILS > > > > Various GDB/MI commands ('-stack-list-arguments', '-stack-list-locals', > > '-stack-list-variables' and so on) take a PRINT-VALUES argument which > > may be '--no-values' (0), '--all-values' (1) or '--simple-values' (2). > > In the '--simple-values' case, the command is supposed to print the > > name, type, and value of variables with simple types, and print only the > > name and type of variables with compound types. > > > > The '--simple-values' argument ought to be suitable for IDEs that need > > to update their user interface with the program's call stack every time > > the program stops. However, it does not take C++ reference types into > > account, and this makes the argument unsuitable for this purpose. > > > > For example, consider the following C++ program: > > > > struct s { > > int v[10]; > > }; > > > > int > > sum(const struct s &s) > > { > > int total = 0; > > for (int i = 0; i < 10; ++i) total += s.v[i]; > > return total; > > } > > > > int > > main(void) > > { > > struct s s = { { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 } }; > > return sum(s); > > } > > > > If we start GDB in MI mode and continue to 'sum', the behaviour of > > '-stack-list-arguments' is as follows: > > > > (gdb) > > -stack-list-arguments --simple-values > > ^done,stack-args=[frame={level="0",args=[{name="s",type="const s &",value="@0x7fffffffe310: {v = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10}}"}]},frame={level="1",args=[]}] > > > > Note that the value of the argument 's' was printed, even though 's' is > > a reference to a structure, which is not a simple value. > > > > See https://github.com/microsoft/MIEngine/pull/673 for a case where this > > behaviour caused Microsoft to avoid the use of '--simple-values' in > > their MIEngine debug adapter, because it caused Visual Studio Code to > > take too long to refresh the call stack in the user interface. > > > > SOLUTIONS > > > > There are two ways we could fix this problem, depending on whether we > > consider the current behaviour to be a bug. > > > > 1. If the current behaviour is a bug, then we can update the behaviour > > of '--simple-values' so that it takes reference types into account: > > that is, a value is simple if it is neither an array, struct, or > > union, nor a reference to an array, struct or union. > > > > In this case we must add a feature to the '-list-features' command so > > that IDEs can detect that it is safe to use the '--simple-values' > > argument when refreshing the call stack. > > > > 2. If the current behaviour is not a bug, then we can add a new option > > for the PRINT-VALUES argument, for example, '--scalar-values' (3), > > that would be suitable for use by IDEs. > > > > In this case we must add a feature to the '-list-features' command > > so that IDEs can detect that the '--scalar-values' argument is > > available for use when refreshing the call stack. > > > > PATCH > > > > This patch implements solution (1) as I think the current behaviour of > > not printing structures, but printing references to structures, is > > contrary to reasonable expectation. > > > > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29554 > > --- > > gdb/NEWS | 8 +++ > > gdb/doc/gdb.texinfo | 7 +++ > > gdb/mi/mi-cmd-stack.c | 9 +-- > > gdb/mi/mi-cmd-var.c | 27 ++++++--- > > gdb/mi/mi-cmds.h | 5 ++ > > gdb/mi/mi-main.c | 7 +-- > > gdb/python/py-framefilter.c | 6 +- > > gdb/testsuite/gdb.mi/mi-stack.exp | 16 ++++- > > gdb/testsuite/gdb.mi/print-simple-values.cc | 62 ++++++++++++++++++++ > > gdb/testsuite/gdb.mi/print-simple-values.exp | 53 +++++++++++++++++ > > 10 files changed, 175 insertions(+), 25 deletions(-) > > create mode 100644 gdb/testsuite/gdb.mi/print-simple-values.cc > > create mode 100644 gdb/testsuite/gdb.mi/print-simple-values.exp > > > > diff --git a/gdb/NEWS b/gdb/NEWS > > index cc262f1f8a6..1971b76a736 100644 > > --- a/gdb/NEWS > > +++ b/gdb/NEWS > > @@ -104,6 +104,14 @@ show always-read-ctf > > without a thread restriction. The same is also true for the 'task' > > field of an Ada task-specific breakpoint. > > > > +** The '--simple-values' argument to the '-stack-list-arguments', > > + '-stack-list-locals', '-stack-list-variables', and '-var-list-children' > > + commands now takes reference types into account: that is, a value is now > > + considered simple if it is neither an array, structure, or union, nor a > > + reference to an array, structure, or union. (Previously all references were > > + considered simple.) Support for this feature can be verified by using the > > + '-list-features' command, which should contain "simple-values-ref-types". > > + > > *** Changes in GDB 13 > > > > * MI version 1 is deprecated, and will be removed in GDB 14. > > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > > index 954f1481dae..dff7ba374bd 100644 > > --- a/gdb/doc/gdb.texinfo > > +++ b/gdb/doc/gdb.texinfo > > @@ -37948,6 +37948,13 @@ option (@pxref{GDB/MI Program Execution}). > > @item data-disassemble-a-option > > Indicates that the @code{-data-disassemble} command supports the @option{-a} > > option (@pxref{GDB/MI Data Manipulation}). > > +@item simple-values-ref-types > > +Indicates that the @code{--simple-values} argument to the > > +@code{-stack-list-arguments}, @code{-stack-list-locals}, > > +@code{-stack-list-variables}, and @code{-var-list-children} commands > > +takes reference types into account: that is, a value is considered > > +simple if it is neither an array, structure, or union, nor a reference > > +to an array, structure, or union. > > @end ftable > > > > @findex -list-target-features > > diff --git a/gdb/mi/mi-cmd-stack.c b/gdb/mi/mi-cmd-stack.c > > index 4c4662ab5d7..406bf85ceec 100644 > > --- a/gdb/mi/mi-cmd-stack.c > > +++ b/gdb/mi/mi-cmd-stack.c > > @@ -645,13 +645,8 @@ list_args_or_locals (const frame_print_options &fp_opts, > > switch (values) > > { > > case PRINT_SIMPLE_VALUES: > > - { > > - struct type *type = check_typedef (sym2->type ()); > > - if (type->code () == TYPE_CODE_ARRAY > > - || type->code () == TYPE_CODE_STRUCT > > - || type->code () == TYPE_CODE_UNION) > > - break; > > - } > > + if (!mi_simple_type_p (sym2->type ())) > > + break; > > /* FALLTHROUGH */ > > > > case PRINT_ALL_VALUES: > > diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c > > index e026c239b87..36eea4619a7 100644 > > --- a/gdb/mi/mi-cmd-var.c > > +++ b/gdb/mi/mi-cmd-var.c > > @@ -335,14 +335,27 @@ mi_print_value_p (struct varobj *var, enum print_values print_values) > > if (type == NULL) > > return 1; > > else > > - { > > - type = check_typedef (type); > > + return mi_simple_type_p (type); > > +} > > > > - /* For PRINT_SIMPLE_VALUES, only print the value if it has a type > > - and that type is not a compound type. */ > > - return (type->code () != TYPE_CODE_ARRAY > > - && type->code () != TYPE_CODE_STRUCT > > - && type->code () != TYPE_CODE_UNION); > > +/* See mi-cmds.h. */ > > + > > +bool > > +mi_simple_type_p (struct type *type) > > +{ > > + type = check_typedef (type); > > + > > + if (TYPE_IS_REFERENCE (type)) > > + type = check_typedef (type->target_type ()); > > + > > + switch (type->code ()) > > + { > > + case TYPE_CODE_ARRAY: > > + case TYPE_CODE_STRUCT: > > + case TYPE_CODE_UNION: > > + return false; > > + default: > > + return true; > > } > > } > > > > diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h > > index 490f50484d9..d867c298df9 100644 > > --- a/gdb/mi/mi-cmds.h > > +++ b/gdb/mi/mi-cmds.h > > @@ -226,4 +226,9 @@ using remove_mi_cmd_entries_ftype > > = gdb::function_view<bool (mi_command *)>; > > extern void remove_mi_cmd_entries (remove_mi_cmd_entries_ftype callback); > > > > +/* Return true if type is a simple type: that is, neither an array, structure, > > + or union, nor a reference to an array, structure, or union. */ > > + > > +extern bool mi_simple_type_p (struct type *type); > > + > > #endif /* MI_MI_CMDS_H */ > > diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c > > index 0013e5dfafd..cedad60d0b8 100644 > > --- a/gdb/mi/mi-main.c > > +++ b/gdb/mi/mi-main.c > > @@ -1656,6 +1656,7 @@ mi_cmd_list_features (const char *command, char **argv, int argc) > > uiout->field_string (NULL, "undefined-command-error-code"); > > uiout->field_string (NULL, "exec-run-start-option"); > > uiout->field_string (NULL, "data-disassemble-a-option"); > > + uiout->field_string (NULL, "simple-values-ref-types"); > > > > if (ext_lang_initialized_p (get_ext_lang_defn (EXT_LANG_PYTHON))) > > uiout->field_string (NULL, "python"); > > @@ -2458,7 +2459,6 @@ static void > > print_variable_or_computed (const char *expression, enum print_values values) > > { > > struct value *val; > > - struct type *type; > > struct ui_out *uiout = current_uiout; > > > > string_file stb; > > @@ -2478,12 +2478,9 @@ print_variable_or_computed (const char *expression, enum print_values values) > > switch (values) > > { > > case PRINT_SIMPLE_VALUES: > > - type = check_typedef (val->type ()); > > type_print (val->type (), "", &stb, -1); > > uiout->field_stream ("type", stb); > > - if (type->code () != TYPE_CODE_ARRAY > > - && type->code () != TYPE_CODE_STRUCT > > - && type->code () != TYPE_CODE_UNION) > > + if (mi_simple_type_p (val->type ())) > > { > > struct value_print_options opts; > > > > diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c > > index 0e8b2409636..e555dc3d879 100644 > > --- a/gdb/python/py-framefilter.c > > +++ b/gdb/python/py-framefilter.c > > @@ -235,14 +235,10 @@ py_print_value (struct ui_out *out, struct value *val, > > if (args_type == MI_PRINT_SIMPLE_VALUES > > || args_type == MI_PRINT_ALL_VALUES) > > { > > - struct type *type = check_typedef (val->type ()); > > - > > if (args_type == MI_PRINT_ALL_VALUES) > > should_print = 1; > > else if (args_type == MI_PRINT_SIMPLE_VALUES > > - && type->code () != TYPE_CODE_ARRAY > > - && type->code () != TYPE_CODE_STRUCT > > - && type->code () != TYPE_CODE_UNION) > > + && mi_simple_type_p (val->type ())) > > should_print = 1; > > } > > else if (args_type != NO_VALUES) > > diff --git a/gdb/testsuite/gdb.mi/mi-stack.exp b/gdb/testsuite/gdb.mi/mi-stack.exp > > index 633e090c13a..777a425894f 100644 > > --- a/gdb/testsuite/gdb.mi/mi-stack.exp > > +++ b/gdb/testsuite/gdb.mi/mi-stack.exp > > @@ -87,6 +87,9 @@ proc test_stack_args_listing {} { > > # -stack-list-arguments 1 1 1 > > # -stack-list-arguments 1 1 3 > > # -stack-list-arguments > > + # -stack-list-arguments 1 1 300 > > + # -stack-list-arguments 2 1 1 > > + # -stack-list-arguments --simple-values 1 1 > > > > mi_gdb_test "231-stack-list-arguments 0" \ > > "231\\^done,stack-args=\\\[frame=\{level=\"0\",args=\\\[\\\]\},frame=\{level=\"1\",args=\\\[name=\"strarg\"\\\]\},frame=\{level=\"2\",args=\\\[name=\"intarg\",name=\"strarg\"\\\]\},frame=\{level=\"3\",args=\\\[name=\"intarg\",name=\"strarg\",name=\"fltarg\"\\\]\},frame=\{level=\"4\",args=\\\[\\\]\}\\\]" \ > > @@ -119,6 +122,14 @@ proc test_stack_args_listing {} { > > mi_gdb_test "235-stack-list-arguments 1 1 300" \ > > "235\\^done,stack-args=\\\[frame=\{level=\"1\",args=\\\[\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\}\\\]\},frame=\{level=\"2\",args=\\\[\{name=\"intarg\",value=\"2\"\},\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\}\\\]\},frame=\{level=\"3\",args=\\\[\{name=\"intarg\",value=\"2\"\},\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\},\{name=\"fltarg\",value=\"3.5\"\}\\\]\},frame=\{level=\"4\",args=\\\[\\\]\}\\\]" \ > > "stack args listing 1 1 300" > > + > > + mi_gdb_test "236-stack-list-arguments 2 1 1" \ > > + "236\\^done,stack-args=\\\[frame=\\{level=\"1\",args=\\\[\\{name=\"strarg\",type=\"char \\*\",value=\"$hex \\\\\"A string argument.\\\\\"\"\\}\\\]\\}\\\]" \ > > + "stack args listing 2 1 1" > > + > > + mi_gdb_test "237-stack-list-arguments --simple-values 1 1" \ > > + "237\\^done,stack-args=\\\[frame=\\{level=\"1\",args=\\\[\\{name=\"strarg\",type=\"char \\*\",value=\"$hex \\\\\"A string argument.\\\\\"\"\\}\\\]\\}\\\]" \ > > + "stack args listing --simple-values 1 1" > > } > > > > proc test_stack_info_depth {} { > > @@ -157,7 +168,6 @@ proc test_stack_locals_listing {} { > > # -stack-list-locals 0 (--no-values) > > # -stack-list-locals 1 (--all-values) > > # -stack-list-locals 2 (--simple-values) > > - # -stack-list-arguments > > > > mi_gdb_test "232-stack-list-locals 0" \ > > "232\\^done,locals=\\\[name=\"A\",name=\"B\",name=\"C\",name=\"D\"\\\]" \ > > @@ -173,6 +183,10 @@ proc test_stack_locals_listing {} { > > "232\\^done,locals=\\\[\{name=\"A\",value=\"1\"\},\{name=\"B\",value=\"2\"\},\{name=\"C\",value=\"3\"\},\{name=\"D\",value=\"\\{0, 1, 2\\}\"\}\\\]" \ > > "stack locals listing of names and values" > > > > + mi_gdb_test "232-stack-list-locals 2" \ > > + "232\\^done,locals=\\\[\{name=\"A\",type=\"int\",value=\"1\"\},\{name=\"B\",type=\"int\",value=\"2\"\},\{name=\"C\",type=\"int\",value=\"3\"\},\{name=\"D\",type=\"int \\\[3\\\]\"\}\\\]" \ > > + "stack locals listing 2" > > + > > mi_gdb_test "232-stack-list-locals --simple-values" \ > > "232\\^done,locals=\\\[\{name=\"A\",type=\"int\",value=\"1\"\},\{name=\"B\",type=\"int\",value=\"2\"\},\{name=\"C\",type=\"int\",value=\"3\"\},\{name=\"D\",type=\"int \\\[3\\\]\"\}\\\]" \ > > "stack locals listing, simple types: names and values, complex type: names and types" > > diff --git a/gdb/testsuite/gdb.mi/print-simple-values.cc b/gdb/testsuite/gdb.mi/print-simple-values.cc > > new file mode 100644 > > index 00000000000..1aad724efb9 > > --- /dev/null > > +++ b/gdb/testsuite/gdb.mi/print-simple-values.cc > > @@ -0,0 +1,62 @@ > > +/* This test case is part of GDB, the GNU debugger. > > + > > + Copyright 2022-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 <http://www.gnu.org/licenses/>. */ > > + > > +/* Test program for PRINT_SIMPLE_VALUES. > > + > > + In the function f: > > + > > + * The arguments i, ir, and irr are ints or references to ints, which > > + must be printed by PRINT_SIMPLE_VALUES. > > + > > + * The arguments a, s, and u are non-scalar values, which must not be > > + printed by PRINT_SIMPLE_VALUES. > > + > > + * The arguments ar, arr, sr, srr, ur, and urr are references to > > + non-scalar values, which must not be printed by > > + PRINT_SIMPLE_VALUES. */ > > + > > +struct s > > +{ > > + int v; > > +}; > > + > > +union u > > +{ > > + int v; > > +}; > > + > > +int > > +f (int i, int &ir, int &&irr, > > + int a[1], int (&ar)[1], int (&&arr)[1], > > + struct s s, struct s &sr, struct s &&srr, > > + union u u, union u &ur, union u &&urr) > > +{ > > + return (i + ir + irr > > + + a[0] + ar[0] + arr[0] > > + + s.v + sr.v + srr.v > > + + u.v + ur.v + urr.v); > > +} > > + > > +int > > +main (void) > > +{ > > + int i = 1, j = 2; > > + int a[1] = { 4 }, b[1] = { 5 }; > > + struct s s = { 7 }, t = { 8 }; > > + union u u = { 10 }, v = { 11 }; > > + return f (i, j, 3, a, b, { 6 }, s, t, { 9 }, u, v, { 12 }); > > +} > > diff --git a/gdb/testsuite/gdb.mi/print-simple-values.exp b/gdb/testsuite/gdb.mi/print-simple-values.exp > > new file mode 100644 > > index 00000000000..9436645df84 > > --- /dev/null > > +++ b/gdb/testsuite/gdb.mi/print-simple-values.exp > > @@ -0,0 +1,53 @@ > > +# Copyright 2022-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 <http://www.gnu.org/licenses/>. > > + > > +# Test of PRINT_SIMPLE_VALUES. > > +# > > +# Test that PRINT_SIMPLE_VALUES distinguishes simple and compound types, > > +# including C++ reference and rvalue reference types. > > + > > +require allow_cplus_tests > > + > > +load_lib mi-support.exp > > +set MIFLAGS "-i=mi" > > + > > +standard_testfile .cc > > + > > +if [build_executable "failed to prepare" $testfile $srcfile {debug c++}] { > > + return -1 > > +} > > + > > +if [mi_clean_restart $binfile] { > > + return > > +} > > + > > +mi_runto_main > > + > > +mi_gdb_test "-break-insert f" "\\^done.*" "set breakpoint on f" > > + > > +mi_send_resuming_command "exec-continue" "exec-continue to breakpoint on f" > > + > > +mi_expect_stop "breakpoint-hit" "f" ".*" ".*" ".*" {.* disp="keep"} \ > > + "run until breakpoint on f" > > + > > +mi_gdb_test "-stack-list-arguments 2" \ > > + "\\^done,stack-args=\\\[frame=\\{level=\"0\",args=\\\[\\{name=\"i\",type=\"int\",value=\"1\"\\},\\{name=\"ir\",type=\"int &\",value=\"@$hex: 2\"\\},\\{name=\"irr\",type=\"int &&\",value=\"@$hex: 3\"\\},\\{name=\"a\",type=\"int \\*\",value=\"$hex\"\\},\\{name=\"ar\",type=\"int \\(&\\)\\\[1\\\]\"\\},\\{name=\"arr\",type=\"int \\(&&\\)\\\[1\\\]\"\\},\\{name=\"s\",type=\"s\"\\},\\{name=\"sr\",type=\"s &\"\\},\\{name=\"srr\",type=\"s &&\"\\},\\{name=\"u\",type=\"u\"\\},\\{name=\"ur\",type=\"u &\"\\},\\{name=\"urr\",type=\"u &&\"\\}\\\]\\},frame=\\{level=\"1\",args=\\\[\\\]\\}\\\]" \ > > + "stack arguments listing 2" > > + > > +mi_gdb_test "-stack-list-arguments --simple-values" \ > > + "\\^done,stack-args=\\\[frame=\\{level=\"0\",args=\\\[\\{name=\"i\",type=\"int\",value=\"1\"\\},\\{name=\"ir\",type=\"int &\",value=\"@$hex: 2\"\\},\\{name=\"irr\",type=\"int &&\",value=\"@$hex: 3\"\\},\\{name=\"a\",type=\"int \\*\",value=\"$hex\"\\},\\{name=\"ar\",type=\"int \\(&\\)\\\[1\\\]\"\\},\\{name=\"arr\",type=\"int \\(&&\\)\\\[1\\\]\"\\},\\{name=\"s\",type=\"s\"\\},\\{name=\"sr\",type=\"s &\"\\},\\{name=\"srr\",type=\"s &&\"\\},\\{name=\"u\",type=\"u\"\\},\\{name=\"ur\",type=\"u &\"\\},\\{name=\"urr\",type=\"u &&\"\\}\\\]\\},frame=\\{level=\"1\",args=\\\[\\\]\\}\\\]" \ > > + "stack arguments listing --simple-values" > > + > > +mi_gdb_exit > > -- > > 2.26.0 > >
Fifth ping. On Tue, 21 Mar 2023 at 09:50, Gareth Rees <grees@undo.io> wrote: > > This is a patch modifying the behaviour of the --simple-values option > but as discussed previously, if GDB maintainers agree that adding a > new option is the best approach, I would be happy to submit a patch > for that solution instead. > > On Sat, 11 Mar 2023 at 11:49, Gareth Rees <grees@undo.io> wrote: > > > > SUMMARY > > > > The '--simple-values' argument to '-stack-list-arguments' and similar > > GDB/MI commands does not take reference types into account, so that > > references to arbitrarily large structures are considered "simple" and > > printed. This means that the '--simple-values' argument cannot be used > > by IDEs when tracing the stack due to the time taken to print large > > structures passed by reference. > > > > DETAILS > > > > Various GDB/MI commands ('-stack-list-arguments', '-stack-list-locals', > > '-stack-list-variables' and so on) take a PRINT-VALUES argument which > > may be '--no-values' (0), '--all-values' (1) or '--simple-values' (2). > > In the '--simple-values' case, the command is supposed to print the > > name, type, and value of variables with simple types, and print only the > > name and type of variables with compound types. > > > > The '--simple-values' argument ought to be suitable for IDEs that need > > to update their user interface with the program's call stack every time > > the program stops. However, it does not take C++ reference types into > > account, and this makes the argument unsuitable for this purpose. > > > > For example, consider the following C++ program: > > > > struct s { > > int v[10]; > > }; > > > > int > > sum(const struct s &s) > > { > > int total = 0; > > for (int i = 0; i < 10; ++i) total += s.v[i]; > > return total; > > } > > > > int > > main(void) > > { > > struct s s = { { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 } }; > > return sum(s); > > } > > > > If we start GDB in MI mode and continue to 'sum', the behaviour of > > '-stack-list-arguments' is as follows: > > > > (gdb) > > -stack-list-arguments --simple-values > > ^done,stack-args=[frame={level="0",args=[{name="s",type="const s &",value="@0x7fffffffe310: {v = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10}}"}]},frame={level="1",args=[]}] > > > > Note that the value of the argument 's' was printed, even though 's' is > > a reference to a structure, which is not a simple value. > > > > See https://github.com/microsoft/MIEngine/pull/673 for a case where this > > behaviour caused Microsoft to avoid the use of '--simple-values' in > > their MIEngine debug adapter, because it caused Visual Studio Code to > > take too long to refresh the call stack in the user interface. > > > > SOLUTIONS > > > > There are two ways we could fix this problem, depending on whether we > > consider the current behaviour to be a bug. > > > > 1. If the current behaviour is a bug, then we can update the behaviour > > of '--simple-values' so that it takes reference types into account: > > that is, a value is simple if it is neither an array, struct, or > > union, nor a reference to an array, struct or union. > > > > In this case we must add a feature to the '-list-features' command so > > that IDEs can detect that it is safe to use the '--simple-values' > > argument when refreshing the call stack. > > > > 2. If the current behaviour is not a bug, then we can add a new option > > for the PRINT-VALUES argument, for example, '--scalar-values' (3), > > that would be suitable for use by IDEs. > > > > In this case we must add a feature to the '-list-features' command > > so that IDEs can detect that the '--scalar-values' argument is > > available for use when refreshing the call stack. > > > > PATCH > > > > This patch implements solution (1) as I think the current behaviour of > > not printing structures, but printing references to structures, is > > contrary to reasonable expectation. > > > > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29554 > > --- > > gdb/NEWS | 8 +++ > > gdb/doc/gdb.texinfo | 7 +++ > > gdb/mi/mi-cmd-stack.c | 9 +-- > > gdb/mi/mi-cmd-var.c | 27 ++++++--- > > gdb/mi/mi-cmds.h | 5 ++ > > gdb/mi/mi-main.c | 7 +-- > > gdb/python/py-framefilter.c | 6 +- > > gdb/testsuite/gdb.mi/mi-stack.exp | 16 ++++- > > gdb/testsuite/gdb.mi/print-simple-values.cc | 62 ++++++++++++++++++++ > > gdb/testsuite/gdb.mi/print-simple-values.exp | 53 +++++++++++++++++ > > 10 files changed, 175 insertions(+), 25 deletions(-) > > create mode 100644 gdb/testsuite/gdb.mi/print-simple-values.cc > > create mode 100644 gdb/testsuite/gdb.mi/print-simple-values.exp > > > > diff --git a/gdb/NEWS b/gdb/NEWS > > index cc262f1f8a6..1971b76a736 100644 > > --- a/gdb/NEWS > > +++ b/gdb/NEWS > > @@ -104,6 +104,14 @@ show always-read-ctf > > without a thread restriction. The same is also true for the 'task' > > field of an Ada task-specific breakpoint. > > > > +** The '--simple-values' argument to the '-stack-list-arguments', > > + '-stack-list-locals', '-stack-list-variables', and '-var-list-children' > > + commands now takes reference types into account: that is, a value is now > > + considered simple if it is neither an array, structure, or union, nor a > > + reference to an array, structure, or union. (Previously all references were > > + considered simple.) Support for this feature can be verified by using the > > + '-list-features' command, which should contain "simple-values-ref-types". > > + > > *** Changes in GDB 13 > > > > * MI version 1 is deprecated, and will be removed in GDB 14. > > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > > index 954f1481dae..dff7ba374bd 100644 > > --- a/gdb/doc/gdb.texinfo > > +++ b/gdb/doc/gdb.texinfo > > @@ -37948,6 +37948,13 @@ option (@pxref{GDB/MI Program Execution}). > > @item data-disassemble-a-option > > Indicates that the @code{-data-disassemble} command supports the @option{-a} > > option (@pxref{GDB/MI Data Manipulation}). > > +@item simple-values-ref-types > > +Indicates that the @code{--simple-values} argument to the > > +@code{-stack-list-arguments}, @code{-stack-list-locals}, > > +@code{-stack-list-variables}, and @code{-var-list-children} commands > > +takes reference types into account: that is, a value is considered > > +simple if it is neither an array, structure, or union, nor a reference > > +to an array, structure, or union. > > @end ftable > > > > @findex -list-target-features > > diff --git a/gdb/mi/mi-cmd-stack.c b/gdb/mi/mi-cmd-stack.c > > index 4c4662ab5d7..406bf85ceec 100644 > > --- a/gdb/mi/mi-cmd-stack.c > > +++ b/gdb/mi/mi-cmd-stack.c > > @@ -645,13 +645,8 @@ list_args_or_locals (const frame_print_options &fp_opts, > > switch (values) > > { > > case PRINT_SIMPLE_VALUES: > > - { > > - struct type *type = check_typedef (sym2->type ()); > > - if (type->code () == TYPE_CODE_ARRAY > > - || type->code () == TYPE_CODE_STRUCT > > - || type->code () == TYPE_CODE_UNION) > > - break; > > - } > > + if (!mi_simple_type_p (sym2->type ())) > > + break; > > /* FALLTHROUGH */ > > > > case PRINT_ALL_VALUES: > > diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c > > index e026c239b87..36eea4619a7 100644 > > --- a/gdb/mi/mi-cmd-var.c > > +++ b/gdb/mi/mi-cmd-var.c > > @@ -335,14 +335,27 @@ mi_print_value_p (struct varobj *var, enum print_values print_values) > > if (type == NULL) > > return 1; > > else > > - { > > - type = check_typedef (type); > > + return mi_simple_type_p (type); > > +} > > > > - /* For PRINT_SIMPLE_VALUES, only print the value if it has a type > > - and that type is not a compound type. */ > > - return (type->code () != TYPE_CODE_ARRAY > > - && type->code () != TYPE_CODE_STRUCT > > - && type->code () != TYPE_CODE_UNION); > > +/* See mi-cmds.h. */ > > + > > +bool > > +mi_simple_type_p (struct type *type) > > +{ > > + type = check_typedef (type); > > + > > + if (TYPE_IS_REFERENCE (type)) > > + type = check_typedef (type->target_type ()); > > + > > + switch (type->code ()) > > + { > > + case TYPE_CODE_ARRAY: > > + case TYPE_CODE_STRUCT: > > + case TYPE_CODE_UNION: > > + return false; > > + default: > > + return true; > > } > > } > > > > diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h > > index 490f50484d9..d867c298df9 100644 > > --- a/gdb/mi/mi-cmds.h > > +++ b/gdb/mi/mi-cmds.h > > @@ -226,4 +226,9 @@ using remove_mi_cmd_entries_ftype > > = gdb::function_view<bool (mi_command *)>; > > extern void remove_mi_cmd_entries (remove_mi_cmd_entries_ftype callback); > > > > +/* Return true if type is a simple type: that is, neither an array, structure, > > + or union, nor a reference to an array, structure, or union. */ > > + > > +extern bool mi_simple_type_p (struct type *type); > > + > > #endif /* MI_MI_CMDS_H */ > > diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c > > index 0013e5dfafd..cedad60d0b8 100644 > > --- a/gdb/mi/mi-main.c > > +++ b/gdb/mi/mi-main.c > > @@ -1656,6 +1656,7 @@ mi_cmd_list_features (const char *command, char **argv, int argc) > > uiout->field_string (NULL, "undefined-command-error-code"); > > uiout->field_string (NULL, "exec-run-start-option"); > > uiout->field_string (NULL, "data-disassemble-a-option"); > > + uiout->field_string (NULL, "simple-values-ref-types"); > > > > if (ext_lang_initialized_p (get_ext_lang_defn (EXT_LANG_PYTHON))) > > uiout->field_string (NULL, "python"); > > @@ -2458,7 +2459,6 @@ static void > > print_variable_or_computed (const char *expression, enum print_values values) > > { > > struct value *val; > > - struct type *type; > > struct ui_out *uiout = current_uiout; > > > > string_file stb; > > @@ -2478,12 +2478,9 @@ print_variable_or_computed (const char *expression, enum print_values values) > > switch (values) > > { > > case PRINT_SIMPLE_VALUES: > > - type = check_typedef (val->type ()); > > type_print (val->type (), "", &stb, -1); > > uiout->field_stream ("type", stb); > > - if (type->code () != TYPE_CODE_ARRAY > > - && type->code () != TYPE_CODE_STRUCT > > - && type->code () != TYPE_CODE_UNION) > > + if (mi_simple_type_p (val->type ())) > > { > > struct value_print_options opts; > > > > diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c > > index 0e8b2409636..e555dc3d879 100644 > > --- a/gdb/python/py-framefilter.c > > +++ b/gdb/python/py-framefilter.c > > @@ -235,14 +235,10 @@ py_print_value (struct ui_out *out, struct value *val, > > if (args_type == MI_PRINT_SIMPLE_VALUES > > || args_type == MI_PRINT_ALL_VALUES) > > { > > - struct type *type = check_typedef (val->type ()); > > - > > if (args_type == MI_PRINT_ALL_VALUES) > > should_print = 1; > > else if (args_type == MI_PRINT_SIMPLE_VALUES > > - && type->code () != TYPE_CODE_ARRAY > > - && type->code () != TYPE_CODE_STRUCT > > - && type->code () != TYPE_CODE_UNION) > > + && mi_simple_type_p (val->type ())) > > should_print = 1; > > } > > else if (args_type != NO_VALUES) > > diff --git a/gdb/testsuite/gdb.mi/mi-stack.exp b/gdb/testsuite/gdb.mi/mi-stack.exp > > index 633e090c13a..777a425894f 100644 > > --- a/gdb/testsuite/gdb.mi/mi-stack.exp > > +++ b/gdb/testsuite/gdb.mi/mi-stack.exp > > @@ -87,6 +87,9 @@ proc test_stack_args_listing {} { > > # -stack-list-arguments 1 1 1 > > # -stack-list-arguments 1 1 3 > > # -stack-list-arguments > > + # -stack-list-arguments 1 1 300 > > + # -stack-list-arguments 2 1 1 > > + # -stack-list-arguments --simple-values 1 1 > > > > mi_gdb_test "231-stack-list-arguments 0" \ > > "231\\^done,stack-args=\\\[frame=\{level=\"0\",args=\\\[\\\]\},frame=\{level=\"1\",args=\\\[name=\"strarg\"\\\]\},frame=\{level=\"2\",args=\\\[name=\"intarg\",name=\"strarg\"\\\]\},frame=\{level=\"3\",args=\\\[name=\"intarg\",name=\"strarg\",name=\"fltarg\"\\\]\},frame=\{level=\"4\",args=\\\[\\\]\}\\\]" \ > > @@ -119,6 +122,14 @@ proc test_stack_args_listing {} { > > mi_gdb_test "235-stack-list-arguments 1 1 300" \ > > "235\\^done,stack-args=\\\[frame=\{level=\"1\",args=\\\[\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\}\\\]\},frame=\{level=\"2\",args=\\\[\{name=\"intarg\",value=\"2\"\},\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\}\\\]\},frame=\{level=\"3\",args=\\\[\{name=\"intarg\",value=\"2\"\},\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\},\{name=\"fltarg\",value=\"3.5\"\}\\\]\},frame=\{level=\"4\",args=\\\[\\\]\}\\\]" \ > > "stack args listing 1 1 300" > > + > > + mi_gdb_test "236-stack-list-arguments 2 1 1" \ > > + "236\\^done,stack-args=\\\[frame=\\{level=\"1\",args=\\\[\\{name=\"strarg\",type=\"char \\*\",value=\"$hex \\\\\"A string argument.\\\\\"\"\\}\\\]\\}\\\]" \ > > + "stack args listing 2 1 1" > > + > > + mi_gdb_test "237-stack-list-arguments --simple-values 1 1" \ > > + "237\\^done,stack-args=\\\[frame=\\{level=\"1\",args=\\\[\\{name=\"strarg\",type=\"char \\*\",value=\"$hex \\\\\"A string argument.\\\\\"\"\\}\\\]\\}\\\]" \ > > + "stack args listing --simple-values 1 1" > > } > > > > proc test_stack_info_depth {} { > > @@ -157,7 +168,6 @@ proc test_stack_locals_listing {} { > > # -stack-list-locals 0 (--no-values) > > # -stack-list-locals 1 (--all-values) > > # -stack-list-locals 2 (--simple-values) > > - # -stack-list-arguments > > > > mi_gdb_test "232-stack-list-locals 0" \ > > "232\\^done,locals=\\\[name=\"A\",name=\"B\",name=\"C\",name=\"D\"\\\]" \ > > @@ -173,6 +183,10 @@ proc test_stack_locals_listing {} { > > "232\\^done,locals=\\\[\{name=\"A\",value=\"1\"\},\{name=\"B\",value=\"2\"\},\{name=\"C\",value=\"3\"\},\{name=\"D\",value=\"\\{0, 1, 2\\}\"\}\\\]" \ > > "stack locals listing of names and values" > > > > + mi_gdb_test "232-stack-list-locals 2" \ > > + "232\\^done,locals=\\\[\{name=\"A\",type=\"int\",value=\"1\"\},\{name=\"B\",type=\"int\",value=\"2\"\},\{name=\"C\",type=\"int\",value=\"3\"\},\{name=\"D\",type=\"int \\\[3\\\]\"\}\\\]" \ > > + "stack locals listing 2" > > + > > mi_gdb_test "232-stack-list-locals --simple-values" \ > > "232\\^done,locals=\\\[\{name=\"A\",type=\"int\",value=\"1\"\},\{name=\"B\",type=\"int\",value=\"2\"\},\{name=\"C\",type=\"int\",value=\"3\"\},\{name=\"D\",type=\"int \\\[3\\\]\"\}\\\]" \ > > "stack locals listing, simple types: names and values, complex type: names and types" > > diff --git a/gdb/testsuite/gdb.mi/print-simple-values.cc b/gdb/testsuite/gdb.mi/print-simple-values.cc > > new file mode 100644 > > index 00000000000..1aad724efb9 > > --- /dev/null > > +++ b/gdb/testsuite/gdb.mi/print-simple-values.cc > > @@ -0,0 +1,62 @@ > > +/* This test case is part of GDB, the GNU debugger. > > + > > + Copyright 2022-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 <http://www.gnu.org/licenses/>. */ > > + > > +/* Test program for PRINT_SIMPLE_VALUES. > > + > > + In the function f: > > + > > + * The arguments i, ir, and irr are ints or references to ints, which > > + must be printed by PRINT_SIMPLE_VALUES. > > + > > + * The arguments a, s, and u are non-scalar values, which must not be > > + printed by PRINT_SIMPLE_VALUES. > > + > > + * The arguments ar, arr, sr, srr, ur, and urr are references to > > + non-scalar values, which must not be printed by > > + PRINT_SIMPLE_VALUES. */ > > + > > +struct s > > +{ > > + int v; > > +}; > > + > > +union u > > +{ > > + int v; > > +}; > > + > > +int > > +f (int i, int &ir, int &&irr, > > + int a[1], int (&ar)[1], int (&&arr)[1], > > + struct s s, struct s &sr, struct s &&srr, > > + union u u, union u &ur, union u &&urr) > > +{ > > + return (i + ir + irr > > + + a[0] + ar[0] + arr[0] > > + + s.v + sr.v + srr.v > > + + u.v + ur.v + urr.v); > > +} > > + > > +int > > +main (void) > > +{ > > + int i = 1, j = 2; > > + int a[1] = { 4 }, b[1] = { 5 }; > > + struct s s = { 7 }, t = { 8 }; > > + union u u = { 10 }, v = { 11 }; > > + return f (i, j, 3, a, b, { 6 }, s, t, { 9 }, u, v, { 12 }); > > +} > > diff --git a/gdb/testsuite/gdb.mi/print-simple-values.exp b/gdb/testsuite/gdb.mi/print-simple-values.exp > > new file mode 100644 > > index 00000000000..9436645df84 > > --- /dev/null > > +++ b/gdb/testsuite/gdb.mi/print-simple-values.exp > > @@ -0,0 +1,53 @@ > > +# Copyright 2022-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 <http://www.gnu.org/licenses/>. > > + > > +# Test of PRINT_SIMPLE_VALUES. > > +# > > +# Test that PRINT_SIMPLE_VALUES distinguishes simple and compound types, > > +# including C++ reference and rvalue reference types. > > + > > +require allow_cplus_tests > > + > > +load_lib mi-support.exp > > +set MIFLAGS "-i=mi" > > + > > +standard_testfile .cc > > + > > +if [build_executable "failed to prepare" $testfile $srcfile {debug c++}] { > > + return -1 > > +} > > + > > +if [mi_clean_restart $binfile] { > > + return > > +} > > + > > +mi_runto_main > > + > > +mi_gdb_test "-break-insert f" "\\^done.*" "set breakpoint on f" > > + > > +mi_send_resuming_command "exec-continue" "exec-continue to breakpoint on f" > > + > > +mi_expect_stop "breakpoint-hit" "f" ".*" ".*" ".*" {.* disp="keep"} \ > > + "run until breakpoint on f" > > + > > +mi_gdb_test "-stack-list-arguments 2" \ > > + "\\^done,stack-args=\\\[frame=\\{level=\"0\",args=\\\[\\{name=\"i\",type=\"int\",value=\"1\"\\},\\{name=\"ir\",type=\"int &\",value=\"@$hex: 2\"\\},\\{name=\"irr\",type=\"int &&\",value=\"@$hex: 3\"\\},\\{name=\"a\",type=\"int \\*\",value=\"$hex\"\\},\\{name=\"ar\",type=\"int \\(&\\)\\\[1\\\]\"\\},\\{name=\"arr\",type=\"int \\(&&\\)\\\[1\\\]\"\\},\\{name=\"s\",type=\"s\"\\},\\{name=\"sr\",type=\"s &\"\\},\\{name=\"srr\",type=\"s &&\"\\},\\{name=\"u\",type=\"u\"\\},\\{name=\"ur\",type=\"u &\"\\},\\{name=\"urr\",type=\"u &&\"\\}\\\]\\},frame=\\{level=\"1\",args=\\\[\\\]\\}\\\]" \ > > + "stack arguments listing 2" > > + > > +mi_gdb_test "-stack-list-arguments --simple-values" \ > > + "\\^done,stack-args=\\\[frame=\\{level=\"0\",args=\\\[\\{name=\"i\",type=\"int\",value=\"1\"\\},\\{name=\"ir\",type=\"int &\",value=\"@$hex: 2\"\\},\\{name=\"irr\",type=\"int &&\",value=\"@$hex: 3\"\\},\\{name=\"a\",type=\"int \\*\",value=\"$hex\"\\},\\{name=\"ar\",type=\"int \\(&\\)\\\[1\\\]\"\\},\\{name=\"arr\",type=\"int \\(&&\\)\\\[1\\\]\"\\},\\{name=\"s\",type=\"s\"\\},\\{name=\"sr\",type=\"s &\"\\},\\{name=\"srr\",type=\"s &&\"\\},\\{name=\"u\",type=\"u\"\\},\\{name=\"ur\",type=\"u &\"\\},\\{name=\"urr\",type=\"u &&\"\\}\\\]\\},frame=\\{level=\"1\",args=\\\[\\\]\\}\\\]" \ > > + "stack arguments listing --simple-values" > > + > > +mi_gdb_exit > > -- > > 2.26.0 > >
Sixth ping. On Tue, 21 Mar 2023 at 09:50, Gareth Rees <grees@undo.io> wrote: > > This is a patch modifying the behaviour of the --simple-values option > but as discussed previously, if GDB maintainers agree that adding a > new option is the best approach, I would be happy to submit a patch > for that solution instead. > > On Sat, 11 Mar 2023 at 11:49, Gareth Rees <grees@undo.io> wrote: > > > > SUMMARY > > > > The '--simple-values' argument to '-stack-list-arguments' and similar > > GDB/MI commands does not take reference types into account, so that > > references to arbitrarily large structures are considered "simple" and > > printed. This means that the '--simple-values' argument cannot be used > > by IDEs when tracing the stack due to the time taken to print large > > structures passed by reference. > > > > DETAILS > > > > Various GDB/MI commands ('-stack-list-arguments', '-stack-list-locals', > > '-stack-list-variables' and so on) take a PRINT-VALUES argument which > > may be '--no-values' (0), '--all-values' (1) or '--simple-values' (2). > > In the '--simple-values' case, the command is supposed to print the > > name, type, and value of variables with simple types, and print only the > > name and type of variables with compound types. > > > > The '--simple-values' argument ought to be suitable for IDEs that need > > to update their user interface with the program's call stack every time > > the program stops. However, it does not take C++ reference types into > > account, and this makes the argument unsuitable for this purpose. > > > > For example, consider the following C++ program: > > > > struct s { > > int v[10]; > > }; > > > > int > > sum(const struct s &s) > > { > > int total = 0; > > for (int i = 0; i < 10; ++i) total += s.v[i]; > > return total; > > } > > > > int > > main(void) > > { > > struct s s = { { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 } }; > > return sum(s); > > } > > > > If we start GDB in MI mode and continue to 'sum', the behaviour of > > '-stack-list-arguments' is as follows: > > > > (gdb) > > -stack-list-arguments --simple-values > > ^done,stack-args=[frame={level="0",args=[{name="s",type="const s &",value="@0x7fffffffe310: {v = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10}}"}]},frame={level="1",args=[]}] > > > > Note that the value of the argument 's' was printed, even though 's' is > > a reference to a structure, which is not a simple value. > > > > See https://github.com/microsoft/MIEngine/pull/673 for a case where this > > behaviour caused Microsoft to avoid the use of '--simple-values' in > > their MIEngine debug adapter, because it caused Visual Studio Code to > > take too long to refresh the call stack in the user interface. > > > > SOLUTIONS > > > > There are two ways we could fix this problem, depending on whether we > > consider the current behaviour to be a bug. > > > > 1. If the current behaviour is a bug, then we can update the behaviour > > of '--simple-values' so that it takes reference types into account: > > that is, a value is simple if it is neither an array, struct, or > > union, nor a reference to an array, struct or union. > > > > In this case we must add a feature to the '-list-features' command so > > that IDEs can detect that it is safe to use the '--simple-values' > > argument when refreshing the call stack. > > > > 2. If the current behaviour is not a bug, then we can add a new option > > for the PRINT-VALUES argument, for example, '--scalar-values' (3), > > that would be suitable for use by IDEs. > > > > In this case we must add a feature to the '-list-features' command > > so that IDEs can detect that the '--scalar-values' argument is > > available for use when refreshing the call stack. > > > > PATCH > > > > This patch implements solution (1) as I think the current behaviour of > > not printing structures, but printing references to structures, is > > contrary to reasonable expectation. > > > > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29554 > > --- > > gdb/NEWS | 8 +++ > > gdb/doc/gdb.texinfo | 7 +++ > > gdb/mi/mi-cmd-stack.c | 9 +-- > > gdb/mi/mi-cmd-var.c | 27 ++++++--- > > gdb/mi/mi-cmds.h | 5 ++ > > gdb/mi/mi-main.c | 7 +-- > > gdb/python/py-framefilter.c | 6 +- > > gdb/testsuite/gdb.mi/mi-stack.exp | 16 ++++- > > gdb/testsuite/gdb.mi/print-simple-values.cc | 62 ++++++++++++++++++++ > > gdb/testsuite/gdb.mi/print-simple-values.exp | 53 +++++++++++++++++ > > 10 files changed, 175 insertions(+), 25 deletions(-) > > create mode 100644 gdb/testsuite/gdb.mi/print-simple-values.cc > > create mode 100644 gdb/testsuite/gdb.mi/print-simple-values.exp > > > > diff --git a/gdb/NEWS b/gdb/NEWS > > index cc262f1f8a6..1971b76a736 100644 > > --- a/gdb/NEWS > > +++ b/gdb/NEWS > > @@ -104,6 +104,14 @@ show always-read-ctf > > without a thread restriction. The same is also true for the 'task' > > field of an Ada task-specific breakpoint. > > > > +** The '--simple-values' argument to the '-stack-list-arguments', > > + '-stack-list-locals', '-stack-list-variables', and '-var-list-children' > > + commands now takes reference types into account: that is, a value is now > > + considered simple if it is neither an array, structure, or union, nor a > > + reference to an array, structure, or union. (Previously all references were > > + considered simple.) Support for this feature can be verified by using the > > + '-list-features' command, which should contain "simple-values-ref-types". > > + > > *** Changes in GDB 13 > > > > * MI version 1 is deprecated, and will be removed in GDB 14. > > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > > index 954f1481dae..dff7ba374bd 100644 > > --- a/gdb/doc/gdb.texinfo > > +++ b/gdb/doc/gdb.texinfo > > @@ -37948,6 +37948,13 @@ option (@pxref{GDB/MI Program Execution}). > > @item data-disassemble-a-option > > Indicates that the @code{-data-disassemble} command supports the @option{-a} > > option (@pxref{GDB/MI Data Manipulation}). > > +@item simple-values-ref-types > > +Indicates that the @code{--simple-values} argument to the > > +@code{-stack-list-arguments}, @code{-stack-list-locals}, > > +@code{-stack-list-variables}, and @code{-var-list-children} commands > > +takes reference types into account: that is, a value is considered > > +simple if it is neither an array, structure, or union, nor a reference > > +to an array, structure, or union. > > @end ftable > > > > @findex -list-target-features > > diff --git a/gdb/mi/mi-cmd-stack.c b/gdb/mi/mi-cmd-stack.c > > index 4c4662ab5d7..406bf85ceec 100644 > > --- a/gdb/mi/mi-cmd-stack.c > > +++ b/gdb/mi/mi-cmd-stack.c > > @@ -645,13 +645,8 @@ list_args_or_locals (const frame_print_options &fp_opts, > > switch (values) > > { > > case PRINT_SIMPLE_VALUES: > > - { > > - struct type *type = check_typedef (sym2->type ()); > > - if (type->code () == TYPE_CODE_ARRAY > > - || type->code () == TYPE_CODE_STRUCT > > - || type->code () == TYPE_CODE_UNION) > > - break; > > - } > > + if (!mi_simple_type_p (sym2->type ())) > > + break; > > /* FALLTHROUGH */ > > > > case PRINT_ALL_VALUES: > > diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c > > index e026c239b87..36eea4619a7 100644 > > --- a/gdb/mi/mi-cmd-var.c > > +++ b/gdb/mi/mi-cmd-var.c > > @@ -335,14 +335,27 @@ mi_print_value_p (struct varobj *var, enum print_values print_values) > > if (type == NULL) > > return 1; > > else > > - { > > - type = check_typedef (type); > > + return mi_simple_type_p (type); > > +} > > > > - /* For PRINT_SIMPLE_VALUES, only print the value if it has a type > > - and that type is not a compound type. */ > > - return (type->code () != TYPE_CODE_ARRAY > > - && type->code () != TYPE_CODE_STRUCT > > - && type->code () != TYPE_CODE_UNION); > > +/* See mi-cmds.h. */ > > + > > +bool > > +mi_simple_type_p (struct type *type) > > +{ > > + type = check_typedef (type); > > + > > + if (TYPE_IS_REFERENCE (type)) > > + type = check_typedef (type->target_type ()); > > + > > + switch (type->code ()) > > + { > > + case TYPE_CODE_ARRAY: > > + case TYPE_CODE_STRUCT: > > + case TYPE_CODE_UNION: > > + return false; > > + default: > > + return true; > > } > > } > > > > diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h > > index 490f50484d9..d867c298df9 100644 > > --- a/gdb/mi/mi-cmds.h > > +++ b/gdb/mi/mi-cmds.h > > @@ -226,4 +226,9 @@ using remove_mi_cmd_entries_ftype > > = gdb::function_view<bool (mi_command *)>; > > extern void remove_mi_cmd_entries (remove_mi_cmd_entries_ftype callback); > > > > +/* Return true if type is a simple type: that is, neither an array, structure, > > + or union, nor a reference to an array, structure, or union. */ > > + > > +extern bool mi_simple_type_p (struct type *type); > > + > > #endif /* MI_MI_CMDS_H */ > > diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c > > index 0013e5dfafd..cedad60d0b8 100644 > > --- a/gdb/mi/mi-main.c > > +++ b/gdb/mi/mi-main.c > > @@ -1656,6 +1656,7 @@ mi_cmd_list_features (const char *command, char **argv, int argc) > > uiout->field_string (NULL, "undefined-command-error-code"); > > uiout->field_string (NULL, "exec-run-start-option"); > > uiout->field_string (NULL, "data-disassemble-a-option"); > > + uiout->field_string (NULL, "simple-values-ref-types"); > > > > if (ext_lang_initialized_p (get_ext_lang_defn (EXT_LANG_PYTHON))) > > uiout->field_string (NULL, "python"); > > @@ -2458,7 +2459,6 @@ static void > > print_variable_or_computed (const char *expression, enum print_values values) > > { > > struct value *val; > > - struct type *type; > > struct ui_out *uiout = current_uiout; > > > > string_file stb; > > @@ -2478,12 +2478,9 @@ print_variable_or_computed (const char *expression, enum print_values values) > > switch (values) > > { > > case PRINT_SIMPLE_VALUES: > > - type = check_typedef (val->type ()); > > type_print (val->type (), "", &stb, -1); > > uiout->field_stream ("type", stb); > > - if (type->code () != TYPE_CODE_ARRAY > > - && type->code () != TYPE_CODE_STRUCT > > - && type->code () != TYPE_CODE_UNION) > > + if (mi_simple_type_p (val->type ())) > > { > > struct value_print_options opts; > > > > diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c > > index 0e8b2409636..e555dc3d879 100644 > > --- a/gdb/python/py-framefilter.c > > +++ b/gdb/python/py-framefilter.c > > @@ -235,14 +235,10 @@ py_print_value (struct ui_out *out, struct value *val, > > if (args_type == MI_PRINT_SIMPLE_VALUES > > || args_type == MI_PRINT_ALL_VALUES) > > { > > - struct type *type = check_typedef (val->type ()); > > - > > if (args_type == MI_PRINT_ALL_VALUES) > > should_print = 1; > > else if (args_type == MI_PRINT_SIMPLE_VALUES > > - && type->code () != TYPE_CODE_ARRAY > > - && type->code () != TYPE_CODE_STRUCT > > - && type->code () != TYPE_CODE_UNION) > > + && mi_simple_type_p (val->type ())) > > should_print = 1; > > } > > else if (args_type != NO_VALUES) > > diff --git a/gdb/testsuite/gdb.mi/mi-stack.exp b/gdb/testsuite/gdb.mi/mi-stack.exp > > index 633e090c13a..777a425894f 100644 > > --- a/gdb/testsuite/gdb.mi/mi-stack.exp > > +++ b/gdb/testsuite/gdb.mi/mi-stack.exp > > @@ -87,6 +87,9 @@ proc test_stack_args_listing {} { > > # -stack-list-arguments 1 1 1 > > # -stack-list-arguments 1 1 3 > > # -stack-list-arguments > > + # -stack-list-arguments 1 1 300 > > + # -stack-list-arguments 2 1 1 > > + # -stack-list-arguments --simple-values 1 1 > > > > mi_gdb_test "231-stack-list-arguments 0" \ > > "231\\^done,stack-args=\\\[frame=\{level=\"0\",args=\\\[\\\]\},frame=\{level=\"1\",args=\\\[name=\"strarg\"\\\]\},frame=\{level=\"2\",args=\\\[name=\"intarg\",name=\"strarg\"\\\]\},frame=\{level=\"3\",args=\\\[name=\"intarg\",name=\"strarg\",name=\"fltarg\"\\\]\},frame=\{level=\"4\",args=\\\[\\\]\}\\\]" \ > > @@ -119,6 +122,14 @@ proc test_stack_args_listing {} { > > mi_gdb_test "235-stack-list-arguments 1 1 300" \ > > "235\\^done,stack-args=\\\[frame=\{level=\"1\",args=\\\[\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\}\\\]\},frame=\{level=\"2\",args=\\\[\{name=\"intarg\",value=\"2\"\},\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\}\\\]\},frame=\{level=\"3\",args=\\\[\{name=\"intarg\",value=\"2\"\},\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\},\{name=\"fltarg\",value=\"3.5\"\}\\\]\},frame=\{level=\"4\",args=\\\[\\\]\}\\\]" \ > > "stack args listing 1 1 300" > > + > > + mi_gdb_test "236-stack-list-arguments 2 1 1" \ > > + "236\\^done,stack-args=\\\[frame=\\{level=\"1\",args=\\\[\\{name=\"strarg\",type=\"char \\*\",value=\"$hex \\\\\"A string argument.\\\\\"\"\\}\\\]\\}\\\]" \ > > + "stack args listing 2 1 1" > > + > > + mi_gdb_test "237-stack-list-arguments --simple-values 1 1" \ > > + "237\\^done,stack-args=\\\[frame=\\{level=\"1\",args=\\\[\\{name=\"strarg\",type=\"char \\*\",value=\"$hex \\\\\"A string argument.\\\\\"\"\\}\\\]\\}\\\]" \ > > + "stack args listing --simple-values 1 1" > > } > > > > proc test_stack_info_depth {} { > > @@ -157,7 +168,6 @@ proc test_stack_locals_listing {} { > > # -stack-list-locals 0 (--no-values) > > # -stack-list-locals 1 (--all-values) > > # -stack-list-locals 2 (--simple-values) > > - # -stack-list-arguments > > > > mi_gdb_test "232-stack-list-locals 0" \ > > "232\\^done,locals=\\\[name=\"A\",name=\"B\",name=\"C\",name=\"D\"\\\]" \ > > @@ -173,6 +183,10 @@ proc test_stack_locals_listing {} { > > "232\\^done,locals=\\\[\{name=\"A\",value=\"1\"\},\{name=\"B\",value=\"2\"\},\{name=\"C\",value=\"3\"\},\{name=\"D\",value=\"\\{0, 1, 2\\}\"\}\\\]" \ > > "stack locals listing of names and values" > > > > + mi_gdb_test "232-stack-list-locals 2" \ > > + "232\\^done,locals=\\\[\{name=\"A\",type=\"int\",value=\"1\"\},\{name=\"B\",type=\"int\",value=\"2\"\},\{name=\"C\",type=\"int\",value=\"3\"\},\{name=\"D\",type=\"int \\\[3\\\]\"\}\\\]" \ > > + "stack locals listing 2" > > + > > mi_gdb_test "232-stack-list-locals --simple-values" \ > > "232\\^done,locals=\\\[\{name=\"A\",type=\"int\",value=\"1\"\},\{name=\"B\",type=\"int\",value=\"2\"\},\{name=\"C\",type=\"int\",value=\"3\"\},\{name=\"D\",type=\"int \\\[3\\\]\"\}\\\]" \ > > "stack locals listing, simple types: names and values, complex type: names and types" > > diff --git a/gdb/testsuite/gdb.mi/print-simple-values.cc b/gdb/testsuite/gdb.mi/print-simple-values.cc > > new file mode 100644 > > index 00000000000..1aad724efb9 > > --- /dev/null > > +++ b/gdb/testsuite/gdb.mi/print-simple-values.cc > > @@ -0,0 +1,62 @@ > > +/* This test case is part of GDB, the GNU debugger. > > + > > + Copyright 2022-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 <http://www.gnu.org/licenses/>. */ > > + > > +/* Test program for PRINT_SIMPLE_VALUES. > > + > > + In the function f: > > + > > + * The arguments i, ir, and irr are ints or references to ints, which > > + must be printed by PRINT_SIMPLE_VALUES. > > + > > + * The arguments a, s, and u are non-scalar values, which must not be > > + printed by PRINT_SIMPLE_VALUES. > > + > > + * The arguments ar, arr, sr, srr, ur, and urr are references to > > + non-scalar values, which must not be printed by > > + PRINT_SIMPLE_VALUES. */ > > + > > +struct s > > +{ > > + int v; > > +}; > > + > > +union u > > +{ > > + int v; > > +}; > > + > > +int > > +f (int i, int &ir, int &&irr, > > + int a[1], int (&ar)[1], int (&&arr)[1], > > + struct s s, struct s &sr, struct s &&srr, > > + union u u, union u &ur, union u &&urr) > > +{ > > + return (i + ir + irr > > + + a[0] + ar[0] + arr[0] > > + + s.v + sr.v + srr.v > > + + u.v + ur.v + urr.v); > > +} > > + > > +int > > +main (void) > > +{ > > + int i = 1, j = 2; > > + int a[1] = { 4 }, b[1] = { 5 }; > > + struct s s = { 7 }, t = { 8 }; > > + union u u = { 10 }, v = { 11 }; > > + return f (i, j, 3, a, b, { 6 }, s, t, { 9 }, u, v, { 12 }); > > +} > > diff --git a/gdb/testsuite/gdb.mi/print-simple-values.exp b/gdb/testsuite/gdb.mi/print-simple-values.exp > > new file mode 100644 > > index 00000000000..9436645df84 > > --- /dev/null > > +++ b/gdb/testsuite/gdb.mi/print-simple-values.exp > > @@ -0,0 +1,53 @@ > > +# Copyright 2022-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 <http://www.gnu.org/licenses/>. > > + > > +# Test of PRINT_SIMPLE_VALUES. > > +# > > +# Test that PRINT_SIMPLE_VALUES distinguishes simple and compound types, > > +# including C++ reference and rvalue reference types. > > + > > +require allow_cplus_tests > > + > > +load_lib mi-support.exp > > +set MIFLAGS "-i=mi" > > + > > +standard_testfile .cc > > + > > +if [build_executable "failed to prepare" $testfile $srcfile {debug c++}] { > > + return -1 > > +} > > + > > +if [mi_clean_restart $binfile] { > > + return > > +} > > + > > +mi_runto_main > > + > > +mi_gdb_test "-break-insert f" "\\^done.*" "set breakpoint on f" > > + > > +mi_send_resuming_command "exec-continue" "exec-continue to breakpoint on f" > > + > > +mi_expect_stop "breakpoint-hit" "f" ".*" ".*" ".*" {.* disp="keep"} \ > > + "run until breakpoint on f" > > + > > +mi_gdb_test "-stack-list-arguments 2" \ > > + "\\^done,stack-args=\\\[frame=\\{level=\"0\",args=\\\[\\{name=\"i\",type=\"int\",value=\"1\"\\},\\{name=\"ir\",type=\"int &\",value=\"@$hex: 2\"\\},\\{name=\"irr\",type=\"int &&\",value=\"@$hex: 3\"\\},\\{name=\"a\",type=\"int \\*\",value=\"$hex\"\\},\\{name=\"ar\",type=\"int \\(&\\)\\\[1\\\]\"\\},\\{name=\"arr\",type=\"int \\(&&\\)\\\[1\\\]\"\\},\\{name=\"s\",type=\"s\"\\},\\{name=\"sr\",type=\"s &\"\\},\\{name=\"srr\",type=\"s &&\"\\},\\{name=\"u\",type=\"u\"\\},\\{name=\"ur\",type=\"u &\"\\},\\{name=\"urr\",type=\"u &&\"\\}\\\]\\},frame=\\{level=\"1\",args=\\\[\\\]\\}\\\]" \ > > + "stack arguments listing 2" > > + > > +mi_gdb_test "-stack-list-arguments --simple-values" \ > > + "\\^done,stack-args=\\\[frame=\\{level=\"0\",args=\\\[\\{name=\"i\",type=\"int\",value=\"1\"\\},\\{name=\"ir\",type=\"int &\",value=\"@$hex: 2\"\\},\\{name=\"irr\",type=\"int &&\",value=\"@$hex: 3\"\\},\\{name=\"a\",type=\"int \\*\",value=\"$hex\"\\},\\{name=\"ar\",type=\"int \\(&\\)\\\[1\\\]\"\\},\\{name=\"arr\",type=\"int \\(&&\\)\\\[1\\\]\"\\},\\{name=\"s\",type=\"s\"\\},\\{name=\"sr\",type=\"s &\"\\},\\{name=\"srr\",type=\"s &&\"\\},\\{name=\"u\",type=\"u\"\\},\\{name=\"ur\",type=\"u &\"\\},\\{name=\"urr\",type=\"u &&\"\\}\\\]\\},frame=\\{level=\"1\",args=\\\[\\\]\\}\\\]" \ > > + "stack arguments listing --simple-values" > > + > > +mi_gdb_exit > > -- > > 2.26.0 > >
>>>>> Gareth Rees via Gdb-patches <gdb-patches@sourceware.org> writes: > Third ping. Sorry about the delay on this. I've applied the patch and fixed up some whitespace problems that git pointed out. I'm going to push it now. Tom
diff --git a/gdb/NEWS b/gdb/NEWS index cc262f1f8a6..1971b76a736 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -104,6 +104,14 @@ show always-read-ctf without a thread restriction. The same is also true for the 'task' field of an Ada task-specific breakpoint. +** The '--simple-values' argument to the '-stack-list-arguments', + '-stack-list-locals', '-stack-list-variables', and '-var-list-children' + commands now takes reference types into account: that is, a value is now + considered simple if it is neither an array, structure, or union, nor a + reference to an array, structure, or union. (Previously all references were + considered simple.) Support for this feature can be verified by using the + '-list-features' command, which should contain "simple-values-ref-types". + *** Changes in GDB 13 * MI version 1 is deprecated, and will be removed in GDB 14. diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 954f1481dae..dff7ba374bd 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -37948,6 +37948,13 @@ option (@pxref{GDB/MI Program Execution}). @item data-disassemble-a-option Indicates that the @code{-data-disassemble} command supports the @option{-a} option (@pxref{GDB/MI Data Manipulation}). +@item simple-values-ref-types +Indicates that the @code{--simple-values} argument to the +@code{-stack-list-arguments}, @code{-stack-list-locals}, +@code{-stack-list-variables}, and @code{-var-list-children} commands +takes reference types into account: that is, a value is considered +simple if it is neither an array, structure, or union, nor a reference +to an array, structure, or union. @end ftable @findex -list-target-features diff --git a/gdb/mi/mi-cmd-stack.c b/gdb/mi/mi-cmd-stack.c index 4c4662ab5d7..406bf85ceec 100644 --- a/gdb/mi/mi-cmd-stack.c +++ b/gdb/mi/mi-cmd-stack.c @@ -645,13 +645,8 @@ list_args_or_locals (const frame_print_options &fp_opts, switch (values) { case PRINT_SIMPLE_VALUES: - { - struct type *type = check_typedef (sym2->type ()); - if (type->code () == TYPE_CODE_ARRAY - || type->code () == TYPE_CODE_STRUCT - || type->code () == TYPE_CODE_UNION) - break; - } + if (!mi_simple_type_p (sym2->type ())) + break; /* FALLTHROUGH */ case PRINT_ALL_VALUES: diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c index e026c239b87..36eea4619a7 100644 --- a/gdb/mi/mi-cmd-var.c +++ b/gdb/mi/mi-cmd-var.c @@ -335,14 +335,27 @@ mi_print_value_p (struct varobj *var, enum print_values print_values) if (type == NULL) return 1; else - { - type = check_typedef (type); + return mi_simple_type_p (type); +} - /* For PRINT_SIMPLE_VALUES, only print the value if it has a type - and that type is not a compound type. */ - return (type->code () != TYPE_CODE_ARRAY - && type->code () != TYPE_CODE_STRUCT - && type->code () != TYPE_CODE_UNION); +/* See mi-cmds.h. */ + +bool +mi_simple_type_p (struct type *type) +{ + type = check_typedef (type); + + if (TYPE_IS_REFERENCE (type)) + type = check_typedef (type->target_type ()); + + switch (type->code ()) + { + case TYPE_CODE_ARRAY: + case TYPE_CODE_STRUCT: + case TYPE_CODE_UNION: + return false; + default: + return true; } } diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h index 490f50484d9..d867c298df9 100644 --- a/gdb/mi/mi-cmds.h +++ b/gdb/mi/mi-cmds.h @@ -226,4 +226,9 @@ using remove_mi_cmd_entries_ftype = gdb::function_view<bool (mi_command *)>; extern void remove_mi_cmd_entries (remove_mi_cmd_entries_ftype callback); +/* Return true if type is a simple type: that is, neither an array, structure, + or union, nor a reference to an array, structure, or union. */ + +extern bool mi_simple_type_p (struct type *type); + #endif /* MI_MI_CMDS_H */ diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index 0013e5dfafd..cedad60d0b8 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -1656,6 +1656,7 @@ mi_cmd_list_features (const char *command, char **argv, int argc) uiout->field_string (NULL, "undefined-command-error-code"); uiout->field_string (NULL, "exec-run-start-option"); uiout->field_string (NULL, "data-disassemble-a-option"); + uiout->field_string (NULL, "simple-values-ref-types"); if (ext_lang_initialized_p (get_ext_lang_defn (EXT_LANG_PYTHON))) uiout->field_string (NULL, "python"); @@ -2458,7 +2459,6 @@ static void print_variable_or_computed (const char *expression, enum print_values values) { struct value *val; - struct type *type; struct ui_out *uiout = current_uiout; string_file stb; @@ -2478,12 +2478,9 @@ print_variable_or_computed (const char *expression, enum print_values values) switch (values) { case PRINT_SIMPLE_VALUES: - type = check_typedef (val->type ()); type_print (val->type (), "", &stb, -1); uiout->field_stream ("type", stb); - if (type->code () != TYPE_CODE_ARRAY - && type->code () != TYPE_CODE_STRUCT - && type->code () != TYPE_CODE_UNION) + if (mi_simple_type_p (val->type ())) { struct value_print_options opts; diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c index 0e8b2409636..e555dc3d879 100644 --- a/gdb/python/py-framefilter.c +++ b/gdb/python/py-framefilter.c @@ -235,14 +235,10 @@ py_print_value (struct ui_out *out, struct value *val, if (args_type == MI_PRINT_SIMPLE_VALUES || args_type == MI_PRINT_ALL_VALUES) { - struct type *type = check_typedef (val->type ()); - if (args_type == MI_PRINT_ALL_VALUES) should_print = 1; else if (args_type == MI_PRINT_SIMPLE_VALUES - && type->code () != TYPE_CODE_ARRAY - && type->code () != TYPE_CODE_STRUCT - && type->code () != TYPE_CODE_UNION) + && mi_simple_type_p (val->type ())) should_print = 1; } else if (args_type != NO_VALUES) diff --git a/gdb/testsuite/gdb.mi/mi-stack.exp b/gdb/testsuite/gdb.mi/mi-stack.exp index 633e090c13a..777a425894f 100644 --- a/gdb/testsuite/gdb.mi/mi-stack.exp +++ b/gdb/testsuite/gdb.mi/mi-stack.exp @@ -87,6 +87,9 @@ proc test_stack_args_listing {} { # -stack-list-arguments 1 1 1 # -stack-list-arguments 1 1 3 # -stack-list-arguments + # -stack-list-arguments 1 1 300 + # -stack-list-arguments 2 1 1 + # -stack-list-arguments --simple-values 1 1 mi_gdb_test "231-stack-list-arguments 0" \ "231\\^done,stack-args=\\\[frame=\{level=\"0\",args=\\\[\\\]\},frame=\{level=\"1\",args=\\\[name=\"strarg\"\\\]\},frame=\{level=\"2\",args=\\\[name=\"intarg\",name=\"strarg\"\\\]\},frame=\{level=\"3\",args=\\\[name=\"intarg\",name=\"strarg\",name=\"fltarg\"\\\]\},frame=\{level=\"4\",args=\\\[\\\]\}\\\]" \ @@ -119,6 +122,14 @@ proc test_stack_args_listing {} { mi_gdb_test "235-stack-list-arguments 1 1 300" \ "235\\^done,stack-args=\\\[frame=\{level=\"1\",args=\\\[\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\}\\\]\},frame=\{level=\"2\",args=\\\[\{name=\"intarg\",value=\"2\"\},\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\}\\\]\},frame=\{level=\"3\",args=\\\[\{name=\"intarg\",value=\"2\"\},\{name=\"strarg\",value=\"$hex \\\\\"A string argument.\\\\\"\"\},\{name=\"fltarg\",value=\"3.5\"\}\\\]\},frame=\{level=\"4\",args=\\\[\\\]\}\\\]" \ "stack args listing 1 1 300" + + mi_gdb_test "236-stack-list-arguments 2 1 1" \ + "236\\^done,stack-args=\\\[frame=\\{level=\"1\",args=\\\[\\{name=\"strarg\",type=\"char \\*\",value=\"$hex \\\\\"A string argument.\\\\\"\"\\}\\\]\\}\\\]" \ + "stack args listing 2 1 1" + + mi_gdb_test "237-stack-list-arguments --simple-values 1 1" \ + "237\\^done,stack-args=\\\[frame=\\{level=\"1\",args=\\\[\\{name=\"strarg\",type=\"char \\*\",value=\"$hex \\\\\"A string argument.\\\\\"\"\\}\\\]\\}\\\]" \ + "stack args listing --simple-values 1 1" } proc test_stack_info_depth {} { @@ -157,7 +168,6 @@ proc test_stack_locals_listing {} { # -stack-list-locals 0 (--no-values) # -stack-list-locals 1 (--all-values) # -stack-list-locals 2 (--simple-values) - # -stack-list-arguments mi_gdb_test "232-stack-list-locals 0" \ "232\\^done,locals=\\\[name=\"A\",name=\"B\",name=\"C\",name=\"D\"\\\]" \ @@ -173,6 +183,10 @@ proc test_stack_locals_listing {} { "232\\^done,locals=\\\[\{name=\"A\",value=\"1\"\},\{name=\"B\",value=\"2\"\},\{name=\"C\",value=\"3\"\},\{name=\"D\",value=\"\\{0, 1, 2\\}\"\}\\\]" \ "stack locals listing of names and values" + mi_gdb_test "232-stack-list-locals 2" \ + "232\\^done,locals=\\\[\{name=\"A\",type=\"int\",value=\"1\"\},\{name=\"B\",type=\"int\",value=\"2\"\},\{name=\"C\",type=\"int\",value=\"3\"\},\{name=\"D\",type=\"int \\\[3\\\]\"\}\\\]" \ + "stack locals listing 2" + mi_gdb_test "232-stack-list-locals --simple-values" \ "232\\^done,locals=\\\[\{name=\"A\",type=\"int\",value=\"1\"\},\{name=\"B\",type=\"int\",value=\"2\"\},\{name=\"C\",type=\"int\",value=\"3\"\},\{name=\"D\",type=\"int \\\[3\\\]\"\}\\\]" \ "stack locals listing, simple types: names and values, complex type: names and types" diff --git a/gdb/testsuite/gdb.mi/print-simple-values.cc b/gdb/testsuite/gdb.mi/print-simple-values.cc new file mode 100644 index 00000000000..1aad724efb9 --- /dev/null +++ b/gdb/testsuite/gdb.mi/print-simple-values.cc @@ -0,0 +1,62 @@ +/* This test case is part of GDB, the GNU debugger. + + Copyright 2022-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 <http://www.gnu.org/licenses/>. */ + +/* Test program for PRINT_SIMPLE_VALUES. + + In the function f: + + * The arguments i, ir, and irr are ints or references to ints, which + must be printed by PRINT_SIMPLE_VALUES. + + * The arguments a, s, and u are non-scalar values, which must not be + printed by PRINT_SIMPLE_VALUES. + + * The arguments ar, arr, sr, srr, ur, and urr are references to + non-scalar values, which must not be printed by + PRINT_SIMPLE_VALUES. */ + +struct s +{ + int v; +}; + +union u +{ + int v; +}; + +int +f (int i, int &ir, int &&irr, + int a[1], int (&ar)[1], int (&&arr)[1], + struct s s, struct s &sr, struct s &&srr, + union u u, union u &ur, union u &&urr) +{ + return (i + ir + irr + + a[0] + ar[0] + arr[0] + + s.v + sr.v + srr.v + + u.v + ur.v + urr.v); +} + +int +main (void) +{ + int i = 1, j = 2; + int a[1] = { 4 }, b[1] = { 5 }; + struct s s = { 7 }, t = { 8 }; + union u u = { 10 }, v = { 11 }; + return f (i, j, 3, a, b, { 6 }, s, t, { 9 }, u, v, { 12 }); +} diff --git a/gdb/testsuite/gdb.mi/print-simple-values.exp b/gdb/testsuite/gdb.mi/print-simple-values.exp new file mode 100644 index 00000000000..9436645df84 --- /dev/null +++ b/gdb/testsuite/gdb.mi/print-simple-values.exp @@ -0,0 +1,53 @@ +# Copyright 2022-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 <http://www.gnu.org/licenses/>. + +# Test of PRINT_SIMPLE_VALUES. +# +# Test that PRINT_SIMPLE_VALUES distinguishes simple and compound types, +# including C++ reference and rvalue reference types. + +require allow_cplus_tests + +load_lib mi-support.exp +set MIFLAGS "-i=mi" + +standard_testfile .cc + +if [build_executable "failed to prepare" $testfile $srcfile {debug c++}] { + return -1 +} + +if [mi_clean_restart $binfile] { + return +} + +mi_runto_main + +mi_gdb_test "-break-insert f" "\\^done.*" "set breakpoint on f" + +mi_send_resuming_command "exec-continue" "exec-continue to breakpoint on f" + +mi_expect_stop "breakpoint-hit" "f" ".*" ".*" ".*" {.* disp="keep"} \ + "run until breakpoint on f" + +mi_gdb_test "-stack-list-arguments 2" \ + "\\^done,stack-args=\\\[frame=\\{level=\"0\",args=\\\[\\{name=\"i\",type=\"int\",value=\"1\"\\},\\{name=\"ir\",type=\"int &\",value=\"@$hex: 2\"\\},\\{name=\"irr\",type=\"int &&\",value=\"@$hex: 3\"\\},\\{name=\"a\",type=\"int \\*\",value=\"$hex\"\\},\\{name=\"ar\",type=\"int \\(&\\)\\\[1\\\]\"\\},\\{name=\"arr\",type=\"int \\(&&\\)\\\[1\\\]\"\\},\\{name=\"s\",type=\"s\"\\},\\{name=\"sr\",type=\"s &\"\\},\\{name=\"srr\",type=\"s &&\"\\},\\{name=\"u\",type=\"u\"\\},\\{name=\"ur\",type=\"u &\"\\},\\{name=\"urr\",type=\"u &&\"\\}\\\]\\},frame=\\{level=\"1\",args=\\\[\\\]\\}\\\]" \ + "stack arguments listing 2" + +mi_gdb_test "-stack-list-arguments --simple-values" \ + "\\^done,stack-args=\\\[frame=\\{level=\"0\",args=\\\[\\{name=\"i\",type=\"int\",value=\"1\"\\},\\{name=\"ir\",type=\"int &\",value=\"@$hex: 2\"\\},\\{name=\"irr\",type=\"int &&\",value=\"@$hex: 3\"\\},\\{name=\"a\",type=\"int \\*\",value=\"$hex\"\\},\\{name=\"ar\",type=\"int \\(&\\)\\\[1\\\]\"\\},\\{name=\"arr\",type=\"int \\(&&\\)\\\[1\\\]\"\\},\\{name=\"s\",type=\"s\"\\},\\{name=\"sr\",type=\"s &\"\\},\\{name=\"srr\",type=\"s &&\"\\},\\{name=\"u\",type=\"u\"\\},\\{name=\"ur\",type=\"u &\"\\},\\{name=\"urr\",type=\"u &&\"\\}\\\]\\},frame=\\{level=\"1\",args=\\\[\\\]\\}\\\]" \ + "stack arguments listing --simple-values" + +mi_gdb_exit