From: Pedro Alves <palves@redhat.com>
To: "Breazeal, Don" <donb@codesourcery.com>, gdb-patches@sourceware.org
Subject: Re: [PATCH 01/16 v2] Refactor native follow-fork
Date: Tue, 09 Sep 2014 10:57:00 -0000 [thread overview]
Message-ID: <540EDD17.30001@redhat.com> (raw)
In-Reply-To: <540A0765.7080602@codesourcery.com>
On 09/05/2014 07:56 PM, Breazeal, Don wrote:
> Hi Pedro,
> Thanks for reviewing this.
>
> On 9/5/2014 7:20 AM, Pedro Alves wrote:
>> linux_child_follow_fork ends up with:
>>
>> static int
>> linux_child_follow_fork (struct target_ops *ops, int follow_child,
>> int detach_fork)
>> {
>> int has_vforked;
>> int parent_pid, child_pid;
>>
>> has_vforked = (inferior_thread ()->pending_follow.kind
>> == TARGET_WAITKIND_VFORKED);
>> parent_pid = ptid_get_lwp (inferior_ptid);
>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> if (parent_pid == 0)
>> parent_pid = ptid_get_pid (inferior_ptid);
>> child_pid
>> = ptid_get_pid (inferior_thread ()->pending_follow.value.related_pid);
>>
>> if (!follow_child)
>> {
>> ...
>> }
>> else
>> {
>> struct lwp_info *child_lp;
>>
>> child_lp = add_lwp (inferior_ptid);
>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> child_lp->stopped = 1;
>> child_lp->last_resume_kind = resume_stop;
>>
>> /* Let the thread_db layer learn about this new process. */
>> check_for_thread_db ();
>> }
>> }
>>
>> Nothing appears to switch inferior_ptid to the child, so seems
>> like we're adding the child_lp with the wrong lwp (and calling
>> check_for_thread_db in the wrong context) ? Is this managing
>> to work by chance because follow_fork_inferior leaves inferior_ptid
>> pointing to the child?
>
> Yes, follow_fork_inferior always sets inferior_ptid to the followed
> inferior.
Ah.
> On entry, linux_child_follow_fork expects inferior_ptid to be
> the followed inferior.
I see. It does make sense.
> So I think it is getting the correct inferior
> from inferior_ptid in these cases. I can change that if you prefer; see
> my question below about acceptable solutions.
>
> Regarding check_for_thread_db, there is something unrelated that I don't
> understand here. If we have reached this function, then aren't we
> guaranteed that PTRACE_O_TRACECLONE is supported, and that we are using
> that instead of libthread_db for detecting thread events? If so, why do
> we need to call check_for_thread_db at all?
Unlike GDBserver, GDB actually still use libthread_db's create/exit event
breakpoints even if PTRACE_O_TRACECLONE is supported. I think this is
just historical at this point, and we could skip those event breakpoints,
optimizing out a bunch of internal stops.
The check_for_thread_db call has another effect -- as mentioned in the
comment:
/* Let the thread_db layer learn about this new process. */
check_for_thread_db ();
if we don't call it, then linux-thread-db.c never adds the child
process to its global thread_db_list list:
/* List of known processes using thread_db, and the required
bookkeeping. */
struct thread_db_info *thread_db_list;
We don't really need to try all the available libthread_db's found
in the path, we could just try try_thread_db_load with the same
libthread_db we had loaded for the parent, or even skip that
and share/refcount the bookkeeping in 'struct thread_db_info'
(dlopen handle, functions pointers, etc.) between parent and child.
This is about the same issue as mentioned just above:
/* Let the shared library layer (solib-svr4) learn about
this new process, relocate the cloned exec, pull in
shared libraries, and install the solib event breakpoint.
If a "cloned-VM" event was propagated better throughout
the core, this wouldn't be required. */
>
> Then this at the top uses the wrong
>> inferior_thread ():
>>
>> has_vforked = (inferior_thread ()->pending_follow.kind
>> == TARGET_WAITKIND_VFORKED);
>>
>>
>> and we're lucky that nothing end up using has_vforked in the
>> follow child path?
>
> You are right, this is incorrect and unnecessary in the case where we
> are following the child.
>
>>
>> I'd much rather we don't have these assumptions in place.
>
> Would an acceptable solution be to move the definitions and assignments
> of has_vforked, parent_pid, and child_pid into the follow-parent case,
> as below?
Yes.
>
> Would you also prefer that on entry to linux_child_follow_fork,
> inferior_ptid is set to the parent like it was before, or would a
> comment explaining that inferior_ptid is expected to be the followed
> inferior be sufficient?
The comment would be sufficient.
Thanks,
Pedro Alves
next prev parent reply other threads:[~2014-09-09 10:57 UTC|newest]
Thread overview: 110+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-07 18:00 [PATCH 00/10] Linux extended-remote fork events Don Breazeal
2014-08-07 18:00 ` [PATCH 06/10] Extended-remote follow fork Don Breazeal
2014-08-07 18:00 ` [PATCH 05/10] GDBserver clone breakpoint list Don Breazeal
2014-08-07 18:00 ` [PATCH 07/10] Extended-remote arch-specific follow fork Don Breazeal
2014-08-07 18:00 ` [PATCH 03/10] Refactor extended ptrace event status Don Breazeal
2014-08-07 18:00 ` [PATCH 04/10] Enhance extended ptrace event setup Don Breazeal
2014-08-13 17:50 ` Breazeal, Don
2014-08-07 18:00 ` [PATCH 01/10] Refactor native follow-fork Don Breazeal
2014-08-07 18:00 ` [PATCH 02/10] Refactor follow-fork message printing Don Breazeal
2014-08-07 18:01 ` [PATCH 08/10] Extended-remote follow vfork Don Breazeal
2014-08-07 18:01 ` [PATCH 10/10] Extended-remote fork event documentation Don Breazeal
2014-08-07 19:31 ` Eli Zaretskii
2014-08-08 15:35 ` Breazeal, Don
2014-08-07 18:01 ` [PATCH 09/10] Extended-remote fork catchpoints Don Breazeal
2014-08-21 0:29 ` [Patch 00/16 v2] Linux extended-remote fork and exec events Don Breazeal
2014-09-04 20:57 ` Breazeal, Don
2014-10-31 23:29 ` [PATCH 00/16 v3] " Don Breazeal
2014-11-12 15:54 ` Pedro Alves
2014-11-13 13:41 ` Pedro Alves
2014-11-13 13:51 ` Pedro Alves
2014-11-13 14:58 ` Pedro Alves
2014-11-13 19:14 ` Pedro Alves
2014-10-31 23:29 ` [PATCH 08/16 v3] Extended-remote follow vfork Don Breazeal
2014-10-31 23:29 ` [PATCH 05/16 v3] GDBserver clone breakpoint list Don Breazeal
2014-10-31 23:29 ` [PATCH 06/16 v3] Extended-remote Linux follow fork Don Breazeal
2014-11-13 13:00 ` Pedro Alves
2014-11-13 18:53 ` Breazeal, Don
2014-11-13 18:59 ` Pedro Alves
2014-11-13 19:06 ` Breazeal, Don
2014-12-06 0:31 ` Breazeal, Don
2015-01-23 12:53 ` Pedro Alves
2015-01-23 17:18 ` Breazeal, Don
[not found] ` <1422222420-25421-1-git-send-email-donb@codesourcery.com>
2015-01-25 21:49 ` [PATCH v4 5/7] Arch-specific remote " Don Breazeal
2015-02-10 16:37 ` Pedro Alves
2015-01-25 21:49 ` [PATCH v4 6/7] Remote follow vfork Don Breazeal
2015-02-10 16:39 ` Pedro Alves
2015-01-25 21:50 ` [PATCH v4 1/7] Identify remote fork event support Don Breazeal
2015-02-10 16:34 ` Pedro Alves
2015-01-25 21:50 ` [PATCH v4 2/7] Clone remote breakpoints Don Breazeal
2015-01-25 21:58 ` [PATCH v4 7/7] Remote fork catch Don Breazeal
2015-01-26 0:07 ` [PATCH v4 3/7 v3] Extended-remote Linux follow fork Don Breazeal
2015-02-10 16:36 ` Pedro Alves
2015-01-26 0:20 ` [PATCH v4 4/7] Target remote " Don Breazeal
2015-01-12 22:39 ` [PATCH 06/16 v3] Extended-remote Linux " Don Breazeal
2015-01-12 22:49 ` Breazeal, Don
2014-10-31 23:29 ` [PATCH 04/16 v3] Determine supported extended-remote features Don Breazeal
2014-11-13 12:59 ` Pedro Alves
2014-11-13 18:28 ` Breazeal, Don
2014-11-13 18:33 ` Pedro Alves
2014-11-13 19:08 ` Pedro Alves
2014-11-13 18:37 ` Breazeal, Don
2014-11-13 18:48 ` Pedro Alves
2014-12-06 0:30 ` Breazeal, Don
2015-01-12 22:36 ` Don Breazeal
2015-01-21 21:02 ` Breazeal, Don
2014-10-31 23:29 ` [PATCH 07/16 v3] Extended-remote arch-specific follow fork Don Breazeal
2014-10-31 23:30 ` [PATCH 09/16 v3] Extended-remote fork catchpoints Don Breazeal
2014-10-31 23:30 ` [PATCH 10/16 v3] Extended-remote fork event documentation Don Breazeal
2014-10-31 23:30 ` [PATCH 12/16 v3] Extended-remote follow exec Don Breazeal
2014-10-31 23:30 ` [PATCH 13/16 v3] Extended-remote exec catchpoints Don Breazeal
2014-10-31 23:30 ` [PATCH 11/16 v3] Extended-remote Linux exit events Don Breazeal
2014-11-13 19:18 ` Pedro Alves
2014-10-31 23:31 ` [PATCH 15/16 v3] Extended-remote exec event documentation Don Breazeal
2014-10-31 23:31 ` [PATCH 16/16 v3] Non-stop follow exec tests Don Breazeal
2014-10-31 23:31 ` [PATCH 14/16 v3] Suppress spurious warnings with extended-remote follow exec Don Breazeal
2014-08-21 0:29 ` [PATCH 01/16 v2] Refactor native follow-fork Don Breazeal
2014-09-05 14:20 ` Pedro Alves
2014-09-05 18:56 ` Breazeal, Don
2014-09-05 20:20 ` Breazeal, Don
2014-09-09 10:57 ` Pedro Alves [this message]
2014-09-08 23:54 ` Breazeal, Don
2014-09-09 11:09 ` Pedro Alves
2014-09-12 16:50 ` Breazeal, Don
2014-09-22 15:53 ` Breazeal, Don
2014-09-26 18:13 ` Pedro Alves
2014-09-29 18:08 ` Breazeal, Don
2014-09-30 10:56 ` Pedro Alves
2014-09-30 18:43 ` Breazeal, Don
2014-08-21 0:30 ` [PATCH 02/16 v2] Refactor follow-fork message printing Don Breazeal
2014-09-26 19:52 ` Pedro Alves
2014-09-26 20:14 ` Breazeal, Don
2014-10-03 23:51 ` Breazeal, Don
2014-10-15 16:08 ` Pedro Alves
2014-10-22 23:47 ` Breazeal, Don
2014-10-24 12:35 ` Pedro Alves
2014-10-24 18:45 ` Breazeal, Don
2014-08-21 0:30 ` [PATCH 04/16 v2] Determine supported extended-remote features Don Breazeal
2014-10-15 16:17 ` Pedro Alves
2014-10-21 23:23 ` Breazeal, Don
2014-10-22 21:48 ` Pedro Alves
2014-10-31 23:38 ` Breazeal, Don
2014-08-21 0:30 ` [PATCH 03/16 v2] Refactor ptrace extended event status Don Breazeal
2014-09-09 11:31 ` Pedro Alves
2014-09-19 18:14 ` [pushed] " Breazeal, Don
2014-08-21 0:31 ` [PATCH 07/16 v2] Extended-remote arch-specific follow fork Don Breazeal
2014-09-19 21:26 ` Breazeal, Don
2014-08-21 0:31 ` [PATCH 05/16 v2] GDBserver clone breakpoint list Don Breazeal
2014-10-15 17:40 ` Pedro Alves
2014-10-31 23:44 ` Breazeal, Don
2014-08-21 0:31 ` [PATCH 06/16 v2] Extended-remote Linux follow fork Don Breazeal
2014-09-19 20:57 ` Breazeal, Don
2014-08-21 0:32 ` [PATCH 08/16 v2] Extended-remote follow vfork Don Breazeal
2014-08-21 0:33 ` [PATCH 10/16 v2] Extended-remote fork event documentation Don Breazeal
2014-08-21 0:33 ` [PATCH 11/16 v2] Extended-remote Linux exit events Don Breazeal
2014-08-21 0:33 ` [PATCH 09/16 v2] Extended-remote fork catchpoints Don Breazeal
2014-08-21 0:34 ` [PATCH 12/16 v2] Extended-remote follow exec Don Breazeal
2014-08-21 0:34 ` [PATCH 13/16 v2] Extended-remote exec catchpoints Don Breazeal
2014-08-21 0:35 ` [PATCH 14/16 v2] Suppress spurious warnings with extended-remote follow exec Don Breazeal
2014-08-21 0:36 ` [PATCH 15/16 v2] Extended-remote exec event documentation Don Breazeal
2014-08-21 0:36 ` [PATCH 16/16 v2] Non-stop follow exec tests Don Breazeal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=540EDD17.30001@redhat.com \
--to=palves@redhat.com \
--cc=donb@codesourcery.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).