* Add a way to invoke redefined (overridden) GDB commands
@ 2020-09-14 9:39 Marco Barisione
2020-09-14 9:39 ` [PATCH 1/2] Move the code to execute a cmd_list_element out from execute_command Marco Barisione
` (3 more replies)
0 siblings, 4 replies; 23+ messages in thread
From: Marco Barisione @ 2020-09-14 9:39 UTC (permalink / raw)
To: gdb-patches
Currently, when a GDB command is redefined, the original implementation is not
available any more. This makes it difficult to build features on top of
existing commands.
Last year I submitted a patch to fix this but I ran out of time to address the
review comments (the original patch was sent on the 28th of October 2019).
These patches restart that work and should address all the comments I got last
time. As the patchea are very different and a long time passed, I'm
submitting as a new series.
My patches add a new "uplevel" command and a new gdb.Command.invoke_uplevel
method inspired by TCL (as initially suggested by Andrew Burgess) so you can
do this:
(gdb) define run
echo Will run!\n
uplevel 0 run
end
(gdb) run
Will run!
[... normal output of run ...]
There are a couple of other things which could be added to make the "uplevel"
command more helpful, but I think they are out of scope and my patches are
already useful as they are.
The first thing is adding a way of accessing the untokenised arguments to a
command via something like "$arg@" (Andrew Burgess suggested "$argv", but
Pedro Alves pointed out that would look like an argument vector).
Another thing which could be added is the ability to do "uplevel -1 ..." to
access the directly redefined command.
This is implemented in Python but I couldn't find an obvious way of doing that
for the "uplevel" command as there's no way of knowing which command you are
currently executing (at least from what I could see).
Maybe it could be implemented in a similar way to how command arguments are
kept around with scoped_user_args_level, so we could keep a stack of all (user
and non-user) commands which are being executed. By checking the latest one
you can know what "uplevel -1" would apply to.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 1/2] Move the code to execute a cmd_list_element out from execute_command
2020-09-14 9:39 Add a way to invoke redefined (overridden) GDB commands Marco Barisione
@ 2020-09-14 9:39 ` Marco Barisione
2020-10-05 9:08 ` Andrew Burgess
2020-09-14 9:39 ` [PATCH 2/2] Add a way to preserve redefined GDB commands for later invocation Marco Barisione
` (2 subsequent siblings)
3 siblings, 1 reply; 23+ messages in thread
From: Marco Barisione @ 2020-09-14 9:39 UTC (permalink / raw)
To: gdb-patches
gdb/ChangeLog:
* gdbcmd.h (execute_cmd_list_command): Add declaration.
* top.c (execute_command): Move out the code to execute a
command from a cmd_list_element.
(execute_cmd_list_command): Add from code originally in
execute_command.
* top.h (execute_cmd_list_command): Add declaration.
---
gdb/gdbcmd.h | 3 +++
gdb/top.c | 66 ++++++++++++++++++++++++++++++----------------------
gdb/top.h | 2 ++
3 files changed, 43 insertions(+), 28 deletions(-)
diff --git a/gdb/gdbcmd.h b/gdb/gdbcmd.h
index 4406094ea5..5be474ec1a 100644
--- a/gdb/gdbcmd.h
+++ b/gdb/gdbcmd.h
@@ -134,6 +134,9 @@ extern struct cmd_list_element *save_cmdlist;
extern void execute_command (const char *, int);
+extern void execute_cmd_list_command (struct cmd_list_element *,
+ const char *, int);
+
/* Execute command P and returns its output. If TERM_OUT,
the output is built using terminal output behaviour such
as cli_styling. */
diff --git a/gdb/top.c b/gdb/top.c
index acd31afb9a..8eae15a488 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -581,7 +581,6 @@ execute_command (const char *p, int from_tty)
const char *arg;
std::string default_args;
std::string default_args_and_arg;
- int was_sync = current_ui->prompt_state == PROMPT_BLOCKED;
line = p;
@@ -641,33 +640,7 @@ execute_command (const char *p, int from_tty)
if (c->deprecated_warn_user)
deprecated_cmd_warning (line);
- /* c->user_commands would be NULL in the case of a python command. */
- if (c->theclass == class_user && c->user_commands)
- execute_user_command (c, arg);
- else if (c->theclass == class_user
- && c->prefixlist && !c->allow_unknown)
- /* If this is a user defined prefix that does not allow unknown
- (in other words, C is a prefix command and not a command
- that can be followed by its args), report the list of
- subcommands. */
- {
- printf_unfiltered
- ("\"%.*s\" must be followed by the name of a subcommand.\n",
- (int) strlen (c->prefixname) - 1, c->prefixname);
- help_list (*c->prefixlist, c->prefixname, all_commands, gdb_stdout);
- }
- else if (c->type == set_cmd)
- do_set_command (arg, from_tty, c);
- else if (c->type == show_cmd)
- do_show_command (arg, from_tty, c);
- else if (!cmd_func_p (c))
- error (_("That is not a command, just a help topic."));
- else if (deprecated_call_command_hook)
- deprecated_call_command_hook (c, arg, from_tty);
- else
- cmd_func (c, arg, from_tty);
-
- maybe_wait_sync_command_done (was_sync);
+ execute_cmd_list_command (c, arg, from_tty);
/* If this command has been post-hooked, run the hook last. */
execute_cmd_post_hook (c);
@@ -690,6 +663,43 @@ execute_command (const char *p, int from_tty)
cleanup_if_error.release ();
}
+/* Execute the C command. */
+
+void
+execute_cmd_list_command (struct cmd_list_element *c, const char *arg,
+ int from_tty)
+{
+ bool was_sync = current_ui->prompt_state == PROMPT_BLOCKED;
+
+ /* c->user_commands would be NULL in the case of a python command. */
+ if (c->theclass == class_user && c->user_commands)
+ execute_user_command (c, arg);
+ else if (c->theclass == class_user
+ && c->prefixlist && !c->allow_unknown)
+ {
+ /* If this is a user defined prefix that does not allow unknown
+ (in other words, C is a prefix command and not a command
+ that can be followed by its args), report the list of
+ subcommands. */
+ printf_unfiltered
+ ("\"%.*s\" must be followed by the name of a subcommand.\n",
+ (int) strlen (c->prefixname) - 1, c->prefixname);
+ help_list (*c->prefixlist, c->prefixname, all_commands, gdb_stdout);
+ }
+ else if (c->type == set_cmd)
+ do_set_command (arg, from_tty, c);
+ else if (c->type == show_cmd)
+ do_show_command (arg, from_tty, c);
+ else if (!cmd_func_p (c))
+ error (_("That is not a command, just a help topic."));
+ else if (deprecated_call_command_hook)
+ deprecated_call_command_hook (c, arg, from_tty);
+ else
+ cmd_func (c, arg, from_tty);
+
+ maybe_wait_sync_command_done (was_sync);
+}
+
/* Run execute_command for P and FROM_TTY. Sends its output to FILE,
do not display it to the screen. BATCH_FLAG will be
temporarily set to true. */
diff --git a/gdb/top.h b/gdb/top.h
index fd99297715..ba6c596da7 100644
--- a/gdb/top.h
+++ b/gdb/top.h
@@ -241,6 +241,8 @@ extern void quit_force (int *, int);
extern void quit_command (const char *, int);
extern void quit_cover (void);
extern void execute_command (const char *, int);
+extern void execute_cmd_list_command (struct cmd_list_element *,
+ const char *, int);
/* If the interpreter is in sync mode (we're running a user command's
list, running command hooks or similars), and we just ran a
--
2.20.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 2/2] Add a way to preserve redefined GDB commands for later invocation
2020-09-14 9:39 Add a way to invoke redefined (overridden) GDB commands Marco Barisione
2020-09-14 9:39 ` [PATCH 1/2] Move the code to execute a cmd_list_element out from execute_command Marco Barisione
@ 2020-09-14 9:39 ` Marco Barisione
2020-09-14 16:18 ` Eli Zaretskii
2020-10-05 10:24 ` Andrew Burgess
2020-09-28 7:54 ` [PING] Add a way to invoke redefined (overridden) GDB commands Marco Barisione
2020-10-12 11:50 ` Pedro Alves
3 siblings, 2 replies; 23+ messages in thread
From: Marco Barisione @ 2020-09-14 9:39 UTC (permalink / raw)
To: gdb-patches
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;
}
\f
/* 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."));
+
+ 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);
+ }
+
+ /* 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)
+ 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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] Add a way to preserve redefined GDB commands for later invocation
2020-09-14 9:39 ` [PATCH 2/2] Add a way to preserve redefined GDB commands for later invocation Marco Barisione
@ 2020-09-14 16:18 ` Eli Zaretskii
2020-09-14 16:51 ` Marco Barisione
2020-10-05 10:24 ` Andrew Burgess
1 sibling, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2020-09-14 16:18 UTC (permalink / raw)
To: Marco Barisione; +Cc: gdb-patches
> From: Marco Barisione <mbarisione@undo.io>
> Date: Mon, 14 Sep 2020 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.
Thanks, the documentation parts are okay.
Though I wonder whether 'uplevel' is really the best name for this.
(No, I don't write Python.)
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] Add a way to preserve redefined GDB commands for later invocation
2020-09-14 16:18 ` Eli Zaretskii
@ 2020-09-14 16:51 ` Marco Barisione
0 siblings, 0 replies; 23+ messages in thread
From: Marco Barisione @ 2020-09-14 16:51 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
On 14 Sep 2020, at 17:18, Eli Zaretskii <eliz@gnu.org> wrote:
> Thanks, the documentation parts are okay.
>
> Though I wonder whether 'uplevel' is really the best name for this.
> (No, I don't write Python.)
I do and I don't think it's immediately clear to the average Python developer
but it's probably better to be consistent between Python and the command.
I added "invoke_" to the name as it matches the existing gdb.Command.invoke
method.
Better suggestions welcome! :)
--
Marco Barisione
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PING] Add a way to invoke redefined (overridden) GDB commands
2020-09-14 9:39 Add a way to invoke redefined (overridden) GDB commands Marco Barisione
2020-09-14 9:39 ` [PATCH 1/2] Move the code to execute a cmd_list_element out from execute_command Marco Barisione
2020-09-14 9:39 ` [PATCH 2/2] Add a way to preserve redefined GDB commands for later invocation Marco Barisione
@ 2020-09-28 7:54 ` Marco Barisione
2020-10-05 7:42 ` Marco Barisione
2020-10-12 11:50 ` Pedro Alves
3 siblings, 1 reply; 23+ messages in thread
From: Marco Barisione @ 2020-09-28 7:54 UTC (permalink / raw)
To: gdb-patches
Ping for the patch series at https://sourceware.org/pipermail/gdb-patches/2020-September/171829.html.
Thanks!
> On 14 Sep 2020, at 10:39, Marco Barisione <mbarisione@undo.io> wrote:
>
> Currently, when a GDB command is redefined, the original implementation is not
> available any more. This makes it difficult to build features on top of
> existing commands.
>
> Last year I submitted a patch to fix this but I ran out of time to address the
> review comments (the original patch was sent on the 28th of October 2019).
> These patches restart that work and should address all the comments I got last
> time. As the patchea are very different and a long time passed, I'm
> submitting as a new series.
>
> My patches add a new "uplevel" command and a new gdb.Command.invoke_uplevel
> method inspired by TCL (as initially suggested by Andrew Burgess) so you can
> do this:
>
> (gdb) define run
> echo Will run!\n
> uplevel 0 run
> end
> (gdb) run
> Will run!
> [... normal output of run ...]
>
>
> There are a couple of other things which could be added to make the "uplevel"
> command more helpful, but I think they are out of scope and my patches are
> already useful as they are.
>
> The first thing is adding a way of accessing the untokenised arguments to a
> command via something like "$arg@" (Andrew Burgess suggested "$argv", but
> Pedro Alves pointed out that would look like an argument vector).
>
> Another thing which could be added is the ability to do "uplevel -1 ..." to
> access the directly redefined command.
> This is implemented in Python but I couldn't find an obvious way of doing that
> for the "uplevel" command as there's no way of knowing which command you are
> currently executing (at least from what I could see).
> Maybe it could be implemented in a similar way to how command arguments are
> kept around with scoped_user_args_level, so we could keep a stack of all (user
> and non-user) commands which are being executed. By checking the latest one
> you can know what "uplevel -1" would apply to.
>
--
Marco Barisione
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PING] Add a way to invoke redefined (overridden) GDB commands
2020-09-28 7:54 ` [PING] Add a way to invoke redefined (overridden) GDB commands Marco Barisione
@ 2020-10-05 7:42 ` Marco Barisione
0 siblings, 0 replies; 23+ messages in thread
From: Marco Barisione @ 2020-10-05 7:42 UTC (permalink / raw)
To: gdb-patches
Second ping for https://sourceware.org/pipermail/gdb-patches/2020-September/171829.html <https://sourceware.org/pipermail/gdb-patches/2020-September/171829.html>.
> On 28 Sep 2020, at 08:54, Marco Barisione <mbarisione@undo.io> wrote:
>
> Ping for the patch series at https://sourceware.org/pipermail/gdb-patches/2020-September/171829.html.
>
> Thanks!
>
>
>> On 14 Sep 2020, at 10:39, Marco Barisione <mbarisione@undo.io> wrote:
>>
>> Currently, when a GDB command is redefined, the original implementation is not
>> available any more. This makes it difficult to build features on top of
>> existing commands.
>>
>> Last year I submitted a patch to fix this but I ran out of time to address the
>> review comments (the original patch was sent on the 28th of October 2019).
>> These patches restart that work and should address all the comments I got last
>> time. As the patchea are very different and a long time passed, I'm
>> submitting as a new series.
>>
>> My patches add a new "uplevel" command and a new gdb.Command.invoke_uplevel
>> method inspired by TCL (as initially suggested by Andrew Burgess) so you can
>> do this:
>>
>> (gdb) define run
>> echo Will run!\n
>> uplevel 0 run
>> end
>> (gdb) run
>> Will run!
>> [... normal output of run ...]
>>
>>
>> There are a couple of other things which could be added to make the "uplevel"
>> command more helpful, but I think they are out of scope and my patches are
>> already useful as they are.
>>
>> The first thing is adding a way of accessing the untokenised arguments to a
>> command via something like "$arg@" (Andrew Burgess suggested "$argv", but
>> Pedro Alves pointed out that would look like an argument vector).
>>
>> Another thing which could be added is the ability to do "uplevel -1 ..." to
>> access the directly redefined command.
>> This is implemented in Python but I couldn't find an obvious way of doing that
>> for the "uplevel" command as there's no way of knowing which command you are
>> currently executing (at least from what I could see).
>> Maybe it could be implemented in a similar way to how command arguments are
>> kept around with scoped_user_args_level, so we could keep a stack of all (user
>> and non-user) commands which are being executed. By checking the latest one
>> you can know what "uplevel -1" would apply to.
>>
>
> --
> Marco Barisione
>
--
Marco Barisione
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/2] Move the code to execute a cmd_list_element out from execute_command
2020-09-14 9:39 ` [PATCH 1/2] Move the code to execute a cmd_list_element out from execute_command Marco Barisione
@ 2020-10-05 9:08 ` Andrew Burgess
2020-10-05 9:40 ` Marco Barisione
0 siblings, 1 reply; 23+ messages in thread
From: Andrew Burgess @ 2020-10-05 9:08 UTC (permalink / raw)
To: Marco Barisione; +Cc: gdb-patches
Hi,
Thanks for working on this. I have a little feedback:
* Marco Barisione <mbarisione@undo.io> [2020-09-14 09:39:24 +0000]:
You should imagine this patch, once applied is being looked at in
isolation, without an understanding that there's a follow on patch.
As such it's usually a nice idea to write a short commit message that
just mentions you're performing this restructuring in preparation for
a follow on patch.
> gdb/ChangeLog:
>
> * gdbcmd.h (execute_cmd_list_command): Add declaration.
> * top.c (execute_command): Move out the code to execute a
> command from a cmd_list_element.
> (execute_cmd_list_command): Add from code originally in
> execute_command.
> * top.h (execute_cmd_list_command): Add declaration.
Why is execute_cmd_list_command declared twice? This doesn't feel
right.
Thanks,
Andrew
> ---
> gdb/gdbcmd.h | 3 +++
> gdb/top.c | 66 ++++++++++++++++++++++++++++++----------------------
> gdb/top.h | 2 ++
> 3 files changed, 43 insertions(+), 28 deletions(-)
>
> diff --git a/gdb/gdbcmd.h b/gdb/gdbcmd.h
> index 4406094ea5..5be474ec1a 100644
> --- a/gdb/gdbcmd.h
> +++ b/gdb/gdbcmd.h
> @@ -134,6 +134,9 @@ extern struct cmd_list_element *save_cmdlist;
>
> extern void execute_command (const char *, int);
>
> +extern void execute_cmd_list_command (struct cmd_list_element *,
> + const char *, int);
> +
> /* Execute command P and returns its output. If TERM_OUT,
> the output is built using terminal output behaviour such
> as cli_styling. */
> diff --git a/gdb/top.c b/gdb/top.c
> index acd31afb9a..8eae15a488 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -581,7 +581,6 @@ execute_command (const char *p, int from_tty)
> const char *arg;
> std::string default_args;
> std::string default_args_and_arg;
> - int was_sync = current_ui->prompt_state == PROMPT_BLOCKED;
>
> line = p;
>
> @@ -641,33 +640,7 @@ execute_command (const char *p, int from_tty)
> if (c->deprecated_warn_user)
> deprecated_cmd_warning (line);
>
> - /* c->user_commands would be NULL in the case of a python command. */
> - if (c->theclass == class_user && c->user_commands)
> - execute_user_command (c, arg);
> - else if (c->theclass == class_user
> - && c->prefixlist && !c->allow_unknown)
> - /* If this is a user defined prefix that does not allow unknown
> - (in other words, C is a prefix command and not a command
> - that can be followed by its args), report the list of
> - subcommands. */
> - {
> - printf_unfiltered
> - ("\"%.*s\" must be followed by the name of a subcommand.\n",
> - (int) strlen (c->prefixname) - 1, c->prefixname);
> - help_list (*c->prefixlist, c->prefixname, all_commands, gdb_stdout);
> - }
> - else if (c->type == set_cmd)
> - do_set_command (arg, from_tty, c);
> - else if (c->type == show_cmd)
> - do_show_command (arg, from_tty, c);
> - else if (!cmd_func_p (c))
> - error (_("That is not a command, just a help topic."));
> - else if (deprecated_call_command_hook)
> - deprecated_call_command_hook (c, arg, from_tty);
> - else
> - cmd_func (c, arg, from_tty);
> -
> - maybe_wait_sync_command_done (was_sync);
> + execute_cmd_list_command (c, arg, from_tty);
>
> /* If this command has been post-hooked, run the hook last. */
> execute_cmd_post_hook (c);
> @@ -690,6 +663,43 @@ execute_command (const char *p, int from_tty)
> cleanup_if_error.release ();
> }
>
> +/* Execute the C command. */
> +
> +void
> +execute_cmd_list_command (struct cmd_list_element *c, const char *arg,
> + int from_tty)
> +{
> + bool was_sync = current_ui->prompt_state == PROMPT_BLOCKED;
> +
> + /* c->user_commands would be NULL in the case of a python command. */
> + if (c->theclass == class_user && c->user_commands)
> + execute_user_command (c, arg);
> + else if (c->theclass == class_user
> + && c->prefixlist && !c->allow_unknown)
> + {
> + /* If this is a user defined prefix that does not allow unknown
> + (in other words, C is a prefix command and not a command
> + that can be followed by its args), report the list of
> + subcommands. */
> + printf_unfiltered
> + ("\"%.*s\" must be followed by the name of a subcommand.\n",
> + (int) strlen (c->prefixname) - 1, c->prefixname);
> + help_list (*c->prefixlist, c->prefixname, all_commands, gdb_stdout);
> + }
> + else if (c->type == set_cmd)
> + do_set_command (arg, from_tty, c);
> + else if (c->type == show_cmd)
> + do_show_command (arg, from_tty, c);
> + else if (!cmd_func_p (c))
> + error (_("That is not a command, just a help topic."));
> + else if (deprecated_call_command_hook)
> + deprecated_call_command_hook (c, arg, from_tty);
> + else
> + cmd_func (c, arg, from_tty);
> +
> + maybe_wait_sync_command_done (was_sync);
> +}
> +
> /* Run execute_command for P and FROM_TTY. Sends its output to FILE,
> do not display it to the screen. BATCH_FLAG will be
> temporarily set to true. */
> diff --git a/gdb/top.h b/gdb/top.h
> index fd99297715..ba6c596da7 100644
> --- a/gdb/top.h
> +++ b/gdb/top.h
> @@ -241,6 +241,8 @@ extern void quit_force (int *, int);
> extern void quit_command (const char *, int);
> extern void quit_cover (void);
> extern void execute_command (const char *, int);
> +extern void execute_cmd_list_command (struct cmd_list_element *,
> + const char *, int);
>
> /* If the interpreter is in sync mode (we're running a user command's
> list, running command hooks or similars), and we just ran a
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/2] Move the code to execute a cmd_list_element out from execute_command
2020-10-05 9:08 ` Andrew Burgess
@ 2020-10-05 9:40 ` Marco Barisione
2020-10-05 17:49 ` Andrew Burgess
0 siblings, 1 reply; 23+ messages in thread
From: Marco Barisione @ 2020-10-05 9:40 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
On 5 Oct 2020, at 10:08, Andrew Burgess <andrew.burgess@embecosm.com> wrote:
>> gdb/ChangeLog:
>>
>> * gdbcmd.h (execute_cmd_list_command): Add declaration.
>> * top.c (execute_command): Move out the code to execute a
>> command from a cmd_list_element.
>> (execute_cmd_list_command): Add from code originally in
>> execute_command.
>> * top.h (execute_cmd_list_command): Add declaration.
>
> Why is execute_cmd_list_command declared twice? This doesn't feel
> right.
On top of gdbcmd.h there’s this comment:
/* ***DEPRECATED*** The gdblib files must not be calling/using things in any
of the possible command languages. If necessary, a hook (that may be
present or not) must be used and set to the appropriate routine by any
command language that cares about it. If you are having to include this
file you are possibly doing things the old way. This file will dissapear.
fnasser@redhat.com */
This deprecation notice was added 20 years ago in commit
<d318976c46b92e4d8640f1310bb7b6b517c8bcf7>.
I couldn't really work out the details, so I just copied what is currently
done for execute_command, which is declared in two header files:
$ git grep '\bexecute_command\b' -- '*.h'
gdbcmd.h:extern void execute_command (const char *, int);
top.h:extern void execute_command (const char *, int);
--
Marco Barisione
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] Add a way to preserve redefined GDB commands for later invocation
2020-09-14 9:39 ` [PATCH 2/2] Add a way to preserve redefined GDB commands for later invocation Marco Barisione
2020-09-14 16:18 ` Eli Zaretskii
@ 2020-10-05 10:24 ` Andrew Burgess
2020-10-05 11:44 ` Marco Barisione
1 sibling, 1 reply; 23+ messages in thread
From: Andrew Burgess @ 2020-10-05 10:24 UTC (permalink / raw)
To: Marco Barisione; +Cc: gdb-patches
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;
> }
> \f
> /* 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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] Add a way to preserve redefined GDB commands for later invocation
2020-10-05 10:24 ` Andrew Burgess
@ 2020-10-05 11:44 ` Marco Barisione
2020-10-05 18:11 ` Andrew Burgess
0 siblings, 1 reply; 23+ messages in thread
From: Marco Barisione @ 2020-10-05 11:44 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
On 5 Oct 2020, at 11:24, Andrew Burgess <andrew.burgess@embecosm.com> wrote:
>> +/* 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.
This is briefly explained in the first email
(https://sourceware.org/pipermail/gdb-patches/2020-September/171829.html).
I didn’t want to do too much for this patch series in case the whole approach
was wrong.
One of the things I didn’t do (for now) was this as I couldn't find an obvious
way for the "uplevel" command to know which command you are currently
executing. An option could be to implement something similar way to how
command arguments are kept around with scoped_user_args_level, so we could
keep a stack of all (user and non-user) commands which are being executed.
By checking the latest one you can know what "uplevel -1" would apply to.
Another question I couldn’t answer is what "uplevel -1 …" would do if used
outside a definition.
Should it show an error or should it just invoke the latest defined command?
In the latter case, wouldn’t it confusing that "-1" has a different meaning
inside or outside commands?
Note that there’s another bit of useful work that I didn’t do, that is the
addition of a way of accessing all the untokenized arguments from a command
definition.
> 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.
How about the case where a command on the same level is invoked?
I considered this (there should be a comment somewhere) but thought it’s
not a real problem as there may be some legitimate cases (for instance
if the arguments are changed) and, if things go wrong, you just get a
max user call depth exceeded.
The same weird but legitimate case may in theory exist also for
invoking a command at a higher level, i.e. the arguments are changed.
> 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.
Is this really what a user would want? Or would they want the ability
to delete commands?
> 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.
I don’t think it would be too difficult but it means that the user must
decide what they want early and they can’t change their minds.
What happens if you accidentally replace a command but you wanted to
override it instead?
> 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.
My goal with my patches was to get rid of hooks in my own code.
> 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.
Good point.
> 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.
Hm, difficult to say how difficult it could be but I will have a look.
There may also be other subtle different behaviours we don’t know about
and which could break existing scripts. Is this acceptable?
--
Marco Barisione
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/2] Move the code to execute a cmd_list_element out from execute_command
2020-10-05 9:40 ` Marco Barisione
@ 2020-10-05 17:49 ` Andrew Burgess
0 siblings, 0 replies; 23+ messages in thread
From: Andrew Burgess @ 2020-10-05 17:49 UTC (permalink / raw)
To: Marco Barisione; +Cc: gdb-patches
* Marco Barisione <mbarisione@undo.io> [2020-10-05 10:40:19 +0100]:
> On 5 Oct 2020, at 10:08, Andrew Burgess <andrew.burgess@embecosm.com> wrote:
> >> gdb/ChangeLog:
> >>
> >> * gdbcmd.h (execute_cmd_list_command): Add declaration.
> >> * top.c (execute_command): Move out the code to execute a
> >> command from a cmd_list_element.
> >> (execute_cmd_list_command): Add from code originally in
> >> execute_command.
> >> * top.h (execute_cmd_list_command): Add declaration.
> >
> > Why is execute_cmd_list_command declared twice? This doesn't feel
> > right.
>
> On top of gdbcmd.h there’s this comment:
> /* ***DEPRECATED*** The gdblib files must not be calling/using things in any
> of the possible command languages. If necessary, a hook (that may be
> present or not) must be used and set to the appropriate routine by any
> command language that cares about it. If you are having to include this
> file you are possibly doing things the old way. This file will dissapear.
> fnasser@redhat.com */
I read this a couple of times and it still doesn't really make much
sense to me.
Maybe someone else on the list will jump in to explain what's going on
here, but assuming they don't I think we should ignore that comment
and make sure the new code we add is as sane as possible.
To that end I think we should have a single declaration, probably in
gdbcmd.h (as the new function seems related to handling gdb commands)
then fix the build fallout, I only had to add new includes to 'top.c'
and 'cli/cli-script.c' so it isn't huge.
One reason for this, which I completely forgot to mention in my first
reply, is that functions should be documented in the header file, and
in the source file just say '/* See gdbcmd.h. */'. Obviously we
don't want to have functions documented twice at the two declaration
points.
Thanks,
Andrew
>
> This deprecation notice was added 20 years ago in commit
> <d318976c46b92e4d8640f1310bb7b6b517c8bcf7>.
> I couldn't really work out the details, so I just copied what is currently
> done for execute_command, which is declared in two header files:
> $ git grep '\bexecute_command\b' -- '*.h'
> gdbcmd.h:extern void execute_command (const char *, int);
> top.h:extern void execute_command (const char *, int);
>
>
> --
> Marco Barisione
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] Add a way to preserve redefined GDB commands for later invocation
2020-10-05 11:44 ` Marco Barisione
@ 2020-10-05 18:11 ` Andrew Burgess
2020-10-06 7:18 ` Marco Barisione
0 siblings, 1 reply; 23+ messages in thread
From: Andrew Burgess @ 2020-10-05 18:11 UTC (permalink / raw)
To: Marco Barisione; +Cc: gdb-patches
* Marco Barisione <mbarisione@undo.io> [2020-10-05 12:44:03 +0100]:
> On 5 Oct 2020, at 11:24, Andrew Burgess <andrew.burgess@embecosm.com> wrote:
> >> +/* 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.
>
> This is briefly explained in the first email
> (https://sourceware.org/pipermail/gdb-patches/2020-September/171829.html).
>
> I didn’t want to do too much for this patch series in case the whole approach
> was wrong.
> One of the things I didn’t do (for now) was this as I couldn't find an obvious
> way for the "uplevel" command to know which command you are currently
> executing. An option could be to implement something similar way to how
> command arguments are kept around with scoped_user_args_level, so we could
> keep a stack of all (user and non-user) commands which are being executed.
> By checking the latest one you can know what "uplevel -1" would
> apply to.
I think this would be a fine approach. I'd pack the command pointer
and arguments into a single object and store these in a newly renamed
scoped_user_args_level I think.
>
> Another question I couldn’t answer is what "uplevel -1 …" would do if used
> outside a definition.
> Should it show an error or should it just invoke the latest defined command?
> In the latter case, wouldn’t it confusing that "-1" has a different meaning
> inside or outside commands?
Yeah I don't think the last case would be a good way to go, but what
about executing the last but one version of the command, this would
seem exactly inline with the behaviour inside a command, so:
(gdb) define cmd_1
echo v1\n
end
(gdb) define cmd_1
echo v2\n
uplevel -1 cmd_1
end
(gdb) cmd_1
v2
v1
(gdb) uplevel 0 cmd_1
v1
(gdb) uplevel -1 cmd
v1
>
> Note that there’s another bit of useful work that I didn’t do, that is the
> addition of a way of accessing all the untokenized arguments from a command
> definition.
>
> > 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.
>
> How about the case where a command on the same level is invoked?
> I considered this (there should be a comment somewhere) but thought it’s
> not a real problem as there may be some legitimate cases (for instance
> if the arguments are changed) and, if things go wrong, you just get a
> max user call depth exceeded.
But at the same level can't you just call the command by name with no
'uplevel' at all?
>
> The same weird but legitimate case may in theory exist also for
> invoking a command at a higher level, i.e. the arguments are
> changed.
Yeah... no.
I'm sure it's possible to craft an example where this might work, but
I think we should start off by blocking this, then if someone comes up
with a really good example of why this is needed, and is the _only_
way to solve there problem then we can rip the check out.
In general I think you could only call to a higher level if you had
full control over how the commands are written and the order they are
registered, in which case (I boldly claim) there would be a better way
to rewrite the code to avoid calling to a higher level.
IF you don't have full control over what was registered later, or the
order in which parts are registered then this would never work.
In short I think blocking this will help far more than it hurts.
>
> > 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.
>
> Is this really what a user would want? Or would they want the ability
> to delete commands?
Deleting too might be useful, but I think the case I gave is not
unique to me, I tried writing a command, I got it slightly wrong.
Sure I can delete and start again, but being able to just do:
(gdb) define --redefine cmd_1
...
would be nice.
>
> > 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.
>
> I don’t think it would be too difficult but it means that the user must
> decide what they want early and they can’t change their minds.
> What happens if you accidentally replace a command but you wanted to
> override it instead?
Well, GDB offers a programmable environment, we don't allow undoing
most (or hardly any) actions, so I think users are OK with the idea
that some action might not be reversible.
>
> > 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.
>
> My goal with my patches was to get rid of hooks in my own code.
>
> > 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.
>
> Good point.
>
> > 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.
>
> Hm, difficult to say how difficult it could be but I will have a look.
>
> There may also be other subtle different behaviours we don’t know about
> and which could break existing scripts. Is this acceptable?
Nothing is completely risk free, and I don't know how well tested the
hooks support is, but it would be nice if we could fully replace hooks
with your code (which is a much better approach).
Thanks,
Andrew
>
> --
> Marco Barisione
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] Add a way to preserve redefined GDB commands for later invocation
2020-10-05 18:11 ` Andrew Burgess
@ 2020-10-06 7:18 ` Marco Barisione
0 siblings, 0 replies; 23+ messages in thread
From: Marco Barisione @ 2020-10-06 7:18 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
On 5 Oct 2020, at 19:11, Andrew Burgess <andrew.burgess@embecosm.com> wrote:
>>> 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.
>>
>> Is this really what a user would want? Or would they want the ability
>> to delete commands?
>
> Deleting too might be useful, but I think the case I gave is not
> unique to me, I tried writing a command, I got it slightly wrong.
> Sure I can delete and start again, but being able to just do:
>
> (gdb) define --redefine cmd_1
> ...
>
> would be nice.
I think I’m onboard with all your suggestions, but I’m not 100% sure about
the details. What happens if you "set confirm off"? That is, what should
be the default answer?
Do we need "define --override" and "define --replace"? What happens if the
command is not already previously defined?
--
Marco Barisione
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Add a way to invoke redefined (overridden) GDB commands
2020-09-14 9:39 Add a way to invoke redefined (overridden) GDB commands Marco Barisione
` (2 preceding siblings ...)
2020-09-28 7:54 ` [PING] Add a way to invoke redefined (overridden) GDB commands Marco Barisione
@ 2020-10-12 11:50 ` Pedro Alves
2020-10-19 17:41 ` Marco Barisione
3 siblings, 1 reply; 23+ messages in thread
From: Pedro Alves @ 2020-10-12 11:50 UTC (permalink / raw)
To: Marco Barisione, gdb-patches
On 9/14/20 10:39 AM, Marco Barisione wrote:
> Currently, when a GDB command is redefined, the original implementation is not
> available any more. This makes it difficult to build features on top of
> existing commands.
>
> Last year I submitted a patch to fix this but I ran out of time to address the
> review comments (the original patch was sent on the 28th of October 2019).
> These patches restart that work and should address all the comments I got last
> time. As the patchea are very different and a long time passed, I'm
> submitting as a new series.
>
> My patches add a new "uplevel" command and a new gdb.Command.invoke_uplevel
> method inspired by TCL (as initially suggested by Andrew Burgess) so you can
> do this:
>
> (gdb) define run
> echo Will run!\n
> uplevel 0 run
> end
> (gdb) run
> Will run!
> [... normal output of run ...]
>
>
> There are a couple of other things which could be added to make the "uplevel"
> command more helpful, but I think they are out of scope and my patches are
> already useful as they are.
So I'm looking at this afresh, and really questioning this "uplevel N"
design. This it not really like TCL's "uplevel". With TCL's uplevel,
you are accessing a different scope or frame, not a previous implementation
of the function that was overwritten. To me, the naming choice is
confusing, from that angle. If someone extends GDB's CLI to gain support
for local variables, then a really-TCL-like uplevel is likely handy, and
then calling that feature "uplevel" would be good.
I also question whether "uplevel N" with "N>0" is really usable, since
in general you don't know what other scripts may have overridden. E.g.,
you never know what "uplevel 3 cmd" will run, since you don't know how
many scripts redefined/overridden cmd.
If we stick with the TCL inspiration, I think a better approach would
be to add support for renaming commands, like TCL's rename command:
https://www.tcl.tk/man/tcl8.4/TclCmd/rename.htm
So a user would do:
(gdb) rename run org_run
(gdb) define run
> echo Will run!\n
> org_run
> end
(gdb) run
Will run!
[... normal output of run ...]
(gdb) org_run
[... normal output of run ...]
(You can find many examples of TCL's rename in use in GDB's testsuite.)
Thanks,
Pedro Alves
>
> The first thing is adding a way of accessing the untokenised arguments to a
> command via something like "$arg@" (Andrew Burgess suggested "$argv", but
> Pedro Alves pointed out that would look like an argument vector).
>
> Another thing which could be added is the ability to do "uplevel -1 ..." to
> access the directly redefined command.
> This is implemented in Python but I couldn't find an obvious way of doing that
> for the "uplevel" command as there's no way of knowing which command you are
> currently executing (at least from what I could see).
> Maybe it could be implemented in a similar way to how command arguments are
> kept around with scoped_user_args_level, so we could keep a stack of all (user
> and non-user) commands which are being executed. By checking the latest one
> you can know what "uplevel -1" would apply to.
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Add a way to invoke redefined (overridden) GDB commands
2020-10-12 11:50 ` Pedro Alves
@ 2020-10-19 17:41 ` Marco Barisione
2020-10-19 18:05 ` Pedro Alves
0 siblings, 1 reply; 23+ messages in thread
From: Marco Barisione @ 2020-10-19 17:41 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On 12 Oct 2020, at 12:50, Pedro Alves <pedro@palves.net> wrote:
> On 9/14/20 10:39 AM, Marco Barisione wrote:
>> Currently, when a GDB command is redefined, the original implementation is not
>> available any more. This makes it difficult to build features on top of
>> existing commands.
>>
>> Last year I submitted a patch to fix this but I ran out of time to address the
>> review comments (the original patch was sent on the 28th of October 2019).
>> These patches restart that work and should address all the comments I got last
>> time. As the patchea are very different and a long time passed, I'm
>> submitting as a new series.
>>
>> My patches add a new "uplevel" command and a new gdb.Command.invoke_uplevel
>> method inspired by TCL (as initially suggested by Andrew Burgess) so you can
>> do this:
>>
>> (gdb) define run
>> echo Will run!\n
>> uplevel 0 run
>> end
>> (gdb) run
>> Will run!
>> [... normal output of run ...]
>>
>>
>> There are a couple of other things which could be added to make the "uplevel"
>> command more helpful, but I think they are out of scope and my patches are
>> already useful as they are.
>
> So I'm looking at this afresh, and really questioning this "uplevel N"
> design. This it not really like TCL's "uplevel". With TCL's uplevel,
> you are accessing a different scope or frame, not a previous implementation
> of the function that was overwritten. To me, the naming choice is
> confusing, from that angle. If someone extends GDB's CLI to gain support
> for local variables, then a really-TCL-like uplevel is likely handy, and
> then calling that feature "uplevel" would be good.
>
> I also question whether "uplevel N" with "N>0" is really usable, since
> in general you don't know what other scripts may have overridden. E.g.,
> you never know what "uplevel 3 cmd" will run, since you don't know how
> many scripts redefined/overridden cmd.
>
> If we stick with the TCL inspiration, I think a better approach would
> be to add support for renaming commands, like TCL's rename command:
>
> https://www.tcl.tk/man/tcl8.4/TclCmd/rename.htm
>
> So a user would do:
>
> (gdb) rename run org_run
> (gdb) define run
>> echo Will run!\n
>> org_run
>> end
> (gdb) run
> Will run!
> [... normal output of run ...]
> (gdb) org_run
> [... normal output of run ...]
>
> (You can find many examples of TCL's rename in use in GDB's testsuite.)
I will try implementing this then, including the semantics for deleting
a command (copied from the TCL command).
A problem with GDB is that commands can have spaces so:
(gdb) rename foo bar
Is ambiguous. Do you want to delete the "foo bar" command or rename the
"foo" command to "bar"?
Even without deleting command, this would be ambiguous:
(gdb) rename foo bar baz
Unless somebody proposes something different I will use "--" to split
the two command names.
For instance:
(gdb) # Rename "foo" to “bar":
(gdb) rename foo -- bar
(gdb) # Rename "foo bar" to "baz":
(gdb) rename foo bar -- baz
(gdb) # Delete "foo bar":
(gdb) rename foo bar --
(gdb) # Invalid:
(gdb) rename --
(gdb) rename -- hello
--
Marco Barisione
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Add a way to invoke redefined (overridden) GDB commands
2020-10-19 17:41 ` Marco Barisione
@ 2020-10-19 18:05 ` Pedro Alves
2020-10-19 18:47 ` Philippe Waroquiers
0 siblings, 1 reply; 23+ messages in thread
From: Pedro Alves @ 2020-10-19 18:05 UTC (permalink / raw)
To: Marco Barisione; +Cc: gdb-patches
On 10/19/20 6:41 PM, Marco Barisione wrote:
> On 12 Oct 2020, at 12:50, Pedro Alves <pedro@palves.net> wrote:
>> On 9/14/20 10:39 AM, Marco Barisione wrote:
>>> Currently, when a GDB command is redefined, the original implementation is not
>>> available any more. This makes it difficult to build features on top of
>>> existing commands.
>>>
>>> Last year I submitted a patch to fix this but I ran out of time to address the
>>> review comments (the original patch was sent on the 28th of October 2019).
>>> These patches restart that work and should address all the comments I got last
>>> time. As the patchea are very different and a long time passed, I'm
>>> submitting as a new series.
>>>
>>> My patches add a new "uplevel" command and a new gdb.Command.invoke_uplevel
>>> method inspired by TCL (as initially suggested by Andrew Burgess) so you can
>>> do this:
>>>
>>> (gdb) define run
>>> echo Will run!\n
>>> uplevel 0 run
>>> end
>>> (gdb) run
>>> Will run!
>>> [... normal output of run ...]
>>>
>>>
>>> There are a couple of other things which could be added to make the "uplevel"
>>> command more helpful, but I think they are out of scope and my patches are
>>> already useful as they are.
>>
>> So I'm looking at this afresh, and really questioning this "uplevel N"
>> design. This it not really like TCL's "uplevel". With TCL's uplevel,
>> you are accessing a different scope or frame, not a previous implementation
>> of the function that was overwritten. To me, the naming choice is
>> confusing, from that angle. If someone extends GDB's CLI to gain support
>> for local variables, then a really-TCL-like uplevel is likely handy, and
>> then calling that feature "uplevel" would be good.
>>
>> I also question whether "uplevel N" with "N>0" is really usable, since
>> in general you don't know what other scripts may have overridden. E.g.,
>> you never know what "uplevel 3 cmd" will run, since you don't know how
>> many scripts redefined/overridden cmd.
>>
>> If we stick with the TCL inspiration, I think a better approach would
>> be to add support for renaming commands, like TCL's rename command:
>>
>> https://www.tcl.tk/man/tcl8.4/TclCmd/rename.htm
>>
>> So a user would do:
>>
>> (gdb) rename run org_run
>> (gdb) define run
>>> echo Will run!\n
>>> org_run
>>> end
>> (gdb) run
>> Will run!
>> [... normal output of run ...]
>> (gdb) org_run
>> [... normal output of run ...]
>>
>> (You can find many examples of TCL's rename in use in GDB's testsuite.)
>
> I will try implementing this then, including the semantics for deleting
> a command (copied from the TCL command).
>
> A problem with GDB is that commands can have spaces so:
> (gdb) rename foo bar
> Is ambiguous. Do you want to delete the "foo bar" command or rename the
> "foo" command to "bar"?
> Even without deleting command, this would be ambiguous:
> (gdb) rename foo bar baz
>
> Unless somebody proposes something different I will use "--" to split
> the two command names.
ISTM that (optional) quotes would be the natural thing here:
(gdb) rename "foo bar" "baz"
(gdb) rename "foo" "bar baz"
I'd rather leave "--" for the typical splitting of options from
other arguments. Commands get that for free if they use
the cli/cli-option.h framework. So "--" would be used like in other
commands, to unambiguously split options from commands that may
start with "-", like:
(gdb) help -
Scroll window backward.
Usage: - [N] [WIN]
Scroll window WIN N lines backwards. Both WIN and N are optional, N
defaults to 1, and WIN defaults to the currently focused window.
(gdb)
(gdb) rename -someoption -- - scroll-backward
Thanks,
Pedro Alves
>
> For instance:
> (gdb) # Rename "foo" to “bar":
> (gdb) rename foo -- bar
> (gdb) # Rename "foo bar" to "baz":
> (gdb) rename foo bar -- baz
> (gdb) # Delete "foo bar":
> (gdb) rename foo bar --
> (gdb) # Invalid:
> (gdb) rename --
> (gdb) rename -- hello
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Add a way to invoke redefined (overridden) GDB commands
2020-10-19 18:05 ` Pedro Alves
@ 2020-10-19 18:47 ` Philippe Waroquiers
2020-10-19 19:28 ` Marco Barisione
2020-10-20 15:15 ` Pedro Alves
0 siblings, 2 replies; 23+ messages in thread
From: Philippe Waroquiers @ 2020-10-19 18:47 UTC (permalink / raw)
To: Pedro Alves, Marco Barisione; +Cc: gdb-patches
On Mon, 2020-10-19 at 19:05 +0100, Pedro Alves wrote:
> ISTM that (optional) quotes would be the natural thing here:
>
> (gdb) rename "foo bar" "baz"
> (gdb) rename "foo" "bar baz"
>
> I'd rather leave "--" for the typical splitting of options from
> other arguments. Commands get that for free if they use
> the cli/cli-option.h framework. So "--" would be used like in other
> commands, to unambiguously split options from commands that may
> start with "-", like:
Effectively better to keep -- to separate options from args.
For what concerns the rename, alias uses = to separate
the 2 commands:
(gdb) h alias
Define a new command that is an alias of an existing command.
Usage: alias [-a] [--] ALIAS = COMMAND [DEFAULT-ARGS...]
...
So, maybe better/more consistent to use = similarly for rename.
I am wondering how rename will interact with alias:
alias are resolved at definition time, so a rename
following an alias might not have the expected effect.
Thanks
Philippe
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Add a way to invoke redefined (overridden) GDB commands
2020-10-19 18:47 ` Philippe Waroquiers
@ 2020-10-19 19:28 ` Marco Barisione
2020-10-20 15:06 ` Pedro Alves
2020-10-20 15:15 ` Pedro Alves
1 sibling, 1 reply; 23+ messages in thread
From: Marco Barisione @ 2020-10-19 19:28 UTC (permalink / raw)
To: Philippe Waroquiers; +Cc: Pedro Alves, gdb-patches
On 19 Oct 2020, at 19:47, Philippe Waroquiers <philippe.waroquiers@skynet.be> wrote:
> On Mon, 2020-10-19 at 19:05 +0100, Pedro Alves wrote:
>> ISTM that (optional) quotes would be the natural thing here:
>>
>> (gdb) rename "foo bar" "baz"
>> (gdb) rename "foo" "bar baz"
>>
>> I'd rather leave "--" for the typical splitting of options from
>> other arguments. Commands get that for free if they use
>> the cli/cli-option.h framework. So "--" would be used like in other
>> commands, to unambiguously split options from commands that may
>> start with "-", like:
> Effectively better to keep -- to separate options from args.
>
> For what concerns the rename, alias uses = to separate
> the 2 commands:
> (gdb) h alias
> Define a new command that is an alias of an existing command.
> Usage: alias [-a] [--] ALIAS = COMMAND [DEFAULT-ARGS...]
> ...
> So, maybe better/more consistent to use = similarly for rename.
>
> I am wondering how rename will interact with alias:
> alias are resolved at definition time, so a rename
> following an alias might not have the expected effect.
That’s a very good point. I didn’t consider aliases and hooks.
My use case is for building on top of existing commands so they can
be extended/tweaked. Aliases and hooks need to follow the new
command, including the ones set by users (so I have no way of knowing
about them).
I can’t think of any way of making a rename command work like this.
Any idea? Otherwise, would my original patch be acceptable if I
changed the name from up level and only use relative levels?
--
Marco Barisione
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Add a way to invoke redefined (overridden) GDB commands
2020-10-19 19:28 ` Marco Barisione
@ 2020-10-20 15:06 ` Pedro Alves
2020-10-20 18:19 ` Marco Barisione
0 siblings, 1 reply; 23+ messages in thread
From: Pedro Alves @ 2020-10-20 15:06 UTC (permalink / raw)
To: Marco Barisione, Philippe Waroquiers; +Cc: gdb-patches
On 10/19/20 8:28 PM, Marco Barisione wrote:
> On 19 Oct 2020, at 19:47, Philippe Waroquiers <philippe.waroquiers@skynet.be> wrote:
>> I am wondering how rename will interact with alias:
>> alias are resolved at definition time, so a rename
>> following an alias might not have the expected effect.
>
> That’s a very good point. I didn’t consider aliases and hooks.
>
> My use case is for building on top of existing commands so they can
> be extended/tweaked. Aliases and hooks need to follow the new
> command, including the ones set by users (so I have no way of knowing
> about them).
> I can’t think of any way of making a rename command work like this.
>
> Any idea?
In my previous example, where it read:
(gdb) rename run org_run
(gdb) define run
...
merge those two commands into a single atomic operation?
Like:
(gdb) rename-define run org_run
> echo Will run!\n
> org_run
> end
(gdb) run
Will run!
[... normal output of run ...]
(gdb) org_run
[... normal output of run ...]
Unlike "rename", "rename-define" (strawman name) would take care
of moving the alias and pre/post hooks to the new implementation.
WDYT?
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Add a way to invoke redefined (overridden) GDB commands
2020-10-19 18:47 ` Philippe Waroquiers
2020-10-19 19:28 ` Marco Barisione
@ 2020-10-20 15:15 ` Pedro Alves
1 sibling, 0 replies; 23+ messages in thread
From: Pedro Alves @ 2020-10-20 15:15 UTC (permalink / raw)
To: Philippe Waroquiers, Marco Barisione; +Cc: gdb-patches
On 10/19/20 7:47 PM, Philippe Waroquiers wrote:
> On Mon, 2020-10-19 at 19:05 +0100, Pedro Alves wrote:
> For what concerns the rename, alias uses = to separate
> the 2 commands:
> (gdb) h alias
> Define a new command that is an alias of an existing command.
> Usage: alias [-a] [--] ALIAS = COMMAND [DEFAULT-ARGS...]
> ...
> So, maybe better/more consistent to use = similarly for rename.
In the "alias" case, the "=" makes sense since we're
setting up an equivalence.
With renaming, isn't "=" a little weird?
(gdb) rename oldname = newname
I guess I'd get used to it.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Add a way to invoke redefined (overridden) GDB commands
2020-10-20 15:06 ` Pedro Alves
@ 2020-10-20 18:19 ` Marco Barisione
2020-10-20 18:32 ` Pedro Alves
0 siblings, 1 reply; 23+ messages in thread
From: Marco Barisione @ 2020-10-20 18:19 UTC (permalink / raw)
To: Pedro Alves; +Cc: Philippe Waroquiers, gdb-patches
On 20 Oct 2020, at 16:06, Pedro Alves <pedro@palves.net> wrote:
> On 10/19/20 8:28 PM, Marco Barisione wrote:
>> On 19 Oct 2020, at 19:47, Philippe Waroquiers <philippe.waroquiers@skynet.be> wrote:
>
>>> I am wondering how rename will interact with alias:
>>> alias are resolved at definition time, so a rename
>>> following an alias might not have the expected effect.
>>
>> That’s a very good point. I didn’t consider aliases and hooks.
>>
>> My use case is for building on top of existing commands so they can
>> be extended/tweaked. Aliases and hooks need to follow the new
>> command, including the ones set by users (so I have no way of knowing
>> about them).
>> I can’t think of any way of making a rename command work like this.
>>
>> Any idea?
>
> In my previous example, where it read:
>
> (gdb) rename run org_run
> (gdb) define run
> ...
>
> merge those two commands into a single atomic operation?
>
> Like:
>
> (gdb) rename-define run org_run
>> echo Will run!\n
>> org_run
>> end
> (gdb) run
> Will run!
> [... normal output of run ...]
> (gdb) org_run
> [... normal output of run ...]
>
> Unlike "rename", "rename-define" (strawman name) would take care
> of moving the alias and pre/post hooks to the new implementation.
I can’t think of any good name for a command so maybe it should just
be an option to define?
(gdb) define -rename-existing orig_run run
echo Will run\n
orig_run
end
Or "define -r" for short.
GDB would not ask if you want to redefine the existing command as I
think that the user’s intention would be quite clear.
Similarly, in Python we could have a "rename_existing" argument to
gdb.Command.__init__ ().
"rename" still seems useful, but hooks/aliases would follow the
rename:
(gdb) define foo
echo Hello world\n
end
(gdb) alias my-alias = foo
(gdb) rename foo bar
(gdb) bar
Hello world
(gdb) foo
Undefined command: “foo". Try "help”.
(gdb) my-alias
Hello world
I think that rename with an "=" would be a bit confusing and would
not provide a way of deleting commands (unlike rename in TCL).
What do you think?
--
Marco Barisione
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Add a way to invoke redefined (overridden) GDB commands
2020-10-20 18:19 ` Marco Barisione
@ 2020-10-20 18:32 ` Pedro Alves
0 siblings, 0 replies; 23+ messages in thread
From: Pedro Alves @ 2020-10-20 18:32 UTC (permalink / raw)
To: Marco Barisione; +Cc: Philippe Waroquiers, gdb-patches
On 10/20/20 7:19 PM, Marco Barisione wrote:
> On 20 Oct 2020, at 16:06, Pedro Alves <pedro@palves.net> wrote:
>> On 10/19/20 8:28 PM, Marco Barisione wrote:
>>> On 19 Oct 2020, at 19:47, Philippe Waroquiers <philippe.waroquiers@skynet.be> wrote:
>>
>>>> I am wondering how rename will interact with alias:
>>>> alias are resolved at definition time, so a rename
>>>> following an alias might not have the expected effect.
>>>
>>> That’s a very good point. I didn’t consider aliases and hooks.
>>>
>>> My use case is for building on top of existing commands so they can
>>> be extended/tweaked. Aliases and hooks need to follow the new
>>> command, including the ones set by users (so I have no way of knowing
>>> about them).
>>> I can’t think of any way of making a rename command work like this.
>>>
>>> Any idea?
>>
>> In my previous example, where it read:
>>
>> (gdb) rename run org_run
>> (gdb) define run
>> ...
>>
>> merge those two commands into a single atomic operation?
>>
>> Like:
>>
>> (gdb) rename-define run org_run
>>> echo Will run!\n
>>> org_run
>>> end
>> (gdb) run
>> Will run!
>> [... normal output of run ...]
>> (gdb) org_run
>> [... normal output of run ...]
>>
>> Unlike "rename", "rename-define" (strawman name) would take care
>> of moving the alias and pre/post hooks to the new implementation.
>
>
> I can’t think of any good name for a command so maybe it should just
> be an option to define?
>
> (gdb) define -rename-existing orig_run run
> echo Will run\n
> orig_run
> end
>
> Or "define -r" for short.
>
> GDB would not ask if you want to redefine the existing command as I
> think that the user’s intention would be quite clear.
>
> Similarly, in Python we could have a "rename_existing" argument to
> gdb.Command.__init__ ().
>
>
> "rename" still seems useful, but hooks/aliases would follow the
> rename:
>
> (gdb) define foo
> echo Hello world\n
> end
> (gdb) alias my-alias = foo
> (gdb) rename foo bar
> (gdb) bar
> Hello world
> (gdb) foo
> Undefined command: “foo". Try "help”.
> (gdb) my-alias
> Hello world
>
> I think that rename with an "=" would be a bit confusing and would
> not provide a way of deleting commands (unlike rename in TCL).
>
>
> What do you think?
+1
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2020-10-20 18:32 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-14 9:39 Add a way to invoke redefined (overridden) GDB commands Marco Barisione
2020-09-14 9:39 ` [PATCH 1/2] Move the code to execute a cmd_list_element out from execute_command Marco Barisione
2020-10-05 9:08 ` Andrew Burgess
2020-10-05 9:40 ` Marco Barisione
2020-10-05 17:49 ` Andrew Burgess
2020-09-14 9:39 ` [PATCH 2/2] Add a way to preserve redefined GDB commands for later invocation Marco Barisione
2020-09-14 16:18 ` Eli Zaretskii
2020-09-14 16:51 ` Marco Barisione
2020-10-05 10:24 ` Andrew Burgess
2020-10-05 11:44 ` Marco Barisione
2020-10-05 18:11 ` Andrew Burgess
2020-10-06 7:18 ` Marco Barisione
2020-09-28 7:54 ` [PING] Add a way to invoke redefined (overridden) GDB commands Marco Barisione
2020-10-05 7:42 ` Marco Barisione
2020-10-12 11:50 ` Pedro Alves
2020-10-19 17:41 ` Marco Barisione
2020-10-19 18:05 ` Pedro Alves
2020-10-19 18:47 ` Philippe Waroquiers
2020-10-19 19:28 ` Marco Barisione
2020-10-20 15:06 ` Pedro Alves
2020-10-20 18:19 ` Marco Barisione
2020-10-20 18:32 ` Pedro Alves
2020-10-20 15:15 ` Pedro Alves
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).