[v2,00/24] Multi-target support
Commit Message
On 10/18/19 9:22 PM, John Baldwin wrote:
> On 10/17/19 3:50 PM, Pedro Alves wrote:
>> Here's v2 of the multi-target patchset, which addresses all the review
>> comments so far, I believe. Patch 15 is new, so all following patches
>> are shifted by one.
>>
>> This time, I've adjusted the host-specific nat files to function API
>> changes. I tried to find spots that would need changes using grep. I
>> built the series on AIX, x86/SPARC Solaris, 64-bit Windows, x86
>> GNU/Linux -m64/-m32, and Aarch64 GNU/Linux. I'm currently running
>> this through the buildbot, will have results tomorrow. I don't expect
>> any serious major issue, if any, as so far runs that completed seem
>> OK.
>
> I (finally) have a patch to fix the build on FreeBSD/amd64 (and probably
> FreeBSD on other platforms) here:
>
> https://github.com/bsdjhb/gdb/commit/e58a36eaef6244d2040ce6f377497ee898978db4
>
Thanks!
> It's a combined patch but has some commentary on bsd-kvm.c which is
> kind of special. That target adds new commands that need to find a
> target to operate on. I opted to have it look at the current inferior
> and if (using a dynamic cast) it is a bsd-kvm target the commands
> modify the state of that inferior. I haven't tested it though as I don't
> use bsd-kvm.c.
I'd rather not merge the bsd-kvm.c parts into my patches as is, for the
reason that it doesn't appear necessary for a minimal keep-working-as-before
change. I think it would be fine as a follow up patch, though.
A couple comments:
- I think we should get away from doing this
inferior_ptid = null_ptid;
exit_inferior_silent (current_inferior ());
in 'bsd_kvm_target::close ()'.
- Class private fields should be named 'm_foo', so m_kd, m_corefile, etc.
For bsd-kvm.c, I _think_ that the only change necessary to keep things
building would be this:
Comments
On 10/29/19 12:13 PM, Pedro Alves wrote:
> On 10/18/19 9:22 PM, John Baldwin wrote:
>> On 10/17/19 3:50 PM, Pedro Alves wrote:
>>> Here's v2 of the multi-target patchset, which addresses all the review
>>> comments so far, I believe. Patch 15 is new, so all following patches
>>> are shifted by one.
>>>
>>> This time, I've adjusted the host-specific nat files to function API
>>> changes. I tried to find spots that would need changes using grep. I
>>> built the series on AIX, x86/SPARC Solaris, 64-bit Windows, x86
>>> GNU/Linux -m64/-m32, and Aarch64 GNU/Linux. I'm currently running
>>> this through the buildbot, will have results tomorrow. I don't expect
>>> any serious major issue, if any, as so far runs that completed seem
>>> OK.
>>
>> I (finally) have a patch to fix the build on FreeBSD/amd64 (and probably
>> FreeBSD on other platforms) here:
>>
>> https://github.com/bsdjhb/gdb/commit/e58a36eaef6244d2040ce6f377497ee898978db4
>>
>
> Thanks!
>
>> It's a combined patch but has some commentary on bsd-kvm.c which is
>> kind of special. That target adds new commands that need to find a
>> target to operate on. I opted to have it look at the current inferior
>> and if (using a dynamic cast) it is a bsd-kvm target the commands
>> modify the state of that inferior. I haven't tested it though as I don't
>> use bsd-kvm.c.
>
> I'd rather not merge the bsd-kvm.c parts into my patches as is, for the
> reason that it doesn't appear necessary for a minimal keep-working-as-before
> change. I think it would be fine as a follow up patch, though.
Ok, that's fine.
> For bsd-kvm.c, I _think_ that the only change necessary to keep things
> building would be this:
>
> --- a/gdb/bsd-kvm.c
> +++ b/gdb/bsd-kvm.c
> @@ -136,7 +136,7 @@ bsd_kvm_target_open (const char *arg, int from_tty)
> core_kd = temp_kd;
> push_target (&bsd_kvm_ops);
>
> - add_thread_silent (bsd_kvm_ptid);
> + add_thread_silent (&bsd_kvm_ops, bsd_kvm_ptid);
> inferior_ptid = bsd_kvm_ptid;
>
> Right? See updated patch below.
Ok, this works for me.
On 1/9/20 7:31 PM, John Baldwin wrote:
> On 10/29/19 12:13 PM, Pedro Alves wrote:
>> For bsd-kvm.c, I _think_ that the only change necessary to keep things
>> building would be this:
>>
>> --- a/gdb/bsd-kvm.c
>> +++ b/gdb/bsd-kvm.c
>> @@ -136,7 +136,7 @@ bsd_kvm_target_open (const char *arg, int from_tty)
>> core_kd = temp_kd;
>> push_target (&bsd_kvm_ops);
>>
>> - add_thread_silent (bsd_kvm_ptid);
>> + add_thread_silent (&bsd_kvm_ops, bsd_kvm_ptid);
>> inferior_ptid = bsd_kvm_ptid;
>>
>> Right? See updated patch below.
>
> Ok, this works for me.
Excellent! I think I'll be merging this soon, just need to
address Aktemur's latest comments.
Thanks,
Pedro Alves
@@ -136,7 +136,7 @@ bsd_kvm_target_open (const char *arg, int from_tty)
core_kd = temp_kd;
push_target (&bsd_kvm_ops);
- add_thread_silent (bsd_kvm_ptid);
+ add_thread_silent (&bsd_kvm_ops, bsd_kvm_ptid);
inferior_ptid = bsd_kvm_ptid;
Right? See updated patch below.
> I did try a simple test of creating two inferiors both of which were
> running /bin/ls and gdb hung trying to run the second inferior via
> 'start'. I suspect this is some kind of bug in the FreeBSD target not
> being multi-target ready that I will have to debug.
Does that work without the multi-target series? I mean, maybe it's more
a multi-process debugging issue than a multi-target issue?
From 3f879fe9334cbd69f50efd0131c2bed5498cde92 Mon Sep 17 00:00:00 2001
From: John Baldwin <jhb@FreeBSD.org>
Date: Fri, 18 Oct 2019 13:12:11 -0700
Subject: [PATCH] Update FreeBSD native build for multi-target-v2.
---
gdb/bsd-kvm.c | 2 +-
gdb/fbsd-nat.c | 33 +++++++++++++++++----------------
2 files changed, 18 insertions(+), 17 deletions(-)
@@ -136,7 +136,7 @@ bsd_kvm_target_open (const char *arg, int from_tty)
core_kd = temp_kd;
push_target (&bsd_kvm_ops);
- add_thread_silent (bsd_kvm_ptid);
+ add_thread_silent (&bsd_kvm_ops, bsd_kvm_ptid);
inferior_ptid = bsd_kvm_ptid;
target_fetch_registers (get_current_regcache (), -1);
@@ -991,11 +991,11 @@ fbsd_enable_proc_events (pid_t pid)
called to discover new threads each time the thread list is updated. */
static void
-fbsd_add_threads (pid_t pid)
+fbsd_add_threads (fbsd_nat_target *target, pid_t pid)
{
int i, nlwps;
- gdb_assert (!in_thread_list (ptid_t (pid)));
+ gdb_assert (!in_thread_list (target, ptid_t (pid)));
nlwps = ptrace (PT_GETNUMLWPS, pid, NULL, 0);
if (nlwps == -1)
perror_with_name (("ptrace"));
@@ -1010,7 +1010,7 @@ fbsd_add_threads (pid_t pid)
{
ptid_t ptid = ptid_t (pid, lwps[i], 0);
- if (!in_thread_list (ptid))
+ if (!in_thread_list (target, ptid))
{
#ifdef PT_LWP_EVENTS
struct ptrace_lwpinfo pl;
@@ -1026,7 +1026,7 @@ fbsd_add_threads (pid_t pid)
fprintf_unfiltered (gdb_stdlog,
"FLWP: adding thread for LWP %u\n",
lwps[i]);
- add_thread (ptid);
+ add_thread (target, ptid);
}
}
}
@@ -1043,7 +1043,7 @@ fbsd_nat_target::update_thread_list ()
#else
prune_threads ();
- fbsd_add_threads (inferior_ptid.pid ());
+ fbsd_add_threads (this, inferior_ptid.pid ());
#endif
}
@@ -1174,7 +1174,7 @@ fbsd_nat_target::resume (ptid_t ptid, int step, enum gdb_signal signo)
if (ptid.lwp_p ())
{
/* If ptid is a specific LWP, suspend all other LWPs in the process. */
- inferior *inf = find_inferior_ptid (ptid);
+ inferior *inf = find_inferior_ptid (this, ptid);
for (thread_info *tp : inf->non_exited_threads ())
{
@@ -1193,7 +1193,7 @@ fbsd_nat_target::resume (ptid_t ptid, int step, enum gdb_signal signo)
{
/* If ptid is a wildcard, resume all matching threads (they won't run
until the process is continued however). */
- for (thread_info *tp : all_non_exited_threads (ptid))
+ for (thread_info *tp : all_non_exited_threads (this, ptid))
if (ptrace (PT_RESUME, tp->ptid.lwp (), NULL, 0) == -1)
perror_with_name (("ptrace"));
ptid = inferior_ptid;
@@ -1239,7 +1239,8 @@ fbsd_nat_target::resume (ptid_t ptid, int step, enum gdb_signal signo)
core, return true. */
static bool
-fbsd_handle_debug_trap (ptid_t ptid, const struct ptrace_lwpinfo &pl)
+fbsd_handle_debug_trap (fbsd_nat_target *target, ptid_t ptid,
+ const struct ptrace_lwpinfo &pl)
{
/* Ignore traps without valid siginfo or for signals other than
@@ -1266,7 +1267,7 @@ fbsd_handle_debug_trap (ptid_t ptid, const struct ptrace_lwpinfo &pl)
if (pl.pl_siginfo.si_code == TRAP_BRKPT)
{
/* Fixup PC for the software breakpoint. */
- struct regcache *regcache = get_thread_regcache (ptid);
+ struct regcache *regcache = get_thread_regcache (target, ptid);
struct gdbarch *gdbarch = regcache->arch ();
int decr_pc = gdbarch_decr_pc_after_break (gdbarch);
@@ -1340,7 +1341,7 @@ fbsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
threads might be skipped during post_attach that
have not yet reported their PL_FLAG_EXITED event.
Ignore EXITED events for an unknown LWP. */
- thread_info *thr = find_thread_ptid (wptid);
+ thread_info *thr = find_thread_ptid (this, wptid);
if (thr != nullptr)
{
if (debug_fbsd_lwp)
@@ -1364,13 +1365,13 @@ fbsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
PL_FLAG_BORN in case the first stop reported after
attaching to an existing process is a PL_FLAG_BORN
event. */
- if (in_thread_list (ptid_t (pid)))
+ if (in_thread_list (this, ptid_t (pid)))
{
if (debug_fbsd_lwp)
fprintf_unfiltered (gdb_stdlog,
"FLWP: using LWP %u for first thread\n",
pl.pl_lwpid);
- thread_change_ptid (ptid_t (pid), wptid);
+ thread_change_ptid (this, ptid_t (pid), wptid);
}
#ifdef PT_LWP_EVENTS
@@ -1380,13 +1381,13 @@ fbsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
threads might be added by fbsd_add_threads that have
not yet reported their PL_FLAG_BORN event. Ignore
BORN events for an already-known LWP. */
- if (!in_thread_list (wptid))
+ if (!in_thread_list (this, wptid))
{
if (debug_fbsd_lwp)
fprintf_unfiltered (gdb_stdlog,
"FLWP: adding thread for LWP %u\n",
pl.pl_lwpid);
- add_thread (wptid);
+ add_thread (this, wptid);
}
ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
return wptid;
@@ -1474,7 +1475,7 @@ fbsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
#endif
#ifdef USE_SIGTRAP_SIGINFO
- if (fbsd_handle_debug_trap (wptid, pl))
+ if (fbsd_handle_debug_trap (this, wptid, pl))
return wptid;
#endif
@@ -1633,7 +1634,7 @@ void
fbsd_nat_target::post_attach (int pid)
{
fbsd_enable_proc_events (pid);
- fbsd_add_threads (pid);
+ fbsd_add_threads (this, pid);
}
#ifdef PL_FLAG_EXEC