| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
I'd like to move some things so they become methods on struct ui. But
first, I think that struct ui and the related things are big enough to
deserve their own file, instead of being scattered through top.{c,h} and
event-top.c.
Change-Id: I15594269ace61fd76ef80a7b58f51ff3ab6979bc
|
|
I originally wrote this patch, because while working on some other
patch, I spotted a regression in the
gdb.multi/multi-target-no-resumed.exp.exp testcase. Debugging the
issue, I realized that the problem was related to how I was using
previous_inferior_ptid to look up the thread the user had last
selected. The problem is that previous_inferior_ptid alone doesn't
tell you which target that ptid is from, and I was just always using
the current target, which was incorrect. Two different targets may
have threads with the same ptid.
I decided to fix this by replacing previous_inferior_ptid with a
strong reference to the thread, called previous_thread.
I have since found a new motivation for this change -- I would like to
tweak "info program" to not rely on get_last_target_status returning a
ptid that still exists in the thread list. With both the follow_fork
changes later in this series, and the step-over-thread-exit changes,
that can happen, as we'll delete threads and not clear the last
waitstatus.
A new update_previous_thread function is added that can be used to
update previous_thread from inferior_ptid. This must be called in
several places that really want to get rid of previous_thread thread,
and reset the thread id counter, otherwise we get regressions like
these:
(gdb) info threads -gid
Id GId Target Id Frame
- * 1 1 Thread 2974541.2974541 "tids-gid-reset" main () at src/gdb/testsuite/gdb.multi/tids-gid-reset.c:21
- (gdb) PASS: gdb.multi/tids-gid-reset.exp: single-inferior: after restart: info threads -gid
+ * 1 2 Thread 2958361.2958361 "tids-gid-reset" main () at src/gdb/testsuite/gdb.multi/tids-gid-reset.c:21
+ (gdb) FAIL: gdb.multi/tids-gid-reset.exp: single-inferior: after restart: info threads -gid
and:
Core was generated by `build/gdb/testsuite/outputs/gdb.reverse/sigall-precsave/si'.
Program terminated with signal SIGTRAP, Trace/breakpoint trap.
#0 gen_ABRT () at src/gdb/testsuite/gdb.reverse/sigall-reverse.c:398
398 kill (getpid (), SIGABRT);
+[Current thread is 1 (LWP 2662066)]
Restored records from core file build/gdb/testsuite/outputs/gdb.reverse/sigall-precsave/sigall.precsave.
#0 gen_ABRT () at src/gdb/testsuite/gdb.reverse/sigall-reverse.c:398
398 kill (getpid (), SIGABRT);
continue
Continuing.
-Program received signal SIGABRT, Aborted.
+Thread 1 received signal SIGABRT, Aborted.
0x00007ffff7dfd55b in kill () at ../sysdeps/unix/syscall-template.S:78
78 ../sysdeps/unix/syscall-template.S: No such file or directory.
-(gdb) PASS: gdb.reverse/sigall-precsave.exp: sig-test-1: get signal ABRT
+(gdb) FAIL: gdb.reverse/sigall-precsave.exp: sig-test-1: get signal ABRT
I.e., GDB was failing to restart the thread counter back to 1, because
the previous_thread thread was being help due to the strong reference.
Tested on GNU/Linux native, gdbserver and gdbserver + "maint set
target-non-stop on".
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <pedro@palves.net>
* infcmd.c (kill_command, detach_command, disconnect_command):
Call update_previous_thread.
* infrun.c (previous_inferior_ptid): Delete.
(previous_thread): New.
(update_previous_thread): New.
(proceed, init_wait_for_inferior): Call update_previous_thread.
(normal_stop): Adjust to compare previous_thread and
inferior_thread. Call update_previous_thread.
* infrun.h (update_previous_thread): Declare.
* target.c (target_pre_inferior, target_preopen): Call
update_previous_thread.
Change-Id: I42779a1ee51a996fa1e8f6e1525c6605dbfd42c7
|
|
Add an observable notified in target_detach just before calling the
detach method on the inferior's target stack. This allows observer to
do some work on the inferior while it's still ptrace-attached, in the
case of a native Linux inferior. Specifically, the amd-dbgapi target
will need it in order to call amd_dbgapi_process_detach before the
process gets ptrace-detached.
Change-Id: I28b6065e251012a4c2db8a600fe13ba31671e3c9
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
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.
|
|
PR gdb/28947
The address_significant gdbarch setting was introduced as a way to remove
non-address bits from pointers, and it is specified by a constant. This
constant represents the number of address bits in a pointer.
Right now AArch64 is the only architecture that uses it, and 56 was a
correct option so far.
But if we are using Pointer Authentication (PAuth), we might use up to 2 bytes
from the address space to store the required information. We could also have
cases where we're using both PAuth and MTE.
We could adjust the constant to 48 to cover those cases, but this doesn't
cover the case where GDB needs to sign-extend kernel addresses after removal
of the non-address bits.
This has worked so far because bit 55 is used to select between kernel-space
and user-space addresses. But trying to clear a range of bits crossing the
bit 55 boundary requires the hook to be smarter.
The following patch renames the gdbarch hook from significant_addr_bit to
remove_non_address_bits and passes a pointer as opposed to the number of
bits. The hook is now responsible for removing the required non-address bits
and sign-extending the address if needed.
While at it, make GDB and GDBServer share some more code for aarch64 and add a
new arch-specific testcase gdb.arch/aarch64-non-address-bits.exp.
Bug-url: https://sourceware.org/bugzilla/show_bug.cgi?id=28947
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
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.
|
|
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.
|
|
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).
|
|
New in this version:
- Better comment in target_kill
- Uncomment line in test to avoid hanging when exiting, when testing on
native-extended-gdbserver
PR 28275 shows that doing a sequence of:
- Run inferior in background (run &)
- kill that inferior
- Run again
We get into this assertion:
/home/smarchi/src/binutils-gdb/gdb/target.c:2590: internal-error: target_wait: Assertion `!proc_target->commit_resumed_state' failed.
#0 internal_error_loc (file=0x5606b344e740 "/home/smarchi/src/binutils-gdb/gdb/target.c", line=2590, fmt=0x5606b344d6a0 "%s: Assertion `%s' failed.") at /home/smarchi/src/binutils-gdb/gdbsupport/errors.cc:54
#1 0x00005606b6296475 in target_wait (ptid=..., status=0x7fffb9390630, options=...) at /home/smarchi/src/binutils-gdb/gdb/target.c:2590
#2 0x00005606b5767a98 in startup_inferior (proc_target=0x5606bfccb2a0 <the_amd64_linux_nat_target>, pid=3884857, ntraps=1, last_waitstatus=0x0, last_ptid=0x0) at /home/smarchi/src/binutils-gdb/gdb/nat/fork-inferior.c:482
#3 0x00005606b4e6c9c5 in gdb_startup_inferior (pid=3884857, num_traps=1) at /home/smarchi/src/binutils-gdb/gdb/fork-child.c:132
#4 0x00005606b50f14a5 in inf_ptrace_target::create_inferior (this=0x5606bfccb2a0 <the_amd64_linux_nat_target>, exec_file=0x604000039f50 "/home/smarchi/build/binutils-gdb/gdb/test", allargs="", env=0x61500000a580, from_tty=1)
at /home/smarchi/src/binutils-gdb/gdb/inf-ptrace.c:105
#5 0x00005606b53b6d23 in linux_nat_target::create_inferior (this=0x5606bfccb2a0 <the_amd64_linux_nat_target>, exec_file=0x604000039f50 "/home/smarchi/build/binutils-gdb/gdb/test", allargs="", env=0x61500000a580, from_tty=1)
at /home/smarchi/src/binutils-gdb/gdb/linux-nat.c:978
#6 0x00005606b512b79b in run_command_1 (args=0x0, from_tty=1, run_how=RUN_NORMAL) at /home/smarchi/src/binutils-gdb/gdb/infcmd.c:468
#7 0x00005606b512c236 in run_command (args=0x0, from_tty=1) at /home/smarchi/src/binutils-gdb/gdb/infcmd.c:526
When running the kill command, commit_resumed_state for the
process_stratum_target (linux-nat, here) is true. After the kill, when
there are no more threads, commit_resumed_state is still true, as
nothing touches this flag during the kill operation. During the
subsequent run command, run_command_1 does:
scoped_disable_commit_resumed disable_commit_resumed ("running");
We would think that this would clear the commit_resumed_state flag of
our native target, but that's not the case, because
scoped_disable_commit_resumed iterates on non-exited inferiors in order
to find active process targets. And after the kill, the inferior is
exited, and the native target was unpushed from it anyway. So
scoped_disable_commit_resumed doesn't touch the commit_resumed_state
flag of the native target, it stays true. When reaching target_wait, in
startup_inferior (to consume the initial expect stop events while the
inferior is starting up and working its way through the shell),
commit_resumed_state is true, breaking the contract saying that
commit_resumed_state is always false when calling the targets' wait
method.
(note: to be correct, I think that startup_inferior should toggle
commit_resumed between the target_wait and target_resume calls, but I'll
ignore that for now)
I can see multiple ways to fix this. In the end, we need
commit_resumed_state to be cleared by the time we get to that
target_wait. It could be done at the end of the kill command, or at the
beginning of the run command.
To keep things in a coherent state, I'd like to make it so that after
the kill command, when the target is left with no threads, its
commit_resumed_state flag is left to false. This way, we can keep
working with the assumption that a target with no threads (and therefore
no running threads) has commit_resumed_state == false.
Do this by adding a scoped_disable_commit_resumed in target_kill. It
clears the target's commit_resumed_state on entry, and leaves it false
if the target does not have any resumed thread on exit. That means,
even if the target has another inferior with stopped threads,
commit_resumed_state will be left to false, which makes sense.
Add a test that tries to cover various combinations of actions done
while an inferior is running (and therefore while commit_resumed_state
is true on the process target).
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28275
Change-Id: I8e6fe6dc1f475055921520e58cab68024039a1e9
Approved-By: Andrew Burgess <aburgess@redhat.com>
|
|
This commit addresses one of the issues identified in PR gdb/28275.
Bug gdb/28275 identifies a number of situations in which this assert:
Assertion `!proc_target->commit_resumed_state' failed.
could be triggered. There's actually a number of similar places where
this assert is found in GDB, the two of interest in gdb/28275 are in
target_wait and target_stop.
In one of the comments:
https://sourceware.org/bugzilla/show_bug.cgi?id=28275#c1
steps to trigger the assertion within target_stop were identified when
using a modified version of the gdb.threads/detach-step-over.exp test
script.
In the gdb.threads/detach-step-over.exp test, we attach to a
multi-threaded inferior, and continue the inferior in asynchronous
(background) mode. Each thread is continuously hitting a conditional
breakpoint where the condition is always false. While the inferior is
running we detach. The goal is that we detach while GDB is performing a
step-over for the conditional breakpoint in at least one thread.
While detaching, if a step-over is in progress, then GDB has to
complete the step over before we can detach. This involves calling
target_stop and target_wait (see prepare_for_detach).
As far as gdb/28275 is concerned, the interesting part here, is the
the process_stratum_target::commit_resumed_state variable must be
false when target_stop and target_wait are called.
This is currently ensured because, in detach_command (infrun.c), we
create an instance of scoped_disable_commit_resumed, this ensures that
when target_detach is called, ::commit_resumed_state will be false.
The modification to the test that I propose here, and which exposed
the bug, is that, instead of using "detach" to detach from the
inferior, we instead use "quit". Quitting GDB after attaching to an
inferior will cause GDB to first detach, and then exit.
When we quit GDB we end up calling target_detach via a different code
path, the stack looks like:
#0 target_detach
#1 kill_or_detach
#2 quit_force
#3 quit_command
Along this path there is no scoped_disable_commit_resumed created.
::commit_resumed_state can be true when we reach prepare_for_detach,
which calls target_wait and target_stop, so the assertion will trigger.
In this commit, I propose fixing this by adding the creation of a
scoped_disable_commit_resumed into target_detach. This will ensure
that ::commit_resumed_state is false when calling prepare_for_detach
from within target_detach.
I did consider placing the scoped_disable_commit_resumed in
prepare_for_detach, however, placing it in target_detach ensures that
the target's commit_resumed_state flag is left to false if the detached
inferior was the last one for that target. It's the same rationale as
for patch "gdb: disable commit resumed in target_kill" that comes later
in this series, but for detach instead of kill.
detach_command still includes a scoped_disable_commit_resumed too, but I
think it is still relevant to cover the resumption at the end of the
function.
Co-Authored-By: Simon Marchi <simon.marchi@efficios.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28275
Change-Id: Ie128f7aba6ef0e018859275eca372e6ea738e96f
|
|
Some class members were changed to bool, but there was
still some assignments or comparisons using 0/1.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
Currently, every internal_error call must be passed __FILE__/__LINE__
explicitly, like:
internal_error (__FILE__, __LINE__, "foo %d", var);
The need to pass in explicit __FILE__/__LINE__ is there probably
because the function predates widespread and portable variadic macros
availability. We can use variadic macros nowadays, and in fact, we
already use them in several places, including the related
gdb_assert_not_reached.
So this patch renames the internal_error function to something else,
and then reimplements internal_error as a variadic macro that expands
__FILE__/__LINE__ itself.
The result is that we now should call internal_error like so:
internal_error ("foo %d", var);
Likewise for internal_warning.
The patch adjusts all calls sites. 99% of the adjustments were done
with a perl/sed script.
The non-mechanical changes are in gdbsupport/errors.h,
gdbsupport/gdb_assert.h, and gdb/gdbarch.py.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Change-Id: Ia6f372c11550ca876829e8fd85048f4502bdcf06
|
|
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
|
|
I don't see why include/gdb/fileio.h is placed there. It's not
installed by "make install", and it's not included by anything outside
of gdb/gdbserver/gdbsupport.
Move its content back to gdbsupport/fileio.h. I have omitted the bits
inside an `#if 0`, since it's obviously not used, as well as the
"limits" constants, which are also unused.
Change-Id: I6fbc2ea10fbe4cfcf15f9f76006b31b99c20e5a9
|
|
This changes the parameter of target_ops::async from int to bool.
Regression tested on x86-64 Fedora 34.
|
|
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.
|
|
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
|
|
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.
|
|
target_read_string takes a byte order parameter, but only uses this to
check whether a given character is zero. This is readily done without
requiring the parameter, so remove it.
|
|
This renames read_string to be an overload of target_read_string.
This makes it more consistent for the eventual merger with gdbserver.
|
|
Commit df22c1e5d53c38f38bce6072bb46de240f9e0e2b handled the case that
a separate debug file was passed as the objfile for a shared library
to svr4_fetch_objfile_link_map. However, a separate debug file can
also be passed for TLS variables in the main executable. In addition,
frv_fetch_objfile_link_map also expects to be passed the original
objfile rather than a separate debug file, so pull the code to resolve
a separate debug file to the main objfile up into
target_translate_tls_address.
|
|
Now that filtered and unfiltered output can be treated identically, we
can unify the printf family of functions. This is done under the name
"gdb_printf". Most of this patch was written by script.
|
|
Now that filtered and unfiltered output can be treated identically, we
can unify the putc family of functions. This is done under the name
"gdb_putc". Most of this patch was written by script.
|
|
Now that filtered and unfiltered output can be treated identically, we
can unify the puts family of functions. This is done under the name
"gdb_puts". Most of this patch was written by script.
|
|
A number of spots call printf_unfiltered only because they are in code
that should not be interrupted by the pager. However, I believe these
cases are all handled by infrun's blanket ban on paging, and so can be
converted to the default (_filtered) API.
After this patch, I think all the remaining _unfiltered calls are ones
that really ought to be. A few -- namely in complete_command -- could
be replaced by a scoped assignment to pagination_enabled, but for the
remainder, the code seems simple enough like this.
|
|
Commit b60cea7 (Make target_wait options use enum flags) broke
deprecated_target_wait_hook usage: there's a commit comment telling
this hook has not been converted.
Rather than trying to mend it, this patch replaces the hook by two
target_wait observers:
target_pre_wait (ptid_t ptid)
target_post_wait (ptid_t event_ptid)
Upon target_wait entry, target_pre_wait is notified with the ptid
passed to target_wait. Upon exit, target_post_wait is notified with
the event ptid returned by target_wait. Should an exception occur,
event_ptid is null_ptid.
This change benefits to Insight (out-of-tree): there's no real use of the
late hook in gdb itself.
|
|
Enabling async mode above the target layer removes duplicate code in
::resume methods of async-capable targets. Commit 5b6d1e4fa4f
("Multi-target support") enabled async mode in do_target_resume after
target_resume returns which is a step in this direction. However,
other callers of target_resume such as target_continue do not enable
async mode. Rather than enabling async mode in each of the callers
after target_resume returns, enable async mode at the end of
target_resume.
|
|
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.
|
|
This simplifies things a bit, as we don't need two variables and think
about reverting target_async_permitted_1 and target_non_stop_enabled_1
values if we can't change the setting.
Change-Id: I36acab045dacf02ae1988486cfdb27c1dff309f6
|
|
Lancelot pointed out that target_announce_attach was missing an
explicit check against nullptr. This patch adds it.
|
|
target_announce_detach was added in commit 0f48b757 ("Factor out
"Detaching from program" message printing"). There, Pedro wrote:
(For now, I left the couple targets that print this a bit differently
alone. Maybe this could be further pulled out into infcmd.c. If we
did that, and those targets want to continue printing differently,
this new function could be converted to a target method.)
It seems to me that the differences aren't very big, and in some cases
other targets handled the output a bit more nicely. In particular,
some targets will print a different message when exec_file==NULL,
rather than printing the same output with an empty string as
exec_file.
This patch incorporates the nicer output into target_announce_detach,
then changes the remaining ports to use this function.
|
|
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.
|
|
This changes one terminal_info implementation, and
default_terminal_info, to use filtered output. Other implementations
of this method already use filtered output.
I can't compile go32-nat.c, so this is a 'best effort' patch.
|
|
This changes the implementations of the target files_info method to
use filtered output. This makes sense because the sole caller of this
method is an ordinary command (info_program_command). This patch
changes this command to use filtered output as well.
|
|
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.
|
|
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.
|
|
The target_async_permitted flag allows a user to override whether a
target can act in async mode or not. In previous commits I have moved
the checking of this flag out of the various ::can_async_p methods and
into the common target.c code.
In this commit I will add some additional assertions into
target_is_async_p and target_async. The rules these assertions are
checking are:
1. A target that returns false for target_can_async_p should never
become "async enabled", and so ::is_async_p should always return
false. This is being checked in target_is_async_p.
2. GDB should never try to enable async mode for a target that
returns false for target_can_async_p, this is checked in
target_async.
There are a few places where we call the ::is_async_p method directly,
in these cases we will obviously not pass through the assert in
target_is_async_p, however, there are also plenty of places where we
do call target_is_async_p so if GDB starts to misbehave we should
catch it quickly enough.
There should be no user visible changes after this commit.
|
|
This commit moves the target_async_permitted check out of each targets
::can_async_p method and into the target_can_async_p wrapper function.
I've left some asserts in the two ::can_async_p methods that I
changed, which will hopefully catch any direct calls to these methods
that might be added in the future.
There should be no user visible changes after this commit.
|
|
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.
|
|
The motivation is to reduce the number of places where unmanaged
pointers are returned from allocation type routines. All of the
callers are updated.
There should be no user visible changes after this commit.
|
|
I stumbled on a bug caused by the fact that a code path read
target_waitstatus::value::sig (expecting it to contain a gdb_signal
value) while target_waitstatus::kind was TARGET_WAITKIND_FORKED. This
meant that the active union field was in fact
target_waitstatus::value::related_pid, and contained a ptid. The read
signal value was therefore garbage, and that caused GDB to crash soon
after. Or, since that GDB was built with ubsan, this nice error
message:
/home/simark/src/binutils-gdb/gdb/linux-nat.c:1271:12: runtime error: load of value 2686365, which is not a valid value for type 'gdb_signal'
Despite being a large-ish change, I think it would be nice to make
target_waitstatus safe against that kind of bug. As already done
elsewhere (e.g. dynamic_prop), validate that the type of value read from
the union matches what is supposed to be the active field.
- Make the kind and value of target_waitstatus private.
- Make the kind initialized to TARGET_WAITKIND_IGNORE on
target_waitstatus construction. This is what most users appear to do
explicitly.
- Add setters, one for each kind. Each setter takes as a parameter the
data associated to that kind, if any. This makes it impossible to
forget to attach the associated data.
- Add getters, one for each associated data type. Each getter
validates that the data type fetched by the user matches the wait
status kind.
- Change "integer" to "exit_status", "related_pid" to "child_ptid",
just because that's more precise terminology.
- Fix all users.
That last point is semi-mechanical. There are a lot of obvious changes,
but some less obvious ones. For example, it's not possible to set the
kind at some point and the associated data later, as some users did.
But in any case, the intent of the code should not change in this patch.
This was tested on x86-64 Linux (unix, native-gdbserver and
native-extended-gdbserver boards). It was built-tested on x86-64
FreeBSD, NetBSD, MinGW and macOS. The rest of the changes to native
files was done as a best effort. If I forgot any place to update in
these files, it should be easy to fix (unless the change happens to
reveal an actual bug).
Change-Id: I0ae967df1ff6e28de78abbe3ac9b4b2ff4ad03b7
|
|
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>
|
|
Rename thread_info::executing to thread_info::m_executing, and make it
private. Add a new get/set member functions, and convert GDB to make
use of these.
The only real change of interest in this patch is in thread.c where I
have deleted the helper function set_executing_thread, and now just
use the new set function thread_info::set_executing. However, the old
helper function set_executing_thread included some code to reset the
thread's stop_pc, so I moved this code into the new function
thread_info::set_executing. However, I don't believe there is
anywhere that this results in a change of behaviour, previously the
executing flag was always set true through a call to
set_executing_thread anyway.
|
|
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
|
|
I don't understand what the sfunc function type in
cmd_list_element::function is for. Compared to cmd_simple_func_ftype,
it has an extra cmd_list_element parameter, giving the callback access
to the cmd_list_element for the command being invoked. This allows
registering the same callback with many commands, and alter the behavior
using the cmd_list_element's context.
From the comment in cmd_list_element, it sounds like at some point it
was the callback function type for set and show functions, hence the
"s". But nowadays, it's used for many more commands that need to access
the cmd_list_element object (see add_catch_command for example).
I don't really see the point of having sfunc at all, since do_sfunc is
just a trivial shim that changes the order of the arguments. All
commands using sfunc could just as well set cmd_list_element::func to
their callback directly.
Therefore, remove the sfunc field in cmd_list_element and everything
that goes with it. Rename cmd_const_sfunc_ftype to cmd_func_ftype and
use it for cmd_list_element::func, as well as for the add_setshow
commands.
Change-Id: I1eb96326c9b511c293c76996cea0ebc51c70fac0
|
|
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
|
|
Commit 408f66864a1a823591b26420410c982174c239a2 ("detach in all-stop
with threads running") regressed "detach" with "target remote":
(gdb) detach
Detaching from program: target:/any/program, process 3671843
Detaching from process 3671843
Ending remote debugging.
[Inferior 1 (process 3671843) detached]
In main
terminate called after throwing an instance of 'gdb_exception_error'
Aborted (core dumped)
Here's the exception above being thrown:
(top-gdb) bt
#0 throw_error (error=TARGET_CLOSE_ERROR, fmt=0x555556035588 "Remote connection closed") at src/gdbsupport/common-exceptions.cc:222
#1 0x0000555555bbaa46 in remote_target::readchar (this=0x555556a11040, timeout=10000) at src/gdb/remote.c:9440
#2 0x0000555555bbb9e5 in remote_target::getpkt_or_notif_sane_1 (this=0x555556a11040, buf=0x555556a11058, forever=0, expecting_notif=0, is_notif=0x0) at src/gdb/remote.c:9928
#3 0x0000555555bbbda9 in remote_target::getpkt_sane (this=0x555556a11040, buf=0x555556a11058, forever=0) at src/gdb/remote.c:10030
#4 0x0000555555bc0e75 in remote_target::remote_hostio_send_command (this=0x555556a11040, command_bytes=13, which_packet=14, remote_errno=0x7fffffffcfd0, attachment=0x0, attachment_len=0x0) at src/gdb/remote.c:12137
#5 0x0000555555bc1b6c in remote_target::remote_hostio_close (this=0x555556a11040, fd=8, remote_errno=0x7fffffffcfd0) at src/gdb/remote.c:12455
#6 0x0000555555bc1bb4 in remote_target::fileio_close (During symbol reading: .debug_line address at offset 0x64f417 is 0 [in module build/gdb/gdb]
this=0x555556a11040, fd=8, remote_errno=0x7fffffffcfd0) at src/gdb/remote.c:12462
#7 0x0000555555c9274c in target_fileio_close (fd=3, target_errno=0x7fffffffcfd0) at src/gdb/target.c:3365
#8 0x000055555595a19d in gdb_bfd_iovec_fileio_close (abfd=0x555556b9f8a0, stream=0x555556b11530) at src/gdb/gdb_bfd.c:439
#9 0x0000555555e09e3f in opncls_bclose (abfd=0x555556b9f8a0) at src/bfd/opncls.c:599
#10 0x0000555555e0a2c7 in bfd_close_all_done (abfd=0x555556b9f8a0) at src/bfd/opncls.c:847
#11 0x0000555555e0a27a in bfd_close (abfd=0x555556b9f8a0) at src/bfd/opncls.c:814
#12 0x000055555595a9d3 in gdb_bfd_close_or_warn (abfd=0x555556b9f8a0) at src/gdb/gdb_bfd.c:626
#13 0x000055555595ad29 in gdb_bfd_unref (abfd=0x555556b9f8a0) at src/gdb/gdb_bfd.c:715
#14 0x0000555555ae4730 in objfile::~objfile (this=0x555556515540, __in_chrg=<optimized out>) at src/gdb/objfiles.c:573
#15 0x0000555555ae955a in std::_Sp_counted_ptr<objfile*, (__gnu_cxx::_Lock_policy)2>::_M_dispose (this=0x555556c20db0) at /usr/include/c++/9/bits/shared_ptr_base.h:377
#16 0x000055555572b7c8 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x555556c20db0) at /usr/include/c++/9/bits/shared_ptr_base.h:155
#17 0x00005555557263c3 in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (this=0x555556bf0588, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/shared_ptr_base.h:730
#18 0x0000555555ae745e in std::__shared_ptr<objfile, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (this=0x555556bf0580, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/shared_ptr_base.h:1169
#19 0x0000555555ae747e in std::shared_ptr<objfile>::~shared_ptr (this=0x555556bf0580, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/shared_ptr.h:103
#20 0x0000555555b1c1dc in __gnu_cxx::new_allocator<std::_List_node<std::shared_ptr<objfile> > >::destroy<std::shared_ptr<objfile> > (this=0x5555564cdd60, __p=0x555556bf0580) at /usr/include/c++/9/ext/new_allocator.h:153
#21 0x0000555555b1bb1d in std::allocator_traits<std::allocator<std::_List_node<std::shared_ptr<objfile> > > >::destroy<std::shared_ptr<objfile> > (__a=..., __p=0x555556bf0580) at /usr/include/c++/9/bits/alloc_traits.h:497
#22 0x0000555555b1b73e in std::__cxx11::list<std::shared_ptr<objfile>, std::allocator<std::shared_ptr<objfile> > >::_M_erase (this=0x5555564cdd60, __position=std::shared_ptr<objfile> (expired, weak count 1) = {get() = 0x555556515540}) at /usr/include/c++/9/bits/stl_list.h:1921
#23 0x0000555555b1afeb in std::__cxx11::list<std::shared_ptr<objfile>, std::allocator<std::shared_ptr<objfile> > >::erase (this=0x5555564cdd60, __position=std::shared_ptr<objfile> (expired, weak count 1) = {get() = 0x555556515540}) at /usr/include/c++/9/bits/list.tcc:158
#24 0x0000555555b19576 in program_space::remove_objfile (this=0x5555564cdd20, objfile=0x555556515540) at src/gdb/progspace.c:210
#25 0x0000555555ae4502 in objfile::unlink (this=0x555556515540) at src/gdb/objfiles.c:487
#26 0x0000555555ae5a12 in objfile_purge_solibs () at src/gdb/objfiles.c:875
#27 0x0000555555c09686 in no_shared_libraries (ignored=0x0, from_tty=1) at src/gdb/solib.c:1236
#28 0x00005555559e3f5f in detach_command (args=0x0, from_tty=1) at src/gdb/infcmd.c:2769
So frame #28 already detached the remote process, and then we're
purging the shared libraries. GDB had opened remote shared libraries
via the target: sysroot, so it tries closing them. GDBserver is
tearing down already, so remote communication breaks down and we close
the remote target and throw TARGET_CLOSE_ERROR.
Note frame #14:
#14 0x0000555555ae4730 in objfile::~objfile (this=0x555556515540, __in_chrg=<optimized out>) at src/gdb/objfiles.c:573
That's a dtor, thus noexcept. That's the reason for the
std::terminate.
Stepping back a bit, why do we still have open remote files if we've
managed to detach already, and, we're debugging with "target remote"?
The reason is that commit 408f66864a1a823591b26420410c982174c239a2
makes detach_command hold a reference to the target, so the remote
target won't be finally closed until frame #28 returns. It's closing
the target that invalidates target file I/O handles.
This commit fixes the issue by not relying on target_close to
invalidate the target file I/O handles, instead invalidate them
immediately in remote_unpush_target. So when GDB purges the solibs,
and we end up in target_fileio_close (frame #7 above), there's nothing
to do, and we don't try to talk with the remote target anymore.
The regression isn't seen when testing with
--target_board=native-gdbserver, because that does "set sysroot" to
disable the "target:" sysroot, for test run speed reasons. So this
commit adds a testcase that explicitly tests detach with "set sysroot
target:".
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <pedro@palves.net>
PR gdb/28080
* remote.c (remote_unpush_target): Invalidate file I/O target
handles.
* target.c (fileio_handles_invalidate_target): Make extern.
* target.h (fileio_handles_invalidate_target): Declare.
gdb/testsuite/ChangeLog:
yyyy-mm-dd Pedro Alves <pedro@palves.net>
PR gdb/28080
* gdb.base/detach-sysroot-target.exp: New.
* gdb.base/detach-sysroot-target.c: New.
Reported-By: Jonah Graham <jonah@kichwacoders.com>
Change-Id: I851234910172f42a1b30e731161376c344d2727d
|
|
Straightforward replacement of get_cmd_context / set_cmd_context with
cmd_list_element methods.
gdb/ChangeLog:
* cli/cli-decode.h (struct cmd_list_element) <set_context,
context>: New.
<context>: Rename to...
<m_context>: ... this.
* cli/cli-decode.c (set_cmd_context, get_cmd_context): Remove.
* command.h (set_cmd_context, get_cmd_context): Remove, use
cmd_list_element::set_context and cmd_list_element::context
everywhere instead.
Change-Id: I5016b0079014e3f17d1aa449ada7954473bf2b5d
|
|
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
|
|
Previously, the prefixname field of struct cmd_list_element was manually
set for prefix commands. This seems verbose and error prone as it
required every single call to functions adding prefix commands to
specify the prefix name while the same information can be easily
generated.
Historically, this was not possible as the prefix field was null for
many commands, but this was fixed in commit
3f4d92ebdf7f848b5ccc9e8d8e8514c64fde1183 by Philippe Waroquiers, so
we can rely on the prefix field being set when generating the prefix
name.
This commit also fixes a use after free in this scenario:
* A command gets created via Python (using the gdb.Command class).
The prefix name member is dynamically allocated.
* An alias to the new command is created. The alias's prefixname is set
to point to the prefixname for the original command with a direct
assignment.
* A new command with the same name as the Python command is created.
* The object for the original Python command gets freed and its
prefixname gets freed as well.
* The alias is updated to point to the new command, but its prefixname
is not updated so it keeps pointing to the freed one.
gdb/ChangeLog:
* command.h (add_prefix_cmd): Remove the prefixname argument as
it can now be generated automatically. Update all callers.
(add_basic_prefix_cmd): Ditto.
(add_show_prefix_cmd): Ditto.
(add_prefix_cmd_suppress_notification): Ditto.
(add_abbrev_prefix_cmd): Ditto.
* cli/cli-decode.c (add_prefix_cmd): Ditto.
(add_basic_prefix_cmd): Ditto.
(add_show_prefix_cmd): Ditto.
(add_prefix_cmd_suppress_notification): Ditto.
(add_prefix_cmd_suppress_notification): Ditto.
(add_abbrev_prefix_cmd): Ditto.
* cli/cli-decode.h (struct cmd_list_element): Replace the
prefixname member variable with a method which generates the
prefix name at runtime. Update all code reading the prefix
name to use the method, and remove all code setting it.
* python/py-cmd.c (cmdpy_destroyer): Remove code to free the
prefixname member as it's now a method.
(cmdpy_function): Determine if the command is a prefix by
looking at prefixlist, not prefixname.
|
