RFA: fix PR 1815

RFA: fix PR 1815

[Tom Tromey]

PR 1815 concerns a crash involving alias commands.

The simplest way to see this is to use "define" to define a command
which is the target of an alias; then invoke the alias.  This usually
crashes.

The bug is that an alias holds a pointer to the target command.
Redefining the target leaves this pointer dangling.

This patch fixes the problem by tracking all aliases and fixing them
up when a command is redefined.  I made delete_cmd static since it is
not needed anywhere else, and this seemed cleaner.  This also rewrites
delete_cmd -- there's no need for two loops here, and IMO the previous
code was unnecessarily obscure.

Built and regtested on x86-64 (compile farm).  New test case included.
Ok?

Tom

:ADDPATCH cli:

2008-11-10  Tom Tromey  <tromey@redhat.com>

	PR gdb/1815:
	* cli/cli-decode.c (delete_cmd): Forward declare.
	(delete_cmd): Now static.  Change return type.  Remove command
	from alias chain.  Rewrite.
	(add_cmd): Initialize new fields.  Update cmd_pointer on all
	aliases.
	(add_alias_cmd): Put command on alias chain.
	* command.h (delete_cmd): Don't declare.
	* cli/cli-decode.h (delete_cmd): Don't declare.
	(struct cmd_list_element) <aliases, alias_chain>: New fields.

2008-11-10  Tom Tromey  <tromey@redhat.com>

	* gdb.base/commands.exp (redefine_backtrace_test): New proc.
	Call it.

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 9cc52ba..6c2b497 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -37,6 +37,9 @@
 
 static void undef_cmd_error (char *, char *);
 
+static struct cmd_list_element *delete_cmd (char *name,
+					    struct cmd_list_element **list);
+
 static struct cmd_list_element *find_cmd (char *command,
 					  int len,
 					  struct cmd_list_element *clist,
@@ -154,9 +157,13 @@ add_cmd (char *name, enum command_class class, void (*fun) (char *, int),
 {
   struct cmd_list_element *c
   = (struct cmd_list_element *) xmalloc (sizeof (struct cmd_list_element));
-  struct cmd_list_element *p;
+  struct cmd_list_element *p, *iter;
 
-  delete_cmd (name, list);
+  /* Turn each alias of the old command into an alias of the new
+     command.  */
+  c->aliases = delete_cmd (name, list);
+  for (iter = c->aliases; iter; iter = iter->alias_chain)
+    iter->cmd_pointer = c;
 
   if (*list == NULL || strcmp ((*list)->name, name) >= 0)
     {
@@ -198,6 +205,7 @@ add_cmd (char *name, enum command_class class, void (*fun) (char *, int),
   c->hookee_pre = NULL;
   c->hookee_post = NULL;
   c->cmd_pointer = NULL;
+  c->alias_chain = NULL;
 
   return c;
 }
@@ -239,7 +247,9 @@ add_alias_cmd (char *name, char *oldname, enum command_class class,
 
   if (old == 0)
     {
-      delete_cmd (name, list);
+      struct cmd_list_element *aliases = delete_cmd (name, list);
+      /* If this happens, it means a programmer error somewhere.  */
+      gdb_assert (!aliases);
       return 0;
     }
 
@@ -252,6 +262,8 @@ add_alias_cmd (char *name, char *oldname, enum command_class class,
   c->allow_unknown = old->allow_unknown;
   c->abbrev_flag = abbrev_flag;
   c->cmd_pointer = old;
+  c->alias_chain = old->aliases;
+  old->aliases = c;
   return c;
 }
 
@@ -616,40 +628,54 @@ add_setshow_zinteger_cmd (char *name, enum command_class class,
 
 /* Remove the command named NAME from the command list.  */
 
-void
+static struct cmd_list_element *
 delete_cmd (char *name, struct cmd_list_element **list)
 {
-  struct cmd_list_element *c;
-  struct cmd_list_element *p;
+  struct cmd_list_element *iter;
+  struct cmd_list_element **previous_chain_ptr;
+  struct cmd_list_element *aliases = NULL;
+
+  previous_chain_ptr = list;
 
-  while (*list && strcmp ((*list)->name, name) == 0)
+  for (iter = *previous_chain_ptr; iter; iter = *previous_chain_ptr)
     {
-      if ((*list)->hookee_pre)
-      (*list)->hookee_pre->hook_pre = 0;   /* Hook slips out of its mouth */
-      if ((*list)->hookee_post)
-      (*list)->hookee_post->hook_post = 0; /* Hook slips out of its bottom  */
-      p = (*list)->next;
-      xfree (* list);
-      *list = p;
+      if (strcmp (iter->name, name) == 0)
+	{
+	  if (iter->hookee_pre)
+	    iter->hookee_pre->hook_pre = 0;
+	  if (iter->hookee_post)
+	    iter->hookee_post->hook_post = 0;
+
+	  /* Update the link.  */
+	  *previous_chain_ptr = iter->next;
+
+	  aliases = iter->aliases;
+
+	  /* If this command was an alias, remove it from the list of
+	     aliases.  */
+	  if (iter->cmd_pointer)
+	    {
+	      struct cmd_list_element **prevp = &iter->cmd_pointer->aliases;
+	      struct cmd_list_element *a = *prevp;
+
+	      while (a != iter)
+		{
+		  prevp = &a->alias_chain;
+		  a = *prevp;
+		}
+	      *prevp = iter->alias_chain;
+	    }
+
+	  xfree (iter);
+
+	  /* We won't see another command with the same name.  */
+	  break;
+	}
+      else
+	previous_chain_ptr = &iter->next;
     }
 
-  if (*list)
-    for (c = *list; c->next;)
-      {
-	if (strcmp (c->next->name, name) == 0)
-	  {
-          if (c->next->hookee_pre)
-            c->next->hookee_pre->hook_pre = 0; /* hooked cmd gets away.  */
-          if (c->next->hookee_post)
-            c->next->hookee_post->hook_post = 0; /* remove post hook */
-                                               /* :( no fishing metaphore */
-	    p = c->next->next;
-	    xfree (c->next);
-	    c->next = p;
-	  }
-	else
-	  c = c->next;
-      }
+  return aliases;
 }
 \f
 /* Shorthands to the commands above. */
diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
index 4e2dbc1..9d27f0f 100644
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -197,6 +197,12 @@ struct cmd_list_element
     /* Pointer to command that is aliased by this one, so the
        aliased command can be located in case it has been hooked.  */
     struct cmd_list_element *cmd_pointer;
+
+    /* Start of a linked list of all aliases of this command.  */
+    struct cmd_list_element *aliases;
+
+    /* Link pointer for aliases on an alias list.  */
+    struct cmd_list_element *alias_chain;
   };
 
 /* API to the manipulation of command lists.  */
@@ -290,8 +296,6 @@ extern char **complete_on_cmdlist (struct cmd_list_element *, char *, char *);
 
 extern char **complete_on_enum (const char *enumlist[], char *, char *);
 
-extern void delete_cmd (char *, struct cmd_list_element **);
-
 extern void help_cmd_list (struct cmd_list_element *, enum command_class,
 			   char *, int, struct ui_file *);
 
diff --git a/gdb/command.h b/gdb/command.h
index cf09c01..5678c9e 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -193,8 +193,6 @@ extern char **complete_on_cmdlist (struct cmd_list_element *, char *, char *);
 
 extern char **complete_on_enum (const char *enumlist[], char *, char *);
 
-extern void delete_cmd (char *, struct cmd_list_element **);
-
 extern void help_cmd (char *, struct ui_file *);
 
 extern void help_list (struct cmd_list_element *, char *,
diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp
index 301b995..41d5bec 100644
--- a/gdb/testsuite/gdb.base/commands.exp
+++ b/gdb/testsuite/gdb.base/commands.exp
@@ -692,6 +692,34 @@ proc if_commands_test {} {
     }
 }
 
+proc redefine_backtrace_test {} {
+    global gdb_prompt
+
+    send_gdb "define backtrace\n"
+    gdb_expect {
+	-re "Really redefine built-in.*$" {
+	    send_gdb "y\n"
+	}
+
+	-re "End with"  {
+	    pass "define backtrace in redefine_backtrace_test"
+	}
+        default {
+	    fail "(timeout or eof) define backtrace in redefine_backtrace_test"
+	}
+    }
+    gdb_test "echo hibob\\n\nend" \
+	    "" \
+	    "enter commands in redefine_backtrace_test"
+
+    gdb_test "backtrace" \
+	    "hibob" \
+	    "execute backtrace command in redefine_backtrace_test"
+    gdb_test "bt" \
+	    "hibob" \
+	    "execute bt command in redefine_backtrace_test"
+}
+
 gdbvar_simple_if_test
 gdbvar_simple_while_test
 gdbvar_complex_if_while_test
@@ -710,3 +738,5 @@ temporary_breakpoint_commands
 stray_arg0_test
 recursive_source_test
 if_commands_test
+# This one should come last, as it redefines "backtrace".
+redefine_backtrace_test

Re: RFA: fix PR 1815

[Joel Brobecker]

> This patch fixes the problem by tracking all aliases and fixing them
> up when a command is redefined.

This is consistent with the way command hooks are tracked.

There is a difference, however, in the fact that hooks are lost
when redifining a command.  Your patch handles this case much better.
I'll open a PR for this problem, and hopefully someone will find the
time to fix it.

> 2008-11-10  Tom Tromey  <tromey@redhat.com>
> 
> 	PR gdb/1815:
> 	* cli/cli-decode.c (delete_cmd): Forward declare.
> 	(delete_cmd): Now static.  Change return type.  Remove command
> 	from alias chain.  Rewrite.
> 	(add_cmd): Initialize new fields.  Update cmd_pointer on all
> 	aliases.
> 	(add_alias_cmd): Put command on alias chain.
> 	* command.h (delete_cmd): Don't declare.
> 	* cli/cli-decode.h (delete_cmd): Don't declare.
> 	(struct cmd_list_element) <aliases, alias_chain>: New fields.

OK.

> 2008-11-10  Tom Tromey  <tromey@redhat.com>
> 
> 	* gdb.base/commands.exp (redefine_backtrace_test): New proc.
> 	Call it.

> @@ -616,40 +628,54 @@ add_setshow_zinteger_cmd (char *name, enum command_class class,
>  
>  /* Remove the command named NAME from the command list.  */
>  
> -void
> +static struct cmd_list_element *

Can you update the command to mention the fact that the function returns
the list of aliases to the command being deleted?

> +    send_gdb "define backtrace\n"
> +    gdb_expect {
> +	-re "Really redefine built-in.*$" {
> +	    send_gdb "y\n"
> +	}
> +
> +	-re "End with"  {
> +	    pass "define backtrace in redefine_backtrace_test"
> +	}
> +        default {
> +	    fail "(timeout or eof) define backtrace in redefine_backtrace_test"
> +	}

I was wondering if this is something that can be implemented with
gdb_test_multiple instead of using gdb_expect/send_gdb.  This particular
case seems a little more complex than usual, so this may not be possible...

Otherwise OK.

-- 
Joel

:REVIEWMAIL:

Re: RFA: fix PR 1815

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> There is a difference, however, in the fact that hooks are lost
Joel> when redifining a command.  Your patch handles this case much better.
Joel> I'll open a PR for this problem, and hopefully someone will find the
Joel> time to fix it.

Thanks.

>> -void
>> +static struct cmd_list_element *

Joel> Can you update the command to mention the fact that the function
Joel> returns the list of aliases to the command being deleted?

Done.

Joel> I was wondering if this is something that can be implemented
Joel> with gdb_test_multiple instead of using gdb_expect/send_gdb.

I thought I tried this, but I redid it now and the appended seems to
work.

Built & regtested on x86-64 (compile farm).
Ok?

Tom

2008-12-09  Tom Tromey  <tromey@redhat.com>

	PR gdb/1815:
	* cli/cli-decode.c (delete_cmd): Forward declare.
	(delete_cmd): Now static.  Change return type.  Remove command
	from alias chain.  Rewrite.
	(add_cmd): Initialize new fields.  Update cmd_pointer on all
	aliases.
	(add_alias_cmd): Put command on alias chain.
	* command.h (delete_cmd): Don't declare.
	* cli/cli-decode.h (delete_cmd): Don't declare.
	(struct cmd_list_element) <aliases, alias_chain>: New fields.

2008-12-09  Tom Tromey  <tromey@redhat.com>

	* gdb.base/commands.exp (redefine_backtrace_test): New proc.
	Call it.

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 9cc52ba..613b8d7 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -37,6 +37,9 @@
 
 static void undef_cmd_error (char *, char *);
 
+static struct cmd_list_element *delete_cmd (char *name,
+					    struct cmd_list_element **list);
+
 static struct cmd_list_element *find_cmd (char *command,
 					  int len,
 					  struct cmd_list_element *clist,
@@ -154,9 +157,13 @@ add_cmd (char *name, enum command_class class, void (*fun) (char *, int),
 {
   struct cmd_list_element *c
   = (struct cmd_list_element *) xmalloc (sizeof (struct cmd_list_element));
-  struct cmd_list_element *p;
+  struct cmd_list_element *p, *iter;
 
-  delete_cmd (name, list);
+  /* Turn each alias of the old command into an alias of the new
+     command.  */
+  c->aliases = delete_cmd (name, list);
+  for (iter = c->aliases; iter; iter = iter->alias_chain)
+    iter->cmd_pointer = c;
 
   if (*list == NULL || strcmp ((*list)->name, name) >= 0)
     {
@@ -198,6 +205,7 @@ add_cmd (char *name, enum command_class class, void (*fun) (char *, int),
   c->hookee_pre = NULL;
   c->hookee_post = NULL;
   c->cmd_pointer = NULL;
+  c->alias_chain = NULL;
 
   return c;
 }
@@ -239,7 +247,9 @@ add_alias_cmd (char *name, char *oldname, enum command_class class,
 
   if (old == 0)
     {
-      delete_cmd (name, list);
+      struct cmd_list_element *aliases = delete_cmd (name, list);
+      /* If this happens, it means a programmer error somewhere.  */
+      gdb_assert (!aliases);
       return 0;
     }
 
@@ -252,6 +262,8 @@ add_alias_cmd (char *name, char *oldname, enum command_class class,
   c->allow_unknown = old->allow_unknown;
   c->abbrev_flag = abbrev_flag;
   c->cmd_pointer = old;
+  c->alias_chain = old->aliases;
+  old->aliases = c;
   return c;
 }
 
@@ -614,42 +626,58 @@ add_setshow_zinteger_cmd (char *name, enum command_class class,
 			NULL, NULL);
 }
 
-/* Remove the command named NAME from the command list.  */
+/* Remove the command named NAME from the command list.  Return the
+   list commands which were aliased to the deleted command.  If the
+   command had no aliases, return NULL.  */
 
-void
+static struct cmd_list_element *
 delete_cmd (char *name, struct cmd_list_element **list)
 {
-  struct cmd_list_element *c;
-  struct cmd_list_element *p;
+  struct cmd_list_element *iter;
+  struct cmd_list_element **previous_chain_ptr;
+  struct cmd_list_element *aliases = NULL;
+
+  previous_chain_ptr = list;
 
-  while (*list && strcmp ((*list)->name, name) == 0)
+  for (iter = *previous_chain_ptr; iter; iter = *previous_chain_ptr)
     {
-      if ((*list)->hookee_pre)
-      (*list)->hookee_pre->hook_pre = 0;   /* Hook slips out of its mouth */
-      if ((*list)->hookee_post)
-      (*list)->hookee_post->hook_post = 0; /* Hook slips out of its bottom  */
-      p = (*list)->next;
-      xfree (* list);
-      *list = p;
+      if (strcmp (iter->name, name) == 0)
+	{
+	  if (iter->hookee_pre)
+	    iter->hookee_pre->hook_pre = 0;
+	  if (iter->hookee_post)
+	    iter->hookee_post->hook_post = 0;
+
+	  /* Update the link.  */
+	  *previous_chain_ptr = iter->next;
+
+	  aliases = iter->aliases;
+
+	  /* If this command was an alias, remove it from the list of
+	     aliases.  */
+	  if (iter->cmd_pointer)
+	    {
+	      struct cmd_list_element **prevp = &iter->cmd_pointer->aliases;
+	      struct cmd_list_element *a = *prevp;
+
+	      while (a != iter)
+		{
+		  prevp = &a->alias_chain;
+		  a = *prevp;
+		}
+	      *prevp = iter->alias_chain;
+	    }
+
+	  xfree (iter);
+
+	  /* We won't see another command with the same name.  */
+	  break;
+	}
+      else
+	previous_chain_ptr = &iter->next;
     }
 
-  if (*list)
-    for (c = *list; c->next;)
-      {
-	if (strcmp (c->next->name, name) == 0)
-	  {
-          if (c->next->hookee_pre)
-            c->next->hookee_pre->hook_pre = 0; /* hooked cmd gets away.  */
-          if (c->next->hookee_post)
-            c->next->hookee_post->hook_post = 0; /* remove post hook */
-                                               /* :( no fishing metaphore */
-	    p = c->next->next;
-	    xfree (c->next);
-	    c->next = p;
-	  }
-	else
-	  c = c->next;
-      }
+  return aliases;
 }
 \f
 /* Shorthands to the commands above. */
diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
index 4e2dbc1..9d27f0f 100644
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -197,6 +197,12 @@ struct cmd_list_element
     /* Pointer to command that is aliased by this one, so the
        aliased command can be located in case it has been hooked.  */
     struct cmd_list_element *cmd_pointer;
+
+    /* Start of a linked list of all aliases of this command.  */
+    struct cmd_list_element *aliases;
+
+    /* Link pointer for aliases on an alias list.  */
+    struct cmd_list_element *alias_chain;
   };
 
 /* API to the manipulation of command lists.  */
@@ -290,8 +296,6 @@ extern char **complete_on_cmdlist (struct cmd_list_element *, char *, char *);
 
 extern char **complete_on_enum (const char *enumlist[], char *, char *);
 
-extern void delete_cmd (char *, struct cmd_list_element **);
-
 extern void help_cmd_list (struct cmd_list_element *, enum command_class,
 			   char *, int, struct ui_file *);
 
diff --git a/gdb/command.h b/gdb/command.h
index cf09c01..5678c9e 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -193,8 +193,6 @@ extern char **complete_on_cmdlist (struct cmd_list_element *, char *, char *);
 
 extern char **complete_on_enum (const char *enumlist[], char *, char *);
 
-extern void delete_cmd (char *, struct cmd_list_element **);
-
 extern void help_cmd (char *, struct ui_file *);
 
 extern void help_list (struct cmd_list_element *, char *,
diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp
index 301b995..7da3f23 100644
--- a/gdb/testsuite/gdb.base/commands.exp
+++ b/gdb/testsuite/gdb.base/commands.exp
@@ -692,6 +692,33 @@ proc if_commands_test {} {
     }
 }
 
+proc redefine_backtrace_test {} {
+    global gdb_prompt
+
+    gdb_test_multiple "define backtrace" "define backtrace" {
+	-re "Really redefine built-in.*$" {
+	    send_gdb "y\n"
+	}
+
+	-re "End with"  {
+	    pass "define backtrace in redefine_backtrace_test"
+	}
+        default {
+	    fail "(timeout or eof) define backtrace in redefine_backtrace_test"
+	}
+    }
+    gdb_test "echo hibob\\n\nend" \
+	    "" \
+	    "enter commands in redefine_backtrace_test"
+
+    gdb_test "backtrace" \
+	    "hibob" \
+	    "execute backtrace command in redefine_backtrace_test"
+    gdb_test "bt" \
+	    "hibob" \
+	    "execute bt command in redefine_backtrace_test"
+}
+
 gdbvar_simple_if_test
 gdbvar_simple_while_test
 gdbvar_complex_if_while_test
@@ -710,3 +737,5 @@ temporary_breakpoint_commands
 stray_arg0_test
 recursive_source_test
 if_commands_test
+# This one should come last, as it redefines "backtrace".
+redefine_backtrace_test

Re: RFA: fix PR 1815

[Joel Brobecker]

> 2008-12-09  Tom Tromey  <tromey@redhat.com>
> 
> 	PR gdb/1815:
> 	* cli/cli-decode.c (delete_cmd): Forward declare.
> 	(delete_cmd): Now static.  Change return type.  Remove command
> 	from alias chain.  Rewrite.
> 	(add_cmd): Initialize new fields.  Update cmd_pointer on all
> 	aliases.
> 	(add_alias_cmd): Put command on alias chain.
> 	* command.h (delete_cmd): Don't declare.
> 	* cli/cli-decode.h (delete_cmd): Don't declare.
> 	(struct cmd_list_element) <aliases, alias_chain>: New fields.
> 
> 2008-12-09  Tom Tromey  <tromey@redhat.com>
> 
> 	* gdb.base/commands.exp (redefine_backtrace_test): New proc.
> 	Call it.

I hope I didn't mislead you with trying to use gdb_test_multiple,
but I'll personally fix it if it turns out that there is a problem.

Approved.

Thanks,
-- 
Joel

Re: RFA: fix PR 1815

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> I hope I didn't mislead you with trying to use gdb_test_multiple,
Joel> but I'll personally fix it if it turns out that there is a problem.

Joel> Approved.

Pedro pointed out there is probably a missing exp_continue.
I'm re-running the test suite with that added.  So, stay tuned...

Tom

Re: RFA: fix PR 1815

[Tom Tromey]

Tom> Pedro pointed out there is probably a missing exp_continue.
Tom> I'm re-running the test suite with that added.  So, stay tuned...

Ok, here's another version.
The only change is the addition of exp_continue.

Tom

2008-12-09  Tom Tromey  <tromey@redhat.com>

	PR gdb/1815:
	* cli/cli-decode.c (delete_cmd): Forward declare.
	(delete_cmd): Now static.  Change return type.  Remove command
	from alias chain.  Rewrite.
	(add_cmd): Initialize new fields.  Update cmd_pointer on all
	aliases.
	(add_alias_cmd): Put command on alias chain.
	* command.h (delete_cmd): Don't declare.
	* cli/cli-decode.h (delete_cmd): Don't declare.
	(struct cmd_list_element) <aliases, alias_chain>: New fields.

2008-12-09  Tom Tromey  <tromey@redhat.com>

	* gdb.base/commands.exp (redefine_backtrace_test): New proc.
	Call it.

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 9cc52ba..613b8d7 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -37,6 +37,9 @@
 
 static void undef_cmd_error (char *, char *);
 
+static struct cmd_list_element *delete_cmd (char *name,
+					    struct cmd_list_element **list);
+
 static struct cmd_list_element *find_cmd (char *command,
 					  int len,
 					  struct cmd_list_element *clist,
@@ -154,9 +157,13 @@ add_cmd (char *name, enum command_class class, void (*fun) (char *, int),
 {
   struct cmd_list_element *c
   = (struct cmd_list_element *) xmalloc (sizeof (struct cmd_list_element));
-  struct cmd_list_element *p;
+  struct cmd_list_element *p, *iter;
 
-  delete_cmd (name, list);
+  /* Turn each alias of the old command into an alias of the new
+     command.  */
+  c->aliases = delete_cmd (name, list);
+  for (iter = c->aliases; iter; iter = iter->alias_chain)
+    iter->cmd_pointer = c;
 
   if (*list == NULL || strcmp ((*list)->name, name) >= 0)
     {
@@ -198,6 +205,7 @@ add_cmd (char *name, enum command_class class, void (*fun) (char *, int),
   c->hookee_pre = NULL;
   c->hookee_post = NULL;
   c->cmd_pointer = NULL;
+  c->alias_chain = NULL;
 
   return c;
 }
@@ -239,7 +247,9 @@ add_alias_cmd (char *name, char *oldname, enum command_class class,
 
   if (old == 0)
     {
-      delete_cmd (name, list);
+      struct cmd_list_element *aliases = delete_cmd (name, list);
+      /* If this happens, it means a programmer error somewhere.  */
+      gdb_assert (!aliases);
       return 0;
     }
 
@@ -252,6 +262,8 @@ add_alias_cmd (char *name, char *oldname, enum command_class class,
   c->allow_unknown = old->allow_unknown;
   c->abbrev_flag = abbrev_flag;
   c->cmd_pointer = old;
+  c->alias_chain = old->aliases;
+  old->aliases = c;
   return c;
 }
 
@@ -614,42 +626,58 @@ add_setshow_zinteger_cmd (char *name, enum command_class class,
 			NULL, NULL);
 }
 
-/* Remove the command named NAME from the command list.  */
+/* Remove the command named NAME from the command list.  Return the
+   list commands which were aliased to the deleted command.  If the
+   command had no aliases, return NULL.  */
 
-void
+static struct cmd_list_element *
 delete_cmd (char *name, struct cmd_list_element **list)
 {
-  struct cmd_list_element *c;
-  struct cmd_list_element *p;
+  struct cmd_list_element *iter;
+  struct cmd_list_element **previous_chain_ptr;
+  struct cmd_list_element *aliases = NULL;
+
+  previous_chain_ptr = list;
 
-  while (*list && strcmp ((*list)->name, name) == 0)
+  for (iter = *previous_chain_ptr; iter; iter = *previous_chain_ptr)
     {
-      if ((*list)->hookee_pre)
-      (*list)->hookee_pre->hook_pre = 0;   /* Hook slips out of its mouth */
-      if ((*list)->hookee_post)
-      (*list)->hookee_post->hook_post = 0; /* Hook slips out of its bottom  */
-      p = (*list)->next;
-      xfree (* list);
-      *list = p;
+      if (strcmp (iter->name, name) == 0)
+	{
+	  if (iter->hookee_pre)
+	    iter->hookee_pre->hook_pre = 0;
+	  if (iter->hookee_post)
+	    iter->hookee_post->hook_post = 0;
+
+	  /* Update the link.  */
+	  *previous_chain_ptr = iter->next;
+
+	  aliases = iter->aliases;
+
+	  /* If this command was an alias, remove it from the list of
+	     aliases.  */
+	  if (iter->cmd_pointer)
+	    {
+	      struct cmd_list_element **prevp = &iter->cmd_pointer->aliases;
+	      struct cmd_list_element *a = *prevp;
+
+	      while (a != iter)
+		{
+		  prevp = &a->alias_chain;
+		  a = *prevp;
+		}
+	      *prevp = iter->alias_chain;
+	    }
+
+	  xfree (iter);
+
+	  /* We won't see another command with the same name.  */
+	  break;
+	}
+      else
+	previous_chain_ptr = &iter->next;
     }
 
-  if (*list)
-    for (c = *list; c->next;)
-      {
-	if (strcmp (c->next->name, name) == 0)
-	  {
-          if (c->next->hookee_pre)
-            c->next->hookee_pre->hook_pre = 0; /* hooked cmd gets away.  */
-          if (c->next->hookee_post)
-            c->next->hookee_post->hook_post = 0; /* remove post hook */
-                                               /* :( no fishing metaphore */
-	    p = c->next->next;
-	    xfree (c->next);
-	    c->next = p;
-	  }
-	else
-	  c = c->next;
-      }
+  return aliases;
 }
 \f
 /* Shorthands to the commands above. */
diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
index 4e2dbc1..9d27f0f 100644
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -197,6 +197,12 @@ struct cmd_list_element
     /* Pointer to command that is aliased by this one, so the
        aliased command can be located in case it has been hooked.  */
     struct cmd_list_element *cmd_pointer;
+
+    /* Start of a linked list of all aliases of this command.  */
+    struct cmd_list_element *aliases;
+
+    /* Link pointer for aliases on an alias list.  */
+    struct cmd_list_element *alias_chain;
   };
 
 /* API to the manipulation of command lists.  */
@@ -290,8 +296,6 @@ extern char **complete_on_cmdlist (struct cmd_list_element *, char *, char *);
 
 extern char **complete_on_enum (const char *enumlist[], char *, char *);
 
-extern void delete_cmd (char *, struct cmd_list_element **);
-
 extern void help_cmd_list (struct cmd_list_element *, enum command_class,
 			   char *, int, struct ui_file *);
 
diff --git a/gdb/command.h b/gdb/command.h
index cf09c01..5678c9e 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -193,8 +193,6 @@ extern char **complete_on_cmdlist (struct cmd_list_element *, char *, char *);
 
 extern char **complete_on_enum (const char *enumlist[], char *, char *);
 
-extern void delete_cmd (char *, struct cmd_list_element **);
-
 extern void help_cmd (char *, struct ui_file *);
 
 extern void help_list (struct cmd_list_element *, char *,
diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp
index 301b995..f6be3ea 100644
--- a/gdb/testsuite/gdb.base/commands.exp
+++ b/gdb/testsuite/gdb.base/commands.exp
@@ -692,6 +692,34 @@ proc if_commands_test {} {
     }
 }
 
+proc redefine_backtrace_test {} {
+    global gdb_prompt
+
+    gdb_test_multiple "define backtrace" "define backtrace" {
+	-re "Really redefine built-in.*$" {
+	    send_gdb "y\n"
+	    exp_continue
+	}
+
+	-re "End with"  {
+	    pass "define backtrace in redefine_backtrace_test"
+	}
+        default {
+	    fail "(timeout or eof) define backtrace in redefine_backtrace_test"
+	}
+    }
+    gdb_test "echo hibob\\n\nend" \
+	    "" \
+	    "enter commands in redefine_backtrace_test"
+
+    gdb_test "backtrace" \
+	    "hibob" \
+	    "execute backtrace command in redefine_backtrace_test"
+    gdb_test "bt" \
+	    "hibob" \
+	    "execute bt command in redefine_backtrace_test"
+}
+
 gdbvar_simple_if_test
 gdbvar_simple_while_test
 gdbvar_complex_if_while_test
@@ -710,3 +738,5 @@ temporary_breakpoint_commands
 stray_arg0_test
 recursive_source_test
 if_commands_test
+# This one should come last, as it redefines "backtrace".
+redefine_backtrace_test

Re: RFA: fix PR 1815

[Joel Brobecker]

> 2008-12-09  Tom Tromey  <tromey@redhat.com>
> 
> 	PR gdb/1815:
> 	* cli/cli-decode.c (delete_cmd): Forward declare.
> 	(delete_cmd): Now static.  Change return type.  Remove command
> 	from alias chain.  Rewrite.
> 	(add_cmd): Initialize new fields.  Update cmd_pointer on all
> 	aliases.
> 	(add_alias_cmd): Put command on alias chain.
> 	* command.h (delete_cmd): Don't declare.
> 	* cli/cli-decode.h (delete_cmd): Don't declare.
> 	(struct cmd_list_element) <aliases, alias_chain>: New fields.
> 
> 2008-12-09  Tom Tromey  <tromey@redhat.com>
> 
> 	* gdb.base/commands.exp (redefine_backtrace_test): New proc.
> 	Call it.

Thanks to you both for fixing the little "exp_continue" nit.
Please go ahead and check the patch in.

-- 
Joel

Re: RFA: fix PR 1815

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> Thanks to you both for fixing the little "exp_continue" nit.
Joel> Please go ahead and check the patch in.

Done.

Tom