[PATCH 2/2] Add a way to preserve redefined GDB commands for later invocation
Andrew Burgess
andrew.burgess@embecosm.com
Mon Oct 5 10:24:41 GMT 2020
Hi,
Thanks for working on this. I think this is a really useful addition
to GDB. I do have some feedback:
* Marco Barisione <mbarisione@undo.io> [2020-09-14 09:39:25 +0000]:
> When a command was redefined, the functionality of the original command
> used to be lost, making it difficult to extend existing commands.
>
> This patch adds an uplevel command (inspired by the TCL command with the
> same name) which allows access to previous implementations of commands
> and a gdb.Command.invoke_uplevel Python method to do the same.
>
> gdb/ChangeLog:
>
> * NEWS: Document the addition of the uplevel command and the
> gdb.Command.invoke_uplevel Python method.
> * cli/cli-decode.c (delete_cmd): Rename to remove_cmd.
> (do_add_cmd): Keep the removed command and and update the chain
> of inmplementations of the same command.
> (add_alias_cmd): Update to reflect the changes to delete_cmd.
> (remove_cmd): Rename from delete_cmd. Preserve removed commands,
> but not aliases, and return them.
> (lookup_uplevel_cmd): Lookup an implementation of a command based
> on a cmd_list_element of the same name and a level.
> * cli/cli-decode.h (struct cmd_list_element): Add uplevel_cmd and
> downlevel_cmd to keep track of redefined commands and commands
> which redefined them.
> * cli/cli-script.c (uplevel_command): Implementation of the
> uplevel command.
> * command.h: Add declaration of lookup_uplevel_cmd.
> * python/py-cmd.c (cmdpy_invoke_uplevel): Implementation of the
> gdb.Command.invoke_uplevel method.
>
> gdb/doc/ChangeLog:
>
> * gdb/doc/gdb.texinfo: Document the uplevel command.
> * python.texi: Document the gdb.Command.invoke_uplevel method.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.base/uplevel.exp: New file to test the uplevel command.
> * gdb.python/py-invoke-uplevel.exp: New file to test the
> gdb.Command.invoke_uplevel method.
> * gdb.python/py-invoke-uplevel.py: New test.
> ---
> gdb/NEWS | 18 ++
> gdb/cli/cli-decode.c | 146 ++++++++--
> gdb/cli/cli-decode.h | 8 +
> gdb/cli/cli-script.c | 71 +++++
> gdb/command.h | 3 +
> gdb/doc/gdb.texinfo | 24 ++
> gdb/doc/python.texi | 43 +++
> gdb/python/py-cmd.c | 75 +++++
> gdb/testsuite/gdb.base/uplevel.exp | 255 +++++++++++++++++
> .../gdb.python/py-invoke-uplevel.exp | 267 ++++++++++++++++++
> gdb/testsuite/gdb.python/py-invoke-uplevel.py | 76 +++++
> 11 files changed, 958 insertions(+), 28 deletions(-)
> create mode 100644 gdb/testsuite/gdb.base/uplevel.exp
> create mode 100644 gdb/testsuite/gdb.python/py-invoke-uplevel.exp
> create mode 100644 gdb/testsuite/gdb.python/py-invoke-uplevel.py
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 0ac0ff18f2..8a2c417902 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -13,6 +13,24 @@
> equivalent of the CLI's "break -qualified" and "dprintf
> -qualified".
>
> +* Python API
> +
> + ** gdb.Command has a new method 'invoke_uplevel' to invoke other
> + implementations of the same command, even if they were overridden by
> + redefining the command. This allows commands to be built around
> + existing commands.
> +
> +* New commands
> +
> +uplevel LEVEL COMMAND [ARGUMENTS]
> + Execute the implementation of a command even if it was redefined.
> +
> + Usage: uplevel LEVEL COMMAND [ARGUMENTS]
> + When a command is originally defined, it's considered at level 0. When
> + the command is redefined the new definition is at level 1, and so on.
> + The UPLEVEL command executes the implementation at LEVEL for COMMAND,
> + passing the specified ARGUMENTS if any.
> +
> *** Changes in GDB 10
>
> * There are new feature names for ARC targets: "org.gnu.gdb.arc.core"
> diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
> index 5a549edd43..5b8889b10b 100644
> --- a/gdb/cli/cli-decode.c
> +++ b/gdb/cli/cli-decode.c
> @@ -30,12 +30,14 @@
>
> static void undef_cmd_error (const char *, const char *);
>
> -static struct cmd_list_element *delete_cmd (const char *name,
> - struct cmd_list_element **list,
> - struct cmd_list_element **prehook,
> - struct cmd_list_element **prehookee,
> - struct cmd_list_element **posthook,
> - struct cmd_list_element **posthookee);
> +static struct cmd_list_element *
> + remove_cmd (const char *name,
> + struct cmd_list_element **list,
> + struct cmd_list_element **aliases,
> + struct cmd_list_element **prehook,
> + struct cmd_list_element **prehookee,
> + struct cmd_list_element **posthook,
> + struct cmd_list_element **posthookee);
>
> static struct cmd_list_element *find_cmd (const char *command,
> int len,
> @@ -182,8 +184,13 @@ do_add_cmd (const char *name, enum command_class theclass,
>
> /* Turn each alias of the old command into an alias of the new
> command. */
> - c->aliases = delete_cmd (name, list, &c->hook_pre, &c->hookee_pre,
> - &c->hook_post, &c->hookee_post);
> + c->uplevel_cmd = remove_cmd (name, list, &c->aliases,
> + &c->hook_pre, &c->hookee_pre, &c->hook_post,
> + &c->hookee_post);
> +
> + if (c->uplevel_cmd != nullptr)
> + c->uplevel_cmd->downlevel_cmd = c;
> +
> for (iter = c->aliases; iter; iter = iter->alias_chain)
> iter->cmd_pointer = c;
> if (c->hook_pre)
> @@ -289,14 +296,15 @@ add_alias_cmd (const char *name, cmd_list_element *old,
> {
> if (old == 0)
> {
> + struct cmd_list_element *aliases;
> struct cmd_list_element *prehook, *prehookee, *posthook, *posthookee;
> - struct cmd_list_element *aliases = delete_cmd (name, list,
> - &prehook, &prehookee,
> - &posthook, &posthookee);
> + struct cmd_list_element *removed_cmd =
> + remove_cmd (name, list, &aliases,
> + &prehook, &prehookee, &posthook, &posthookee);
>
> /* If this happens, it means a programmer error somewhere. */
> - gdb_assert (!aliases && !prehook && !prehookee
> - && !posthook && ! posthookee);
> + gdb_assert (!removed_cmd && !aliases && !prehook && !prehookee &&
> + !posthook && !posthookee);
> return 0;
> }
>
> @@ -901,15 +909,36 @@ add_setshow_zuinteger_cmd (const char *name, enum command_class theclass,
> NULL, NULL);
> }
>
> -/* 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. The various *HOOKs are set to
> - the pre- and post-hook commands for the deleted command. If the
> - command does not have a hook, the corresponding out parameter is
> +/* Remove the command or alias named NAME from the command list.
> +
> + If NAME refers to a real command (not an alias), then the removed
> + command is returned. Otherwise, if NAME refers to an alias, the alias
> + gets destroyed and NULLPTR is returned.
> + If no command called NAME is found nothing happens and NULLPTR is
> + returned.
> +
> + Real commands are returned so that their implementation can be
> + preserved to be used with the uplevel command.
> + On the other hand, aliases, while implemented using cmd_list_element,
> + don't look like real commands from a user point of view:
> + * You cannot define a hook for an alias.
> + * No "Redefine command ..." question is asked when define a command
> + with the same name as an alias.
> + Because of this, aliases are deleted when a command with the same name
> + is defined.
> +
> + *ALIASES is set to the list of commands which were aliased to the
> + removed command. If the command had no aliases, this is set to
> + NULL.
> +
> + The various *HOOKs are set to the pre- and post-hook commands for
> + the deleted command. If the command does not have a hook, the
> + corresponding out parameter is
> set to NULL. */
>
> static struct cmd_list_element *
> -delete_cmd (const char *name, struct cmd_list_element **list,
> +remove_cmd (const char *name, struct cmd_list_element **list,
> + struct cmd_list_element **aliases,
> struct cmd_list_element **prehook,
> struct cmd_list_element **prehookee,
> struct cmd_list_element **posthook,
> @@ -917,8 +946,8 @@ delete_cmd (const char *name, struct cmd_list_element **list,
> {
> struct cmd_list_element *iter;
> struct cmd_list_element **previous_chain_ptr;
> - struct cmd_list_element *aliases = NULL;
>
> + *aliases = NULL;
> *prehook = NULL;
> *prehookee = NULL;
> *posthook = NULL;
> @@ -929,8 +958,12 @@ delete_cmd (const char *name, struct cmd_list_element **list,
> {
> if (strcmp (iter->name, name) == 0)
> {
> - if (iter->destroyer)
> + bool is_alias = iter->cmd_pointer != nullptr;
> + if (is_alias && iter->destroyer)
> + /* Call the destroyer before doing the rest of the cleanup
> + in case it accesses other fields. */
> iter->destroyer (iter, iter->context);
> +
> if (iter->hookee_pre)
> iter->hookee_pre->hook_pre = 0;
> *prehook = iter->hook_pre;
> @@ -942,12 +975,13 @@ delete_cmd (const char *name, struct cmd_list_element **list,
>
> /* Update the link. */
> *previous_chain_ptr = iter->next;
> + iter->next = nullptr;
>
> - aliases = iter->aliases;
> + *aliases = iter->aliases;
>
> - /* If this command was an alias, remove it from the list of
> - aliases. */
> - if (iter->cmd_pointer)
> + /* If the cmd_list_element is an alias and not a real command,
> + remove it from the list of aliases and destroy it. */
> + if (is_alias)
> {
> struct cmd_list_element **prevp = &iter->cmd_pointer->aliases;
> struct cmd_list_element *a = *prevp;
> @@ -958,9 +992,10 @@ delete_cmd (const char *name, struct cmd_list_element **list,
> a = *prevp;
> }
> *prevp = iter->alias_chain;
> - }
>
> - delete iter;
> + delete iter;
> + iter = nullptr;
> + }
>
> /* We won't see another command with the same name. */
> break;
> @@ -969,7 +1004,7 @@ delete_cmd (const char *name, struct cmd_list_element **list,
> previous_chain_ptr = &iter->next;
> }
>
> - return aliases;
> + return iter;
> }
>
> /* Shorthands to the commands above. */
> @@ -1773,6 +1808,61 @@ lookup_cmd_1 (const char **text, struct cmd_list_element *clist,
> }
> }
>
> +/* Look up and return the command with level LEVEL for the chain of
> + commands of which C is part.
> +
> + When a command gets redefined, the original implementation is
> + preserved and a chain of CMD_LIST_ELEMENT (with the same name) i
> + formed. This function, given a CMD_LIST_ELEMENT C part of a chain
> + of CMD_LIST_ELEMENTs, where level indicates which element to return.
> +
> + If LEVEL is positive then it's an absolute number identifying the
> + element to loop up, where 0 is the first command to be defined, 1 is
> + the second, and so on. If LEVEL is negative, then it's relative to C,
> + for instance -1 is the command which was redefined by C.
> +
> + If the command is not found, NULLPTR is returned. */
> +
> +struct cmd_list_element *
> +lookup_uplevel_cmd (struct cmd_list_element *c, int level)
> +{
> + if (!c)
> + return nullptr;
> +
> + int i;
> + struct cmd_list_element *curr_cmd = nullptr;
> +
> + if (level >= 0)
> + {
> + /* Relative to the top-level command (i.e. the first one to be
> + defined), going downlevel. */
> + struct cmd_list_element *top_level_cmd = c;
> + while (top_level_cmd->uplevel_cmd != nullptr)
> + top_level_cmd = top_level_cmd->uplevel_cmd;
> +
> + for (curr_cmd = top_level_cmd, i = 0;
> + curr_cmd != nullptr;
> + curr_cmd = curr_cmd->downlevel_cmd, i++)
> + {
> + if (i == level)
> + return curr_cmd;
> + }
> + }
> + else
> + {
> + /* Relative to c, going uplevel. */
> + for (curr_cmd = c, i = 0;
> + curr_cmd != nullptr;
> + curr_cmd = curr_cmd->uplevel_cmd, i--)
> + {
> + if (i == level)
> + return curr_cmd;
> + }
> + }
> +
> + return nullptr;
> +}
> +
> /* All this hair to move the space to the front of cmdtype */
>
> static void
> diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
> index e8ce362922..79f8ed6f09 100644
> --- a/gdb/cli/cli-decode.h
> +++ b/gdb/cli/cli-decode.h
> @@ -223,6 +223,14 @@ struct cmd_list_element
> /* Pointer to command strings of user-defined commands */
> counted_command_line user_commands;
>
> + /* Pointer to command that was redefined by this command.
> + This is kept around as it can still be invoked by the redefined
> + command, for instance using the "uplevel" command. */
> + struct cmd_list_element *uplevel_cmd = nullptr;
> + /* Pointer to command that redefined this command, if any. This is
> + the reverse of UPLEVEL_CMD. */
> + struct cmd_list_element *downlevel_cmd = nullptr;
> +
> /* Pointer to command that is hooked by this one, (by hook_pre)
> so the hook can be removed when this one is deleted. */
> struct cmd_list_element *hookee_pre = nullptr;
> diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
> index da4a41023a..5e4613ac6a 100644
> --- a/gdb/cli/cli-script.c
> +++ b/gdb/cli/cli-script.c
> @@ -1666,6 +1666,68 @@ show_user_1 (struct cmd_list_element *c, const char *prefix, const char *name,
>
> }
>
> +/* Implementation of the "uplevel" command. */
> +
> +static void
> +uplevel_command (const char *arg, int from_tty)
> +{
> + /* Extract the level argument and update arg to point to the remaining
> + string. */
> + size_t level_end_offset;
> + int level;
> + try
> + {
> + level = std::stoi (arg, &level_end_offset);
> + }
> + catch (const std::exception &)
> + {
> + level = -1;
> + }
> + if (level < 0)
> + error (_("uplevel command requires a non-negative number for the "
> + "level argument."));
I don't understand why the Python API allows for negative values while
the CLI command does not. In my mind, negative values are the only
really useful way to use this feature as it should be considered bad
practice to assume that there are not some unknown number of overrides
above you in the command stack.
> +
> + arg = &arg[level_end_offset];
> + arg = skip_spaces (arg);
> + if (arg[0] == '\0')
> + error (_("uplevel command requires a command to execute."));
> +
> + /* Extract the command and update arg to point after it. */
> + std::string default_args;
> + struct cmd_list_element *c = lookup_cmd (&arg, cmdlist, "",
> + &default_args, false, true);
> + gdb_assert (c != nullptr);
> +
> + /* Build the command line based on the default arguments and the
> + * remaining string in arg. */
> + std::string default_args_and_arg;
> + if (!default_args.empty ())
> + {
> + if (*arg != '\0')
> + default_args_and_arg = default_args + ' ' + arg;
> + else
> + default_args_and_arg = default_args;
> + arg = default_args_and_arg.c_str ();
> + }
> +
> + if (*arg == '\0')
> + {
> + /* Pass null arg rather than an empty one. */
> + arg = *arg == '\0' ? nullptr : arg;
> + }
> +
> + /* Find the actual command to execute (based on c and level). */
> + c = lookup_uplevel_cmd (c, level);
> + if (c == nullptr)
> + {
> + error (_("Command unavailable at level %d."), level);
> + }
Single statement bodies don't get { ... }.
> +
> + /* And finally execute it. */
> + execute_cmd_list_command (c, arg, from_tty);
> +}
> +
> +
> void _initialize_cli_script ();
> void
> _initialize_cli_script ()
> @@ -1709,4 +1771,13 @@ The conditional expression must follow the word `if' and must in turn be\n\
> followed by a new line. The nested commands must be entered one per line,\n\
> and should be terminated by the word 'else' or `end'. If an else clause\n\
> is used, the same rules apply to its nested commands as to the first ones."));
> +
> + add_com ("uplevel", class_support, uplevel_command, _("\
> +Execute the implementation of a command even if it was redefined.\n\
> +\n\
> +Usage: uplevel LEVEL COMMAND [ARGUMENTS]\n\
> +When a command is originally defined, it's considered at level 0. When\n\
> +the command is redefined the new definition is at level 1, and so on.\n\
> +The UPLEVEL command executes the implementation at LEVEL for COMMAND,\n\
> +passing the specified ARGUMENTS if any."));
> }
> diff --git a/gdb/command.h b/gdb/command.h
> index 22e43de3c1..d042e798d2 100644
> --- a/gdb/command.h
> +++ b/gdb/command.h
> @@ -284,6 +284,9 @@ extern struct cmd_list_element *lookup_cmd_1 (const char **,
> std::string *,
> int);
>
> +extern struct cmd_list_element *
> + lookup_uplevel_cmd (struct cmd_list_element *, int);
> +
> extern struct cmd_list_element *deprecate_cmd (struct cmd_list_element *,
> const char * );
>
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 8bff27c940..138cc227b1 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -26890,6 +26890,30 @@ Used inside a user-defined command, this tells @value{GDBN} that this
> command should not be repeated when the user hits @key{RET}
> (@pxref{Command Syntax, repeat last command}).
>
> +@kindex uplevel
> +@cindex execute redefined commands
> +@item uplevel @var{level} @var{command} @r{[}@var{arguments}@r{]}
> +When commands get redefined, the original implementation is preserved so
> +the new implementation can build on top of an existing command.
> +The @code{uplevel} command invokes the implementation of the command at
> +the specified @code{level}, where level 0 is the original implementation,
> +1 is the first redefinition, and so on.
> +
> +For instance, you can modify the @code{run} command to print a message
> +before running an inferior:
> +@example
> +(gdb) define run
> +Really redefine built-in command "run"? (y or n) y
> +Type commands for definition of "run".
> +End with a line saying just "end".
> +>echo Will run now!\n
> +>uplevel 0 run
> +>end
> +(gdb) run
> +Will run now!
> +...
> +@end example
> +
> @kindex help user-defined
> @item help user-defined
> List all user-defined commands and all python commands defined in class
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index 9bb9f3c2a6..f0f00ddfe1 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -3721,6 +3721,49 @@ print gdb.string_to_argv ("1 2\ \\\"3 '4 \"5' \"6 '7\"")
>
> @end defun
>
> +@defun Command.invoke_uplevel (argument, from_tty, a@r{[}level=-1@r{]})
> +When commands get redefined, the original implementation is preserved so
> +the new implementation can build on top of an existing command.
> +The @code{invoke_uplevel} method invokes the implementation of the command
> +at the specified @code{level}, where level 0 is the original
> +implementation, 1 is the first redefinition, and so on.
> +
> +Negative values for @code{level} are relative to the current
> +implementation, so a @code{level} of -1 (the default if the argument is
> +not specified) will invoke the implementation which was overridden by the
> +instance on which this method was called.
> +
> +The meaning of the @code{argument} and @code{from_tty} arguments is the
> +same as for the @code{invoke} method.
> +
> +The following example shows a reimplementation of the @code{echo} command
> +which prints extra text to the beginning and end of the message:
> +
> +@smallexample
> +class RedefinedEcho (gdb.Command):
> + def __init__ (self):
> + super (RedefinedEcho, self).__init__ ("echo", gdb.COMMAND_SUPPORT)
> +
> + def invoke (self, arg, from_tty):
> + print ("Before")
> + # Call the previous implementation of the "echo" command.
> + self.invoke_uplevel (arg)
I tried this demo and it doesn't work, you need to pass `from_tty`
through to `invoke_uplevel`.
> + print ("After")
> +
> +RedefinedEcho ()
> +@end smallexample
> +
> +For instance:
> +@smallexample
> +(@value{GDBP}) echo Hello World\n
> +Before
> +Hello World
> +After
> +(@value{GDBP})
> +@end smallexample
> +
> +@end defun
> +
> @cindex completion of Python commands
> @defun Command.complete (text, word)
> This method is called by @value{GDBN} when the user attempts
> diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c
> index 760208f52b..57dad034bc 100644
> --- a/gdb/python/py-cmd.c
> +++ b/gdb/python/py-cmd.c
> @@ -411,6 +411,64 @@ gdbpy_parse_command_name (const char *name,
> return NULL;
> }
>
> +/* Python function called to invoke the implementatinon of a command, even
> + if the implementatinonon was overridden by a redefinition of the same
> + command.
> +
> + This corresponds to the "uplevel" command, but the level can also be
> + negative to indicate a relative level. For instance, -1 indicates the
> + command overridden by the definition of SELF. */
> +
> +static PyObject *
> +cmdpy_invoke_uplevel (PyObject *self, PyObject *py_args, PyObject *kw)
> +{
> + cmdpy_object *obj = (cmdpy_object *) self;
> +
> + static const char *keywords[] = { "argument", "from_tty", "level", NULL };
> + const char *argument = NULL;
> + PyObject *from_tty_obj = NULL;
> + int level = -1;
> +
> + if (!gdb_PyArg_ParseTupleAndKeywords (py_args, kw,
> + "sO|i",
> + keywords,
> + &argument,
> + &from_tty_obj,
> + &level))
> + return NULL;
> +
> + int from_tty = PyObject_IsTrue (from_tty_obj);
> +
> + cmd_list_element *target_cmd = lookup_uplevel_cmd (obj->command, level);
> + if (target_cmd == nullptr)
> + {
> + PyErr_Format (gdbpy_gdb_error,
> + _("No implementation of command '%s' at level %d"),
> + obj->command->name, level);
> + return NULL;
> + }
> +
> + /* target_cmd may be the same as obj->command which means that this
> + command is calling itself.
> + We don't deal with this case in any special way as there are
> + potential legitimate uses of it, for instance if argument changes.
> + In the worse case, Python will quickly raise a RecursionError (or
> + RuntimeError for Python < 3.5) due to reaching the maximum recursion
> + limit. */
> +
> + try
> + {
> + execute_cmd_list_command (target_cmd, argument, from_tty);
> + }
> + catch (const gdb_exception &except)
> + {
> + GDB_PY_HANDLE_EXCEPTION (except);
> + }
> +
> +
> + Py_RETURN_NONE;
> +}
> +
> /* Object initializer; sets up gdb-side structures for command.
>
> Use: __init__(NAME, COMMAND_CLASS [, COMPLETER_CLASS][, PREFIX]]).
> @@ -642,6 +700,23 @@ static PyMethodDef cmdpy_object_methods[] =
> { "dont_repeat", cmdpy_dont_repeat, METH_NOARGS,
> "Prevent command repetition when user enters empty line." },
>
> + { "invoke_uplevel", (PyCFunction) cmdpy_invoke_uplevel,
> + METH_VARARGS | METH_KEYWORDS,
> + "invoke_uplevel (argument, from_tty, level=-1)\n\
> +Execute the implementation of the command at the specified level.\n\
> +\n\
> +When commands get redefined, the original implementation is preserved so \
> +the new implementation can build on top of an existing command. \
> +The invoke_uplevel method invokes the implementation of the command at \
> +the specified level.\n\
> +\n\
> +Positive values for level are absolute indices where 0 is the original \
> +implmentation, 1 is the first redefinition, and so on.\n\
> +\n\
> +Negative values for level are relative to the current implementation, so \
> +a level of -1 will invoke the implementation which was redefined by \
> +self." },
> +
> { 0 }
> };
>
> diff --git a/gdb/testsuite/gdb.base/uplevel.exp b/gdb/testsuite/gdb.base/uplevel.exp
> new file mode 100644
> index 0000000000..dd6c978ac5
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/uplevel.exp
> @@ -0,0 +1,255 @@
> +# Copyright (C) 2009-2020 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/>.
> +
> +# This file is part of the GDB testsuite. It tests the uplevel command.
> +
> +standard_testfile
> +
> +clean_restart
> +
> +proc test_uplevel_existing { } {
> + with_test_prefix "uplevel with existing" {
> + gdb_test_exact \
> + "uplevel 0 echo Hello\\n" \
> + "Hello"
> +
> + gdb_test_exact \
> + "uplevel 1 echo Hello\\n" \
> + "Command unavailable at level 1."
> +
> + gdb_test_exact \
> + "uplevel 123 echo Hello\\n" \
> + "Command unavailable at level 123."
> + }
> +}
> +
> +proc test_overwrite_builtin { } {
> + with_test_prefix "overwrite builtin" {
> + gdb_define_cmd "delete" {
> + "echo Will call delete with \$arg0.\\n"
> + "uplevel 0 delete \$arg0"
> + }
> +
> + gdb_test_exact \
> + "delete 123" \
> + "Will call delete with 123.\nNo breakpoint number 123."
> +
> + # Same as above as the command at level 1 is the redefinition we
> + # added.
> + gdb_test_exact \
> + "uplevel 1 delete 123" \
> + "Will call delete with 123.\nNo breakpoint number 123."
> +
> + # Invoke the original delete directly.
> + gdb_test_exact \
> + "uplevel 0 delete 123" \
> + "No breakpoint number 123."
> + }
> +}
> +
> +proc test_overwrite_user_defined {} {
> + with_test_prefix "overwrite user defined" {
> + gdb_define_cmd "my-command" {
> + "echo Level 0: \$arg0\\n"
> + }
> + gdb_define_cmd "my-command" {
> + "echo Level 1: \$arg0\\n"
> + "uplevel 0 my-command \$arg0"
> + }
> + gdb_define_cmd "my-command" {
> + "echo THIS SHOULD NOT BE CALLED"
> + "uplevel 1 my-command \$arg0"
> + }
> + # Skip level 2 on purpose.
> + gdb_define_cmd "my-command" {
> + "echo Level 3: \$arg0\\n"
> + "uplevel 1 my-command \$arg0"
> + }
> +
> + gdb_test_exact \
> + "my-command hello" \
> + "Level 3: hello\nLevel 1: hello\nLevel 0: hello"
> + gdb_test_exact \
> + "uplevel 3 my-command hello" \
> + "Level 3: hello\nLevel 1: hello\nLevel 0: hello"
> + gdb_test_exact \
> + "uplevel 1 my-command hello" \
> + "Level 1: hello\nLevel 0: hello"
> + gdb_test_exact \
> + "uplevel 0 my-command hello" \
> + "Level 0: hello"
> + }
> +}
> +
> +proc test_aliases {} {
> + # Define a command and an alias to it.
> + with_test_prefix "aliases" {
> + gdb_define_cmd "aliased-cmd" {
> + "echo Original command\\n"
> + }
> +
> + gdb_test_no_output "alias alias-to-cmd = aliased-cmd"
> +
> + # The alias should initially just call the aliased command.
> + with_test_prefix "original alias" {
> + gdb_test_exact \
> + "alias-to-cmd" \
> + "Original command"
> + }
> +
> + # When the original aliased command gets redefined, the alias gets
> + # redirected to the new command (which then can access the original
> + # command with uplevel).
> + gdb_define_cmd "aliased-cmd" {
> + "echo New aliased command\\n"
> + "uplevel 0 aliased-cmd"
> + }
> +
> + with_test_prefix "new alias" {
> + gdb_test_exact \
> + "alias-to-cmd" \
> + "New aliased command\nOriginal command"
> + }
> +
> + # When a command with the same name as an alias gets defined, the old
> + # alias is lost.
> + # This means that, if the command does an "uplevel 0 ..." then it's
> + # going to call itself.
> + gdb_define_cmd "alias-to-cmd" {
> + "if !\$alias_to_cmd_recursing"
> + " echo First call into user command\\n"
> + " set \$alias_to_cmd_recursing = 1"
> + " uplevel 0 alias-to-cmd"
> + "else"
> + " echo Recursed into user command\\n"
> + "end"
> + }
> +
> + with_test_prefix "alias overwritten by command" {
> + gdb_test_exact \
> + "alias-to-cmd" \
> + "First call into user command\nRecursed into user command"
> + }
> + }
> +}
> +
> +proc test_hooks {} {
> + with_test_prefix "hooks moved to redefined commands" {
> + # Define a command and hooks for it.
> + gdb_define_cmd "cmd-with-hooks" {
> + "echo Original command\\n"
> + }
> + gdb_define_cmd "hook-cmd-with-hooks" {
> + "echo Pre\\n"
> + }
> + gdb_define_cmd "hookpost-cmd-with-hooks" {
> + "echo Post\\n"
> + }
> +
> + # Redefine the command.
> + gdb_define_cmd "cmd-with-hooks" {
> + "echo New: before\\n"
> + "uplevel 0 cmd-with-hooks"
> + "echo New: after\\n"
> + }
> +
> + # The hooks should now apply to the new command.
> + gdb_test_exact \
> + "cmd-with-hook" \
> + "Pre\nNew: before\nOriginal command\nNew: after\nPost"
> + }
> +
> + with_test_prefix "redefining hooks" {
> + # Define a command with a hook.
> + gdb_define_cmd "another-cmd-with-hooks" {
> + "echo Command\\n"
> + }
> + gdb_define_cmd "hook-another-cmd-with-hooks" {
> + "echo Original hook\\n"
> + }
> +
> + # Redefine the hook.
> + gdb_define_cmd "hook-another-cmd-with-hooks" {
> + "echo New hook: before\\n"
> + "uplevel 0 hook-another-cmd-with-hooks"
> + "echo New hook: after\\n"
> + }
> +
> + # Call the command, the new hook should be called, not the original
> + # one.
> + gdb_test_exact \
> + "another-cmd-with-hook" \
> + "New hook: before\nOriginal hook\nNew hook: after\nCommand"
> + }
> +}
> +
> +proc test_invalid_level {} {
> + with_test_prefix "invalid level" {
> + gdb_test_exact \
> + "uplevel" \
> + "uplevel command requires a non-negative number for the level argument."
> +
> + gdb_test_exact \
> + "uplevel not-a-number" \
> + "uplevel command requires a non-negative number for the level argument."
> +
> + gdb_test_exact \
> + "uplevel -1" \
> + "uplevel command requires a non-negative number for the level argument."
> +
> + gdb_test_exact \
> + "uplevel 999999999999999999999999999999999" \
> + "uplevel command requires a non-negative number for the level argument."
> + }
> +}
> +
> +proc test_invalid_command {} {
> + with_test_prefix "invalid command" {
> + gdb_test_exact \
> + "uplevel 0" \
> + "uplevel command requires a command to execute."
> +
> + gdb_test_exact \
> + "uplevel 0 THIS_DOES_NOT_EXIST" \
> + "Undefined command: \"THIS_DOES_NOT_EXIST\". Try \"help\"."
> + }
> +}
> +
> +proc test_infinite_recursion {} {
> + with_test_prefix "infinite recursion" {
> + # Define a command which calls itself via uplevel.
> + gdb_define_cmd "infinite-recursion" {
> + "uplevel 0 infinite-recursion"
> + }
> +
> + gdb_test_exact \
> + "infinite-recursion" \
> + "Max user call depth exceeded -- command aborted."
> + }
> +}
> +
> +with_test_prefix "uplevel" {
> + # Allow the redefinition of commands.
> + gdb_test_no_output "set confirm off"
> +
> + test_uplevel_existing
> + test_overwrite_builtin
> + test_overwrite_user_defined
> + test_aliases
> + test_hooks
> + test_invalid_level
> + test_invalid_command
> + test_infinite_recursion
> +}
> diff --git a/gdb/testsuite/gdb.python/py-invoke-uplevel.exp b/gdb/testsuite/gdb.python/py-invoke-uplevel.exp
> new file mode 100644
> index 0000000000..b141534b16
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-invoke-uplevel.exp
> @@ -0,0 +1,267 @@
> +# Copyright (C) 2009-2020 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/>.
> +
> +# This file is part of the GDB testsuite. It tests accessing overridden
> +# commands.
> +
> +load_lib gdb-python.exp
> +
> +standard_testfile
> +
> +# Skip all tests if Python scripting is not enabled.
> +gdb_exit
> +gdb_start
> +if { [skip_python_tests] } { continue }
> +
> +# Prepare for testing.
> +#
> +# This quits GDB (if running), starts a new one, and loads any required
> +# external scripts.
> +
> +proc prepare_gdb {} {
> + global srcdir subdir testfile
> +
> + gdb_exit
> + gdb_start
> + gdb_reinitialize_dir $srcdir/$subdir
> +
> + gdb_test_no_output "set confirm off"
> +
> + # Load the code which adds commands.
> + set remote_python_file \
> + [gdb_remote_download host ${srcdir}/${subdir}/${testfile}.py]
> + gdb_test_no_output "source ${remote_python_file}" "load python file"
> +}
> +
> +# Add a command called CMD through the Python API.
> +#
> +# The command will print messages including the string MSG when invoked.
> +
> +proc add_py_cmd { cmd msg { level "None" } } {
> + gdb_test_no_output "python TestCommand ('${cmd}', '${msg}', $level)"
> +}
> +
> +# Add a command called CMD through GDB scripting.
> +#
> +# The command will print messages including the string MSG when invoked,
> +# but not invoke the uplevel command XXX
> +
> +proc add_gdb_script_cmd { cmd msg } {
> + gdb_define_cmd $cmd [list "echo gdb: ${msg}\\n"]
> +}
> +
> +# Define an alias.
> +
> +proc define_alias { alias_name original_name } {
> + gdb_test_no_output "alias ${alias_name} = ${original_name}"
> +}
> +
> +# test_sequence_exact CMD LIST
> +#
> +# Like gdb_test_exact but, for convenience, it accepts a list of lines
> +# instead of a single line.
> +
> +proc test_sequence_exact { cmd lines } {
> + set expected_string [join $lines "\n"]
> + gdb_test_exact $cmd $expected_string
> +}
> +
> +proc test_delete_overridden_by_py {} {
> + with_test_prefix "override delete with Python" {
> + prepare_gdb
> + define_alias "new-delete" "del"
> + add_py_cmd "delete" "new delete"
> +
> + set expected_list {
> + "py-before: new delete"
> + "No breakpoint number 9999."
> + "py-after: new delete"
> + }
> +
> + test_sequence_exact "delete 9999" $expected_list
> + test_sequence_exact "del 9999" $expected_list
> + test_sequence_exact "new-delete 9999" $expected_list
> + }
> +}
> +
> +proc test_gdb_script_overridden_by_py {} {
> + with_test_prefix "override a defined command with a Python one" {
> + prepare_gdb
> + add_gdb_script_cmd "new-command" "new command"
> + define_alias "new-alias" "new-command"
> + add_py_cmd "new-command" "new command"
> +
> + set expected_list {
> + "py-before: new command"
> + "gdb: new command"
> + "py-after: new command"
> + }
> +
> + test_sequence_exact "new-command" $expected_list
> + test_sequence_exact "new-alias" $expected_list
> + }
> +}
> +
> +proc test_py_overridden_by_py {} {
> + with_test_prefix "override a Python command with another Python one" {
> + prepare_gdb
> + add_py_cmd "new-command" "new command 1"
> + add_py_cmd "new-command" "new command 2"
> +
> + test_sequence_exact "new-command" {
> + "py-before: new command 2"
> + "py-before: new command 1"
> + "py-invalid-level -1: new command 1"
> + "py-after: new command 1"
> + "py-after: new command 2"
> + }
> + }
> +}
> +
> +proc test_override_many_py {} {
> + with_test_prefix "override delete with many commands" {
> + prepare_gdb
> + define_alias "new-delete-defined-early" "delete"
> + add_py_cmd "delete" "py delete 1"
> + add_py_cmd "delete" "py delete 2"
> + define_alias "new-delete-defined-middle" "delete"
> + add_py_cmd "delete" "py delete 3"
> + add_py_cmd "delete" "py delete 4"
> + define_alias "new-delete-defined-late" "delete"
> +
> + set expected_list {
> + "py-before: py delete 4"
> + "py-before: py delete 3"
> + "py-before: py delete 2"
> + "py-before: py delete 1"
> + "No breakpoint number 9999."
> + "py-after: py delete 1"
> + "py-after: py delete 2"
> + "py-after: py delete 3"
> + "py-after: py delete 4"
> + }
> +
> + # Long command form.
> + test_sequence_exact "delete 9999" $expected_list
> +
> + # Short command form.
> + test_sequence_exact "del 9999" $expected_list
> +
> + # Aliases.
> + # There are three aliases: one defined before any command, one after
> + # some commands are defined, and one defined after all the commands
> + # were defined. When they were defined should be irrelevant and they
> + # should all work the same.
> + test_sequence_exact "new-delete-defined-early 9999" $expected_list
> + test_sequence_exact "new-delete-defined-middle 9999" $expected_list
> + test_sequence_exact "new-delete-defined-late 9999" $expected_list
> + }
> +}
> +
> +proc test_last_invokes_at_level { level expected_list } {
> + with_test_prefix "invoke_uplevel with level=$level" {
> + prepare_gdb
> + add_py_cmd "delete" "new delete at level 1"
> + add_py_cmd "delete" "new delete at level 2"
> + add_py_cmd "delete" "new delete at level 3" $level
> +
> + test_sequence_exact "delete 9999" $expected_list
> + }
> +}
> +
> +proc test_last_invokes_at_levels { } {
> + # Invoke the top-level command both via absolute and relative levels.
> + test_last_invokes_at_level 0 {
> + "py-before: new delete at level 3"
> + "No breakpoint number 9999."
> + "py-after: new delete at level 3"
> + }
> + test_last_invokes_at_level -3 {
> + "py-before: new delete at level 3"
> + "No breakpoint number 9999."
> + "py-after: new delete at level 3"
> + }
> +
> + # First redefined command.
> + test_last_invokes_at_level 1 {
> + "py-before: new delete at level 3"
> + "py-before: new delete at level 1"
> + "No breakpoint number 9999."
> + "py-after: new delete at level 1"
> + "py-after: new delete at level 3"
> + }
> + test_last_invokes_at_level -2 {
> + "py-before: new delete at level 3"
> + "py-before: new delete at level 1"
> + "No breakpoint number 9999."
> + "py-after: new delete at level 1"
> + "py-after: new delete at level 3"
> + }
> +
> + # Second redefined command.
> + test_last_invokes_at_level 2 {
> + "py-before: new delete at level 3"
> + "py-before: new delete at level 2"
> + "py-before: new delete at level 1"
> + "No breakpoint number 9999."
> + "py-after: new delete at level 1"
> + "py-after: new delete at level 2"
> + "py-after: new delete at level 3"
> + }
> + test_last_invokes_at_level -1 {
> + "py-before: new delete at level 3"
> + "py-before: new delete at level 2"
> + "py-before: new delete at level 1"
> + "No breakpoint number 9999."
> + "py-after: new delete at level 1"
> + "py-after: new delete at level 2"
> + "py-after: new delete at level 3"
> + }
> +
> + # Third redefined command, i.e. the command calls itself again.
> + # This is caught in TestCommand and prints a "py-recursion: ..."
> + # message.
> + test_last_invokes_at_level 3 {
> + "py-before: new delete at level 3"
> + "py-recursion: new delete at level 3"
> + "py-after: new delete at level 3"
> + }
> +}
> +
> +proc test_last_invokes_at_invalid_level { level } {
> + test_last_invokes_at_level $level [subst {
> + "py-before: new delete at level 3"
> + "py-invalid-level ${level}: new delete at level 3"
> + "py-after: new delete at level 3"
> + }]
> +}
> +
> +proc test_last_invokes_at_invalid_levels { } {
> + test_last_invokes_at_invalid_level 4
> + test_last_invokes_at_invalid_level -4
> + test_last_invokes_at_invalid_level 999
> + test_last_invokes_at_invalid_level -999
> +}
> +
> +set current_lang "c"
> +
> +with_test_prefix "overriding commands" {
> + test_delete_overridden_by_py
> + test_gdb_script_overridden_by_py
> + test_py_overridden_by_py
> + test_override_many_py
> + test_last_invokes_at_levels
> + test_last_invokes_at_invalid_levels
> +}
> diff --git a/gdb/testsuite/gdb.python/py-invoke-uplevel.py b/gdb/testsuite/gdb.python/py-invoke-uplevel.py
> new file mode 100644
> index 0000000000..c79e12e6b1
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-invoke-uplevel.py
> @@ -0,0 +1,76 @@
> +# Copyright (C) 2008-2020 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/>.
> +
> +# This file is part of the GDB testsuite. It tests command overriding
> +# support in Python.
> +
> +import gdb
> +
> +
> +class TestCommand (gdb.Command):
> +
> + def __init__ (self, cmd_name, msg, level):
> + """
> + Initialises a command called CMD_NAME which, when invoked, prints
> + MSG.
> +
> + LEVEL is either an integer to pass to invoke_uplevel or None to
> + avoid passing any level argument to invoke_uplevel.
> + """
> + self._cmd_name = cmd_name
> + self._msg = msg
> + self.level = level
> +
> + self._already_doing_invoke = False
> +
> + gdb.Command.__init__ (self, cmd_name, gdb.COMMAND_NONE)
> +
> + def invoke (self, args, from_tty):
> + # If this command is invoked by itself, inform the test of this
> + # and don't end up in infinite recursion.
> + if self._already_doing_invoke:
> + print ('py-recursion: %s' % self._msg)
> + return
> +
> + print ('py-before: %s' % self._msg)
> +
> + # If the class was instantiated without a level argument, then
> + # don't pass one to invoke_uplevel. Otherwise pass the specified
> + # value.
> + # We don't just use -1 as default in __init__ to test that the
> + # default for invoke_uplevel is correct.
> + invoke_uplevel_kwargs = {}
> + if self.level is not None:
> + invoke_uplevel_kwargs["level"] = self.level
> + expected_level_in_msg = self.level
> + else:
> + expected_level_in_msg = -1
> +
> + self._already_doing_invoke = True
> + try:
> + self.invoke_uplevel (args, from_tty, **invoke_uplevel_kwargs)
> + except gdb.error as exc:
> + expected_exc_msg = (
> + "No implementation of command '%s' at level %d" %
> + (self._cmd_name, expected_level_in_msg))
> + if str (exc) == expected_exc_msg:
> + print ('py-invalid-level %d: %s' %
> + (expected_level_in_msg, self._msg))
> + else:
> + raise
> + finally:
> + self._already_doing_invoke = False
> +
> + print ('py-after: %s' % self._msg)
> --
> 2.20.1
>
I think that there is another useful check that should be added to
this patch, I think we should prohibit calling a command further down
the command stack. That is we should make this invalid:
define cmd_1
echo this is level 0 cmd_1\n
uplevel 1 cmd_1
end
define cmd_1
echo this is level 1 cmd_1\n
uplevel 0 cmd_1
end
What I'd like to see is that when we execute 'uplevel 1 cmd_1' GDB
throws an error about '... requested command at higher level
than current command...' or similar.
The above will eventually error with 'Max user call depth exceeded --
command aborted.', but I think explicitly catching this case would
improve the user experience.
Similarly doing something like the above in Python will trigger an
error about maximum recursion exceeded from inside Python, but I think
explicitly catching this case would be an improvement.
To implement the above you'll likely need a global for current command
level. It might be neat to expose this inside GDB as
$_gdb_command_level or similar.
I also wonder how we might handle _really_ redefining a command now
that past commands hang around.
When writing the above cmd_1 example the first thing I did was open
GDB and try to write the commands at the CLI. I made a typo when
writing the second version of cmd_1. So now I'm stuck with a chain:
working level 0 cmd_1 --> broken level 1 cmd_1
Sure I can write a level 2 version that knows to skip over the broken
level 1, but it might be nice if there was a flag somewhere that I
could pass to say, no, really replace the last version of this command
please.
Also, when I define cmd_1 (at the CLI) I get asked:
Redefine command "cmd_1"? (y or n)
Maybe this could/should be changed to:
"cmd_1" already exists, type 'o' to override, 'r' to replace, or 'q' to quit:
I'm thinking about the pager prompt which is more than just y/n. I
haven't looked how easy it would be to do something similar here.
One last thing I note is that this new functionality is very similar
to the existing "hook-" and "hookpost-" functionality, though I think
this new approach is much better.
I think as a minimum the documentation for the old hook approach
should be updated to indicate that that technique is deprecated, and
this new approach should be used instead.
Where I really think we should be going is adding a patch #3 to this
series that replaces the implementation of the old functionality using
your new code. Meaning, when someone writes:
define hook-echo
echo before\n
end
GDB would internally change this to effectively be:
define echo
echo before\n
uplevel -1 echo
end
this will work for many trivial cases. There's a few nasty corners,
like hooks for sub-commands (see the docs on hooking 'target remote'),
the pseudo-command 'stop', and calling the same command from within a
hook.
I (personally) think that if we could convert most of the hook
functionality over to use your new approach, then if there are a few
corners that can't be converted, so long was we document this in the
NEWS file it would be OK to remove the old hook support - this might
allow some good code cleanup.
Thanks,
Andrew
More information about the Gdb-patches
mailing list