Don't treat references to compound values as "simple".

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

diff --git a/gdb/NEWS b/gdb/NEWS
index e3c095de09e71bd363312f4588362fe27f30f65a..6aa0d5171f26e165bd12d0ee5d7e68d0918c7c59 100644 (file)
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -119,6 +119,14 @@ info main
    CLI, that the MI allowed it was a long standing bug, which has now
    been fixed.
 
+** 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".
+
 * Python API
 
   ** The gdb.unwinder.Unwinder.name attribute is now read-only.

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index ed9cec646fa43a07b3b0eec0f0294baabfadba1e..8c4177c19010d146e50080183daf72b69e3dddf4 100644 (file)
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -38009,6 +38009,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 b8169a67e4426e052db4a513f95500098d830f9e..0172bf9814d57de652ddba7109b0946b38f59340 100644 (file)
--- 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 e026c239b87a887fb9dbbcb6f12c65340b861d5d..36eea4619a755ff39dd8351cb78d9274362c5dec 100644 (file)
--- 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 490f50484d9dc16659fe5934f6ddde59e73797cb..d867c298df98c77c59ac0e927c477a1430f83b76 100644 (file)
--- 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 35c74c407ee8967d9cffe3f936934fede6a20b61..840663e0fb4ecb305298aa811293cb870838b4ea 100644 (file)
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1657,6 +1657,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");
@@ -2459,7 +2460,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;
@@ -2479,12 +2479,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 0e8b24096367c6d1b2083bda87ca9692942d2f61..e555dc3d8797f879bd7141c3d2c70cc618bb60d2 100644 (file)
--- 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 633e090c13a1cf7df2a21c1d0bdfa1196b5fff02..777a425894f84c709302e4751aae0e5877e44c4f 100644 (file)
--- 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 (file)
index 0000000..00969e6
--- /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 (file)
index 0000000..9436645
--- /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