[binutils-gdb] Fix the problems reported by prefix check of command-def-selftests.c

[binutils-gdb] Fix the problems reported by prefix check of command-def-selftests.c

[Philippe Waroquiers]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=3f4d92ebdf7f848b5ccc9e8d8e8514c64fde1183

commit 3f4d92ebdf7f848b5ccc9e8d8e8514c64fde1183
Author: Philippe Waroquiers <philippe.waroquiers@skynet.be>
Date:   Sat May 9 20:56:55 2020 +0200

    Fix the problems reported by prefix check of command-def-selftests.c
    
    The next commit updates command-def-selftests.c to detect missing
    or wrong prefix commands in a list of subcommands.
    This command structure selftest detects a series of problems
    that are fixed by this commit.
    
    Many commands have a null prefix command, e.g.
        (gdb) maintenance selftest command_str
        Running selftest command_structure_invariants.
        list 0x560417949cb8 reachable via prefix 'append binary '.  command 'memory' has null prefixcmd
        list 0x560417949cb8 reachable via prefix 'append binary '.  command 'value' has null prefixcmd
        ...
    
    Most of these are fixed by the following changes:
      * do_add_cmd searches the prefix command having the list
        in which the command is added.
        This ensures that a command defined after its prefix command
        gets the correct prefix command.
      * Due to the GDB initialization order, a GDB file can define
        a subcommand before the prefix command is defined.
        So, have add_prefix_cmd calling a new recursive function
        'update_prefix_field_of_prefix_commands' to set the prefix
        command of all sub-commands that are now reachable from
        this newly defined prefix command.  Note that this recursive
        call replaces the function 'set_prefix_cmd' that was providing
        a partial solution to this problem.
    
    Following that, 2 python commands (defined after all the other GDB
    commands) got a wrong prefix command, e.g. "info frame-filter" has
    as prefix command the "i" alias of "info".  This is fixed by having
    lookup_cmd_for_prefixlist returning the aliased command rather than
    the alias.
    
    After that, one remaining problem:
        (gdb) maintenance selftest command_str
        Running selftest command_structure_invariants.
        list 0x55f320272298 reachable via prefix 'set remote '.  command 'system-call-allowed' has null prefixcmd
        Self test failed: self-test failed at ../../classfix/gdb/unittests/command-def-selftests.c:196
        Ran 1 unit tests, 1 failed
        (gdb)
    
    Caused by initialize_remote_fileio that was taking the address of
    its arguments remote_set_cmdlist and remote_show_cmdlist instead
    of receiving the correct values to use as list.
    
    2020-05-15  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
    
            * cli/cli-decode.c (lookup_cmd_for_prefix): Return the aliased command
            as prefix, not one of its aliases.
            (set_cmd_prefix): Remove.
            (do_add_cmd): Centralize the setting of the prefix of a command, when
            command is defined after its full chain of prefix commands.
            (add_alias_cmd): Remove call to set_cmd_prefix, as do_add_cmd does it.
            (add_setshow_cmd_full): Likewise.
            (update_prefix_field_of_prefixed_commands): New function.
            (add_prefix_cmd): Replace non working call to set_cmd_prefix by
            update_prefix_field_of_prefixed_commands.
            * gdb/remote-fileio.c (initialize_remote_fileio): Use the real
            addresses of remote_set_cmdlist and remote_show_cmdlist given
            as argument, not the address of an argument.
            * gdb/remote-fileio.h (initialize_remote_fileio): Likewise.
            * gdb/remote.c (_initialize_remote): Likewise.

Diff:
---
 gdb/ChangeLog        | 18 ++++++++++++
 gdb/cli/cli-decode.c | 80 ++++++++++++++++++++++++++++++----------------------
 gdb/remote-fileio.c  |  8 +++---
 gdb/remote-fileio.h  |  4 +--
 gdb/remote.c         |  2 +-
 5 files changed, 71 insertions(+), 41 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e5708da7d4b..bdb1db961ac 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,21 @@
+2020-05-15  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
+
+	* cli/cli-decode.c (lookup_cmd_for_prefix): Return the aliased command
+	as prefix, not one of its aliases.
+	(set_cmd_prefix): Remove.
+	(do_add_cmd): Centralize the setting of the prefix of a command, when
+	command is defined after its full chain of prefix commands.
+	(add_alias_cmd): Remove call to set_cmd_prefix, as do_add_cmd does it.
+	(add_setshow_cmd_full): Likewise.
+	(update_prefix_field_of_prefixed_commands): New function.
+	(add_prefix_cmd): Replace non working call to set_cmd_prefix by
+	update_prefix_field_of_prefixed_commands.
+	* gdb/remote-fileio.c (initialize_remote_fileio): Use the real
+	addresses of remote_set_cmdlist and remote_show_cmdlist given
+	as argument, not the address of an argument.
+	* gdb/remote-fileio.h (initialize_remote_fileio): Likewise.
+	* gdb/remote.c (_initialize_remote): Likewise.
+
 2020-05-15  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
 
 	* cli/cli-cmds.c (alias_command): Check for an existing alias
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 78b89010846..be36eb67321 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -61,7 +61,11 @@ lookup_cmd_for_prefixlist (struct cmd_list_element **key,
       if (p->prefixlist == NULL)
 	continue;
       else if (p->prefixlist == key)
-	return p;
+	{
+	  /* If we found an alias, we must return the aliased
+	     command.  */
+	  return p->cmd_pointer ? p->cmd_pointer : p;
+	}
 
       q = lookup_cmd_for_prefixlist (key, *(p->prefixlist));
       if (q != NULL)
@@ -71,27 +75,6 @@ lookup_cmd_for_prefixlist (struct cmd_list_element **key,
   return NULL;
 }
 
-static void
-set_cmd_prefix (struct cmd_list_element *c, struct cmd_list_element **list)
-{
-  struct cmd_list_element *p;
-
-  /* Check to see if *LIST contains any element other than C.  */
-  for (p = *list; p != NULL; p = p->next)
-    if (p != c)
-      break;
-
-  if (p == NULL)
-    {
-      /* *SET_LIST only contains SET.  */
-      p = lookup_cmd_for_prefixlist (list, setlist);
-
-      c->prefix = p ? (p->cmd_pointer ? p->cmd_pointer : p) : p;
-    }
-  else
-    c->prefix = p->prefix;
-}
-
 static void
 print_help_for_command (struct cmd_list_element *c, const char *prefix,
 			int recurse, struct ui_file *stream);
@@ -229,6 +212,13 @@ do_add_cmd (const char *name, enum command_class theclass,
       p->next = c;
     }
 
+  /* Search the prefix cmd of C, and assigns it to C->prefix.
+     See also add_prefix_cmd and update_prefix_field_of_prefixed_commands.  */
+  struct cmd_list_element *prefixcmd = lookup_cmd_for_prefixlist (list,
+								  cmdlist);
+  c->prefix = prefixcmd;
+
+
   return c;
 }
 
@@ -330,7 +320,6 @@ add_alias_cmd (const char *name, cmd_list_element *old,
   c->alias_chain = old->aliases;
   old->aliases = c;
 
-  set_cmd_prefix (c, list);
   return c;
 }
 
@@ -349,6 +338,37 @@ add_alias_cmd (const char *name, const char *oldname,
 }
 
 
+/* Update the prefix field of all sub-commands of the prefix command C.
+   We must do this when a prefix command is defined as the GDB init sequence
+   does not guarantee that a prefix command is created before its sub-commands.
+   For example, break-catch-sig.c initialization runs before breakpoint.c
+   initialization, but it is breakpoint.c that creates the "catch" command used
+   by the "catch signal" command created by break-catch-sig.c.  */
+
+static void
+update_prefix_field_of_prefixed_commands (struct cmd_list_element *c)
+{
+  for (cmd_list_element *p = *c->prefixlist; p != NULL; p = p->next)
+    {
+      p->prefix = c;
+
+      /* We must recursively update the prefix field to cover
+	 e.g.  'info auto-load libthread-db' where the creation
+	 order was:
+           libthread-db
+           auto-load
+           info
+	 In such a case, when 'auto-load' was created by do_add_cmd,
+         the 'libthread-db' prefix field could not be updated, as the
+	 'auto-load' command was not yet reachable by
+	    lookup_cmd_for_prefixlist (list, cmdlist)
+	    that searches from the top level 'cmdlist'.  */
+      if (p->prefixlist != nullptr)
+	update_prefix_field_of_prefixed_commands (p);
+    }
+}
+
+
 /* Like add_cmd but adds an element for a command prefix: a name that
    should be followed by a subcommand to be looked up in another
    command list.  PREFIXLIST should be the address of the variable
@@ -362,20 +382,14 @@ add_prefix_cmd (const char *name, enum command_class theclass,
 		struct cmd_list_element **list)
 {
   struct cmd_list_element *c = add_cmd (name, theclass, fun, doc, list);
-  struct cmd_list_element *p;
 
   c->prefixlist = prefixlist;
   c->prefixname = prefixname;
   c->allow_unknown = allow_unknown;
 
-  if (list == &cmdlist)
-    c->prefix = NULL;
-  else
-    set_cmd_prefix (c, list);
-
-  /* Update the field 'prefix' of each cmd_list_element in *PREFIXLIST.  */
-  for (p = *prefixlist; p != NULL; p = p->next)
-    p->prefix = c;
+  /* Now that prefix command C is defined, we need to set the prefix field
+     of all prefixed commands that were defined before C itself was defined.  */
+  update_prefix_field_of_prefixed_commands (c);
 
   return c;
 }
@@ -555,8 +569,6 @@ add_setshow_cmd_full (const char *name,
   if (set_func != NULL)
     set_cmd_sfunc (set, set_func);
 
-  set_cmd_prefix (set, set_list);
-
   show = add_set_or_show_cmd (name, show_cmd, theclass, var_type, var,
 			      full_show_doc, show_list);
   show->doc_allocated = 1;
diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c
index df470fd86df..7450e848602 100644
--- a/gdb/remote-fileio.c
+++ b/gdb/remote-fileio.c
@@ -1295,15 +1295,15 @@ show_system_call_allowed (const char *args, int from_tty)
 }
 
 void
-initialize_remote_fileio (struct cmd_list_element *remote_set_cmdlist,
-			  struct cmd_list_element *remote_show_cmdlist)
+initialize_remote_fileio (struct cmd_list_element **remote_set_cmdlist,
+			  struct cmd_list_element **remote_show_cmdlist)
 {
   add_cmd ("system-call-allowed", no_class,
 	   set_system_call_allowed,
 	   _("Set if the host system(3) call is allowed for the target."),
-	   &remote_set_cmdlist);
+	   remote_set_cmdlist);
   add_cmd ("system-call-allowed", no_class,
 	   show_system_call_allowed,
 	   _("Show if the host system(3) call is allowed for the target."),
-	   &remote_show_cmdlist);
+	   remote_show_cmdlist);
 }
diff --git a/gdb/remote-fileio.h b/gdb/remote-fileio.h
index ab73f9ba7db..f206b040913 100644
--- a/gdb/remote-fileio.h
+++ b/gdb/remote-fileio.h
@@ -37,8 +37,8 @@ extern void remote_fileio_reset (void);
 
 /* Called from _initialize_remote ().  */
 extern void initialize_remote_fileio (
-  struct cmd_list_element *remote_set_cmdlist,
-  struct cmd_list_element *remote_show_cmdlist);
+  struct cmd_list_element **remote_set_cmdlist,
+  struct cmd_list_element **remote_show_cmdlist);
 
 /* Unpack a struct fio_stat.  */
 extern void remote_fileio_to_host_stat (struct fio_stat *fst,
diff --git a/gdb/remote.c b/gdb/remote.c
index 5b1fa848536..812ab8bc1b3 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -14876,5 +14876,5 @@ Specify \"unlimited\" to display all the characters."),
 				       &setdebuglist, &showdebuglist);
 
   /* Eventually initialize fileio.  See fileio.c */
-  initialize_remote_fileio (remote_set_cmdlist, remote_show_cmdlist);
+  initialize_remote_fileio (&remote_set_cmdlist, &remote_show_cmdlist);
 }