This changes gdb to use the C++17 [[fallthrough]] attribute rather
than special comments.
This was mostly done by script, but I neglected a few spellings and so
also fixed it up by hand.
I suspect this fixes the bug mentioned below, by switching to a
standard approach that, presumably, clang supports.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=23159
Approved-By: John Baldwin <jhb@FreeBSD.org>
Approved-By: Luis Machado <luis.machado@arm.com>
Approved-By: Pedro Alves <pedro@palves.net>
Since GDB now requires C++17, we don't need the internally maintained
gdb::optional implementation. This patch does the following replacing:
- gdb::optional -> std::optional
- gdb::in_place -> std::in_place
- #include "gdbsupport/gdb_optional.h" -> #include <optional>
This change has mostly been done automatically. One exception is
gdbsupport/thread-pool.* which did not use the gdb:: prefix as it
already lives in the gdb namespace.
Change-Id: I19a92fa03e89637bab136c72e34fd351524f65e9
Approved-By: Tom Tromey <tom@tromey.com>
Approved-By: Pedro Alves <pedro@palves.net>
gdb::make_unique is a wrapper around std::make_unique when compiled with
C++17. Now that C++17 is required, use std::make_unique directly in the
codebase, and remove gdb::make_unique.
Change-Id: I80b615e46e4b7c097f09d78e579a9bdce00254ab
Approved-By: Tom Tromey <tom@tromey.com>
Approved-By: Pedro Alves <pedro@palves.net
Remove get_current_regcache, inlining the call to get_thread_regcache in
callers. When possible, pass the right thread_info object known from
the local context. Otherwise, fall back to passing `inferior_thread ()`.
This makes the reference to global context bubble up one level, a small
step towards the long term goal of reducing the number of references to
global context (or rather, moving those references as close as possible
to the top of the call tree).
No behavior change expected.
Change-Id: Ifa6980c88825d803ea586546b6b4c633c33be8d6
Now that so_list lists are implemented using intrusive_list, it doesn't
really make sense for the element type to be named "_list". Rename to
just `struct shobj` (`struct so` was deemed to be not greppable enough).
Change-Id: I1063061901298bb40fee73bf0cce44cd12154c0e
Approved-By: Pedro Alves <pedro@palves.net>
Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
Replace the hand-made linked list implementation with intrusive_list,
simplying management of list items.
Change-Id: I7f55fd88325bb197cc655c9be5a2ec966d8cc48d
Approved-By: Pedro Alves <pedro@palves.net>
Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
Change these two fields, simplifying memory management and copying.
Change-Id: If2559284c515721e71e1ef56ada8b64667eebe55
Approved-By: Pedro Alves <pedro@palves.net>
Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
A subsequent patch changes so_list to be linked using
intrusive_list. Iterating an intrusive_list yields some references to
the list elements. Convert some functions accepting so_list objects to
take references, to make things easier and more natural. Add const
where possible and convenient.
Change-Id: Id5ab5339c3eb6432e809ad14782952d6a45806f3
Approved-By: Pedro Alves <pedro@palves.net>
Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com>
This function is just a wrapper around the current inferior's gdbarch.
I find that having that wrapper just obscures where the arch is coming
from, and that it's often used as "I don't know which arch to use so
I'll use this magical target_gdbarch function that gets me an arch" when
the arch should in fact come from something in the context (a thread,
objfile, symbol, etc). I think that removing it and inlining
`current_inferior ()->arch ()` everywhere will make it a bit clearer
where that arch comes from and will trigger people into reflecting
whether this is the right place to get the arch or not.
Change-Id: I79f14b4e4934c88f91ca3a3155f5fc3ea2fadf6b
Reviewed-By: John Baldwin <jhb@FreeBSD.org>
Approved-By: Andrew Burgess <aburgess@redhat.com>
I noticed a comment by an include and remembered that I think these
don't really provide much value -- sometimes they are just editorial,
and sometimes they are obsolete. I think it's better to just remove
them. Tested by rebuilding.
Approved-By: Andrew Burgess <aburgess@redhat.com>
This replaces some casts to 'tracepoint *' with checked_static_cast.
Some functions are changed to accept a 'tracepoint *' now, for better
type safety.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Following the commit f818c32ba4 ("gdb/mi: fix ^running record with
multiple MI interpreters"), I thought it would make sense to make
current_token a field of mi_interp. This variable contains the token of
the currently handled MI command, like the 222 in:
222-exec-continue
I didn't find any bug related to that, it's just a "that seems nicer"
cleanup, since the current token is a fundamentally per-interp thing.
mi_execute_command needs a check similar to what we already have in
mi_cmd_gdb_exit: when invoked from Python's gdb.execute_mi, the current
interpreter is not an mi_interp. When using the Python gdb.execute_mi
function, there is no such concept of token, so we can just skip that.
There should be no user-visible change.
Change-Id: Ib52b3c0cba4b7c9d805b432c809692a86e4945ad
Approved-By: Tom Tromey <tom@tromey.com>
I noticed that the "usage" error for -var-set-frozen mentioned the
wrong command name. Then I looked through the whole file and found a
couple other spots that didn't mention the command name at all. This
patch fixes all of these.
Reviewed-by: Kevin Buettner <kevinb@redhat.com>
Currently, each target backend is responsible for printing "[Thread
...exited]" before deleting a thread. This leads to unnecessary
differences between targets, like e.g. with the remote target, we
never print such messages, even though we do print "[New Thread ...]".
E.g., debugging the gdb.threads/attach-many-short-lived-threads.exp
with gdbserver, letting it run for a bit, and then pressing Ctrl-C, we
currently see:
(gdb) c
Continuing.
^C[New Thread 3850398.3887449]
[New Thread 3850398.3887500]
[New Thread 3850398.3887551]
[New Thread 3850398.3887602]
[New Thread 3850398.3887653]
...
Thread 1 "attach-many-sho" received signal SIGINT, Interrupt.
0x00007ffff7e6a23f in __GI___clock_nanosleep (clock_id=clock_id@entry=0, flags=flags@entry=0, req=req@entry=0x7fffffffda80, rem=rem@entry=0x7fffffffda80)
at ../sysdeps/unix/sysv/linux/clock_nanosleep.c:78
78 in ../sysdeps/unix/sysv/linux/clock_nanosleep.c
(gdb)
Above, we only see "New Thread" notifications, even though threads
were deleted.
After this patch, we'll see:
(gdb) c
Continuing.
^C[Thread 3558643.3577053 exited]
[Thread 3558643.3577104 exited]
[Thread 3558643.3577155 exited]
[Thread 3558643.3579603 exited]
...
[New Thread 3558643.3597415]
[New Thread 3558643.3600015]
[New Thread 3558643.3599965]
...
Thread 1 "attach-many-sho" received signal SIGINT, Interrupt.
0x00007ffff7e6a23f in __GI___clock_nanosleep (clock_id=clock_id@entry=0, flags=flags@entry=0, req=req@entry=0x7fffffffda80, rem=rem@entry=0x7fffffffda80)
at ../sysdeps/unix/sysv/linux/clock_nanosleep.c:78
78 in ../sysdeps/unix/sysv/linux/clock_nanosleep.c
(gdb) q
This commit fixes this by moving the thread exit printing to common
code instead, triggered from within delete_thread (or rather,
set_thread_exited).
There's one wrinkle, though. While most targest want to print:
[Thread ... exited]
the Windows target wants to print:
[Thread ... exited with code <exit_code>]
... and sometimes wants to suppress the notification for the main
thread. To address that, this commits adds a delete_thread_with_code
function, only used by that target (so far).
This fix was originally posted as part of a larger series:
https://inbox.sourceware.org/gdb-patches/20221212203101.1034916-1-pedro@palves.net/
But didn't really need to be part of that series. In order to get
this fix merged sooner, I (Andrew Burgess) have rebased this commit
outside of the original series. Any bugs introduced while splitting
this patch out and rebasing, are entirely my own.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30129
Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
Remove the static mi_parse::make functions, and instead use the
mi_parse constructor.
This is a partial revert of the commit:
commit fde3f93adb
Date: Mon Mar 20 10:56:55 2023 -0600
Introduce "static constructor" for mi_parse
which introduced the mi_parse::make functions, though after discussion
on the list the reasons for seem to have been lost[1]. Given there
are no test regressions when moving back to using the constructors, I
propose we should do that for now.
There should be no user visible changes after this commit.
[1] https://inbox.sourceware.org/gdb-patches/20230404-dap-loaded-sources-v2-5-93f229095e03@adacore.com/
Approved-By: Tom Tromey <tom@tromey.com>
Have the mi_out_new function return a std::unique_ptr instead of a raw
pointer. Update the two uses of mi_out_new.
There should be no user visible changes after this commit.
Approved-By: Tom Tromey <tom@tromey.com>
This commit extends the breakpoint mechanism to allow for inferior
specific breakpoints (but not watchpoints in this commit).
As GDB gains better support for multiple connections, and so for
running multiple (possibly unrelated) inferiors, then it is not hard
to imagine that a user might wish to create breakpoints that apply to
any thread in a single inferior. To achieve this currently, the user
would need to create a condition possibly making use of the $_inferior
convenience variable, which, though functional, isn't the most user
friendly.
This commit adds a new 'inferior' keyword that allows for the creation
of inferior specific breakpoints.
Inferior specific breakpoints are automatically deleted when the
associated inferior is removed from GDB, this is similar to how
thread-specific breakpoints are deleted when the associated thread is
deleted.
Watchpoints are already per-program-space, which in most cases mean
watchpoints are already inferior specific. There is a small window
where inferior-specific watchpoints might make sense, which is after a
vfork, when two processes are sharing the same address space.
However, I'm leaving that as an exercise for another day. For now,
attempting to use the inferior keyword with a watchpoint will give an
error, like this:
(gdb) watch a8 inferior 1
Cannot use 'inferior' keyword with watchpoints
A final note on the implementation: currently, inferior specific
breakpoints, like thread-specific breakpoints, are inserted into every
inferior, GDB then checks once the inferior stops if we are in the
correct thread or inferior, and resumes automatically if we stopped in
the wrong thread/inferior.
An obvious optimisation here is to only insert breakpoint locations
into the specific program space (which mostly means inferior) that
contains either the inferior or thread we are interested in. This
would reduce the number times GDB has to stop and then resume again in
a multi-inferior setup.
I have a series on the mailing list[1] that implements this
optimisation for thread-specific breakpoints. Once this series has
landed I'll update that series to also handle inferior specific
breakpoints in the same way. For now, inferior specific breakpoints
are just slightly less optimal, but this is no different to
thread-specific breakpoints in a multi-inferior debug session, so I
don't see this as a huge problem.
[1] https://inbox.sourceware.org/gdb-patches/cover.1685479504.git.aburgess@redhat.com/
Simon noticed a crash that could be caused via new Python
gdb.execute_mi function. Looking into this, I found a few unchecked
casts to mi_interp, like:
- struct mi_interp *mi = (struct mi_interp *) command_interp ();
This patch replaces all such casts with safer variants.
For -gdb-exit and mi_load_progress, I chose to have the functions
simply not generate any output. It didn't seem useful to do so.
Some casts I eliminated by adding a parameter to a function. Then, in
mi_execute_command, I changed the code to use
gdb::checked_static_cast. This is appropriate because this particular
overload can only be called by the MI interpreter.
There does not seem to be a very good way to test -gdb-exit.
Regression tested on x86-64 Fedora 36.
This changes mi_parse::command to be a unique_xmalloc_ptr and fixes up
all the uses. This avoids some manual memory management. std::string
is not used here due to how the Python API works -- this approach
avoids an extra copy there.
Reviewed-by: Keith Seitz <keiths@redhat.com>
This changes the MI "token" to be a std::string, removing some manual
memory management. It also makes current_token 'const' in order to
support this change.
Reviewed-by: Keith Seitz <keiths@redhat.com>
I found the documentation for -dprintf-insert a bit unclear. It
didn't mention the possibility of multiple arguments, and I also
noticed that it implied that the format parameter is optional, which
it is not.
While looking into this I also noticed a few comments in the
implementation that could also be improved.
Then, I noticed a repeated call to strlen in a loop condition, so I
fixed this up as well.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Same idea as previous patches, but for about_to_proceed. We only need
(and want, as far as the mi_interp implementation is concerned) to
notify the interpreter that caused the proceed.
Change-Id: Id259bca10dbc3d43d46607ff7b95243a9cbe2f89
Same idea as previous patches, but for inferior_disappeared.
For symmetry with on_inferior_appeared, I named this one
on_inferior_disappeared, despite the observer being called
inferior_exit. This is called when detaching an inferior, so I think
that calling it "disappeared" is a bit less misleading (the observer
should probably be renamed later).
Change-Id: I372101586bc9454997953c1e540a2a6685f53ef6
Same idea as previous patches, but for inferior_added.
mi_interp::init avoided using mi_inferior_added, since, as the comment
used to say, it would notify all MI interpreters. Now, it's easy to
only notify the new interpreter, so it's possible to just call the
on_inferior_added method in mi_interp::init.
Change-Id: I0eddbd5367217d1c982516982089913019ef309f
Same as previous patches, but for sync_execution_done. Except that
here, we only want to notify the interpreter that is executing the
command, not all interpreters.
Change-Id: I729c719447b5c5f29af65dbf6fed9132e2cd308b
