mirror of
https://github.com/espressif/binutils-gdb.git
synced 2025-12-19 01:19:41 +08:00
6e7c49c2dd9ab0803894925c2ec422ea5aeddfb8
803 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Gustavo Romero
|
7202f41f5f |
gdb: Introduce is_address_tagged target hook
This commit introduces a new target hook, target_is_address_tagged, which is used instead of the gdbarch_tagged_address_p gdbarch hook in the upper layer (printcmd.c). This change enables easy specialization of memory tagging address check per target in the future. As target_is_address_tagged continues to utilize the gdbarch_tagged_address_p hook, there is no change in behavior for all the targets that use the new target hook (i.e., the remote.c, aarch64-linux-nat.c, and corelow.c targets). Just the gdbarch_tagged_address_p signature is changed for convenience, since target_is_address_tagged takes the address to be checked as a CORE_ADDR type. Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org> Approved-By: Luis Machado <luis.machado@arm.com> Tested-By: Luis Machado <luis.machado@arm.com> |
||
|
Andrew Burgess
|
1d506c26d9 |
Update copyright year range in header of all files managed by GDB
This commit is the result of the following actions:
- Running gdb/copyright.py to update all of the copyright headers to
include 2024,
- Manually updating a few files the copyright.py script told me to
update, these files had copyright headers embedded within the
file,
- Regenerating gdbsupport/Makefile.in to refresh it's copyright
date,
- Using grep to find other files that still mentioned 2023. If
these files were updated last year from 2022 to 2023 then I've
updated them this year to 2024.
I'm sure I've probably missed some dates. Feel free to fix them up as
you spot them.
|
||
|
Lancelot Six
|
6b09f1342c |
gdb: Replace gdb::optional with std::optional
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> |
||
|
Simon Marchi
|
7438771288 |
gdb: remove regcache's address space
While looking at the regcache code, I noticed that the address space (passed to regcache when constructing it, and available through regcache::aspace) wasn't relevant for the regcache itself. Callers of regcache::aspace use that method because it appears to be a convenient way of getting the address space for a thread, if you already have the regcache. But there is always another way to get the address space, as the callers pretty much always know which thread they are dealing with. The regcache code itself doesn't use the address space. This patch removes anything related to address_space from the regcache code, and updates callers to get it from the thread in context. This removes a bit of unnecessary complexity from the regcache code. The current get_thread_arch_regcache function gets an address_space for the given thread using the target_thread_address_space function (which calls the target_ops::thread_address_space method). This suggest that there might have been the intention of supporting per-thread address spaces. But digging through the history, I did not find any such case. Maybe this method was just added because we needed a way to get an address space from a ptid (because constructing a regcache required an address space), and this seemed like the right way to do it, I don't know. The only implementations of thread_address_space and process_stratum_target::thread_address_space and linux_nat_target::thread_address_space, which essentially just return the inferior's address space. And thread_address_space is only used in the current get_thread_arch_regcache, which gets removed. So, I think that the thread_address_space target method can be removed, and we can assume that it's fine to use the inferior's address space everywhere. Callers of regcache::aspace are updated to get the address space from the relevant inferior, either using some context they already know about, or in last resort using the current global context. So, to summarize: - remove everything in regcache related to address spaces - in particular, remove get_thread_arch_regcache, and rename get_thread_arch_aspace_regcache to get_thread_arch_regcache - remove target_ops::thread_address_space, and target_thread_address_space - adjust all users of regcache::aspace to get the address space another way Change-Id: I04fd41b22c83fe486522af7851c75bcfb31c88c7 |
||
|
Pedro Alves
|
65c459abeb |
Thread options & clone events (core + remote)
A previous patch taught GDB about a new TARGET_WAITKIND_THREAD_CLONED event kind, and made the Linux target report clone events. A following patch will teach Linux GDBserver to do the same thing. However, for remote debugging, it wouldn't be ideal for GDBserver to report every clone event to GDB, when GDB only cares about such events in some specific situations. Reporting clone events all the time would be potentially chatty. We don't enable thread create/exit events all the time for the same reason. Instead we have the QThreadEvents packet. QThreadEvents is target-wide, though. This patch makes GDB instead explicitly request that the target reports clone events or not, on a per-thread basis. In order to be able to do that with GDBserver, we need a new remote protocol feature. Since a following patch will want to enable thread exit events on per-thread basis too, the packet introduced here is more generic than just for clone events. It lets you enable/disable a set of options at once, modelled on Linux ptrace's PTRACE_SETOPTIONS. IOW, this commit introduces a new QThreadOptions packet, that lets you specify a set of per-thread event options you want to enable. The packet accepts a list of options/thread-id pairs, similarly to vCont, processed left to right, with the options field being a number interpreted as a bit mask of options. The only option defined in this commit is GDB_THREAD_OPTION_CLONE (0x1), which ask the remote target to report clone events. Another patch later in the series will introduce another option. For example, this packet sets option "1" (clone events) on thread p1000.2345: QThreadOptions;1:p1000.2345 and this clears options for all threads of process 1000, and then sets option "1" (clone events) on thread p1000.2345: QThreadOptions;0:p1000.-1;1:p1000.2345 This clears options of all threads of all processes: QThreadOptions;0 The target reports the set of supported options by including "QThreadOptions=<supported options>" in its qSupported response. infrun is then tweaked to enable GDB_THREAD_OPTION_CLONE when stepping over a breakpoint. Unlike PTRACE_SETOPTIONS, fork/vfork/clone children do NOT inherit their parent's thread options. This is so that GDB can send e.g., "QThreadOptions;0;1:TID" without worrying about threads it doesn't know about yet. Documentation for this new remote protocol feature is included in a documentation patch later in the series. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=19675 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27830 Reviewed-By: Andrew Burgess <aburgess@redhat.com> Change-Id: Ie41e5093b2573f14cf6ac41b0b5804eba75be37e |
||
|
Pedro Alves
|
0d36baa9af |
Step over clone syscall w/ breakpoint, TARGET_WAITKIND_THREAD_CLONED
(A good chunk of the problem statement in the commit log below is
Andrew's, adjusted for a different solution, and for covering
displaced stepping too. The testcase is mostly Andrew's too.)
This commit addresses bugs gdb/19675 and gdb/27830, which are about
stepping over a breakpoint set at a clone syscall instruction, one is
about displaced stepping, and the other about in-line stepping.
Currently, when a new thread is created through a clone syscall, GDB
sets the new thread running. With 'continue' this makes sense
(assuming no schedlock):
- all-stop mode, user issues 'continue', all threads are set running,
a newly created thread should also be set running.
- non-stop mode, user issues 'continue', other pre-existing threads
are not affected, but as the new thread is (sort-of) a child of the
thread the user asked to run, it makes sense that the new threads
should be created in the running state.
Similarly, if we are stopped at the clone syscall, and there's no
software breakpoint at this address, then the current behaviour is
fine:
- all-stop mode, user issues 'stepi', stepping will be done in place
(as there's no breakpoint to step over). While stepping the thread
of interest all the other threads will be allowed to continue. A
newly created thread will be set running, and then stopped once the
thread of interest has completed its step.
- non-stop mode, user issues 'stepi', stepping will be done in place
(as there's no breakpoint to step over). Other threads might be
running or stopped, but as with the continue case above, the new
thread will be created running. The only possible issue here is
that the new thread will be left running after the initial thread
has completed its stepi. The user would need to manually select
the thread and interrupt it, this might not be what the user
expects. However, this is not something this commit tries to
change.
The problem then is what happens when we try to step over a clone
syscall if there is a breakpoint at the syscall address.
- For both all-stop and non-stop modes, with in-line stepping:
+ user issues 'stepi',
+ [non-stop mode only] GDB stops all threads. In all-stop mode all
threads are already stopped.
+ GDB removes s/w breakpoint at syscall address,
+ GDB single steps just the thread of interest, all other threads
are left stopped,
+ New thread is created running,
+ Initial thread completes its step,
+ [non-stop mode only] GDB resumes all threads that it previously
stopped.
There are two problems in the in-line stepping scenario above:
1. The new thread might pass through the same code that the initial
thread is in (i.e. the clone syscall code), in which case it will
fail to hit the breakpoint in clone as this was removed so the
first thread can single step,
2. The new thread might trigger some other stop event before the
initial thread reports its step completion. If this happens we
end up triggering an assertion as GDB assumes that only the
thread being stepped should stop. The assert looks like this:
infrun.c:5899: internal-error: int finish_step_over(execution_control_state*): Assertion `ecs->event_thread->control.trap_expected' failed.
- For both all-stop and non-stop modes, with displaced stepping:
+ user issues 'stepi',
+ GDB starts the displaced step, moves thread's PC to the
out-of-line scratch pad, maybe adjusts registers,
+ GDB single steps the thread of interest, [non-stop mode only] all
other threads are left as they were, either running or stopped.
In all-stop, all other threads are left stopped.
+ New thread is created running,
+ Initial thread completes its step, GDB re-adjusts its PC,
restores/releases scratchpad,
+ [non-stop mode only] GDB resumes the thread, now past its
breakpoint.
+ [all-stop mode only] GDB resumes all threads.
There is one problem with the displaced stepping scenario above:
3. When the parent thread completed its step, GDB adjusted its PC,
but did not adjust the child's PC, thus that new child thread
will continue execution in the scratch pad, invoking undefined
behavior. If you're lucky, you see a crash. If unlucky, the
inferior gets silently corrupted.
What is needed is for GDB to have more control over whether the new
thread is created running or not. Issue #1 above requires that the
new thread not be allowed to run until the breakpoint has been
reinserted. The only way to guarantee this is if the new thread is
held in a stopped state until the single step has completed. Issue #3
above requires that GDB is informed of when a thread clones itself,
and of what is the child's ptid, so that GDB can fixup both the parent
and the child.
When looking for solutions to this problem I considered how GDB
handles fork/vfork as these have some of the same issues. The main
difference between fork/vfork and clone is that the clone events are
not reported back to core GDB. Instead, the clone event is handled
automatically in the target code and the child thread is immediately
set running.
Note we have support for requesting thread creation events out of the
target (TARGET_WAITKIND_THREAD_CREATED). However, those are reported
for the new/child thread. That would be sufficient to address in-line
stepping (issue #1), but not for displaced-stepping (issue #3). To
handle displaced-stepping, we need an event that is reported to the
_parent_ of the clone, as the information about the displaced step is
associated with the clone parent. TARGET_WAITKIND_THREAD_CREATED
includes no indication of which thread is the parent that spawned the
new child. In fact, for some targets, like e.g., Windows, it would be
impossible to know which thread that was, as thread creation there
doesn't work by "cloning".
The solution implemented here is to model clone on fork/vfork, and
introduce a new TARGET_WAITKIND_THREAD_CLONED event. This event is
similar to TARGET_WAITKIND_FORKED and TARGET_WAITKIND_VFORKED, except
that we end up with a new thread in the same process, instead of a new
thread of a new process. Like FORKED and VFORKED, THREAD_CLONED
waitstatuses have a child_ptid property, and the child is held stopped
until GDB explicitly resumes it. This addresses the in-line stepping
case (issues #1 and #2).
The infrun code that handles displaced stepping fixup for the child
after a fork/vfork event is thus reused for THREAD_CLONE, with some
minimal conditions added, addressing the displaced stepping case
(issue #3).
The native Linux backend is adjusted to unconditionally report
TARGET_WAITKIND_THREAD_CLONED events to the core.
Following the follow_fork model in core GDB, we introduce a
target_follow_clone target method, which is responsible for making the
new clone child visible to the rest of GDB.
Subsequent patches will add clone events support to the remote
protocol and gdbserver.
displaced_step_in_progress_thread becomes unused with this patch, but
a new use will reappear later in the series. To avoid deleting it and
readding it back, this patch marks it with attribute unused, and the
latter patch removes the attribute again. We need to do this because
the function is static, and with no callers, the compiler would warn,
(error with -Werror), breaking the build.
This adds a new gdb.threads/stepi-over-clone.exp testcase, which
exercises stepping over a clone syscall, with displaced stepping vs
inline stepping, and all-stop vs non-stop. We already test stepping
over clone syscalls with gdb.base/step-over-syscall.exp, but this test
uses pthreads, while the other test uses raw clone, and this one is
more thorough. The testcase passes on native GNU/Linux, but fails
against GDBserver. GDBserver will be fixed by a later patch in the
series.
Co-authored-by: Andrew Burgess <aburgess@redhat.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=19675
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27830
Change-Id: I95c06024736384ae8542a67ed9fdf6534c325c8e
Reviewed-By: Andrew Burgess <aburgess@redhat.com>
|
||
|
Simon Marchi
|
25b5a04e85 |
gdb: remove target_section_table typedef
Remove this typedef. I think that hiding the real type (std::vector) behind a typedef just hinders readability. Change-Id: I80949da3392f60a2826c56c268e0ec6f503ad79f Approved-By: Pedro Alves <pedro@palves.net> Reviewed-By: Reviewed-By: Lancelot Six <lancelot.six@amd.com> |
||
|
Simon Marchi
|
99d9c3b92c |
gdb: remove target_gdbarch
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> |
||
|
Tom Tromey
|
ef0f16ccf8 |
Remove explanatory comments from includes
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> |
||
|
Tom Tromey
|
01bccc56af |
Use gdb::checked_static_cast for tracepoints
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> |
||
|
Tom Tromey
|
358be6e72d |
Read Ada main name from executable, not inferior
An upstream bug report points out this bug: if the user switches from one Ada executable to another without "kill"ing the inferior, then the "start" command will fail. What happens here is that the Ada "main" name is found in a constant string in the executable. But, if the inferior is running, then the process_stratum target reads from the inferior memory. This patch fixes the problem by changing the main name code to set trust-readonly-sections, causing the target stack to read from the executable instead. I looked briefly at changing GNAT to emit DW_AT_main_subprogram instead, but this looks to be pretty involved. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=25811 |
||
|
John Baldwin
|
a388ab0b86 |
gdb: Store an x86_xsave_layout in i386_gdbarch_tdep.
This structure is fetched from the current target in i386_gdbarch_init via a new "fetch_x86_xsave_layout" target method. Approved-By: Simon Marchi <simon.marchi@efficios.com> |
||
|
Tom de Vries
|
1f08d32460 |
[gdb/build] Return gdb::array_view in thread_info_to_thread_handle
In remote_target::thread_info_to_thread_handle we return a copy:
...
gdb::byte_vector
remote_target::thread_info_to_thread_handle (struct thread_info *tp)
{
remote_thread_info *priv = get_remote_thread_info (tp);
return priv->thread_handle;
}
...
Fix this by returning a gdb::array_view instead:
...
gdb::array_view<const gdb_byte>
remote_target::thread_info_to_thread_handle (struct thread_info *tp)
...
Tested on x86_64-linux.
This fixes the build when building with -std=c++20.
Approved-By: Pedro Alves <pedro@palves.net>
|
||
|
Tom Tromey
|
5768573861 |
Remove target_close
I noticed that target_close is only called in two places: solib-svr4.c, and target_ops_ref_policy::decref. This patch fixes the former by changing target_bfd_reopen to return a target_ops_up and then fixing the sole caller. Then it removes target_close by inlining its body into the decref method. The advantage of this approach is that targets are now automatically managed. Regression tested on x86-64 Fedora 38. Approved-By: Andrew Burgess <aburgess@redhat.com> |
||
|
Tom de Vries
|
33b5899fc0 |
[gdb] Fix typos
Fix a few typos: - implemention -> implementation - convertion(s) -> conversion(s) - backlashes -> backslashes - signoring -> ignoring - (un)ambigious -> (un)ambiguous - occured -> occurred - hidding -> hiding - temporarilly -> temporarily - immediatelly -> immediately - sillyness -> silliness - similiar -> similar - porkuser -> pokeuser - thats -> that - alway -> always - supercede -> supersede - accomodate -> accommodate - aquire -> acquire - priveleged -> privileged - priviliged -> privileged - priviledges -> privileges - privilige -> privilege - recieve -> receive - (p)refered -> (p)referred - succesfully -> successfully - successfuly -> successfully - responsability -> responsibility - wether -> whether - wich -> which - disasbleable -> disableable - descriminant -> discriminant - construcstor -> constructor - underlaying -> underlying - underyling -> underlying - structureal -> structural - appearences -> appearances - terciarily -> tertiarily - resgisters -> registers - reacheable -> reachable - likelyhood -> likelihood - intepreter -> interpreter - disassemly -> disassembly - covnersion -> conversion - conviently -> conveniently - atttribute -> attribute - struction -> struct - resonable -> reasonable - popupated -> populated - namespaxe -> namespace - intialize -> initialize - identifer(s) -> identifier(s) - expection -> exception - exectuted -> executed - dungerous -> dangerous - dissapear -> disappear - completly -> completely - (inter)changable -> (inter)changeable - beakpoint -> breakpoint - automativ -> automatic - alocating -> allocating - agressive -> aggressive - writting -> writing - reguires -> requires - registed -> registered - recuding -> reducing - opeartor -> operator - ommitted -> omitted - modifing -> modifying - intances -> instances - imbedded -> embedded - gdbaarch -> gdbarch - exection -> execution - direcive -> directive - demanged -> demangled - decidely -> decidedly - argments -> arguments - agrument -> argument - amespace -> namespace - targtet -> target - supress(ed) -> suppress(ed) - startum -> stratum - squence -> sequence - prompty -> prompt - overlow -> overflow - memember -> member - languge -> language - geneate -> generate - funcion -> function - exising -> existing - dinking -> syncing - destroh -> destroy - clenaed -> cleaned - changep -> changedp (name of variable) - arround -> around - aproach -> approach - whould -> would - symobl -> symbol - recuse -> recurse - outter -> outer - freeds -> frees - contex -> context Tested on x86_64-linux. Reviewed-By: Tom Tromey <tom@tromey.com> |
||
|
Joel Brobecker
|
213516ef31 |
Update copyright year range in header of all files managed by GDB
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. |
||
|
Andrew Burgess
|
c8181f706f |
gdb: remove the pop_all_targets (and friends) global functions
This commit removes the global functions pop_all_targets, pop_all_targets_above, and pop_all_targets_at_and_above, and makes them methods on the inferior class. As the pop_all_targets functions will unpush each target, which decrements the targets reference count, it is possible that the target might be closed. Right now, closing a target, in some cases, depends on the current inferior being set correctly, that is, to the inferior from which the target was popped. To facilitate this I have used switch_to_inferior_no_thread within the new methods. Previously it was the responsibility of the caller to ensure that the correct inferior was selected. In a couple of places (event-top.c and top.c) I have been able to remove a previous switch_to_inferior_no_thread call. In remote_unpush_target (remote.c) I have left the switch_to_inferior_no_thread call as it is required for the generic_mourn_inferior call. |
||
|
Andrew Burgess
|
9678f8fe97 |
gdb: remove decref_target
The decref_target function is not really needed. Calling target_ops::decref will just redirect to decref_target anyway, so why not just rename decref_target to target_ops::decref? That's what this commit does. It's not exactly renaming to target_ops::decref, because the decref functionality is handled by a policy class, so the new name is now target_ops_ref_policy::decref. There should be no user visible change after this commit. |
||
|
Andrew Burgess
|
91e3d1d1a5 |
gdb: have target_stack automate reference count handling
This commit changes the target_stack class from using a C style array
of 'target_ops *' to using a C++ std::array<target_ops_ref, ...>. The
benefit of this change is that some of the reference counting of
target_ops objects is now done automatically.
This commit fixes a crash in gdb.python/py-inferior.exp where GDB
crashes at exit, leaving a core file behind.
The crash occurs in connpy_connection_dealloc, and is actually
triggered by this assert:
gdb_assert (conn_obj->target == nullptr);
Now a little aside...
... the assert is never actually printed, instead GDB crashes due
to calling a pure virtual function. The backtrace at the point of
crash looks like this:
#7 0x00007fef7e2cf747 in std::terminate() () from /lib64/libstdc++.so.6
#8 0x00007fef7e2d0515 in __cxa_pure_virtual () from /lib64/libstdc++.so.6
#9 0x0000000000de334d in target_stack::find_beneath (this=0x4934d78, t=0x2bda270 <the_dummy_target>) at ../../s>
#10 0x0000000000df4380 in inferior::find_target_beneath (this=0x4934b50, t=0x2bda270 <the_dummy_target>) at ../.>
#11 0x0000000000de2381 in target_ops::beneath (this=0x2bda270 <the_dummy_target>) at ../../src/gdb/target.c:3047
#12 0x0000000000de68aa in target_ops::supports_terminal_ours (this=0x2bda270 <the_dummy_target>) at ../../src/gd>
#13 0x0000000000dde6b9 in target_supports_terminal_ours () at ../../src/gdb/target.c:1112
#14 0x0000000000ee55f1 in internal_vproblem(internal_problem *, const char *, int, const char *, typedef __va_li>
Notice in frame #12 we called target_ops::supports_terminal_ours,
however, this is the_dummy_target, which is of type dummy_target,
and so we should have called dummy_target::supports_terminal_ours.
I believe the reason we ended up in the wrong implementation of
supports_terminal_ours (which is a virtual function) is because we
made the call during GDB's shut-down, and, I suspect, the vtables
were in a weird state.
Anyway, the point of this patch is not to fix GDB's ability to
print an assert during exit, but to address the root cause of the
assert. With that aside out of the way, we can return to the main
story...
Connections are represented in Python with gdb.TargetConnection
objects (or its sub-classes). The assert in question confirms that
when a gdb.TargetConnection is deallocated, the underlying GDB
connection has itself been removed from GDB. If this is not true then
we risk creating multiple different gdb.TargetConnection objects for
the same connection, which would be bad.
To ensure that we have one gdb.TargetConnection object for each
connection, the all_connection_objects map exists, this maps the
process_stratum_target object (the connection) to the
gdb.TargetConnection object that represents the connection.
When a connection is removed in GDB the connection_removed observer
fires, which we catch with connpy_connection_removed, this function
then sets conn_obj->target to nullptr, and removes the corresponding
entry from the all_connection_objects map.
The first issue here is that connpy_connection_dealloc is being called
as part of GDB's exit code, which is run after the Python interpreter
has been shut down. The connpy_connection_dealloc function is used to
deallocate the gdb.TargetConnection Python object. Surely it is
wrong for us to be deallocating Python objects after the interpreter
has been shut down.
The reason why connpy_connection_dealloc is called during GDB's exit
is that the global all_connection_objects map is still holding a
reference to the gdb.TargetConnection object. When the map is
destroyed during GDB's exit, the gdb.TargetConnection objects within
the map can finally be deallocated.
The reason why all_connection_objects has contents when GDB exits, and
the reason the assert fires, is that, when GDB exits, there are still
some connections that have not yet been removed from GDB, that is,
they have a non-zero reference count.
If we take a look at quit_force (top.c) you can see that, for each
inferior, we call pop_all_targets before we (later in the function)
call do_final_cleanups. It is the do_final_cleanups call that is
responsible for shutting down the Python interpreter. The
pop_all_targets calls should, in theory, cause all the connections to
be removed from GDB.
That this isn't working indicates that some targets have a non-zero
reference count even after this final pop_all_targets call, and
indeed, when I debug GDB, that is what I see.
I tracked the problem down to delete_inferior where we do some house
keeping, and then delete the inferior object, which calls
inferior::~inferior.
In neither delete_inferior or inferior::~inferior do we call
pop_all_targets, and it is this missing call that means we leak some
references to the target_ops objects on the inferior's target_stack.
In this commit I will provide a partial fix for the problem. I say
partial fix, but this will actually be enough to resolve the crash.
In a later commit I will provide the final part of the fix.
As mentioned at the start of the commit message, this commit changes
the m_stack in target_stack to hold target_ops_ref objects. This
means that when inferior::~inferior is called, and m_stack is
released, we automatically decrement the target_ops reference count.
With this change in place we no longer leak any references, and now,
in quit_force the final pop_all_targets calls will release the final
references. This means that the targets will be correctly closed at
this point, which means the connections will be removed from GDB and
the Python objects deallocated before the Python interpreter shuts
down.
There's a slight oddity in target_stack::unpush, where we std::move
the reference out of m_stack like this:
auto ref = std::move (m_stack[stratum]);
the `ref' isn't used explicitly, but it serves to hold the
target_ops_ref until the end of the scope while allowing the m_stack
entry to be reset back to nullptr. The alternative would be to
directly set the m_stack entry to nullptr, like this:
m_stack[stratum] = nullptr;
The problem here is that when we set the m_stack entry to nullptr we
first decrement the target_ops reference count, and then set the array
entry to nullptr.
If the decrement means that the target_ops object reaches a zero
reference count then the target_ops object will be closed by calling
target_close. In target_close we ensure that the target being closed
is not in any inferiors target_stack.
As we decrement before clearing, then this check in target_close will
fail, and an assert will trigger.
By using std::move to move the reference out of m_stack, this clears
the m_stack entry, meaning the inferior no longer contains the
target_ops in its target_stack. Now when the REF object goes out of
scope and the reference count is decremented, target_close can run
successfully.
I've made use of the Python connection_removed listener API to add a
test for this issue. The test installs a listener and then causes
delete_inferior to be called, we can then see that the connection is
then correctly removed (because the listener triggers).
|
||
|
Simon Marchi
|
3fe639b81b |
gdb: constify auxv parse functions
Constify the input parameters of the various auxv parse functions, they don't need to modify the raw auxv data. Change-Id: I13eacd5ab8e925ec2b5c1f7722cbab39c41516ec |
||
|
Simon Marchi
|
31282a8491 |
gdb: constify target_stack::is_pushed
The target_ops parameters here can be made const. Change-Id: Ibc18b17d6b21d06145251a03e68aca90538117d6 |
||
|
Simon Marchi
|
b872057a63 |
gdbsupport: convert FILEIO_* macros to an enum
Converting from free-form macros to an enum gives a bit of type-safety. This caught places where we would assign host error numbers to what should contain a target fileio error number, for instance in target_fileio_pread. I added the FILEIO_SUCCESS enumerator, because remote.c:remote_hostio_parse_result initializes the remote_errno output variable to 0. It seems better to have an explicit enumerator than to assign a value for which there is no enumerator. I considered initializing this variable to FILEIO_EUNKNOWN instead, such that if the remote side replies with an error and omits the errno value, we'll get an errno that represents an error instead of 0 (which reprensents no error). But it's not clear what the consequences of that change would be, so I prefer to err on the side of caution and just keep the existing behavior (there is no intended change in behavior with this patch). Note that remote_hostio_parse_resul still reads blindly what the remote side sends as a target errno into this variable, so we can still end up with a nonsensical value here. It's not good, but out of the scope of this patch. Convert host_to_fileio_error and fileio_errno_to_host to return / accept a fileio_error instead of an int, and cascade the change in the whole chain that uses that. Change-Id: I454b0e3fcf0732447bc872252fa8e57d138b0e03 |
||
|
Tom Tromey
|
4a570176b4 |
Change target_ops::async to accept bool
This changes the parameter of target_ops::async from int to bool. Regression tested on x86-64 Fedora 34. |
||
|
Tom Tromey
|
0e90c44162 |
Constify target_pid_to_exec_file
This changes target_pid_to_exec_file and target_ops::pid_to_exec_file to return a "const char *". I couldn't build many of these targets, but did examine the code by hand -- also, as this only affects the return type, it's normally pretty safe. This brings gdb and gdbserver a bit closer, and allows for the removal of a const_cast as well. |
||
|
Pedro Alves
|
d51926f06a |
Slightly tweak and clarify target_resume's interface
The current target_resume interface is a bit odd & non-intuitive.
I've found myself explaining it a couple times the recent past, while
reviewing patches that assumed STEP/SIGNAL always applied to the
passed in PTID. It goes like this today:
- if the passed in PTID is a thread, then the step/signal request is
for that thread.
- otherwise, if PTID is a wildcard (all threads or all threads of
process), the step/signal request is for inferior_ptid, and PTID
indicates which set of threads run free.
Because GDB always switches the current thread to "leader" thread
being resumed/stepped/signalled, we can simplify this a bit to:
- step/signal are always for inferior_ptid.
- PTID indicates the set of threads that run free.
Still not ideal, but it's a minimal change and at least there are no
special cases this way.
That's what this patch does. It renames the PTID parameter to
SCOPE_PTID, adds some assertions to target_resume, and tweaks
target_resume's description. In addition, it also renames PTID to
SCOPE_PTID in the remote and linux-nat targets, and simplifies their
implementation a little bit. Other targets could do the same, but
they don't have to.
Change-Id: I02a2ec2ab3a3e9b191de1e9a84f55c17cab7daaf
|
||
|
Tom Tromey
|
b17c7ab380 |
Move target_read_string to target/target.c
This moves the two overloads of target_read_string to a new file, target/target.c, and updates both gdb and gdbserver to build this. |
||
|
Andrew Burgess
|
c8154ce0d6 |
gdb: move gdb_disassembly_flag into a new disasm-flags.h file
While working on the disassembler I was getting frustrated. Every time I touched disasm.h it seemed like every file in GDB would need to be rebuilt. Surely the disassembler can't be required by that many parts of GDB, right? Turns out that disasm.h is included in target.h, so pretty much every file was being rebuilt! The only thing from disasm.h that target.h needed is the gdb_disassembly_flag enum, as this is part of the target_ops api. In this commit I move gdb_disassembly_flag into its own file. This is then included in target.h and disasm.h, after which, the number of files that depend on disasm.h is much reduced. I also audited all the other includes of disasm.h and found that the includes in mep-tdep.c and python/py-registers.c are no longer needed, so I've removed these. Now, after changing disasm.h, GDB rebuilds much quicker. There should be no user visible changes after this commit. |
||
|
Markus Metzger
|
696c0d5ef2 |
gdb, gdbserver: update thread identifier in enable_btrace target method
The enable_btrace target method takes a ptid_t to identify the thread on which tracing shall be enabled. Change this to thread_info * to avoid translating back and forth between the two. This will be used in a subsequent patch. |
||
|
Tom Tromey
|
bc521517b7 |
Introduce target_announce_attach
This introduces target_announce_attach, by analog with target_announce_detach. Then it converts existing targets to use this, rather than emitting their own output by hand. |
||
|
Joel Brobecker
|
4a94e36819 |
Automatic Copyright Year update after running gdb/copyright.py
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. |
||
|
Andrew Burgess
|
200fd2874d |
gdb: make post_startup_inferior a virtual method on inf_ptrace_target
While working on a later patch that required me to understand how GDB
starts up inferiors, I was confused by the
target_ops::post_startup_inferior method.
The post_startup_inferior target function is only called from
inf_ptrace_target::create_inferior.
Part of the target class hierarchy looks like this:
inf_child_target
|
'-- inf_ptrace_target
|
|-- linux_nat_target
|
|-- fbsd_nat_target
|
|-- nbsd_nat_target
|
|-- obsd_nat_target
|
'-- rs6000_nat_target
Every sub-class of inf_ptrace_target, except rs6000_nat_target,
implements ::post_startup_inferior. The rs6000_nat_target picks up
the implementation of ::post_startup_inferior not from
inf_ptrace_target, but from inf_child_target.
No descendent of inf_child_target, outside the inf_ptrace_target
sub-tree, implements ::post_startup_inferior, which isn't really
surprising, as they would never see the method called (remember, the
method is only called from inf_ptrace_target::create_inferior).
What I find confusing is the role inf_child_target plays in
implementing, what is really a helper function for just one of its
descendents.
In this commit I propose that we formally make ::post_startup_inferior
a helper function of inf_ptrace_target. To do this I will remove the
::post_startup_inferior from the target_ops API, and instead make this
a protected, pure virtual function on inf_ptrace_target.
I'll remove the empty implementation of ::post_startup_inferior from
the inf_child_target class, and add a new empty implementation to the
rs6000_nat_target class.
All the other descendents of inf_ptrace_target already provide an
implementation of this method and so don't need to change beyond
making the method protected within their class declarations.
To me, this makes much more sense now. The helper function, which is
only called from within the inf_ptrace_target class, is now a part of
the inf_ptrace_target class.
The only way in which this change is visible to a user is if the user
turns on 'set debug target 1'. With this debug flag on, prior to this
patch the user would see something like:
-> native->post_startup_inferior (...)
<- native->post_startup_inferior (2588939)
After this patch these lines are no longer present, as the
post_startup_inferior is no longer a top level target method. For me,
this is an acceptable change.
|
||
|
Andrew Burgess
|
0c1e6e265b |
gdb: introduce a new overload of target_can_async_p
There are a few places where we call the target_ops::can_async_p member function directly, instead of using the target_can_async_p wrapper. In some of these places this is because we need to ask before the target has been pushed, and in another location (in target.c) it seems unnecessary to go through the wrapper when we are already in target.c code. However, in the next commit I'd like to hoist some common checks out of target specific code into target.c. To achieve this, in this commit, I introduce a new overload of target_can_async_p which takes a target_ops pointer, and calls the ::can_async_p method directly. I then make use of the new overload where appropriate. There should be no user visible changes after this commit. |
||
|
Simon Marchi
|
25558938d0 |
gdb: change thread_info::name to unique_xmalloc_ptr, add helper function
This started out as changing thread_info::name to a unique_xmalloc_ptr. That showed that almost all users of that field had the same logic to get a thread's name: use thread_info::name if non-nullptr, else ask the target. Factor out this logic in a new thread_name free function. Make the field private (rename to m_name) and add some accessors. Change-Id: Iebdd95f4cd21fbefc505249bd1d05befc466a2fc |
||
|
Tom Tromey
|
c80e29dba9 |
Change get_ada_task_ptid parameter type
get_ada_task_ptid currently takes a 'long' as its 'thread' parameter type. However, on some platforms this is actually a pointer, and using 'long' can sometimes end up with the value being sign-extended. This sign extension can cause problems later, if the tid is then later used as an address again. This patch changes the parameter type to ULONGEST and updates all the uses. This approach preserves sign extension on the targets where it is apparently intended, while avoiding it on others. Co-Authored-By: John Baldwin <jhb@FreeBSD.org> |
||
|
Simon Marchi
|
82d1f134cc |
gdb: follow-fork: push target and add thread in target_follow_fork
In the context of ROCm-gdb [1], the ROCm target sits on top of the linux-nat target. when a process forks, it needs to carry over some data from the forking inferior to the fork child inferior. Ideally, the ROCm target would implement the follow_fork target_ops method, but there are some small problems. This patch fixes these, which helps the ROCm target, but also makes things more consistent and a bit nicer in general, I believe. The main problem is: when follow-fork-mode is "parent", target_follow_fork is called with the parent as the current inferior. When it's "child", target_follow_fork is called with the child as the current inferior. This means that target_follow_fork is sometimes called on the parent's target stack and sometimes on the child's target stack. The parent's target stack may contain targets above the process target, such as the ROCm target. So if follow-fork-child is "parent", the ROCm target would get notified of the fork and do whatever is needed. But the child's target stack, at that moment, only contains the exec and process target copied over from the parent. The child's target stack is set up by follow_fork_inferior, before calling target_follow_fork. In that case, the ROCm target wouldn't get notified of the fork. For consistency, I think it would be good to always call target_follow_fork on the parent inferior's target stack. I think it makes sense as a way to indicate "this inferior has called fork, do whatever is needed". The desired outcome of the fork (whether an inferior is created for the child, do we need to detach from the child) can be indicated by passed parameter. I therefore propose these changes: - make follow_fork_inferior always call target_follow_fork with the parent as the current inferior. That lets all targets present on the parent's target stack do some fork-related handling and push themselves on the fork child's target stack if needed. For this purpose, pass the child inferior down to target_follow_fork and follow_fork implementations. This is nullptr if no inferior is created for the child, because we want to detach from it. - as a result, in follow_fork_inferior, detach from the parent inferior (if needed) only after the target_follow_fork call. This is needed because we want to call target_follow_fork before the parent's target stack is torn down. - hand over to the targets in the parent's target stack (including the process target) the responsibility to push themselves, if needed, to the child's target stack. Also hand over the responsibility to the process target, at the same time, to create the child's initial thread (just like we do for follow_exec). - pass the child inferior to exec_on_vfork, so we don't need to swap the current inferior between parent and child. Nothing in exec_on_vfork depends on the current inferior, after this change. Although this could perhaps be replaced with just having the exec target implement follow_fork and push itself in the child's target stack, like the process target does... We would just need to make sure the process target calls beneath()->follow_fork(...). I'm not sure about this one. gdb/ChangeLog: * target.h (struct target_ops) <follow_fork>: Add inferior* parameter. (target_follow_fork): Likewise. * target.c (default_follow_fork): Likewise. (target_follow_fork): Likewise. * fbsd-nat.h (class fbsd_nat_target) <follow_fork>: Likewise. (fbsd_nat_target::follow_fork): Likewise, and call inf_ptrace_target::follow_fork. * linux-nat.h (class linux_nat_target) <follow_fork>: Likewise. * linux-nat.c (linux_nat_target::follow_fork): Likewise, and call inf_ptrace_target::follow_fork. * obsd-nat.h (obsd_nat_target) <follow_fork>: Likewise. * obsd-nat.c (obsd_nat_target::follow_fork): Likewise, and call inf_ptrace_target::follow_fork. * remote.c (class remote_target) <follow_fork>: Likewise. (remote_target::follow_fork): Likewise, and call process_stratum_target::follow_fork. * process-stratum-target.h (class process_stratum_target) <follow_fork>: New. * process-stratum-target.c (process_stratum_target::follow_fork): New. * target-delegates.c: Re-generate. [1] https://github.com/ROCm-Developer-Tools/ROCgdb Change-Id: I460bd0af850f0485e8aed4b24c6d8262a4c69929 |
||
|
Simon Marchi
|
3a849a3454 |
gdb: pass child_ptid and fork kind to target_ops::follow_fork
This is a small cleanup I think would be nice, that I spotted while doing the following patch. gdb/ChangeLog: * target.h (struct target_ops) <follow_fork>: Add ptid and target_waitkind parameters. (target_follow_fork): Likewise. * target.c (default_follow_fork): Likewise. (target_follow_fork): Likewise. * fbsd-nat.h (class fbsd_nat_target) <follow_fork>: Likewise. * fbsd-nat.c (fbsd_nat_target::follow_fork): Likewise. * linux-nat.h (class linux_nat_target) <follow_fork>: Likewise. * linux-nat.c (linux_nat_target::follow_fork): Likewise. * obsd-nat.h (class obsd_nat_target) <follow_fork>: Likewise. * obsd-nat.c (obsd_nat_target::follow_fork): Likewise. * remote.c (class remote_target) <follow_fork>: Likewise. * target-debug.h (target_debug_print_target_waitkind): New. * target-delegates.c: Re-generate. Change-Id: I5421a542f2e19100a22b74cc333d2b235d0de3c8 |
||
|
Pedro Alves
|
d7cb0ef35b |
Fix detach with target remote (PR gdb/28080)
Commit |
||
|
Tom de Vries
|
fbf3c4b979 |
[gdb/tdep] Use pid to choose process 64/32-bitness
In a linux kernel mailing list discussion, it was mentioned that "gdb has
this odd thing where it takes the 64-bit vs 32-bit data for the whole process
from one thread, and picks the worst possible thread to do it (ie explicitly
not even the main thread, ...)" [1].
The picking of the thread is done here in
x86_linux_nat_target::read_description:
...
/* GNU/Linux LWP ID's are process ID's. */
tid = inferior_ptid.lwp ();
if (tid == 0)
tid = inferior_ptid.pid (); /* Not a threaded program. */
...
To understand what this code does, let's investigate a scenario in which
inferior_ptid.lwp () != inferior_ptid.pid ().
Say we start exec jit-attach-pie, identified with pid x. The main thread
starts another thread that sleeps, and then the main thread waits for the
sleeping thread. So we have two threads, identified with LWP IDs x and x+1:
...
PID LWP CMD
x x ./jit-attach-pie
x x+1 ./jit-attach-pie
...
[ The thread with LWP x is known as the thread group leader. ]
When attaching to this exec using the pid, gdb does a stop_all_threads which
iterates over all the threads, first LWP x, and then LWP x+1.
So the state we arrive with at x86_linux_nat_target::read_description is:
...
(gdb) p inferior_ptid
$1 = {m_pid = x, m_lwp = x+1, m_tid = 0}
...
and consequently we probe 64/32-bitness from thread LWP x+1.
[ Note that this is different from when gdb doesn't attach but instead
launches the exec itself, in which case there's just one thread to begin with,
and consequently the probed thread is LWP x. ]
According to aforementioned remark, a better choice would have been the main
thread, that is, LWP x.
This patch implement that choice, by simply doing:
...
tid = inferior_ptid.pid ();
...
The fact that gdb makes a per-process permanent choice for 64/32-bitness is a
problem in itself: each thread can be in either 64 or 32 bit mode, and change
forth and back. That is a problem that this patch doesn't fix.
Now finally: why does this matter in the context of the linux kernel
discussion? The discussion was related to a patch that exposed io_uring
threads to user-space. This made it possible that one of those threads would
be picked out to select 64/32-bitness. Given that such threads are atypical
user-space threads in the sense that they don't return to user-space and don't
have a userspace register state, reading their registers returns garbage, and
so it could f.i. occur that in a 64-bit process with all normal user-space
threads in 64-bit mode, the probing would return 32-bit.
It may be that this is worked-around on the kernel side by providing userspace
register state in those threads such that current gdb is happy. Nevertheless,
it seems prudent to fix this on the gdb size as well.
Tested on x86_64-linux.
[1] https://lore.kernel.org/io-uring/CAHk-=wh0KoEZXPYMGkfkeVEerSCEF1AiCZSvz9TRrx=Kj74D+Q@mail.gmail.com/
gdb/ChangeLog:
2021-05-23 Tom de Vries <tdevries@suse.de>
PR tdep/27822
* target.h (struct target_ops): Mention target_thread_architecture in
read_description comment.
* x86-linux-nat.c (x86_linux_nat_target::read_description): Use
pid to determine if process is 64-bit or 32-bit.
* aarch64-linux-nat.c (aarch64_linux_nat_target::read_description):
Same.
* ppc-linux-nat.c (ppc_linux_nat_target::read_description): Same.
* riscv-linux-nat.c (riscv_linux_nat_target::read_description): Same.
* s390-linux-nat.c (s390_linux_nat_target::read_description): Same.
* arm-linux-nat.c (arm_linux_nat_target::read_description): Same.
Likewise, use pid to determine if kernel supports reading VFP
registers.
|
||
|
Simon Marchi
|
294c36eb6a |
gdb: on exec, delegate pushing / unpushing target and adding thread to target_ops::follow_exec
On "exec", some targets need to unpush themselves from the inferior, and do some bookkeeping, like forgetting the data associated to the exec'ing inferior. One such example is the thread-db target. It does so in a special case in thread_db_target::wait, just before returning the TARGET_WAITKIND_EXECD event to its caller. We have another such case in the context of rocm-gdb [1], where the "rocm" target is pushed on top of the linux-nat target. When an exec happens, we want to unpush the rocm target from the exec'ing inferior to close some file descriptors that refer to the pre-exec address space and forget about that inferior. We then want to push the target on the inferior in which execution continues, to open the file descriptors for the post-exec address space. I think that a good way to address this cleanly is to do all this in the target_ops::follow_exec implementations. Make the process_stratum_target::follow_exec implementation have the default behavior of pushing itself to the new inferior's target stack (if execution continues in a new inferior) and add the initial thread. remote_target::follow_exec is an example of process target that wants to do a bit more than the default behavior. So it calls process_stratum_target::follow_exec first and does the extra work second. linux-thread-db (a non-process target) implements follow_exec to do some bookeeping (forget about that process' data), before handing down the event down to the process target (which hits process_stratum_target::follow_exec). gdb/ChangeLog: * target.h (struct target_ops) <follow_exec>: Add ptid_t parameter. (target_follow_exec): Likewise. * target.c (target_follow_exec): Add ptid_t parameter. * infrun.c (follow_exec): Adjust call to target_follow_exec, don't push target nor create thread. * linux-thread-db.c (class thread_db_target) <follow_exec>: New. (thread_db_target::wait): Just return on TARGET_WAITKIND_EXECD. (thread_db_target::follow_exec): New. * remote.c (class remote_target) <follow_exec>: Add ptid_t parameter. (remote_target::follow_exec): Call process_stratum_target::follow_exec. * target-delegates.c: Re-generate. Change-Id: I3f96d0ba3ea0dde6540b7e1b4d5cdb01635088c8 |
||
|
Simon Marchi
|
2af87c859f |
gdb: call target_follow_exec when "set follow-exec-mode" is "same"
target_follow_exec is currently only called in the "follow-exec-mode ==
new" branch of follow_exec, not the "follow-exec-mode == same" branch.
I think it would make sense to call it regardless of the mode to let
targets do some necessary handling.
This is needed in the context of rocm-gdb [1], where a target is pushed
on top of the linux-nat target. On exec, it needs to do some
bookkeeping, close some file descriptors / handles that were related to
the process pre-exec and open some new ones for the process post-exec.
However, by looking at the only in-tree implementation of
target_ops::follow_exec, remote_target::follow_exec, I found that it
would be useful for the extended-remote target too, to align its
behavior with native debugging (although I think that behavior is not
very user-friendly, see PR 27745 [2]).
Using two programs, one (let's call it "execer") that execs the other
(let's call it "execee"), with native:
$ ./gdb -q -nx --data-directory=data-directory ./execer
Reading symbols from ./execer...
(gdb) r
Starting program: /home/simark/build/binutils-gdb/gdb/execer
I am execer
process 1495622 is executing new program: /home/simark/build/binutils-gdb/gdb/execee
I am execee
[Inferior 1 (process 1495622) exited normally]
(gdb) r
Starting program: /home/simark/build/binutils-gdb/gdb/execee
I am execee
[Inferior 1 (process 1495626) exited normally]
And now with gdbserver (some irrelevant output lines removed for brevity):
$ ./gdbserver --once --multi :1234
...
$ ./gdb -q -nx --data-directory=data-directory ./execer -ex "set remote exec-file /home/simark/build/binutils-gdb/gdb/execer" -ex "tar ext :1234"
Reading symbols from ./execer...
Remote debugging using :1234
(gdb) r
Starting program: /home/simark/build/binutils-gdb/gdb/execer
process 1495724 is executing new program: /home/simark/build/binutils-gdb/gdb/execee
[Inferior 1 (process 1495724) exited normally]
(gdb) r
`target:/home/simark/build/binutils-gdb/gdb/execee' has disappeared; keeping its symbols.
Starting program: target:/home/simark/build/binutils-gdb/gdb/execee
warning: Build ID mismatch between current exec-file target:/home/simark/build/binutils-gdb/gdb/execee
and automatically determined exec-file target:/home/simark/build/binutils-gdb/gdb/execer
exec-file-mismatch handling is currently "ask"
Reading /home/simark/build/binutils-gdb/gdb/execer from remote target...
Load new symbol table from "target:/home/simark/build/binutils-gdb/gdb/execer"? (y or n)
When handling the exec, GDB updates the exec-file of the inferior to be
the execee. This means that a subsequent "run" will run the execee, not
the original executable (execer).
remote_target::follow_exec is meant to update the "remote exec-file",
which is the file on the remote system that will be executed if you
"run" the inferior, to the execee as well. However, this is not called
when follow-exec-mode is same, because target_follow_exec is not called
in this branch. As a result, GDB thinks the inferior is executing
execee but the remote side is really executing execer, hence the
mismatch message.
By calling target_follow_exec in the "same" branch of the follow_exec
function, we ensure that everybody agrees, and we get the same behavior
with the extended-remote target as we get with the native target, the
execee is executed on the second run:
$ ./gdbserver --once --multi :1234
...
$ ./gdb -q -nx --data-directory=data-directory ./execer -ex "set remote exec-file /home/simark/build/binutils-gdb/gdb/execer" -ex "tar ext :1234"
Reading symbols from ./execer...
Remote debugging using :1234
(gdb) r
Starting program: /home/simark/build/binutils-gdb/gdb/execer
process 1501445 is executing new program: /home/simark/build/binutils-gdb/gdb/execee
[Inferior 1 (process 1501445) exited normally]
(gdb) r
`target:/home/simark/build/binutils-gdb/gdb/execee' has disappeared; keeping its symbols.
Starting program: target:/home/simark/build/binutils-gdb/gdb/execee
[Inferior 1 (process 1501447) exited normally]
(gdb)
This scenario is tested in gdb.base/foll-exec-mode.exp, and in fact this
patch fixes the test for me when using
--target_board=native-extended-gdbserver.
gdb/ChangeLog:
* infrun.c (follow_exec): Call target_follow_fork when
follow-exec-mode is same.
* target.h (target_follow_fork): Improve doc.
[1] https://github.com/ROCm-Developer-Tools/ROCgdb
[2] https://sourceware.org/bugzilla/show_bug.cgi?id=27745
Change-Id: I4ee84a875e39bf3f8eaf3e6789a4bfe23a2a430e
|
||
|
Simon Marchi
|
e97007b64a |
gdb: make target_ops::follow_fork return void
I noticed that all implementations return false, so target_ops::follow_fork doesn't really need to return a value. Change it to return void. gdb/ChangeLog: * target.h (struct target_ops) <follow_fork>: Return void. (target_follow_fork): Likewise. * target.c (default_follow_fork): Likewise. (target_follow_fork): Likewise. * infrun.c (follow_fork_inferior): Adjust. * fbsd-nat.h (class fbsd_nat_target) <follow_fork>: Return void. * fbsd-nat.c (fbsd_nat_target:::follow_fork): Likewise. * linux-nat.h (class linux_nat_target) <follow_fork>: Likewise. * linux-nat.c (linux_nat_target::follow_fork): Return void. * obsd-nat.h (class obsd_nat_target) <follow_fork>: Return void. * obsd-nat.c (obsd_nat_target::follow_fork): Likewise. * remote.c (class remote_target) <follow_fork>: Likewise. (remote_target::follow_fork): Likewise. * target-delegates.c: Re-generate. Change-Id: If908c2f68b29fa275be2b0b9deb41e4c6a1b7879 |
||
|
Simon Marchi
|
b4b1a226df |
gdb: defer commit resume until all available events are consumed
Rationale
---------
Let's say you have multiple threads hitting a conditional breakpoint
at the same time, and all of these are going to evaluate to false.
All these threads will need to be resumed.
Currently, GDB fetches one target event (one SIGTRAP representing the
breakpoint hit) and decides that the thread should be resumed. It
calls resume and commit_resume immediately. It then fetches the
second target event, and does the same, until it went through all
threads.
The result is therefore something like:
- consume event for thread A
- resume thread A
- commit resume (affects thread A)
- consume event for thread B
- resume thread B
- commit resume (affects thread B)
- consume event for thread C
- resume thread C
- commit resume (affects thread C)
For targets where it's beneficial to group resumptions requests (most
likely those that implement target_ops::commit_resume), it would be
much better to have:
- consume event for thread A
- resume thread A
- consume event for thread B
- resume thread B
- consume event for thread C
- resume thread C
- commit resume (affects threads A, B and C)
Implementation details
----------------------
To achieve this, this patch adds another check in
maybe_set_commit_resumed_all_targets to avoid setting the
commit-resumed flag of targets that readily have events to provide to
infrun.
To determine if a target has events readily available to report, this
patch adds an `has_pending_events` target_ops method. The method
returns a simple bool to say whether or not it has pending events to
report.
Testing
=======
To test this, I start GDBserver with a program that spawns multiple
threads:
$ ../gdbserver/gdbserver --once :1234 ~/src/many-threads-stepping-over-breakpoints/many-threads-stepping-over-breakpoints
I then connect with GDB and install a conditional breakpoint that always
evaluates to false (and force the evaluation to be done by GDB):
$ ./gdb -nx --data-directory=data-directory \
/home/simark/src/many-threads-stepping-over-breakpoints/many-threads-stepping-over-breakpoints \
-ex "set breakpoint condition-evaluation host" \
-ex "set pag off" \
-ex "set confirm off" \
-ex "maint set target-non-stop on" \
-ex "tar rem :1234" \
-ex "tb main" \
-ex "b 13 if 0" \
-ex c \
-ex "set debug infrun" \
-ex "set debug remote 1" \
-ex "set debug displaced"
I then do "continue" and look at the log.
The remote target receives a bunch of stop notifications for all
threads that have hit the breakpoint. infrun consumes and processes
one event, decides it should not cause a stop, prepares a displaced
step, after which we should see:
[infrun] maybe_set_commit_resumed_all_process_targets: not requesting commit-resumed for target remote, target has pending events
Same for a second thread (since we have 2 displaced step buffers).
For the following threads, their displaced step is deferred since
there are no more buffers available.
After consuming the last event the remote target has to offer, we get:
[infrun] maybe_set_commit_resumed_all_process_targets: enabling commit-resumed for target remote
[infrun] maybe_call_commit_resumed_all_process_targets: calling commit_resumed for target remote
[remote] Sending packet: $vCont;s:p14d16b.14d1b1;s:p14d16b.14d1b2#55
[remote] Packet received: OK
Without the patch, there would have been one vCont;s just after each
prepared displaced step.
gdb/ChangeLog:
yyyy-mm-dd Simon Marchi <simon.marchi@efficios.com>
Pedro Alves <pedro@palves.net>
* async-event.c (async_event_handler_marked): New.
* async-event.h (async_event_handler_marked): Declare.
* infrun.c (maybe_set_commit_resumed_all_targets): Switch to
inferior before calling target method. Don't commit-resumed if
target_has_pending_events is true.
* remote.c (remote_target::has_pending_events): New.
* target-delegates.c: Regenerate.
* target.c (target_has_pending_events): New.
* target.h (target_ops::has_pending_events): New target method.
(target_has_pending_events): New.
Change-Id: I18112ba19a1ff4986530c660f530d847bb4a1f1d
|
||
|
Simon Marchi
|
1192f124a3 |
gdb: generalize commit_resume, avoid commit-resuming when threads have pending statuses
The rationale for this patch comes from the ROCm port [1], the goal
being to reduce the number of back and forths between GDB and the
target when doing successive operations. I'll start with explaining
the rationale and then go over the implementation. In the ROCm / GPU
world, the term "wave" is somewhat equivalent to a "thread" in GDB.
So if you read if from a GPU stand point, just s/thread/wave/.
ROCdbgapi, the library used by GDB [2] to communicate with the GPU
target, gives the illusion that it's possible for the debugger to
control (start and stop) individual threads. But in reality, this is
not how it works. Under the hood, all threads of a queue are
controlled as a group. To stop one thread in a group of running ones,
the state of all threads is retrieved from the GPU, all threads are
destroyed, and all threads but the one we want to stop are re-created
from the saved state. The net result, from the point of view of GDB,
is that the library stopped one thread. The same thing goes if we
want to resume one thread while others are running: the state of all
running threads is retrieved from the GPU, they are all destroyed, and
they are all re-created, including the thread we want to resume.
This leads to some inefficiencies when combined with how GDB works,
here are two examples:
- Stopping all threads: because the target operates in non-stop mode,
when the user interface mode is all-stop, GDB must stop all threads
individually when presenting a stop. Let's suppose we have 1000
threads and the user does ^C. GDB asks the target to stop one
thread. Behind the scenes, the library retrieves 1000 thread
states and restores the 999 others still running ones. GDB asks
the target to stop another one. The target retrieves 999 thread
states and restores the 998 remaining ones. That means that to
stop 1000 threads, we did 1000 back and forths with the GPU. It
would have been much better to just retrieve the states once and
stop there.
- Resuming with pending events: suppose the 1000 threads hit a
breakpoint at the same time. The breakpoint is conditional and
evaluates to true for the first thread, to false for all others.
GDB pulls one event (for the first thread) from the target, decides
that it should present a stop, so stops all threads using
stop_all_threads. All these other threads have a breakpoint event
to report, which is saved in `thread_info::suspend::waitstatus` for
later. When the user does "continue", GDB resumes that one thread
that did hit the breakpoint. It then processes the pending events
one by one as if they just arrived. It picks one, evaluates the
condition to false, and resumes the thread. It picks another one,
evaluates the condition to false, and resumes the thread. And so
on. In between each resumption, there is a full state retrieval
and re-creation. It would be much nicer if we could wait a little
bit before sending those threads on the GPU, until it processed all
those pending events.
To address this kind of performance issue, ROCdbgapi has a concept
called "forward progress required", which is a boolean state that
allows its user (i.e. GDB) to say "I'm doing a bunch of operations,
you can hold off putting the threads on the GPU until I'm done" (the
"forward progress not required" state). Turning forward progress back
on indicates to the library that all threads that are supposed to be
running should now be really running on the GPU.
It turns out that GDB has a similar concept, though not as general,
commit_resume. One difference is that commit_resume is not stateful:
the target can't look up "does the core need me to schedule resumed
threads for execution right now". It is also specifically linked to
the resume method, it is not used in other contexts. The target
accumulates resumption requests through target_ops::resume calls, and
then commits those resumptions when target_ops::commit_resume is
called. The target has no way to check if it's ok to leave resumed
threads stopped in other target methods.
To bridge the gap, this patch generalizes the commit_resume concept in
GDB to match the forward progress concept of ROCdbgapi. The current
name (commit_resume) can be interpreted as "commit the previous resume
calls". I renamed the concept to "commit_resumed", as in "commit the
threads that are resumed".
In the new version, we have two things:
- the commit_resumed_state field in process_stratum_target: indicates
whether GDB requires target stacks using this target to have
resumed threads committed to the execution target/device. If
false, an execution target is allowed to leave resumed threads
un-committed at the end of whatever method it is executing.
- the commit_resumed target method: called when commit_resumed_state
transitions from false to true. While commit_resumed_state was
false, the target may have left some resumed threads un-committed.
This method being called tells it that it should commit them back
to the execution device.
Let's take the "Stopping all threads" scenario from above and see how
it would work with the ROCm target with this change. Before stopping
all threads, GDB would set the target's commit_resumed_state field to
false. It would then ask the target to stop the first thread. The
target would retrieve all threads' state from the GPU and mark that
one as stopped. Since commit_resumed_state is false, it leaves all
the other threads (still resumed) stopped. GDB would then proceed to
call target_stop for all the other threads. Since resumed threads are
not committed, this doesn't do any back and forth with the GPU.
To simplify the implementation of targets, this patch makes it so that
when calling certain target methods, the contract between the core and
the targets guarantees that commit_resumed_state is false. This way,
the target doesn't need two paths, one for commit_resumed_state ==
true and one for commit_resumed_state == false. It can just assert
that commit_resumed_state is false and work with that assumption.
This also helps catch places where we forgot to disable
commit_resumed_state before calling the method, which represents a
probable optimization opportunity. The commit adds assertions in the
target method wrappers (target_resume and friends) to have some
confidence that this contract between the core and the targets is
respected.
The scoped_disable_commit_resumed type is used to disable the commit
resumed state of all process targets on construction, and selectively
re-enable it on destruction (see below for criteria). Note that it
only sets the process_stratum_target::commit_resumed_state flag. A
subsequent call to maybe_call_commit_resumed_all_targets is necessary
to call the commit_resumed method on all target stacks with process
targets that got their commit_resumed_state flag turned back on. This
separation is because we don't want to call the commit_resumed methods
in scoped_disable_commit_resumed's destructor, as they may throw.
On destruction, commit-resumed is not re-enabled for a given target
if:
1. this target has no threads resumed, or
2. this target has at least one resumed thread with a pending status
known to the core (saved in thread_info::suspend::waitstatus).
The first point is not technically necessary, because a proper
commit_resumed implementation would be a no-op if the target has no
resumed threads. But since we have a flag do to a quick check, it
shouldn't hurt.
The second point is more important: together with the
scoped_disable_commit_resumed instance added in fetch_inferior_event,
it makes it so the "Resuming with pending events" described above is
handled efficiently. Here's what happens in that case:
1. The user types "continue".
2. Upon destruction, the scoped_disable_commit_resumed in the
`proceed` function does not enable commit-resumed, as it sees some
threads have pending statuses.
3. fetch_inferior_event is called to handle another event, the
breakpoint hit evaluates to false, and that thread is resumed.
Because there are still more threads with pending statuses, the
destructor of scoped_disable_commit_resumed in
fetch_inferior_event still doesn't enable commit-resumed.
4. Rinse and repeat step 3, until the last pending status is handled
by fetch_inferior_event. In that case,
scoped_disable_commit_resumed's destructor sees there are no more
threads with pending statues, so it asks the target to commit
resumed threads.
This allows us to avoid all unnecessary back and forths, there is a
single commit_resumed call once all pending statuses are processed.
This change required remote_target::remote_stop_ns to learn how to
handle stopping threads that were resumed but pending vCont. The
simplest example where that happens is when using the remote target in
all-stop, but with "maint set target-non-stop on", to force it to
operate in non-stop mode under the hood. If two threads hit a
breakpoint at the same time, GDB will receive two stop replies. It
will present the stop for one thread and save the other one in
thread_info::suspend::waitstatus.
Before this patch, when doing "continue", GDB first resumes the thread
without a pending status:
Sending packet: $vCont;c:p172651.172676#f3
It then consumes the pending status in the next fetch_inferior_event
call:
[infrun] do_target_wait_1: Using pending wait status status->kind = stopped, signal = GDB_SIGNAL_TRAP for Thread 1517137.1517137.
[infrun] target_wait (-1.0.0, status) =
[infrun] 1517137.1517137.0 [Thread 1517137.1517137],
[infrun] status->kind = stopped, signal = GDB_SIGNAL_TRAP
It then realizes it needs to stop all threads to present the stop, so
stops the thread it just resumed:
[infrun] stop_all_threads: Thread 1517137.1517137 not executing
[infrun] stop_all_threads: Thread 1517137.1517174 executing, need stop
remote_stop called
Sending packet: $vCont;t:p172651.172676#04
This is an unnecessary resume/stop. With this patch, we don't commit
resumed threads after proceeding, because of the pending status:
[infrun] maybe_commit_resumed_all_process_targets: not requesting commit-resumed for target extended-remote, a thread has a pending waitstatus
When GDB handles the pending status and stop_all_threads runs, we stop a
resumed but pending vCont thread:
remote_stop_ns: Enqueueing phony stop reply for thread pending vCont-resume (1520940, 1520976, 0)
That thread was never actually resumed on the remote stub / gdbserver,
so we shouldn't send a packet to the remote side asking to stop the
thread.
Note that there are paths that resume the target and then do a
synchronous blocking wait, in sort of nested event loop, via
wait_sync_command_done. For example, inferior function calls, or any
run control command issued from a breakpoint command list. We handle
that making wait_sync_command_one a "sync" point -- force forward
progress, or IOW, force-enable commit-resumed state.
gdb/ChangeLog:
yyyy-mm-dd Simon Marchi <simon.marchi@efficios.com>
Pedro Alves <pedro@palves.net>
* infcmd.c (run_command_1, attach_command, detach_command)
(interrupt_target_1): Use scoped_disable_commit_resumed.
* infrun.c (do_target_resume): Remove
target_commit_resume call.
(commit_resume_all_targets): Remove.
(maybe_set_commit_resumed_all_targets): New.
(maybe_call_commit_resumed_all_targets): New.
(enable_commit_resumed): New.
(scoped_disable_commit_resumed::scoped_disable_commit_resumed)
(scoped_disable_commit_resumed::~scoped_disable_commit_resumed)
(scoped_disable_commit_resumed::reset)
(scoped_disable_commit_resumed::reset_and_commit)
(scoped_enable_commit_resumed::scoped_enable_commit_resumed)
(scoped_enable_commit_resumed::~scoped_enable_commit_resumed):
New.
(proceed): Use scoped_disable_commit_resumed and
maybe_call_commit_resumed_all_targets.
(fetch_inferior_event): Use scoped_disable_commit_resumed.
* infrun.h (struct scoped_disable_commit_resumed): New.
(maybe_call_commit_resumed_all_process_targets): New.
(struct scoped_enable_commit_resumed): New.
* mi/mi-main.c (exec_continue): Use scoped_disable_commit_resumed.
* process-stratum-target.h (class process_stratum_target):
<commit_resumed_state>: New.
* record-full.c (record_full_wait_1): Change commit_resumed_state
around calling commit_resumed.
* remote.c (class remote_target) <commit_resume>: Rename to...
<commit_resumed>: ... this.
(struct stop_reply): Move up.
(remote_target::commit_resume): Rename to...
(remote_target::commit_resumed): ... this. Check if there is any
thread pending vCont resume.
(remote_target::remote_stop_ns): Generate stop replies for resumed
but pending vCont threads.
(remote_target::wait_ns): Add gdb_assert.
* target-delegates.c: Regenerate.
* target.c (target_wait, target_resume): Assert that the current
process_stratum target isn't in commit-resumed state.
(defer_target_commit_resume): Remove.
(target_commit_resume): Remove.
(target_commit_resumed): New.
(make_scoped_defer_target_commit_resume): Remove.
(target_stop): Assert that the current process_stratum target
isn't in commit-resumed state.
* target.h (struct target_ops) <commit_resume>: Rename to ...
<commit_resumed>: ... this.
(target_commit_resume): Remove.
(target_commit_resumed): New.
(make_scoped_defer_target_commit_resume): Remove.
* top.c (wait_sync_command_done): Use
scoped_enable_commit_resumed.
[1] https://github.com/ROCm-Developer-Tools/ROCgdb/
[2] https://github.com/ROCm-Developer-Tools/ROCdbgapi
Change-Id: I836135531a29214b21695736deb0a81acf8cf566
|
||
|
Simon Marchi
|
328d42d87e |
gdb: remove current_top_target function
The current_top_target function is a hidden dependency on the current inferior. Since I'd like to slowly move towards reducing our dependency on the global current state, remove this function and make callers use current_inferior ()->top_target () There is no expected change in behavior, but this one step towards making those callers use the inferior from their context, rather than refer to the global current inferior. gdb/ChangeLog: * target.h (current_top_target): Remove, make callers use the current inferior instead. * target.c (current_top_target): Remove. Change-Id: Iccd457036f84466cdaa3865aa3f9339a24ea001d |
||
|
Simon Marchi
|
d777bf0df2 |
gdb: move all "current target" wrapper implementations to target.c
The following patch removes the current_top_target function, replacing uses with `current_inferior ()->top_target ()`. This is a problem for uses in target.h, because they don't have access to the current_inferior function and the inferior structure: target.h can't include inferior.h, otherwise that would make a cyclic inclusion. Avoid this by moving all implementations of the wrappers that call target methods with the current target to target.c. Many of them are changed from a macro to a function, which is an improvement for readability and debuggability, IMO. target_shortname and target_longname were not function-like macros, so a few adjustments are needed. gdb/ChangeLog: * target.h (target_shortname): Change to function declaration. (target_longname): Likewise. (target_attach_no_wait): Likewise. (target_post_attach): Likewise. (target_prepare_to_store): Likewise. (target_supports_enable_disable_tracepoint): Likewise. (target_supports_string_tracing): Likewise. (target_supports_evaluation_of_breakpoint_conditions): Likewise. (target_supports_dumpcore): Likewise. (target_dumpcore): Likewise. (target_can_run_breakpoint_commands): Likewise. (target_files_info): Likewise. (target_post_startup_inferior): Likewise. (target_insert_fork_catchpoint): Likewise. (target_remove_fork_catchpoint): Likewise. (target_insert_vfork_catchpoint): Likewise. (target_remove_vfork_catchpoint): Likewise. (target_insert_exec_catchpoint): Likewise. (target_remove_exec_catchpoint): Likewise. (target_set_syscall_catchpoint): Likewise. (target_rcmd): Likewise. (target_can_lock_scheduler): Likewise. (target_can_async_p): Likewise. (target_is_async_p): Likewise. (target_execution_direction): Likewise. (target_extra_thread_info): Likewise. (target_pid_to_exec_file): Likewise. (target_thread_architecture): Likewise. (target_find_memory_regions): Likewise. (target_make_corefile_notes): Likewise. (target_get_bookmark): Likewise. (target_goto_bookmark): Likewise. (target_stopped_by_watchpoint): Likewise. (target_stopped_by_sw_breakpoint): Likewise. (target_supports_stopped_by_sw_breakpoint): Likewise. (target_stopped_by_hw_breakpoint): Likewise. (target_supports_stopped_by_hw_breakpoint): Likewise. (target_have_steppable_watchpoint): Likewise. (target_can_use_hardware_watchpoint): Likewise. (target_region_ok_for_hw_watchpoint): Likewise. (target_can_do_single_step): Likewise. (target_insert_watchpoint): Likewise. (target_remove_watchpoint): Likewise. (target_insert_hw_breakpoint): Likewise. (target_remove_hw_breakpoint): Likewise. (target_can_accel_watchpoint_condition): Likewise. (target_can_execute_reverse): Likewise. (target_get_ada_task_ptid): Likewise. (target_filesystem_is_local): Likewise. (target_trace_init): Likewise. (target_download_tracepoint): Likewise. (target_can_download_tracepoint): Likewise. (target_download_trace_state_variable): Likewise. (target_enable_tracepoint): Likewise. (target_disable_tracepoint): Likewise. (target_trace_start): Likewise. (target_trace_set_readonly_regions): Likewise. (target_get_trace_status): Likewise. (target_get_tracepoint_status): Likewise. (target_trace_stop): Likewise. (target_trace_find): Likewise. (target_get_trace_state_variable_value): Likewise. (target_save_trace_data): Likewise. (target_upload_tracepoints): Likewise. (target_upload_trace_state_variables): Likewise. (target_get_raw_trace_data): Likewise. (target_get_min_fast_tracepoint_insn_len): Likewise. (target_set_disconnected_tracing): Likewise. (target_set_circular_trace_buffer): Likewise. (target_set_trace_buffer_size): Likewise. (target_set_trace_notes): Likewise. (target_get_tib_address): Likewise. (target_set_permissions): Likewise. (target_static_tracepoint_marker_at): Likewise. (target_static_tracepoint_markers_by_strid): Likewise. (target_traceframe_info): Likewise. (target_use_agent): Likewise. (target_can_use_agent): Likewise. (target_augmented_libraries_svr4_read): Likewise. (target_log_command): Likewise. * target.c (target_shortname): New. (target_longname): New. (target_attach_no_wait): New. (target_post_attach): New. (target_prepare_to_store): New. (target_supports_enable_disable_tracepoint): New. (target_supports_string_tracing): New. (target_supports_evaluation_of_breakpoint_conditions): New. (target_supports_dumpcore): New. (target_dumpcore): New. (target_can_run_breakpoint_commands): New. (target_files_info): New. (target_post_startup_inferior): New. (target_insert_fork_catchpoint): New. (target_remove_fork_catchpoint): New. (target_insert_vfork_catchpoint): New. (target_remove_vfork_catchpoint): New. (target_insert_exec_catchpoint): New. (target_remove_exec_catchpoint): New. (target_set_syscall_catchpoint): New. (target_rcmd): New. (target_can_lock_scheduler): New. (target_can_async_p): New. (target_is_async_p): New. (target_execution_direction): New. (target_extra_thread_info): New. (target_pid_to_exec_file): New. (target_thread_architecture): New. (target_find_memory_regions): New. (target_make_corefile_notes): New. (target_get_bookmark): New. (target_goto_bookmark): New. (target_stopped_by_watchpoint): New. (target_stopped_by_sw_breakpoint): New. (target_supports_stopped_by_sw_breakpoint): New. (target_stopped_by_hw_breakpoint): New. (target_supports_stopped_by_hw_breakpoint): New. (target_have_steppable_watchpoint): New. (target_can_use_hardware_watchpoint): New. (target_region_ok_for_hw_watchpoint): New. (target_can_do_single_step): New. (target_insert_watchpoint): New. (target_remove_watchpoint): New. (target_insert_hw_breakpoint): New. (target_remove_hw_breakpoint): New. (target_can_accel_watchpoint_condition): New. (target_can_execute_reverse): New. (target_get_ada_task_ptid): New. (target_filesystem_is_local): New. (target_trace_init): New. (target_download_tracepoint): New. (target_can_download_tracepoint): New. (target_download_trace_state_variable): New. (target_enable_tracepoint): New. (target_disable_tracepoint): New. (target_trace_start): New. (target_trace_set_readonly_regions): New. (target_get_trace_status): New. (target_get_tracepoint_status): New. (target_trace_stop): New. (target_trace_find): New. (target_get_trace_state_variable_value): New. (target_save_trace_data): New. (target_upload_tracepoints): New. (target_upload_trace_state_variables): New. (target_get_raw_trace_data): New. (target_get_min_fast_tracepoint_insn_len): New. (target_set_disconnected_tracing): New. (target_set_circular_trace_buffer): New. (target_set_trace_buffer_size): New. (target_set_trace_notes): New. (target_get_tib_address): New. (target_set_permissions): New. (target_static_tracepoint_marker_at): New. (target_static_tracepoint_markers_by_strid): New. (target_traceframe_info): New. (target_use_agent): New. (target_can_use_agent): New. (target_augmented_libraries_svr4_read): New. (target_log_command): New. * bfin-tdep.c (bfin_sw_breakpoint_from_kind): Adjust. * infrun.c (set_schedlock_func): Adjust. * mi/mi-main.c (exec_reverse_continue): Adjust. * reverse.c (exec_reverse_once): Adjust. * sh-tdep.c (sh_sw_breakpoint_from_kind): Adjust. * tui/tui-stack.c (tui_locator_window::make_status_line): Adjust. * remote-sim.c (gdbsim_target::detach): Adjust. (gdbsim_target::files_info): Adjust. Change-Id: I72ef56e9a25adeb0b91f1ad05e34c89f77ebeaa8 |
||
|
Luis Machado
|
dbe692af2d |
New target methods for memory tagging support
This patch starts adding some of the generic pieces to accomodate memory tagging. We have three new target methods: - supports_memory_tagging: Checks if the target supports memory tagging. This defaults to false for targets that don't support memory tagging. - fetch_memtags: Fetches the allocation tags associated with a particular memory range [address, address + length). The default is to return 0 without returning any tags. This should only be called if memory tagging is supported. - store_memtags: Stores a set of allocation tags for a particular memory range [address, address + length). The default is to return 0. This should only be called if memory tagging is supported. gdb/ChangeLog: 2021-03-24 Luis Machado <luis.machado@linaro.org> * remote.c (remote_target) <supports_memory_tagging>: New method override. <fetch_memtags>: New method override. <store_memtags>: New method override. (remote_target::supports_memory_tagging): New method. (remote_target::fetch_memtags): New method. (remote_target::store_memtags): New method. * target-delegates.c: Regenerate. * target.h (struct target_ops) <supports_memory_tagging>: New virtual method. <fetch_memtags>: New virtual method. <store_memtags>: New virtual method. (target_supports_memory_tagging): Define. (target_fetch_memtags): Define. (target_store_memtags): Define. * target-debug.h (target_debug_print_size_t) (target_debug_print_const_gdb_byte_vector_r) (target_debug_print_gdb_byte_vector_r): New functions. |
||
|
Simon Marchi
|
66848ebca8 |
gdb: remote target_longname
I noticed it was unused. gdb/ChangeLog: * target.h (target_longname): Remove. Change-Id: Id4f514ea038a6d8d40e11179db587b11793cbbd8 |
||
|
Simon Marchi
|
c8fbd44a01 |
gdb: remove target_is_pushed free function
Same principle as the previous patches. gdb/ChangeLog: * target.h (target_is_pushed): Remove, update callers to use inferior::target_is_pushed instead. * target.c (target_is_pushed): Remove. Change-Id: I9862e6205acc65672da807cbe4b46cde009e7b9d |
||
|
Simon Marchi
|
02980c5645 |
gdb: remove push_target free functions
Same as the previous patch, but for the push_target functions. The implementation of the move variant is moved to a new overload of inferior::push_target. gdb/ChangeLog: * target.h (push_target): Remove, update callers to use inferior::push_target. * target.c (push_target): Remove. * inferior.h (class inferior) <push_target>: New overload. Change-Id: I5a95496666278b8f3965e5e8aecb76f54a97c185 |
||
|
Simon Marchi
|
fadf6add30 |
gdb: remove unpush_target free function
unpush_target unpushes the passed-in target from the current inferior's target stack. Calling it is therefore an implicit dependency on the current global inferior. Remove that function and make the callers use the inferior::unpush_target method directly. This sometimes allows using the inferior from the context rather than the global current inferior. target_unpusher::operator() now needs to be implemented in target.c, otherwise target.h and inferior.h both need to include each other, and that wouldn't work. gdb/ChangeLog: * target.h (unpush_target): Remove, update all callers to use `inferior::unpush_target` instead. (struct target_unpusher) <operator()>: Just declare. * target.c (unpush_target): Remove. (target_unpusher::operator()): New. Change-Id: Ia5172dfb3f373e0a75b991885b50322ca2142a8c |