Now that filtered and unfiltered output can be treated identically, we
can unify the printf family of functions. This is done under the name
"gdb_printf". Most of this patch was written by script.
Now that filtered and unfiltered output can be treated identically, we
can unify the puts family of functions. This is done under the name
"gdb_puts". Most of this patch was written by script.
I think it only really makes sense to call wrap_here with an argument
consisting solely of spaces. Given this, it seemed better to me that
the argument be an int, rather than a string. This patch is the
result. Much of it was written by a script.
This commit attempts to improve the help text that is generated for
gdb.Parameter objects when the user fails to provide their own
documentation.
Documentation for a gdb.Parameter is currently pulled from two
sources: the class documentation string, and the set_doc/show_doc
class attributes. Thus, a fully documented parameter might look like
this:
class Param_All (gdb.Parameter):
"""This is the class documentation string."""
show_doc = "Show the state of this parameter"
set_doc = "Set the state of this parameter"
def get_set_string (self):
val = "on"
if (self.value == False):
val = "off"
return "Test Parameter has been set to " + val
def __init__ (self, name):
super (Param_All, self).__init__ (name, gdb.COMMAND_DATA, gdb.PARAM_BOOLEAN)
self._value = True
Param_All ('param-all')
Then in GDB we see this:
(gdb) help set param-all
Set the state of this parameter
This is the class documentation string.
Which is fine. But, if the user skips both of the documentation parts
like this:
class Param_None (gdb.Parameter):
def get_set_string (self):
val = "on"
if (self.value == False):
val = "off"
return "Test Parameter has been set to " + val
def __init__ (self, name):
super (Param_None, self).__init__ (name, gdb.COMMAND_DATA, gdb.PARAM_BOOLEAN)
self._value = True
Param_None ('param-none')
Now in GDB we see this:
(gdb) help set param-none
This command is not documented.
This command is not documented.
That's not great, the duplicated text looks a bit weird. If we drop
different parts we get different results. Here's what we get if the
user drops the set_doc and show_doc attributes:
(gdb) help set param-doc
This command is not documented.
This is the class documentation string.
That kind of sucks, we say it's undocumented, then proceed to print
the documentation. Finally, if we drop the class documentation but
keep the set_doc and show_doc:
(gdb) help set param-set-show
Set the state of this parameter
This command is not documented.
That seems OK.
So, I think there's room for improvement.
With this patch, for the four cases above we now see this:
# All values provided by the user, no change in this case:
(gdb) help set param-all
Set the state of this parameter
This is the class documentation string.
# Nothing provided by the user, the first string is now different:
(gdb) help set param-none
Set the current value of 'param-none'.
This command is not documented.
# Only the class documentation is provided, the first string is
# changed as in the previous case:
(gdb) help set param-doc
Set the current value of 'param-doc'.
This is the class documentation string.
# Only the set_doc and show_doc are provided, this case is unchanged
# from before the patch:
(gdb) help set param-set-show
Set the state of this parameter
This command is not documented.
The one place where this change might be considered a negative is when
dealing with prefix commands. If we create a prefix command but don't
supply the set_doc / show_doc strings, then this is what we saw before
my patch:
(gdb) python Param_None ('print param-none')
(gdb) help set print
set print, set pr, set p
Generic command for setting how things print.
List of set print subcommands:
... snip ...
set print param-none -- This command is not documented.
... snip ...
And after my patch:
(gdb) python Param_None ('print param-none')
(gdb) help set print
set print, set pr, set p
Generic command for setting how things print.
List of set print subcommands:
... snip ...
set print param-none -- Set the current value of 'print param-none'.
... snip ...
This seems slightly less helpful than before, but I don't think its
terrible.
Additionally, I've changed what we print when the get_show_string
method is not provided in Python.
Back when gdb.Parameter was first added to GDB, we didn't provide a
show function when registering the internal command object within
GDB. As a result, GDB would make use of its "magic" mangling of the
show_doc string to create a sentence that would display the current
value (see deprecated_show_value_hack in cli/cli-setshow.c).
However, when we added support for the get_show_string method to
gdb.Parameter, there was an attempt to maintain backward compatibility
by displaying the show_doc string with the current value appended, see
get_show_value in py-param.c. Unfortunately, this isn't anywhere
close to what deprecated_show_value_hack does, and the results are
pretty poor, for example, this is GDB before my patch:
(gdb) show param-none
This command is not documented. off
I think we can all agree that this is pretty bad.
After my patch, we how show this:
(gdb) show param-none
The current value of 'param-none' is "off".
Which at least is a real sentence, even if it's not very informative.
This patch does change the way that the Python API behaves slightly,
but only in the cases when the user has missed providing GDB with some
information. In most cases I think the new behaviour is a lot better,
there's the one case (noted above) which is a bit iffy, but I think is
still OK.
I've updated the existing gdb.python/py-parameter.exp test to cover
the modified behaviour.
Finally, I've updated the documentation to (I hope) make it clearer
how the various bits of help text come together.
While looking into the language-capturing issue, I found another way
to crash gdb using parameters from Python:
(gdb) python print(gdb.parameter('endian'))
(This is related to PR python/12188, though this patch isn't going to
fix what that bug is really about.)
The problem here is that the global variable that underlies the
"endian" parameter is initialized to NULL. However, that's not a
valid value for an "enum" set/show parameter.
My understanding is that, in gdb, an "enum" parameter's underlying
variable must have a value that is "==" (not just strcmp-equal) to one
of the values coming from the enum array. This invariant is relied on
in various places.
I started this patch by fixing the problem with "endian". Then I
added some assertions to add_setshow_enum_cmd to try to catch other
problems of the same type.
This patch fixes all the problems that I found. I also looked at all
the calls to add_setshow_enum_cmd to ensure that they were all
included in the gdb I tested. I think they are: there are no calls in
nat-* files, or in remote-sim.c; and I was trying a build with all
targets, Python, and Guile enabled.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=12188
This commit brings all the changes made by running gdb/copyright.py
as per GDB's Start of New Year Procedure.
For the avoidance of doubt, all changes in this commits were
performed by the script.
I don't think it's very useful to show deprecated aliases to the
user. It encourages the user to use them, when the goal is the
opposite.
For example, before:
(gdb) help set index-cache enabled
set index-cache enabled, set index-cache off, set index-cache on
alias set index-cache off = set index-cache enabled off
alias set index-cache on = set index-cache enabled on
Enable the index cache.
When on, enable the use of the index cache.
(gdb) help set index-cache on
Warning: 'set index-cache on', an alias for the command 'set index-cache enabled', is deprecated.
Use 'set index-cache enabled on'.
set index-cache enabled, set index-cache off, set index-cache on
alias set index-cache off = set index-cache enabled off
alias set index-cache on = set index-cache enabled on
Enable the index cache.
When on, enable the use of the index cache.
After:
(gdb) help set index-cache enabled
Enable the index cache.
When on, enable the use of the index cache.
(gdb) help set index-cache on
Warning: 'set index-cache on', an alias for the command 'set index-cache enabled', is deprecated.
Use 'set index-cache enabled on'.
Enable the index cache.
When on, enable the use of the index cache.
Change-Id: I989b618a5ad96ba975367e9d16db95523cd57a4c
Now that we use intrusive list to link aliases, it becomes easier to
pass cmd_list_element arguments by const-reference rather than by
pointer to some functions, change a few.
Change-Id: Id0df648ed26e9447da0671fc2c858981cda31df8
Change the manually-implemented linked list to use intrusive_list. This
is not strictly necessary, but it makes the code much simpler.
Change-Id: Idd08090ebf2db8bdcf68e85ef72a9635f1584ccc
The motivation is to reduce the number of places where unmanaged
pointers are returned from allocation type routines. All of the
callers are updated.
There should be no user visible changes after this commit.
The getter and setter in struct setting always receive and return values
by const reference. This is not necessary for scalar values (like bool
and int), but more importantly it makes it a bit annoying to write a
getter, you have to use a scratch static variable or something similar
that you can refer to:
const bool &
my_getter ()
{
static bool value;
value = function_returning_bool ();
return value;
}
Change the getter and setter function signatures to receive and return
value by value instead of by reference, when the underlying data type is
scalar. This means that string-based settings will still use
references, but all others will be by value. The getter above would
then be re-written as:
bool
my_getter ()
{
return function_returning_bool ();
}
This is useful for a patch later in this series that defines a boolean
setting with a getter and a setter.
Change-Id: Ieca3a2419fcdb75a6f75948b2c920b548a0af0fd
Remove two unnecessary nullptr checks. If aliases is nullptr, then the
for loops will simply be skipped.
Change-Id: I9132063bb17798391f8d019af305383fa8e0229f
There's a common pattern to call add_basic_prefix_cmd and
add_show_prefix_cmd to add matching set and show commands. Add the
add_setshow_prefix_cmd function to factor that out and use it at a few
places.
Change-Id: I6e9e90a30e9efb7b255bf839cac27b85d7069cfd
The main motivation behind this improvement is to help the
implementation of a patch Simon Marchi is preparing to fix a bug when
MI or Python try to access parameters that are inferior dependent (see
PR/28085).
This commit extends the previous ones, which introduces the setting
object to represent a static variable whose value can be set or shown
with the appropriate commands. This patch proposes that a setting can
either contain a pointer to a static variable holding a setting, or
pointers to a pair of setter and getter callback functions.
The callbacks functions can be used to retrieve or change the value with
custom logic. This is useful when the source of truth for a given
setting is not contained in the variable pointed to by the setting
instance.
Given that the callback function call is hidden within the setting
abstraction introduced earlier, none of the sites accessing the setting
needs to be updated. The registered getter or setter is used whatever
the way to access it is (through MI, Python, Guile, the "with" command
and the $_gdb_setting / $_gdb_setting_str convenience functions).
All the add_setshow_*_cmd are given a new overload that will accept the
pair of function pointers (set / get functions) instead of the pointer
to a global variable.
Tested on GNU/Linux x86_64 with no regression observed.
Change-Id: Ieb81fef57550632ff66e6aa85f637372a226be8c
Co-authored-by: Simon Marchi <simon.marchi@polymtl.ca>
String-like settings (var_string, var_filename, var_optional_filename,
var_string_noescape) currently take a pointer to a `char *` storage
variable (typically global) that holds the setting's value. I'd like to
"mordernize" this by changing them to use an std::string for storage.
An obvious reason is that string operations on std::string are often
easier to write than with C strings. And they avoid having to do any
manual memory management.
Another interesting reason is that, with `char *`, nullptr and an empty
string often both have the same meaning of "no value". String settings
are initially nullptr (unless initialized otherwise). But when doing
"set foo" (where `foo` is a string setting), the setting now points to
an empty string. For example, solib_search_path is nullptr at startup,
but points to an empty string after doing "set solib-search-path". This
leads to some code that needs to check for both to check for "no value".
Or some code that converts back and forth between NULL and "" when
getting or setting the value. I find this very error-prone, because it
is very easy to forget one or the other. With std::string, we at least
know that the variable is not "NULL". There is only one way of
representing an empty string setting, that is with an empty string.
I was wondering whether the distinction between NULL and "" would be
important for some setting, but it doesn't seem so. If that ever
happens, it would be more C++-y and self-descriptive to use
optional<string> anyway.
Actually, there's one spot where this distinction mattered, it's in
init_history, for the test gdb.base/gdbinit-history.exp. init_history
sets the history filename to the default ".gdb_history" if it sees that
the setting was never set - if history_filename is nullptr. If
history_filename is an empty string, it means the setting was explicitly
cleared, so it leaves it as-is. With the change to std::string, this
distinction doesn't exist anymore. This can be fixed by moving the code
that chooses a good default value for history_filename to
_initialize_top. This is ran before -ex commands are processed, so an
-ex command can then clear that value if needed (what
gdb.base/gdbinit-history.exp tests).
Another small improvement, in my opinion is that we can now easily
give string parameters initial values, by simply initializing the global
variables, instead of xstrdup-ing it in the _initialize function.
In Python and Guile, when registering a string-like parameter, we
allocate (with new) an std::string that is owned by the param_smob (in
Guile) and the parmpy_object (in Python) objects.
This patch started by changing all relevant add_setshow_* commands to
take an `std::string *` instead of a `char **` and fixing everything
that failed to build. That includes of course all string setting
variable and their uses.
string_option_def now uses an std::string also, because there's a
connection between options and settings (see
add_setshow_cmds_for_options).
The add_path function in source.c is really complex and twisted, I'd
rather not try to change it to work on an std::string right now.
Instead, I added an overload that copies the std:string to a `char *`
and back. This means more copying, but this is not used in a hot path
at all, so I think it is acceptable.
Change-Id: I92c50a1bdd8307141cdbacb388248e4e4fc08c93
Co-authored-by: Lancelot SIX <lsix@lancelotsix.com>
cmd_list_element can contain a pointer to data that can be set and / or
shown. This is achieved with the void* VAR member which points to the
data that can be accessed, while the VAR_TYPE member (of type enum
var_types) indicates how to interpret the data pointed to.
With this pattern, the user of the cmd_list_element needs to know what
is the storage type associated with a given VAR_TYPES in order to do
the proper casting. No automatic safeguard is available to prevent
miss-use of the pointer. Client code typically looks something like:
switch (c->var_type)
{
case var_zuinteger:
unsigned int v = *(unsigned int*) c->var;
...
break;
case var_boolean:
bool v = *(bool *) c->var;
...
break;
...
}
This patch proposes to add an abstraction around the var_types and void*
pointer pair. The abstraction is meant to prevent the user from having
to handle the cast and verify that the data is read or written as a type
that is coherent with the setting's var_type. This is achieved by
introducing the struct setting which exposes a set of templated get /
set member functions. The template parameter is the type of the
variable that holds the referred variable.
Using those accessors allows runtime checks to be inserted in order to
ensure that the data pointed to has the expected type. For example,
instantiating the member functions with bool will yield something
similar to:
const bool &get<bool> () const
{
gdb_assert (m_var_type == var_boolean);
gdb_assert (m_var != nullptr);
return *static_cast<bool *> (m_var);
}
void set<bool> (const bool &var)
{
gdb_assert (m_var_type == var_boolean);
gdb_assert (m_var != nullptr);
*static_cast<bool *> (m_var) = var;
}
Using the new abstraction, our initial example becomes:
switch (c->var_type)
{
case var_zuinteger:
unsigned int v = c->var->get<unsigned int> ();
...
break;
case var_boolean:
bool v = c->var->get<bool> ();
...
break;
...
}
While the call site is still similar, the introduction of runtime checks
help ensure correct usage of the data.
In order to avoid turning the bulk of add_setshow_cmd_full into a
templated function, and following a suggestion from Pedro Alves, a
setting can be constructed from a pre validated type erased reference to
a variable. This is what setting::erased_args is used for.
Introducing an opaque abstraction to describe a setting will also make
it possible to use callbacks to retrieve or set the value of the setting
on the fly instead of pointing to a static chunk of memory. This will
be done added in a later commit.
Given that a cmd_list_element may or may not reference a setting, the
VAR and VAR_TYPES members of the struct are replaced with a
gdb::optional<setting> named VAR.
Few internal function signatures have been modified to take into account
this new abstraction:
-The functions value_from_setting, str_value_from_setting and
get_setshow_command_value_string used to have a 'cmd_list_element *'
parameter but only used it for the VAR and VAR_TYPE member. They now
take a 'const setting &' parameter instead.
- Similarly, the 'void *' and a 'enum var_types' parameters of
pascm_param_value and gdbpy_parameter_value have been replaced with a
'const setting &' parameter.
No user visible change is expected after this patch.
Tested on GNU/Linux x86_64, with no regression noticed.
Co-authored-by: Simon Marchi <simon.marchi@polymtl.ca>
Change-Id: Ie1d08c3ceb8b30b3d7bf1efe036eb8acffcd2f34
I don't understand what the sfunc function type in
cmd_list_element::function is for. Compared to cmd_simple_func_ftype,
it has an extra cmd_list_element parameter, giving the callback access
to the cmd_list_element for the command being invoked. This allows
registering the same callback with many commands, and alter the behavior
using the cmd_list_element's context.
From the comment in cmd_list_element, it sounds like at some point it
was the callback function type for set and show functions, hence the
"s". But nowadays, it's used for many more commands that need to access
the cmd_list_element object (see add_catch_command for example).
I don't really see the point of having sfunc at all, since do_sfunc is
just a trivial shim that changes the order of the arguments. All
commands using sfunc could just as well set cmd_list_element::func to
their callback directly.
Therefore, remove the sfunc field in cmd_list_element and everything
that goes with it. Rename cmd_const_sfunc_ftype to cmd_func_ftype and
use it for cmd_list_element::func, as well as for the add_setshow
commands.
Change-Id: I1eb96326c9b511c293c76996cea0ebc51c70fac0
After browsing the CLI code for quite a while and trying really hard, I
reached the conclusion that I can't give a meaningful explanation of
what "sfunc" and "cfunc" functions are, in cmd_list_element. I don't
see a logic at all. That makes it very difficult to do any kind of
change. Unless somebody can make sense out of all that, I'd like to try
to retro-fit some logic in the cmd_list_element callback function code
so that we can understand what is going on, do some cleanups and add new
features.
The first change is about "cfunc". I can't figure out what the "c" in
cfunc means. It's not const, because there's already "const" in
"cmd_const_cfunc_ftype", and the previous "cmd_cfunc_ftype" had nothing
const.. It's not "cmd" or "command", because there's already "cmd" in
"cmd_const_cfunc_ftype".
The "main" command callback, cmd_list_element::func, has three
parameters, whereas cfunc has two. It is missing the cmd_list_element
parameter. So the only reason I see for cfunc to exist is to be a shim
between the three and two parameter versions. Most commands don't need
to receive the cmd_list_element object, so adding it everywhere would be
long and would just add more unnecessary boilerplate. So since this is
the "simple" version of the callback, compared to the "full", I suggest
renaming cmd_const_cfunc_ftype into cmd_simple_func_ftype, as well as
everything (like the utility functions) that goes with it.
Change-Id: I4e46cacfd77a66bc1cbf683f6a362072504b7868
I propose removing the context parameter from add_setshow_enum_cmd. It
was useful before add_setshow_enum_cmd returned both created commands,
as the caller couldn't easily set the context itself. But now, I think
it's fine to just let the caller do it.
gdb/ChangeLog:
* command.h (add_setshow_enum_cmd): Remove context parameter.
* cli/cli-decode.c (add_setshow_enum_cmd): Likewise, and don't
set context.
* cli/cli-style.c (cli_style_option::add_setshow_commands): Set
context here.
Change-Id: I377c4e6820ec9d5069492ed28f4cba342ce1336e
Same idea as previous patch, but for add_alias_cmd. Remove the overload
that accepts the target command as a string (the target command name),
leaving only the one that takes the cmd_list_element.
gdb/ChangeLog:
* command.h (add_alias_cmd): Accept target as
cmd_list_element. Update callers.
Change-Id: I546311f411e9e7da9302322d6ffad4e6c56df266
Same idea as previous patch, but for add_info_alias.
gdb/ChangeLog:
* command.h (add_info_alias): Accept target as
cmd_list_element. Update callers.
Change-Id: If830d423364bf42d7bea5ac4dd3a81adcfce6f7a
The alias creation functions currently accept a name to specify the
target command. They pass this to add_alias_cmd, which needs to lookup
the target command by name.
Given that:
- We don't support creating an alias for a command before that command
exists.
- We always use add_info_alias just after creating that target command,
and therefore have access to the target command's cmd_list_element.
... change add_com_alias to accept the target command as a
cmd_list_element (other functions are done in subsequent patches). This
ensures we don't create the alias before the target command, because you
need to get the cmd_list_element from somewhere when you call the alias
creation function. And it avoids an unecessary command lookup. So it
seems better to me in every aspect.
gdb/ChangeLog:
* command.h (add_com_alias): Accept target as
cmd_list_element. Update callers.
Change-Id: I24bed7da57221cc77606034de3023fedac015150
Some add_set_show commands return a single cmd_list_element, the one for
the "set" command. A subsequent patch will need to access the show
command's cmd_list_element as well. Change these functions to return a
new structure type that holds both pointers.
I initially only modified add_setshow_boolean_cmd (the one I needed),
but I think it's better to change the whole chain to keep everything in
sync.
gdb/ChangeLog:
* command.h (set_show_commands): New.
(add_setshow_enum_cmd, add_setshow_auto_boolean_cmd,
add_setshow_boolean_cmd, add_setshow_filename_cmd,
add_setshow_string_cmd, add_setshow_string_noescape_cmd,
add_setshow_optional_filename_cmd, add_setshow_integer_cmd,
add_setshow_uinteger_cmd, add_setshow_zinteger_cmd,
add_setshow_zuinteger_cmd, add_setshow_zuinteger_unlimited_cmd):
Return set_show_commands. Adjust callers.
* cli/cli-decode.c (add_setshow_cmd_full): Return
set_show_commands, remove result parameters, adjust callers.
Change-Id: I17492b01b76002d09effc84830f9c6db26f1db7a
Same idea as the previous patches, but for whether a command is a
"command class help" command. I think this one is particularly useful,
because it's not obvious when reading code what "c->func == NULL" means.
Remove the cmd_func_p function, which does kind of the same thing as
cmd_list_element::is_command_class_help (except it doesn't give a clue
about the semantic of a NULL func value).
gdb/ChangeLog:
* cli/cli-decode.h (cmd_list_element) <is_command_class_help>:
New, use it.
* command.h (cmd_func_p): Remove.
* cli/cli-decode.c (cmd_func_p): Remove.
Change-Id: I521a3e1896dc93a5babe1493d18f5eb071e1b3b7
Same idea as the previous patch, but for prefix instead of alias.
gdb/ChangeLog:
* cli/cli-decode.h (cmd_list_element) <is_prefix>: New, use it.
Change-Id: I76a9d2e82fc8d7429904424674d99ce6f9880e2b
Add the cmd_list_element::is_alias helper to check whether a command is
an alias. I find it easier to understand the intention in:
if (c->is_alias ())
than
if (c->alias_target != nullptr)
Change all the spots that are reading alias_target just to compare it to
NULL/nullptr to use is_alias instead.
gdb/ChangeLog:
* cli/cli-decode.h (cmd_list_element) <is_alias>: New, use it.
Change-Id: I26ed56f99ee47fe884fdfedf87016501631693ce
cmd_pointer is another field whose name I found really not clear. Yes,
it's a pointer to a command, the type tells me that. But what's the
relationship of that command to the current command? This field
contains, for an alias, the command that it aliases. So I think that
the name "alias_target" would be more appropriate.
Also, rename "old" parameters to "target" in the functions that add
aliases.
gdb/ChangeLog:
* cli/cli-decode.h (cmd_list_element) <cmd_pointer>: Rename
to...
<alias_target>: ... this.
(add_alias_cmd): Rename old to target.
(add_info_alias): Rename old_name to target_name.
(add_com_alias): Likewise.
Change-Id: I8db36c6dd799fae155f7acd3805f6d62d98befa9
While browsing this code, I found the name "prefixlist" really
confusing. I kept reading it as "list of prefixes". Which it isn't:
it's a list of sub-commands, for a prefix command. I think that
renaming it to "subcommands" would make things clearer.
gdb/ChangeLog:
* Rename "prefixlist" parameters to "subcommands" throughout.
* cli/cli-decode.h (cmd_list_element) <prefixlist>: Rename to...
<subcommands>: ... this.
* cli/cli-decode.c (lookup_cmd_for_prefixlist): Rename to...
(lookup_cmd_with_subcommands): ... this.
Change-Id: I150da10d03052c2420aa5b0dee41f422e2a97928
I don't think this can ever happen, that we add an alias command and
pass a nullptr old (target) command. Remove the "if" handling this,
replace with an assert.
gdb/ChangeLog:
* cli/cli-decode.c (add_alias_cmd): Don't handle old == 0.
Change-Id: Ibb39e8dc4e0c465fa42e6826215f30a0a0aef932
I don't think this method really benefits from being implemented in the
header file, especially because it's recursive, it can't be inlined.
Move it to the source file, so it's no re-compiled by every CU
including cli/cli-decode.h.
I also noticed this method could be const, make it so.
gdb/ChangeLog:
* cli/cli-decode.h (prefixname): Make const, move implementation
to cli/cli-decode.c.
* cli/cli-decode.c (cmd_list_element::prefixname): New.
Change-Id: I1597cace98d9a4ba71f51f1f495e73cc07b5dcf3
Previously, the prefixname field of struct cmd_list_element was manually
set for prefix commands. This seems verbose and error prone as it
required every single call to functions adding prefix commands to
specify the prefix name while the same information can be easily
generated.
Historically, this was not possible as the prefix field was null for
many commands, but this was fixed in commit
3f4d92ebdf by Philippe Waroquiers, so
we can rely on the prefix field being set when generating the prefix
name.
This commit also fixes a use after free in this scenario:
* A command gets created via Python (using the gdb.Command class).
The prefix name member is dynamically allocated.
* An alias to the new command is created. The alias's prefixname is set
to point to the prefixname for the original command with a direct
assignment.
* A new command with the same name as the Python command is created.
* The object for the original Python command gets freed and its
prefixname gets freed as well.
* The alias is updated to point to the new command, but its prefixname
is not updated so it keeps pointing to the freed one.
gdb/ChangeLog:
* command.h (add_prefix_cmd): Remove the prefixname argument as
it can now be generated automatically. Update all callers.
(add_basic_prefix_cmd): Ditto.
(add_show_prefix_cmd): Ditto.
(add_prefix_cmd_suppress_notification): Ditto.
(add_abbrev_prefix_cmd): Ditto.
* cli/cli-decode.c (add_prefix_cmd): Ditto.
(add_basic_prefix_cmd): Ditto.
(add_show_prefix_cmd): Ditto.
(add_prefix_cmd_suppress_notification): Ditto.
(add_prefix_cmd_suppress_notification): Ditto.
(add_abbrev_prefix_cmd): Ditto.
* cli/cli-decode.h (struct cmd_list_element): Replace the
prefixname member variable with a method which generates the
prefix name at runtime. Update all code reading the prefix
name to use the method, and remove all code setting it.
* python/py-cmd.c (cmdpy_destroyer): Remove code to free the
prefixname member as it's now a method.
(cmdpy_function): Determine if the command is a prefix by
looking at prefixlist, not prefixname.
In code dealing with commands, there's a pattern repeated a few times of
calling lookup_cmd with some speficic arguments and then using strcmp
on the returned command to check for an exact match.
As a later patch would add a few more similar lines of code, this patch
adds a new lookup_cmd_exact function which simplify this use case.
gdb/ChangeLog:
* cli/cli-decode.c (lookup_cmd_exact): Add.
* cli/cli-script.c (do_define_command): Use lookup_cmd_exact.
(define_prefix_command): Ditto.
* command.h: Add lookup_cmd_exact.
This commits the result of running gdb/copyright.py as per our Start
of New Year procedure...
gdb/ChangeLog
Update copyright year range in copyright header of all GDB files.
After Andrew's latest patch, I noticed that the deprecation warnings
could use the (so-called) title style when printing command names.
This patch implements this idea.
gdb/ChangeLog
2020-12-15 Tom Tromey <tromey@adacore.com>
* cli/cli-decode.c (deprecated_cmd_warning): Use title style for
command names.
gdb/testsuite/ChangeLog
2020-12-15 Tom Tromey <tromey@adacore.com>
* gdb.base/style.exp: Add deprecation tests.
Consider this GDB session:
(gdb) define set xxx_yyy
Type commands for definition of "set xxx_yyy".
End with a line saying just "end".
>echo in set xxx_yyy command\n
>end
(gdb) alias set qqq_aaa=set xxx_yyy
(gdb) maintenance deprecate set qqq_aaa
(gdb) set qqq_aaa
Warning: 'qqq_aaa', an alias for the command 'xxx_yyy' is deprecated.
No alternative known.
in set xxx_yyy command
(gdb)
Notice the warning mentions 'qqq_aaa' and 'xxx_yyy', I consider this
to be wrong. I think the proper warning should read:
(gdb) set qqq_aaa
Warning: 'set qqq_aaa', an alias for the command 'set xxx_yyy', is deprecated.
No alternative known.
With the 'set' prefixes added and a comma before the final 'is
deprecated'. That is what this patch does. The expected results are
updated as needed.
gdb/ChangeLog:
* cli/cli-decode.c (deprecated_cmd_warning): Ignore the prefix
result from lookup_cmd_composition_1, use the prefixes from both
the command and the alias instead.
(lookup_cmd_composition_1): Initial prefix command is the based on
the search list being passed in. Simplify the logic for tracking
the prefix command. Replace a use of alloca with a local
std::string.
gdb/testsuite/ChangeLog:
* gdb.base/commands.exp: Update expected results.
Rewrite deprecated_cmd_warning to be i18n friendly. While I'm going
through the function I also cleaned up some whitespace issues,
replaced uses of NULL with nullptr, and moved some comments to avoid
having to add { ... }.
Though the message being printed has a 'Warning: ' prefix I could have
changed from using printf_filtered to use warning, however, I haven't
done that in this commit as that would change what GDB outputs and I
wanted this commit NOT to change the output.
There should be no user visible changes after this commit.
gdb/ChangeLog:
* cli/cli-decode.c (deprecated_cmd_warning): Use nullptr instead
of NULL. Don't print message piece by piece, but sentence at a
time to allow internationalisation. Some whitespace cleanup.
I noticed that deprecated aliases that have a prefix don't give a
deprecated command warning. For example looking in mi/mi-main.c we
see this:
c = add_alias_cmd ("target-async", "mi-async", class_run, 0, &setlist);
deprecate_cmd (c, "set mi-async");
c = add_alias_cmd ("target-async", "mi-async", class_run, 0, &showlist);
deprecate_cmd (c, "show mi-async");
So both 'set target-async' and 'show target-async' are deprecated and
should be giving a warning, however, in use we see no warning given.
This is a consequence of how the code that should give this
warning (deprecated_cmd_warning) performs a second command lookup in
order to distinguish between aliases and real commands, and that the
code that calls this (lookup_cmd_1) strips off prefix commands as it
calls itself recursively.
As a result when we are considering an alias like 'set target-async'
we first enter lookup_cmd_1 with text = "set target-async", we spot
the 'set' command prefix and then recursively call lookup_cmd_1 with
text = "target-async".
We spot that 'target-async' is a known alias but that it is
deprecated, and so call deprecated_cmd_warning passing in the value of
text, which remember is now "target-async".
In deprecated_cmd_warning we again perform a command lookup starting
from the top-level cmdlist, but now we're trying to find just
"target-async", this fails (as this command requires the 'set' prefix,
and so no warning is given.
I resolved this issue by passing a command list to the function
deprecated_cmd_warning, this is the list in which the command can be
found.
A new test is added to cover this case.
However, there is an additional problem which will be addressed in a
subsequent patch.
Consider this GDB session:
(gdb) define set xxx_yyy
Type commands for definition of "set xxx_yyy".
End with a line saying just "end".
>echo in set xxx_yyy command\n
>end
(gdb) alias set qqq_aaa=set xxx_yyy
(gdb) maintenance deprecate set qqq_aaa
(gdb) set qqq_aaa
Warning: 'qqq_aaa', an alias for the command 'xxx_yyy' is deprecated.
No alternative known.
in set xxx_yyy command
(gdb)
Notice the warning mentions 'qqq_aaa' and 'xxx_yyy', I consider this
to be wrong. I think the proper warning should read:
(gdb) set qqq_aaa
Warning: 'set qqq_aaa', an alias for the command 'set xxx_yyy' is deprecated.
No alternative known.
With the 'set' prefixes added. A later patch will resolve this
issue.
gdb/ChangeLog:
PR cli/15104
* cli/cli-decode.c (lookup_cmd_1): Pass command list to
deprecated_cmd_warning.
(deprecated_cmd_warning): Take extra parameter, call
lookup_cmd_composition_1 and pass new parameter through.
(lookup_cmd_composition_1): New function, takes implementation of
lookup_cmd_composition but with extra parameter.
(lookup_cmd_composition): Now calls lookup_cmd_composition_1
passing in cmdlist.
* command.h (deprecated_cmd_warning): Add extra parameter to
declaration.
* top.c (execute_command): Pass cmdlist to deprecated_cmd_warning.
gdb/testsuite/ChangeLog:
PR cli/15104
* gdb.base/commands.exp: Add additional tests.
* gdb.base/completion.exp: Add additional tests.
Consider this gdb session, where on line #3 tab completion is used:
(gdb) alias xxx_yyy_zzz=break
(gdb) maint deprecate xxx_yyy_zzz
(gdb) xxx_yyy_<TAB>
The third line then updates to look like this:
(gdb) xxx_yyy_Warning: 'xxx_yyy_zzz', an alias for the command 'break' is deprecated.
No alternative known.
zzz
What's happened is during tab completion the alias has been resolved
to the actual command being aliased, and at this stage the warning is
issued. Clearly this is not what we want during tab completion.
In this commit I add a new parameter to the lookup function, a boolean
that indicates if the lookup is being done as part of completion.
This flag is used to suppress the warning. Now we get the expected
behaviour, the alias completes without any warning, but the warning is
still given once the user executes the alias.
gdb/ChangeLog:
* cli/cli-decode.c (lookup_cmd_1): Move header comment into
command.h, add extra parameter, and use this to guard giving a
warning.
* command.h (lookup_cmd_1): Add comment from cli/cli-decode.c,
include argument names in declaration, add new argument.
* completer.c (complete_line_internal_1): Remove unneeded
brackets, pass extra argument to lookup_cmd_1.
gdb/testsuite/ChangeLog:
* gdb.base/completion.exp: Add additional tests.
The cmd_type function only has a single caller, which is in the CLI
implementation code. This patch removes the function, and moves the
cmd_types enum definition from command.h to cli-decode.h, fixing an 18
year old FIXME.
gdb/ChangeLog
2020-06-28 Tom Tromey <tom@tromey.com>
* command.h (cmd_types): Remove.
(cmd_type): Don't declare.
* cli/cli-decode.h (enum cmd_types): Uncomment. No longer a
typedef.
* cli/cli-cmds.c (setting_cmd): Use cmd->type directly.
* cli/cli-decode.c (cmd_type): Remove.
Currently, a user can define an alias, but cannot have default
arguments for this alias.
This patch modifies the 'alias' command so that default args can
be provided.
(gdb) h alias
Define a new command that is an alias of an existing command.
Usage: alias [-a] [--] ALIAS = COMMAND [DEFAULT-ARGS...]
ALIAS is the name of the alias command to create.
COMMAND is the command being aliased to.
Options:
-a
Specify that ALIAS is an abbreviation of COMMAND.
Abbreviations are not used in command completion..
GDB will automatically prepend the provided DEFAULT-ARGS to the list
of arguments explicitly provided when using ALIAS.
Use "help aliases" to list all user defined aliases and their default args.
Examples:
Make "spe" an alias of "set print elements":
alias spe set print elements
Make "elms" an alias of "elements" in the "set print" command:
alias -a set print elms set print elements
Make "btf" an alias of "backtrace -full -past-entry -past-main" :
alias btf = backtrace -full -past-entry -past-main
Make "wLapPeu" an alias of 2 nested "with":
alias wLapPeu = with language pascal -- with print elements unlimited --
(gdb)
The way 'default-args' is implemented makes it trivial to set default
args also for GDB commands (such as "backtrace") and for GDB pre-defined
aliases (such as "bt"). It was however deemed better to not allow to
define default arguments for pre-defined commands and aliases, to avoid
users believing that e.g. default args for "backtrace" would apply to "bt".
If needed, default-args could be allowed for GDB predefined commands
and aliases by adding a command
'set default-args GDB_COMMAND_OR_PREDEFINED_ALIAS [DEFAULT-ARGS...]'.
* 'alias' command now has a completer that helps to complete:
- ALIAS (if the user defines an alias after a prefix),
- the aliased COMMAND
- the possible options for the aliased COMMAND.
* Help and apropos commands show the definitions of the aliases
that have default arguments, e.g.
(gdb) help backtrace
backtrace, btf, where, bt
alias btf = backtrace -full -past-entry -past-main
Print backtrace of all stack frames, or innermost COUNT frames.
Usage: backtrace [OPTION]... [QUALIFIER]... [COUNT | -COUNT]
Options:
-entry-values no|only|preferred|if-needed|both|compact|default
Set printing of function arguments at function entry.
...
gdb/ChangeLog
2020-06-22 Philippe Waroquiers <philippe.waroquiers@skynet.be>
* cli/cli-cmds.c (lookup_cmd_for_default_args)
(alias_command_completer)
(make_alias_options_def_group): New functions.
(alias_opts, alias_option_defs): New struct and array.
(alias_usage_error): Update usage.
(alias_command): Handles optional DEFAULT-ARGS... arguments.
Use option framework.
(_initialize_cli_cmds): Update alias command help.
Update aliases command help.
(show_user):
Add NULL for new default_args lookup_cmd argument.
(valid_command_p): Rename to validate_aliased_command.
Add NULL for new default_args lookup_cmd argument. Verify that the
aliased_command has no default args.
* cli/cli-decode.c (help_cmd): Show aliases definitions.
(lookup_cmd_1, lookup_cmd): New argument default_args.
(add_alias_cmd):
Add NULL for new default_args lookup_cmd argument.
(print_help_for_command): Show default args under the layout
alias some_alias = some_aliased_cmd some_alias_default_arg.
* cli/cli-decode.h (struct cmd_list_element): New member default_args.
xfree default_args in destructor.
* cli/cli-script.c (process_next_line, do_define_command):
Add NULL for new default_args lookup_cmd argument.
* command.h: Declare new default_args argument in lookup_cmd
and lookup_cmd_1.
* completer.c (complete_line_internal_1):
Add NULL for new default_args lookup_cmd or lookup_cmd_1 argument.
* guile/scm-cmd.c (gdbscm_parse_command_name): Likewise.
* guile/scm-param.c (add_setshow_generic, pascm_parameter_defined_p):
Likewise.
* infcmd.c (_initialize_infcmd): Likewise.
* python/py-auto-load.c (gdbpy_initialize_auto_load): Likewise.
* python/py-cmd.c (gdbpy_parse_command_name): Likewise.
* python/py-param.c (add_setshow_generic): Likewise.
* remote.c (_initialize_remote): Likewise.
* top.c (execute_command): Prepend default_args if command has some.
(set_verbose):
Add NULL for new default_args lookup_cmd or lookup_cmd_1 argument.
* tracepoint.c (validate_actionline, encode_actions_1):
Add NULL for new default_args lookup_cmd or lookup_cmd_1 argument.
This commit finally does the (small) change that started this patch
series.
It ensures that the class_alias is only used for user-defined aliases.
So, the few GDB pre-defined aliases that were using the 'class_alias'
class are now using a real help class, typically the class of
the aliased command.
gdb/ChangeLog
2020-05-15 Philippe Waroquiers <philippe.waroquiers@skynet.be>
* command.h (enum command_class): Improve comments, document
that class_alias is for user-defined aliases, give the class
name for each class, remove unused class_xdb.
* cli/cli-decode.c (add_com_alias): Document THECLASS intended usage.
* breakpoint.c (_initialize_breakpoint): Replace class_alias
by a precise class.
* infcmd.c (_initialize_infcmd): Likewise.
* reverse.c (_initialize_reverse): Likewise.
* stack.c (_initialize_stack): Likewise.
* symfile.c (_initialize_symfile): Likewise.
* tracepoint.c (_initialize_tracepoint): Likewise.
gdb/testsuite/ChangeLog
2020-05-15 Philippe Waroquiers <philippe.waroquiers@skynet.be>
* gdb.base/alias.exp: Verify 'help aliases' shows user defined aliases.
Similarly to 'help CLASS', apropos possibly shows several
times the same help (for the command and for each of its aliases).
This patch changes 'apropos' so that the help for a command and
all its aliases is shown once.
So, apropos_cmd now skips all aliases/abbreviations, as these are printed
as part of the help of the aliased command.
When 'apropos' prints the help of a command, function 'help_cmd' now
unconditionally print the command name and its possible aliases (as we must
indicate to the user the command/aliases for which the help is printed).
When 'help somecommand' prints the help of a command, if the command is not
aliased, the command name is not printed (to avoid a useless first line), but if
it has aliases, then the command name and all its aliases are now printed.
In addition to provide to the user the choice of the best way to
type a command, it also avoids the strange behaviour that the output
of 'help somealias' does not mention somealias.
gdb/ChangeLog
2020-05-15 Philippe Waroquiers <philippe.waroquiers@skynet.be>
* cli/cli-decode.c (apropos_cmd): Produce output for aliases
when their aliased command is traversed.
(help_cmd): Add fput_command_names_styled call to
output command name and aliases when command has an alias.
gdb/testsuite/ChangeLog
2020-05-15 Philippe Waroquiers <philippe.waroquiers@skynet.be>
* gdb.base/help.exp: Test apropos and help for commands
having aliases. Fixed comments not starting with an
upper-case letter or not finishing with a dot.
Currently, help CLASS possibly shows several times the same help,
as it shows it once for the command, and once for each alias.
The final objective of this patch series is to have class_alias used only
for user defined aliases, not anymore for aliases predefined by GDB.
The command 'help aliases' will then only show the user defined aliases.
So, the idea is that GDB predefined aliases will be shown together
with their aliased command.
This commit changes 'help CLASS' so that a command is shown once in the output,
with all its aliases.
This ensures:
* that the user has only to read once the same help text
* and sees the command and all its aliases in a glance, a.o. allowing
the user to choose the preferred way (e.g. the shortest one,
or the most mnemonic one) to type the command.
For example, the old output:
(gdb) help stack
...
List of commands:
backtrace -- Print backtrace of all stack frames, or innermost COUNT frames.
bt -- Print backtrace of all stack frames, or innermost COUNT frames.
...
(note that 'where' is not shown in this output)
becomes
(gdb) help stack
...
List of commands:
backtrace, where, bt -- Print backtrace of all stack frames, or innermost COUNT frames.
...
The output layout chosen is to have the command first, followed by all its
aliases separated by a comma. Note that the command and alias names are
title-styled. For sure, other layouts could be discussed, but this one is IMO
readable and compact.
The function 'help_cmd_list' can be simplified by removing the prefix argument,
as the prefixname of a command can now be retrieved in the GDB command tree
structure.
This also fixes the fact that 'help aliases' wrongly shows a long
list of (non-alias) when defining an alias for a prefix command.
For example, after:
(gdb) alias montre = show
then
(gdb) help aliases
shows hundreds of sub-commands starting with the non aliased command,
such as:
montre -- Generic command for showing things about the debugger.
show ada -- Generic command for showing Ada-specific settings.
show ada print-signatures -- Show whether the output of formal ...
....
'help_cmd_list' is also made static, as it is only used inside cli-decode.c.
Note that the 'help CLASS' is somewhat broken, in the sense that it
sometimes shows too many commands (commands not belonging to CLASS)
and sometimes shows not enough commands (not showing some commands
belonging to CLASS).
For example, 'help breakpoints' shows the command
'disable pretty-printer' and 'disable unwinder', not related to breakpoints.
On the other end, 'help stack' does not show 'disable unwinder'
while 'disable unwinder' is defined in unwinders.py as belonging to class_stack.
Fixing the missing commands is easy to do,
but fixing the excess commands is not straightforward, as many
subcommands have a class 'no_class' or 'all_class'.
Possibly, some of this might be improved/fixed in another patch series.
With this patch series, the 'abbrev flag' has as only remaining purpose
to avoid having the abbreviation alias appearing in the completion list,
so change 'help alias' accordingly.
gdb/ChangeLog
2020-05-15 Philippe Waroquiers <philippe.waroquiers@skynet.be>
* cli/cli-decode.h (help_cmd_list): Remove declaration.
* cli/cli-decode.c (help_cmd_list): Declare as static,
remove prefix argument, use bool for recurse arg, rework to show the aliases of
a command together with the command.
(fput_command_name_styled, fput_command_names_styled): New functions.
(print_help_for_command): Remove prefix arg, use bool for recurse arg, use
fput_command_name_styled.
(help_list, help_all): Update callers to remove prefix arg and use bool recurse.
* cli/cli-cmds.c (_initialize_cli_cmds): Update alias_command doc.
gdb/testsuite/ChangeLog
2020-05-15 Philippe Waroquiers <philippe.waroquiers@skynet.be>
* gdb.base/alias.exp: Update help output check.
cmd_show_list function implements the 'show' command.
cmd_show_list output is inconsistent: it sometimes shows a prefix
and sometimes does not.
For example, in the below, you see that there is a prefix before
each value, except for 'enabled'.
(gdb) show style
style address background: The "address" style background color is: none
style address foreground: The "address" style foreground color is: blue
style address intensity: The "address" style display intensity is: normal
enabled: CLI output styling is enabled.
style filename background: The "filename" style background color is: none
...
There are other inconsistencies or bugs e.g. in
the below we see twice insn-number-max, once with a prefix
and once without prefix : last line, just before the value of
instruction-history-size which is itself without prefix.
(gdb) show record
record btrace bts buffer-size: The record/replay bts buffer size is 65536.
record btrace cpu: btrace cpu is 'auto'.
record btrace pt buffer-size: The record/replay pt buffer size is 16384.
record btrace replay-memory-access: Replay memory access is read-only.
record full insn-number-max: Record/replay buffer limit is 200000.
record full memory-query: Whether query if PREC cannot record memory change of next instruction is off.
record full stop-at-limit: Whether record/replay stops when record/replay buffer becomes full is on.
function-call-history-size: Number of functions to print in "record function-call-history" is 10.
insn-number-max: instruction-history-size: Number of instructions to print in "record instruction-history" is 10.
(gdb)
Also, some values are output several times due to some aliases, so avoid outputting duplicated
values by skipping all aliases.
Now that the command structure has a correct 'back-pointer' from a command
to its prefix command, we can simplify cmd_show_list by removing its prefix argument
and at the same time fix the output inconsistencies and bugs.
gdb/ChangeLog
2020-05-15 Philippe Waroquiers <philippe.waroquiers@skynet.be>
* cli/cli-setshow.h (cmd_show_list): Remove prefix argument.
* cli/cli-decode.c (do_show_prefix_cmd): Likewise.
* command.h (cmd_show_list): Likewise.
* dwarf2/index-cache.c (show_index_cache_command): Likewise.
* cli/cli-setshow.c (cmd_show_list): Use the prefix to produce the output. Skip aliases.
gdb/testsuite/ChangeLog
2020-05-15 Philippe Waroquiers <philippe.waroquiers@skynet.be>
* gdb.base/default.exp: Update output following fixes.
The next commit updates command-def-selftests.c to detect missing
or wrong prefix commands in a list of subcommands.
This command structure selftest detects a series of problems
that are fixed by this commit.
Many commands have a null prefix command, e.g.
(gdb) maintenance selftest command_str
Running selftest command_structure_invariants.
list 0x560417949cb8 reachable via prefix 'append binary '. command 'memory' has null prefixcmd
list 0x560417949cb8 reachable via prefix 'append binary '. command 'value' has null prefixcmd
...
Most of these are fixed by the following changes:
* do_add_cmd searches the prefix command having the list
in which the command is added.
This ensures that a command defined after its prefix command
gets the correct prefix command.
* Due to the GDB initialization order, a GDB file can define
a subcommand before the prefix command is defined.
So, have add_prefix_cmd calling a new recursive function
'update_prefix_field_of_prefix_commands' to set the prefix
command of all sub-commands that are now reachable from
this newly defined prefix command. Note that this recursive
call replaces the function 'set_prefix_cmd' that was providing
a partial solution to this problem.
Following that, 2 python commands (defined after all the other GDB
commands) got a wrong prefix command, e.g. "info frame-filter" has
as prefix command the "i" alias of "info". This is fixed by having
lookup_cmd_for_prefixlist returning the aliased command rather than
the alias.
After that, one remaining problem:
(gdb) maintenance selftest command_str
Running selftest command_structure_invariants.
list 0x55f320272298 reachable via prefix 'set remote '. command 'system-call-allowed' has null prefixcmd
Self test failed: self-test failed at ../../classfix/gdb/unittests/command-def-selftests.c:196
Ran 1 unit tests, 1 failed
(gdb)
Caused by initialize_remote_fileio that was taking the address of
its arguments remote_set_cmdlist and remote_show_cmdlist instead
of receiving the correct values to use as list.
2020-05-15 Philippe Waroquiers <philippe.waroquiers@skynet.be>
* cli/cli-decode.c (lookup_cmd_for_prefix): Return the aliased command
as prefix, not one of its aliases.
(set_cmd_prefix): Remove.
(do_add_cmd): Centralize the setting of the prefix of a command, when
command is defined after its full chain of prefix commands.
(add_alias_cmd): Remove call to set_cmd_prefix, as do_add_cmd does it.
(add_setshow_cmd_full): Likewise.
(update_prefix_field_of_prefixed_commands): New function.
(add_prefix_cmd): Replace non working call to set_cmd_prefix by
update_prefix_field_of_prefixed_commands.
* gdb/remote-fileio.c (initialize_remote_fileio): Use the real
addresses of remote_set_cmdlist and remote_show_cmdlist given
as argument, not the address of an argument.
* gdb/remote-fileio.h (initialize_remote_fileio): Likewise.
* gdb/remote.c (_initialize_remote): Likewise.
