This commit is the result of running the gdb/copyright.py script,
which automated the update of the copyright year range for all
source files managed by the GDB project to be updated to include
year 2023.
Consider a hello world a.out, started using gdbserver:
...
$ gdbserver --once 127.0.0.1:2345 ./a.out
Process ./a.out created; pid = 15743
Listening on port 2345
...
that we can connect to using gdb:
...
$ gdb -ex "target remote 127.0.0.1:2345"
Remote debugging using 127.0.0.1:2345
Reading /home/vries/a.out from remote target...
...
0x00007ffff7dd4550 in _start () from target:/lib64/ld-linux-x86-64.so.2
(gdb)
...
After that, we can for instance quit with confirmation:
...
(gdb) quit
A debugging session is active.
Inferior 1 [process 16691] will be killed.
Quit anyway? (y or n) y
$
...
Or, kill with confirmation and quit:
...
(gdb) kill
Kill the program being debugged? (y or n) y
[Inferior 1 (process 16829) killed]
(gdb) quit
$
...
Or, monitor exit, kill with confirmation, and quit:
...
(gdb) monitor exit
(gdb) kill
Kill the program being debugged? (y or n) y
Remote connection closed
(gdb) quit
$
...
But when doing monitor exit followed by quit with confirmation, we get the gdb
prompt back, requiring us to do quit once more:
...
(gdb) monitor exit
(gdb) quit
A debugging session is active.
Inferior 1 [process 16944] will be killed.
Quit anyway? (y or n) y
Remote connection closed
(gdb) quit
$
...
So, the first quit didn't quit. This happens as follows:
- quit_command calls query_if_trace_running
- a TARGET_CLOSE_ERROR is thrown
- it's caught in remote_target::get_trace_status, but then
rethrown because it's TARGET_CLOSE_ERROR
- catch_command_errors catches the error, at which point the quit command
has been aborted.
The TARGET_CLOSE_ERROR is defined as:
...
/* Target throwing an error has been closed. Current command should be
aborted as the inferior state is no longer valid. */
TARGET_CLOSE_ERROR,
...
so in a way this is expected behaviour. But aborting quit because the inferior
state (which we've already confirmed we're not interested in) is no longer
valid, and having to type quit again seems pointless.
Furthermore, the purpose of not catching errors thrown by
query_if_trace_running as per commit 2f9d54cfce ("make -gdb-exit call
disconnect_tracing too, and don't lose history if the target errors on
"quit""), was to make sure that error (_("Not confirmed.") had effect.
Fix this in quit_command by catching only the TARGET_CLOSE_ERROR exception
during query_if_trace_running and reporting it:
...
(gdb) monitor exit
(gdb) quit
A debugging session is active.
Inferior 1 [process 19219] will be killed.
Quit anyway? (y or n) y
Remote connection closed
$
...
Tested on x86_64-linux.
PR server/15746
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=15746
Approved-By: Tom Tromey <tom@tromey.com>
This changes GDB to use frame_info_ptr instead of frame_info *
The substitution was done with multiple sequential `sed` commands:
sed 's/^struct frame_info;/class frame_info_ptr;/'
sed 's/struct frame_info \*/frame_info_ptr /g' - which left some
issues in a few files, that were manually fixed.
sed 's/\<frame_info \*/frame_info_ptr /g'
sed 's/frame_info_ptr $/frame_info_ptr/g' - used to remove whitespace
problems.
The changed files were then manually checked and some 'sed' changes
undone, some constructors and some gets were added, according to what
made sense, and what Tromey originally did
Co-Authored-By: Bruno Larsen <blarsen@redhat.com>
Approved-by: Tom Tomey <tom@tromey.com>
This commit changes the format of 'disassemble /r' to match GNU
objdump. Specifically, GDB will now display the instruction bytes in
as 'objdump --wide --disassemble' does.
Here is an example for RISC-V before this patch:
(gdb) disassemble /r 0x0001018e,0x0001019e
Dump of assembler code from 0x1018e to 0x1019e:
0x0001018e <call_me+66>: 03 26 84 fe lw a2,-24(s0)
0x00010192 <call_me+70>: 83 25 c4 fe lw a1,-20(s0)
0x00010196 <call_me+74>: 61 65 lui a0,0x18
0x00010198 <call_me+76>: 13 05 85 6a addi a0,a0,1704
0x0001019c <call_me+80>: f1 22 jal 0x10368 <printf>
End of assembler dump.
And here's an example after this patch:
(gdb) disassemble /r 0x0001018e,0x0001019e
Dump of assembler code from 0x1018e to 0x1019e:
0x0001018e <call_me+66>: fe842603 lw a2,-24(s0)
0x00010192 <call_me+70>: fec42583 lw a1,-20(s0)
0x00010196 <call_me+74>: 6561 lui a0,0x18
0x00010198 <call_me+76>: 6a850513 addi a0,a0,1704
0x0001019c <call_me+80>: 22f1 jal 0x10368 <printf>
End of assembler dump.
There are two differences here. First, the instruction bytes after
the patch are grouped based on the size of the instruction, and are
byte-swapped to little-endian order.
Second, after the patch, GDB now uses the bytes-per-line hint from
libopcodes to add whitespace padding after the opcode bytes, this
means that in most cases the instructions are nicely aligned.
It is still possible for a very long instruction to intrude into the
disassembled text space. The next example is x86-64, before the
patch:
(gdb) disassemble /r main
Dump of assembler code for function main:
0x0000000000401106 <+0>: 55 push %rbp
0x0000000000401107 <+1>: 48 89 e5 mov %rsp,%rbp
0x000000000040110a <+4>: c7 87 d8 00 00 00 01 00 00 00 movl $0x1,0xd8(%rdi)
0x0000000000401114 <+14>: b8 00 00 00 00 mov $0x0,%eax
0x0000000000401119 <+19>: 5d pop %rbp
0x000000000040111a <+20>: c3 ret
End of assembler dump.
And after the patch:
(gdb) disassemble /r main
Dump of assembler code for function main:
0x0000000000401106 <+0>: 55 push %rbp
0x0000000000401107 <+1>: 48 89 e5 mov %rsp,%rbp
0x000000000040110a <+4>: c7 87 d8 00 00 00 01 00 00 00 movl $0x1,0xd8(%rdi)
0x0000000000401114 <+14>: b8 00 00 00 00 mov $0x0,%eax
0x0000000000401119 <+19>: 5d pop %rbp
0x000000000040111a <+20>: c3 ret
End of assembler dump.
Most instructions are aligned, except for the very long instruction.
Notice too that for x86-64 libopcodes doesn't request that GDB group
the instruction bytes. This matches the behaviour of objdump.
In case the user really wants the old behaviour, I have added a new
modifier 'disassemble /b', this displays the instruction byte at a
time. For x86-64, which never groups instruction bytes, /b and /r are
equivalent, but for RISC-V, using /b gets the old layout back (except
that the whitespace for alignment is still present). Consider our
original RISC-V example, this time using /b:
(gdb) disassemble /b 0x0001018e,0x0001019e
Dump of assembler code from 0x1018e to 0x1019e:
0x0001018e <call_me+66>: 03 26 84 fe lw a2,-24(s0)
0x00010192 <call_me+70>: 83 25 c4 fe lw a1,-20(s0)
0x00010196 <call_me+74>: 61 65 lui a0,0x18
0x00010198 <call_me+76>: 13 05 85 6a addi a0,a0,1704
0x0001019c <call_me+80>: f1 22 jal 0x10368 <printf>
End of assembler dump.
Obviously, this patch is a potentially significant change to the
behaviour or /r. I could have added /b with the new behaviour and
left /r alone. However, personally, I feel the new behaviour is
significantly better than the old, hence, I made /r be what I consider
the "better" behaviour.
The reason I prefer the new behaviour is that, when I use /r, I almost
always want to manually decode the instruction for some reason, and
having the bytes displayed in "instruction order" rather than memory
order, just makes this easier.
The 'record instruction-history' command also takes a /r modifier, and
has been modified in the same way as disassemble; /r gets the new
behaviour, and /b has been added to retain the old behaviour.
Finally, the MI command -data-disassemble, is unchanged in behaviour,
this command now requests the raw bytes of the instruction, which is
equivalent to the /b modifier. This means that the MI output will
remain backward compatible.
I noticed a couple of initialization functions that aren't really
needed, and that currently require explicit calls in gdb_init. This
patch removes these functions, simplifying gdb a little.
Regression tested on x86-64 Fedora 34.
Currently, GDB internally uses the term "location" for both the
location specification the user input (linespec, explicit location, or
an address location), and for actual resolved locations, like the
breakpoint locations, or the result of decoding a location spec to
SaLs. This is expecially confusing in the breakpoints module, as
struct breakpoint has these two fields:
breakpoint::location;
breakpoint::loc;
"location" is the location spec, and "loc" is the resolved locations.
And then, we have a method called "locations()", which returns the
resolved locations as range...
The location spec type is presently called event_location:
/* Location we used to set the breakpoint. */
event_location_up location;
and it is described like this:
/* The base class for all an event locations used to set a stop event
in the inferior. */
struct event_location
{
and even that is incorrect... Location specs are used for finding
actual locations in the program in scenarios that have nothing to do
with stop events. E.g., "list" works with location specs.
To clean all this confusion up, this patch renames "event_location" to
"location_spec" throughout, and then all the variables that hold a
location spec, they are renamed to include "spec" in their name, like
e.g., "location" -> "locspec". Similarly, functions that work with
location specs, and currently have just "location" in their name are
renamed to include "spec" in their name too.
Change-Id: I5814124798aa2b2003e79496e78f95c74e5eddca
Same idea as previous patch, but for symtab::objfile. I find
it clearer without this wrapper, as it shows that the objfile is
common to all symtabs of a given compunit. Otherwise, you could think
that each symtab (of a given compunit) can have a specific objfile.
Change-Id: Ifc0dbc7ec31a06eefa2787c921196949d5a6fcc6
I think the symtab::dirname method is bogus, or at least very
misleading. It makes you think that it returns the directory that was
used to find that symtab's file during compilation (i.e. the directory
the file refers to in the DWARF line header file table), or the
directory part of the symtab's filename maybe. In fact, it returns the
compilation unit's directory, which is the CWD of the compiler, at
compilation time. At least for DWARF, if the symtab's filename is
relative, it will be relative to that directory. But if the symtab's
filename is absolute, then the directory returned by symtab::dirname has
nothing to do with the symtab's filename.
Remove symtab::dirname to avoid this confusion, change all users to
fetch the same information through the compunit. At least, it will be
clear that this is a compunit property, not a symtab property.
Change-Id: I2894c3bf3789d7359a676db3c58be2c10763f5f0
Various spots in gdb currently know about the wrap buffer, and so are
careful to call wrap_here to be certain that all output has been
flushed.
Now that the pager is just an ordinary stream, this isn't needed, and
a simple call to gdb_flush is enough.
Similarly, there are places where gdb prints to gdb_stderr, but first
flushes gdb_stdout. stderr_file already flushes gdb_stdout, so these
aren't needed.
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 vprintf family of functions: vprintf_filtered,
vprintf_unfiltered, vfprintf_filtered and vfprintf_unfiltered. (For
the gdb_stdout variants, recall that only printf_unfiltered gets truly
unfiltered output at this point.) This removes one such function and
renames the remaining two to "gdb_vprintf". All callers are updated.
Much of this patch was written by script.
This rewrites the output pager as a ui_file implementation.
A new header is introduced to declare the pager class. The
implementation remains in utils.c for the time being, because there
are some static globals there that must be used by this code. (This
could be cleaned up at some future date.)
I went through all the text output in gdb to ensure that this change
should be ok. There are a few cases:
* Any existing call to printf_unfiltered is required to be avoid the
pager. This is ensured directly in the implementation.
* All remaining calls to the f*_unfiltered functions -- the ones that
take an explicit ui_file -- either send to an unfiltered stream
(e.g., gdb_stderr), which is obviously ok; or conditionally send to
gdb_stdout
I investigated all such calls by searching for:
grep -e '\bf[a-z0-9_]*_unfiltered' *.[chyl] */*.[ch] | grep -v gdb_stdlog | grep -v gdb_stderr
This yields a number of candidates to check.
* The breakpoint _print_recreate family, and
save_trace_state_variables. These are used for "save" commands
and so are fine.
* Things printing to a temporary stream. Obviously ok.
* Disassembly selftests.
* print_gdb_help - this is non-obvious, but ok because paging isn't
yet enabled at this point during startup.
* serial.c - doens't use gdb_stdout
* The code in compile/. This is all printing to a file.
* DWARF DIE dumping - doesn't reference gdb_stdout.
* Calls to the _filtered form -- these are all clearly ok, because if
they are using gdb_stdout, then filtering will still apply; and if
not, then filtering never applied and still will not.
Therefore, at this point, there is no longer any distinction between
all the other _filtered and _unfiltered calls, and they can be
unified.
In this patch, take special note of the vfprintf_maybe_filtered and
ui_file::vprintf change. This is one instance of the above idea,
erasing the distinction between filtered and unfiltered -- in this
part of the change, the "unfiltered_output" flag is never passe to
cli_ui_out. Subsequent patches will go much further in this
direction.
Also note the can_emit_style_escape changes in ui-file.c. Checking
against gdb_stdout or gdb_stderr was always a bit of a hack; and now
it is no longer needed, because this is decision can be more fully
delegated to the particular ui_file implementation.
ui_file::can_page is removed, because this patch removed the only call
to it.
I think this is the main part of fixing PR cli/7234.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=7234
GDB already has a flag to suppress printing notification events, such
as thread and inferior context switches, on the CLI. This is used
internally when executing commands. Make the flag available to the
user via a new command. This is expected to be useful in scripts.
For instance, suppose that when Inferior 1 gets to a certain state,
you want to add and set up a new inferior using the commands below,
but you also want to have a reduced/clean output.
define do-setup
printf "Setting up Inferior 2...\n"
add-inferior -exec a.out
inferior 2
break file.c:3
run
inferior 1
printf "Done\n"
end
Currently, GDB prints
(gdb) do-setup
Setting up Inferior 2...
[New inferior 2]
Added inferior 2 on connection 1 (native)
[Switching to inferior 2 [<null>] (/tmp/a.out)]
Breakpoint 2 at 0x1155: file file.c, line 3.
Thread 2.1 "a.out" hit Breakpoint 2, main () at file.c:3
3 return 0;
[Switching to inferior 1 [process 7670] (/tmp/test)]
[Switching to thread 1.1 (process 7670)]
#0 main () at test.c:2
2 int a = 1;
Done
GDB's Python API make it possible to capture and return GDB's output,
but this does not work for all the streams. In particular, CLI
notification events are not captured:
(gdb) python gdb.execute("do-setup", False, True)
[Switching to inferior 2 [<null>] (/tmp/a.out)]
Thread 2.1 "a.out" hit Breakpoint 2, main () at file.c:3
3 return 0;
[Switching to inferior 1 [process 8263] (/tmp/test)]
[Switching to thread 1.1 (process 8263)]
#0 main () at test.c:2
2 int a = 1;
You can use the new "set suppress-cli-notifications" command to
suppress the output:
(gdb) set suppress-cli-notifications on
(gdb) do-setup
Setting up Inferior 2...
[New inferior 2]
Added inferior 2 on connection 1 (native)
Breakpoint 2 at 0x1155: file file.c, line 3.
Done
While working on the previous commit to fix PR cli/28665, I noticed
that the 'edit' command would suffer from the same problem. That is,
something like:
(gdb) edit task 123
would cause GDB to break. For a full explanation of what's going on
here, see the commit message for the previous commit.
As with the previous commit, this issue can be prevented by detecting,
and throwing, a junk at the end of the line error earlier, before
calling decode_line_1.
So, that's what this commit does. I've also added some tests for this
issue.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28665
In PR cli/28665, it was reported that GDB would crash when given a
command like:
(gdb) list task 123
The problem here is that in cli/cli-cmd.c:list_command, the string
'task 123' is passed to string_to_event_location in find a location
specification. However, this location parsing understands about
breakpoint conditions, and so, will stop parsing when it sees
something that looks like a condition, in this case, the 'task 123'
looks like a breakpoint condition.
As a result, the location we get back from string_to_event_location
has no actual location specification attached to it. The actual call
path is:
list_command
string_to_event_location
string_to_event_location_basic
new_linespec_location
In new_linespec_location we call linespec_lex_to_end, which looks at
'task 123' and decides that there's nothing there that describes a
location. As such, in new_linespec_location, the spec_string field of
the location is left as nullptr.
Back in list_command we then call decode_line_1, which calls
event_location_to_sals, which calls parse_linespec, which takes the
spec_string we found earlier, and tries to converts this into a list
of sals.
However, parse_linespec is not intended to be passed a nullptr, for
example, calling is_ada_operator will try to access through the
nullptr, causing undefined behaviour. But there are other cases
within parse_linespec which don't expect to see a nullptr.
When looking at how to fix this issue, I first considered having
linespec_lex_to_end detect the problem. That function understands
when the first thing in the linespec is a condition keyword, and so,
could throw an error saying something like: "no linespec before
condition keyword", however, this is not going to work, at least, not
without additional changes to GDB, it is valid to place a breakpoint
like:
(gdb) break task 123
This will place a breakpoint at the current location with the
condition 'task 123', and changing linespec_lex_to_end breaks this
behaviour.
So, next, I considered what would happen if I added a condition to an
otherwise valid list command, this is what I see:
(gdb) list file.c:1 task 123
Junk at end of line specification.
(gdb)
So, then I wondered, could we just pull the "Junk" detection forward,
so that we throw the error earlier, before we call decode_line_1?
It turns out that yes we can. Well, sort of.
It is simpler, I think, to add a separate check into the list_command
function, after calling string_to_event_location, but before calling
decode_line_1. We know when we call string_to_event_location that the
string in question is not empty, so, after calling
string_to_event_location, if non of the string has been consumed, then
the content of the string must be junk - it clearly doesn't look like
a location specification.
I've reused the same "Junk at end of line specification." error for
consistency, and added a few tests to cover this issue.
While the first version of this patch was on the mailing list, a
second bug PR gdb/28797 was raised. This was for a very similar
issue, but this time the problem command was:
(gdb) list ,,
Here the list command understands about the first comma, list can have
two arguments separated by a comma, and the first argument can be
missing. So we end up trying to parse the second command "," as a
linespec.
However, in linespec_lex_to_end, we will stop parsing a linespec at a
comma, so, in the above case we end up with an empty linespec (between
the two commas), and, like above, this results in the spec_string
being nullptr.
As with the previous case, I've resolved this issue by adding an extra
check for junk at the end of the line - after parsing (or failing to
parse) the nothing between the two commas, we still have the "," left
at the end of the list command line - when we see this we can throw
the same "junk at the end of the line" error, and all is good.
I've added tests for this case too.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28665
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28797
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.
Many otherwise ordinary commands choose to use unfiltered output
rather than filtered. I don't think there's any reason for this, so
this changes many such commands to use filtered output instead.
Note that complete_command is not touched due to a comment there
explaining why unfiltered output is believed to be used.
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.
This command adds the "exit" command as an alias for the "quit"
command, as requested in PR gdb/28406.
The documentation is also updated to mention this new command.
Tested on x86_64-linux.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28406
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 'show user' command (which shows the definition of non-python/scheme
user defined commands) is currently missing a completer. This is
mentioned in PR 16238. Having one can improve the user experience.
In this commit I propose an implementation for such completer as well as
the associated tests.
Tested on x86_64 GNU/Linux.
All feedbacks are welcome.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16238
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 bug fixed by this [1] patch was caused by an out-of-bounds access to
a value's content. The code gets the value's content (just a pointer)
and then indexes it with a non-sensical index.
This made me think of changing functions that return value contents to
return array_views instead of a plain pointer. This has the advantage
that when GDB is built with _GLIBCXX_DEBUG, accesses to the array_view
are checked, making bugs more apparent / easier to find.
This patch changes the return types of these functions, and updates
callers to call .data() on the result, meaning it's not changing
anything in practice. Additional work will be needed (which can be done
little by little) to make callers propagate the use of array_view and
reap the benefits.
[1] https://sourceware.org/pipermail/gdb-patches/2021-September/182306.html
Change-Id: I5151f888f169e1c36abe2cbc57620110673816f3
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
In some situations it is possible that a user might not want GDB to
try and access source code files, for example, the source code might
be stored on a slow to access network file system.
It is almost certainly possible that using some combination of 'set
directories' and/or 'set substitute-path' a user can trick GDB into
being unable to find the source files, but this feels like a rather
crude way to solve the problem.
In this commit a new option is add that stops GDB from opening and
reading the source files. A user can run with source code reading
disabled if this is required, then re-enable later if they decide
that they now want to view the source code.
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
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
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
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
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
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.
Before this patch:
(gdb) source ~/script.scm
ERROR: In procedure apply-smob/1:
ERROR: In procedure primitive-load-path: Unable to find file "~/script.scm" in load path
Error while executing Scheme code.
(gdb)
This is because the path is not tilde expanded. In contrast, when
sourcing a .py or .gdb script the path is tilde expanded.
This commit fixes this oversight, and allows the above source command
to work as expected.
The tilde expansion is done in the generic GDB code before we call the
sourcer function for any particular extension language.
gdb/ChangeLog:
* cli/cli-cmds.c: Add 'gdbsupport/gdb_tilde_expand.h'
include.
(source_script_with_search): Perform tilde expansion.
gdb/testsuite/ChangeLog:
* gdb.guile/guile.exp: Add an extra test.
Commands "set debug remote" and "set remotetimeout" are defined in
cli/cli-cmds.c, I think it would make more sense for them to be in
remote.c.
gdb/ChangeLog:
* cli/cli-cmds.c (show_remote_debug): Remove.
(show_remote_timeout): Remove.
(_initialize_cli_cmds): Don't register commands.
* remote.c (show_remote_debug): Move here.
(show_remote_timeout): Move here.
(_initialize_remote): Register commands.
Change-Id: Ic4d81888aa4f8dde89d1d29397ef19a08951b80b
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.
We currently have two flushing commands 'flushregs' and 'maint
flush-symbol-cache'. I'm planning to add at least one more so I
thought it might be nice if we bundled these together into one place.
And so I created the 'maint flush ' command prefix. Currently there
are two commands:
(gdb) maint flush symbol-cache
(gdb) maint flush register-cache
Unfortunately, even though both of the existing flush commands are
maintenance commands, I don't know how keen we about deleting existing
commands for fear of breaking things in the wild. So, both of the
existing flush commands 'maint flush-symbol-cache' and 'flushregs' are
still around as deprecated aliases to the new commands.
I've updated the testsuite to use the new command syntax, and updated
the documentation too.
gdb/ChangeLog:
* NEWS: Mention new commands, and that the old commands are now
deprecated.
* cli/cli-cmds.c (maintenanceflushlist): Define.
* cli/cli-cmds.h (maintenanceflushlist): Declare.
* maint.c (_initialize_maint_cmds): Initialise
maintenanceflushlist.
* regcache.c: Add 'cli/cli-cmds.h' include.
(reg_flush_command): Add header comment.
(_initialize_regcache): Create new 'maint flush register-cache'
command, make 'flushregs' an alias.
* symtab.c: Add 'cli/cli-cmds.h' include.
(_initialize_symtab): Create new 'maint flush symbol-cache'
command, make old command an alias.
gdb/doc/ChangeLog:
* gdb.texinfo (Symbols): Document 'maint flush symbol-cache'.
(Maintenance Commands): Document 'maint flush register-cache'.
gdb/testsuite/ChangeLog:
* gdb.base/c-linkage-name.exp: Update to use new 'maint flush ...'
commands.
* gdb.base/killed-outside.exp: Likewise.
* gdb.opt/inline-bt.exp: Likewise.
* gdb.perf/gmonster-null-lookup.py: Likewise.
* gdb.perf/gmonster-print-cerr.py: Likewise.
* gdb.perf/gmonster-ptype-string.py: Likewise.
* gdb.python/py-unwind.exp: Likewise.
I noticed a couple of spots where the "disassemble" could style its
output, but currently does not. This patch adds styling to the
function name at the start of the disassembly, and any addresses
printed there.
gdb/ChangeLog
2020-10-08 Tom Tromey <tom@tromey.com>
* cli/cli-cmds.c (print_disassembly): Style function name and
addresses. Add _() wrappers.
gdb/testsuite/ChangeLog
2020-10-08 Tom Tromey <tom@tromey.com>
* gdb.base/style.exp: Check that "main"'s name is styled.
Andrew Burgess pointed out a regression, which he described in
PR symtab/26270:
================
After commit:
commit bcfe6157ca (refs/bisect/bad)
Date: Fri Apr 24 15:35:01 2020 -0600
Use the linkage name if it exists
The disassembler no longer demangles function names in its output. So
we see things like this:
(gdb) disassemble tree_insert
Dump of assembler code for function _Z11tree_insertP4nodei:
....
Instead of this:
(gdb) disassemble tree_insert
Dump of assembler code for function tree_insert(node*, int):
....
This is because find_pc_partial_function now returns the linkage name
rather than the demangled name.
================
This patch fixes the problem by introducing a new "overload" of
find_pc_partial_function, which returns the general_symbol_info rather
than simply the name. This lets the disassemble command choose which
name to show.
Regression tested on x86-64 Fedora 32.
gdb/ChangeLog
2020-07-28 Tom Tromey <tromey@adacore.com>
PR symtab/26270:
* symtab.h (find_pc_partial_function_sym): Declare.
* cli/cli-cmds.c (disassemble_command): Use
find_pc_partial_function_sym. Check asm_demangle.
* blockframe.c (cache_pc_function_sym): New global.
(cache_pc_function_name): Remove.
(clear_pc_function_cache): Update.
(find_pc_partial_function_sym): New function, from
find_pc_partial_function.
(find_pc_partial_function): Rewrite using
find_pc_partial_function_sym.
gdb/testsuite/ChangeLog
2020-07-28 Andrew Burgess <andrew.burgess@embecosm.com>
PR symtab/26270:
* gdb.cp/disasm-func-name.cc: New file.
* gdb.cp/disasm-func-name.exp: New file.
Pedro pointed out that disassemble/m should be documented after
disassemble/s, because /m is deprecated. This patch does so, and adds
a usage line.
Regression tested on x86-64 Fedora 32.
gdb/ChangeLog
2020-07-28 Tom Tromey <tromey@adacore.com>
* cli/cli-cmds.c (_initialize_cli_cmds): Rearrange "disassemble"
help. Add usage.
