gdb/python/guile: fix segfault from nested prefix command creation

A commit I recently pushed:

  commit 0b5023cc71d3af8b18e10e6599a3f9381bc15265
  Date:   Sat Apr 12 09:15:53 2025 +0100

      gdb/python/guile: user created prefix commands get help list

can trigger a segfault if a user tries to create nested prefix
commands.  For example, this will trigger a crash:

  (gdb) python gdb.ParameterPrefix("prefix-1", gdb.COMMAND_NONE)
  (gdb) python gdb.ParameterPrefix("prefix-1 prefix-2", gdb.COMMAND_NONE)

  Fatal signal: Segmentation fault
  ... etc ...

If the user adds an actual parameter under 'prefix-1' before creating
'prefix-2', then everything is fine:

  (gdb) python gdb.ParameterPrefix("prefix-1", gdb.COMMAND_NONE)
  (gdb) python gdb.Parameter('prefix-1 param-1', gdb.COMMAND_NONE, gdb.PARAM_BOOLEAN)
  (gdb) python gdb.ParameterPrefix("prefix-1 prefix-2", gdb.COMMAND_NONE)

The mistake in the above patch is in how gdbpy_parse_command_name is
used.  The BASE_LIST output argument from this function points to the
list of commands for the prefix, not to the prefix command itself.

So when gdbpy_parse_command_name is called for 'prefix-1 prefix-2',
BASE_LIST points to the list of commands associated with 'prefix-1',
not to the actual 'prefix-1' cmd_list_element.

Back in cmdpy_init, from where gdbpy_parse_command_name was called, I
was walking back from the first entry in BASE_LIST to figure out if
this was a "show" prefix command or not.  However, if BASE_LIST is
empty then there is no first item, and this would trigger the
segfault.

The solution it to extend gdbpy_parse_command_name to also return the
prefix cmd_list_element in addition to the existing values.  With this
done, and cmdpy_init updated, the segfault is now avoided.

There's a new test that would trigger the crash without the patch.

And, of course, the above commit also broke guile in the exact same
way.  And the fix is exactly the same.  And there's a guile test too.

NOTE: We should investigate possibly sharing some of this boiler plate
helper code between Python and Guile.  But not in this commit.

Approved-By: Tom Tromey <tom@tromey.com>

diff --git a/gdb/guile/guile-internal.h b/gdb/guile/guile-internal.h
index 3101882fb2c1023ff91eb00f01c1419b3abad135..60a8bf3a8db8b3bcf23827f5bf73e8a65b233abb 100644 (file)
--- a/gdb/guile/guile-internal.h
+++ b/gdb/guile/guile-internal.h
@@ -449,10 +449,11 @@ extern const struct block *bkscm_scm_to_block
 
 /* scm-cmd.c */
 
-extern char *gdbscm_parse_command_name (const char *name,
-                                       const char *func_name, int arg_pos,
-                                       struct cmd_list_element ***base_list,
-                                       struct cmd_list_element **start_list);
+extern char *gdbscm_parse_command_name
+  (const char *name, const char *func_name, int arg_pos,
+   struct cmd_list_element ***base_list,
+   struct cmd_list_element **start_list,
+   struct cmd_list_element **prefix_cmd = nullptr);
 
 extern int gdbscm_valid_command_class_p (int command_class);
 

diff --git a/gdb/guile/scm-cmd.c b/gdb/guile/scm-cmd.c
index 565d58890b82382756f393bb73130a4dc61199d6..74646f35ea2618a470517396d67bf4830171d333 100644 (file)
--- a/gdb/guile/scm-cmd.c
+++ b/gdb/guile/scm-cmd.c
@@ -457,11 +457,13 @@ cmdscm_completer (struct cmd_list_element *command,
    name of the new command.  All earlier words must be existing prefix
    commands.
 
-   *BASE_LIST is set to the final prefix command's list of
-   *sub-commands.
+   *BASE_LIST is set to the final prefix command's list of sub-commands.
 
    START_LIST is the list in which the search starts.
 
+   When PREFIX_CMD is not NULL then *PREFIX_CMD is set to the prefix
+   command itself, or NULL, if there is no prefix command.
+
    This function returns the xmalloc()d name of the new command.
    On error a Scheme exception is thrown.  */
 
@@ -469,13 +471,17 @@ char *
 gdbscm_parse_command_name (const char *name,
                           const char *func_name, int arg_pos,
                           struct cmd_list_element ***base_list,
-                          struct cmd_list_element **start_list)
+                          struct cmd_list_element **start_list,
+                          struct cmd_list_element **prefix_cmd)
 {
   struct cmd_list_element *elt;
   int len = strlen (name);
   int i, lastchar;
   char *msg;
 
+  if (prefix_cmd != nullptr)
+    *prefix_cmd = nullptr;
+
   /* Skip trailing whitespace.  */
   for (i = len - 1; i >= 0 && (name[i] == ' ' || name[i] == '\t'); --i)
     ;
@@ -522,6 +528,8 @@ gdbscm_parse_command_name (const char *name,
   if (elt->is_prefix ())
     {
       *base_list = elt->subcommands;
+      if (prefix_cmd != nullptr)
+       *prefix_cmd = elt;
       return result.release ();
     }
 
@@ -743,8 +751,9 @@ gdbscm_register_command_x (SCM self)
   if (cmdscm_is_valid (c_smob))
     scm_misc_error (FUNC_NAME, _("command is already registered"), SCM_EOL);
 
+  struct cmd_list_element *prefix_cmd = nullptr;
   cmd_name = gdbscm_parse_command_name (c_smob->name, FUNC_NAME, SCM_ARG1,
-                                       &cmd_list, &cmdlist);
+                                       &cmd_list, &cmdlist, &prefix_cmd);
   c_smob->cmd_name = gdbscm_gc_xstrdup (cmd_name);
   xfree (cmd_name);
 
@@ -769,11 +778,14 @@ gdbscm_register_command_x (SCM self)
                 'set prefix' the user will get the help text listing all
                 of the sub-commands, and for 'show prefix', the user will
                 see all of the sub-command values.  */
-             cmd_list_element *first = *cmd_list;
-             while (first->prefix != nullptr)
-               first = first->prefix;
-
-             bool is_show = first->subcommands == &showlist;
+             if (prefix_cmd != nullptr)
+               {
+                 while (prefix_cmd->prefix != nullptr)
+                   prefix_cmd = prefix_cmd->prefix;
+               }
+
+             bool is_show = (prefix_cmd != nullptr
+                             && prefix_cmd->subcommands == &showlist);
 
              if (is_show)
                cmd = add_show_prefix_cmd (c_smob->cmd_name,

diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c
index 5b4f8138aeade55366f7a1b1391b6f932dc16452..e8c29042430b3b907bbe1a205272bd199d9081c1 100644 (file)
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -334,24 +334,30 @@ cmdpy_completer (struct cmd_list_element *command,
    name of the new command.  All earlier words must be existing prefix
    commands.
 
-   *BASE_LIST is set to the final prefix command's list of
-   *sub-commands.
+   *BASE_LIST is set to the final prefix command's list of sub-commands.
 
    START_LIST is the list in which the search starts.
 
+   When PREFIX_CMD is not NULL then *PREFIX_CMD is set to the prefix
+   command itself, or NULL, if there is no prefix command.
+
    This function returns the name of the new command.  On error sets the Python
    error and returns NULL.  */
 
 gdb::unique_xmalloc_ptr<char>
 gdbpy_parse_command_name (const char *name,
                          struct cmd_list_element ***base_list,
-                         struct cmd_list_element **start_list)
+                         struct cmd_list_element **start_list,
+                         struct cmd_list_element **prefix_cmd)
 {
   struct cmd_list_element *elt;
   int len = strlen (name);
   int i, lastchar;
   const char *prefix_text2;
 
+  if (prefix_cmd != nullptr)
+    *prefix_cmd = nullptr;
+
   /* Skip trailing whitespace.  */
   for (i = len - 1; i >= 0 && (name[i] == ' ' || name[i] == '\t'); --i)
     ;
@@ -393,6 +399,8 @@ gdbpy_parse_command_name (const char *name,
   if (elt->is_prefix ())
     {
       *base_list = elt->subcommands;
+      if (prefix_cmd != nullptr)
+       *prefix_cmd = elt;
       return result;
     }
 
@@ -467,8 +475,9 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
       return -1;
     }
 
+  cmd_list_element *prefix_cmd = nullptr;
   gdb::unique_xmalloc_ptr<char> cmd_name
-    = gdbpy_parse_command_name (name, &cmd_list, &cmdlist);
+    = gdbpy_parse_command_name (name, &cmd_list, &cmdlist, &prefix_cmd);
   if (cmd_name == nullptr)
     return -1;
 
@@ -525,11 +534,14 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
                 'set prefix' the user will get the help text listing all
                 of the sub-commands, and for 'show prefix', the user will
                 see all of the sub-command values.  */
-             cmd_list_element *first = *cmd_list;
-             while (first->prefix != nullptr)
-               first = first->prefix;
-
-             bool is_show = first->subcommands == &showlist;
+             if (prefix_cmd != nullptr)
+               {
+                 while (prefix_cmd->prefix != nullptr)
+                   prefix_cmd = prefix_cmd->prefix;
+               }
+
+             bool is_show = (prefix_cmd != nullptr
+                             && prefix_cmd->subcommands == &showlist);
 
              if (is_show)
                cmd = add_show_prefix_cmd (cmd_name.get (),

diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index bdccb265441e7f7fb3382abc7736b30b0a6dadf2..7f4237eecc2171b90d19e144e2002e58a9fa2a40 100644 (file)
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -510,7 +510,8 @@ PyObject *gdbpy_string_to_argv (PyObject *self, PyObject *args);
 PyObject *gdbpy_parameter_value (const setting &var);
 gdb::unique_xmalloc_ptr<char> gdbpy_parse_command_name
   (const char *name, struct cmd_list_element ***base_list,
-   struct cmd_list_element **start_list);
+   struct cmd_list_element **start_list,
+   struct cmd_list_element **prefix_cmd = nullptr);
 PyObject *gdbpy_register_tui_window (PyObject *self, PyObject *args,
                                     PyObject *kw);
 

diff --git a/gdb/testsuite/gdb.guile/scm-parameter.exp b/gdb/testsuite/gdb.guile/scm-parameter.exp
index bf0d7df5f26aa9fce1f5e1375977199f3c85aad4..e35428a8495281308ed8cd06b1fe5670bc531ed0 100644 (file)
--- a/gdb/testsuite/gdb.guile/scm-parameter.exp
+++ b/gdb/testsuite/gdb.guile/scm-parameter.exp
@@ -428,6 +428,16 @@ gdb_test_multiline "create set/show foo1 prefix commands" \
     "   #:prefix? #t))" "" \
     "end"
 
+gdb_test_multiline "create set/show foo1 baz1 prefix commands" \
+    "guile" "" \
+    "(register-command! (make-command \"set foo1 baz1\"" "" \
+    "   #:command-class COMMAND_OBSCURE" "" \
+    "   #:prefix? #t))" "" \
+    "(register-command! (make-command \"show foo1 baz1\"" "" \
+    "   #:command-class COMMAND_OBSCURE" "" \
+    "   #:prefix? #t))" "" \
+    "end"
+
 gdb_test_multiline "create 'foo bar' parameter" \
     "guile" "" \
     "(register-parameter! (make-parameter \"foo bar\"" "" \

diff --git a/gdb/testsuite/gdb.python/py-parameter-prefix.exp b/gdb/testsuite/gdb.python/py-parameter-prefix.exp
index 69cbb90ab214aac597d6261679809b5502cdd59f..eb09fe7e85da428f98189fe2f5e86fcda0462828 100644 (file)
--- a/gdb/testsuite/gdb.python/py-parameter-prefix.exp
+++ b/gdb/testsuite/gdb.python/py-parameter-prefix.exp
@@ -330,7 +330,53 @@ proc_with_prefix test_dont_repeat {} {
        "repeat show prefix-12 xxx yyy"
 }
 
+# Create a parameter prefixm, and immediately add another prefix under
+# the first.  The important thing here is that the second prefix is
+# created into an otherwise empty prefix as this triggered a bug at
+# one point.
+proc_with_prefix test_nested {} {
+    gdb_test_multiline "Create nested parameter prefixes" \
+       "python" "" \
+       "gdb.ParameterPrefix('prefix-13', gdb.COMMAND_NONE)" "" \
+       "gdb.ParameterPrefix('prefix-13 prefix-14', gdb.COMMAND_NONE)" "" \
+       "gdb.Parameter('prefix-13 param-1', gdb.COMMAND_NONE, gdb.PARAM_BOOLEAN)" "" \
+       "gdb.Parameter('prefix-13 param-2', gdb.COMMAND_NONE, gdb.PARAM_BOOLEAN)" "" \
+       "gdb.Parameter('prefix-13 prefix-14 param-3', gdb.COMMAND_NONE, gdb.PARAM_BOOLEAN)" "" \
+       "gdb.Parameter('prefix-13 prefix-14 param-4', gdb.COMMAND_NONE, gdb.PARAM_BOOLEAN)" "" \
+       "end" ""
+
+    gdb_test "show prefix-13 prefix-14" \
+       [multi_line \
+            "^prefix-13 prefix-14 param-3:  The current value of 'prefix-13 prefix-14 param-3' is \"off\"\\." \
+            "prefix-13 prefix-14 param-4:  The current value of 'prefix-13 prefix-14 param-4' is \"off\"\\."]
+
+    gdb_test "show prefix-13" \
+       [multi_line \
+            "^prefix-13 param-1:  The current value of 'prefix-13 param-1' is \"off\"\\." \
+            "prefix-13 param-2:  The current value of 'prefix-13 param-2' is \"off\"\\." \
+            "prefix-13 prefix-14 param-3:  The current value of 'prefix-13 prefix-14 param-3' is \"off\"\\." \
+            "prefix-13 prefix-14 param-4:  The current value of 'prefix-13 prefix-14 param-4' is \"off\"\\."]
+
+    gdb_test "set prefix-13 prefix-14" \
+       [multi_line \
+            "" \
+            "set prefix-13 prefix-14 param-3 -- Set the current value of 'prefix-13 prefix-14 param-3'\\." \
+            "set prefix-13 prefix-14 param-4 -- Set the current value of 'prefix-13 prefix-14 param-4'\\." \
+            "" \
+            ".*"]
+
+    gdb_test "set prefix-13" \
+       [multi_line \
+            "" \
+            "set prefix-13 param-1 -- Set the current value of 'prefix-13 param-1'\\." \
+            "set prefix-13 param-2 -- Set the current value of 'prefix-13 param-2'\\." \
+            "set prefix-13 prefix-14 -- This command is not documented\\." \
+            "" \
+            ".*"]
+}
+
 test_basic_usage
 test_simple_sub_class
 test_prefix_with_invoke
 test_dont_repeat
+test_nested