From: Tom Tromey <tom@tromey.com>
To: Pedro Alves <palves@redhat.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH 3/3] Per-inferior thread list, thread ranges/iterators, down with ALL_THREADS, etc.
Date: Mon, 01 Oct 2018 17:12:00 -0000 [thread overview]
Message-ID: <878t3hlhh7.fsf@tromey.com> (raw)
In-Reply-To: <20181001103252.5150-4-palves@redhat.com> (Pedro Alves's message of "Mon, 1 Oct 2018 11:32:52 +0100")
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> As preparation for multi-target, this patch makes each inferior have
Pedro> its own thread list.
This is great.
Pedro> Lastly, the solution I settled with was to replace the ALL_THREADS /
Pedro> ALL_NON_EXITED_THREADS / ALL_INFERIORS macros with (C++20-like) ranges
Pedro> and iterators, such that you can instead naturaly iterate over
Pedro> threads/inferiors using range-for, like e.g,.:
This is my preferred approach as well. I wouldn't mind if we got rid of
all the iteration macros, and (by default) changed callback-based
iteration to for-range with iterators.
I like filtered_iterator and safe_iterator quite a bit. I think I had a
use for them already but just didn't think to do it this way.
Pedro> init_wait_for_inferior is currently responsible for discarding skipped
Pedro> inline frames, which had to be moved elsewhere.
Note that ASAN pointed out a use-after-free bug in
clear_inline_frame_state. It can be called sometimes after the
thread_info has been deleted, so:
return pid == state.thread->inf->pid;
... references freed memory.
I wonder if your patch fixes this. If not, maybe I have a patch on my
asan branch (or maybe that was one of the ones I had to rewrite, I can't
recall at the moment).
Pedro> * inline-frame.c (clear_inline_frame_state(thread_info*)): New
Pedro> overload.
Pedro> * inline-frame.h (clear_inline_frame_state(thread_info*)): New
Pedro> overload.
I didn't see these in the patch.
Pedro> +/* Filter for filtered_iterator. Filters out exited inferiors. */
Pedro> +
Pedro> +struct exited_inferior_filter
Pedro> +{
Pedro> + bool operator() (inferior *inf)
Pedro> + {
Pedro> + return (inf->pid != 0);
Nit: the parens. There were some other instances of this. Maybe we
don't care.
Pedro> + /* Returns a range adapter covering the inferior's non-exited
Pedro> + threads. Used like this:
Pedro> +
Pedro> + for (thread_info *thr : inf->non_exited_threads ())
Pedro> + [...]
Pedro> + */
Pedro> + inf_non_exited_threads_range non_exited_threads ()
Pedro> + { return inf_non_exited_threads_range (this->thread_list); }
Pedro> +
Pedro> + inline safe_inf_threads_range threads_safe ()
Pedro> + { return safe_inf_threads_range (this->thread_list); }
Nit: threads_safe should have an intro comment.
Pedro> +#include "inferior-iter.h"
I've been using "common/..." but it seems to vary in gdb.
Pedro> +void
Pedro> +all_threads_iterator::advance ()
Pedro> +{
Pedro> + /* The loop below is written in the natural way as-if we'd always
Pedro> + start at the beginning of the inferior list. This fast forwards
Pedro> + the algorithm to the actual current position. */
Pedro> + goto start;
I did a double take on this one, but it does make sense.
Tom
next prev parent reply other threads:[~2018-10-01 17:12 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-01 10:32 [PATCH 0/3] Per-inferior thread list, multi-target prep Pedro Alves
2018-10-01 10:32 ` [PATCH 1/3] Avoid find_thread_ptid with null_ptid Pedro Alves
2018-10-01 15:54 ` Tom Tromey
2018-10-02 17:08 ` Pedro Alves
2018-10-02 21:14 ` Tom Tromey
2018-10-01 10:33 ` [PATCH 2/3] Fix follow_exec latent problem Pedro Alves
2018-10-01 10:33 ` [PATCH 3/3] Per-inferior thread list, thread ranges/iterators, down with ALL_THREADS, etc Pedro Alves
2018-10-01 16:33 ` John Baldwin
2018-10-02 13:21 ` Pedro Alves
2018-10-03 17:34 ` John Baldwin
2018-10-04 19:16 ` Pedro Alves
2018-10-01 17:12 ` Tom Tromey [this message]
2018-10-03 12:12 ` Pedro Alves
2018-11-22 16:28 ` [PATCH 0/3] Per-inferior thread list, multi-target prep Pedro Alves
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=878t3hlhh7.fsf@tromey.com \
--to=tom@tromey.com \
--cc=gdb-patches@sourceware.org \
--cc=palves@redhat.com \
/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).