* [review] infrun: mark an exited thread non-executing when attempting to stop
@ 2019-10-18 13:27 Tankut Baris Aktemur (Code Review)
2019-11-04 8:36 ` Tankut Baris Aktemur (Code Review)
` (20 more replies)
0 siblings, 21 replies; 22+ messages in thread
From: Tankut Baris Aktemur (Code Review) @ 2019-10-18 13:27 UTC (permalink / raw)
To: gdb-patches
Tankut Baris Aktemur has uploaded a new change for review.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/133
......................................................................
infrun: mark an exited thread non-executing when attempting to stop
In stop_all_threads, GDB sends signals to other threads in an attempt
to stop them. While in a typical scenario the expected wait status is
TARGET_WAITKIND_STOPPED, it is possible that the thread GDB attempted
to stop has already terminated. If so, a waitstatus other than
TARGET_WAITKIND_STOPPED would be received. In that case, mark the
thread as not-executing and set its state to THREAD_EXITED.
If a wait status that denotes thread termination is ignored, GDB goes
into an infinite loop in stop_all_threads.
E.g.:
~~~
$ gdb ./a.out
(gdb) start
...
(gdb) add-inferior -exec ./a.out
...
(gdb) inferior 2
...
(gdb) start
...
(gdb) set schedule-multiple on
(gdb) set debug infrun 2
(gdb) continue
Continuing.
infrun: clear_proceed_status_thread (process 23419)
infrun: clear_proceed_status_thread (process 23703)
infrun: proceed (addr=0xffffffffffffffff, signal=GDB_SIGNAL_DEFAULT)
infrun: proceed: resuming process 23419
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 23419] at 0x55555555514e
infrun: infrun_async(1)
infrun: prepare_to_wait
infrun: proceed: resuming process 23703
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 23703] at 0x55555555514e
infrun: prepare_to_wait
infrun: target_wait (-1.0.0, status) =
infrun: 23703.23703.0 [process 23703],
infrun: status->kind = exited, status = 42
infrun: handle_inferior_event status->kind = exited, status = 42
[Inferior 2 (process 23703) exited with code 052]
infrun: stop_waiting
infrun: stop_all_threads
infrun: stop_all_threads, pass=0, iterations=0
infrun: process 23419 executing, need stop
infrun: target_wait (-1.0.0, status) =
infrun: 23419.23419.0 [process 23419],
infrun: status->kind = exited, status = 42
infrun: stop_all_threads status->kind = exited, status = 42 process 23419
infrun: process 23419 executing, already stopping
infrun: target_wait (-1.0.0, status) =
infrun: -1.0.0 [process -1],
infrun: status->kind = no-resumed
infrun: stop_all_threads status->kind = no-resumed process -1
infrun: process 23419 executing, already stopping
infrun: target_wait (-1.0.0, status) =
infrun: -1.0.0 [process -1],
infrun: status->kind = no-resumed
infrun: stop_all_threads status->kind = no-resumed process -1
infrun: process 23419 executing, already stopping
infrun: target_wait (-1.0.0, status) =
infrun: -1.0.0 [process -1],
infrun: status->kind = no-resumed
infrun: stop_all_threads status->kind = no-resumed process -1
infrun: process 23419 executing, already stopping
infrun: target_wait (-1.0.0, status) =
infrun: -1.0.0 [process -1],
infrun: status->kind = no-resumed
infrun: stop_all_threads status->kind = no-resumed process -1
infrun: process 23419 executing, already stopping
~~~
And this polling goes on forever. This patch prevents the infinite
looping behavior.
gdb/ChangeLog:
2019-10-18 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
* infrun.c (stop_all_threads): Do NOT ignore
TARGET_WAITKIND_NO_RESUMED, TARGET_WAITKIND_THREAD_EXITED,
TARGET_WAITKIND_EXITED, TARGET_WAITKIND_SIGNALLED wait statuses
received from threads we attempt to stop; mark the corresponding
thread as THREAD_EXITED and not-executing.
Change-Id: I7cec98f40283773b79255d998511da434e9cd408
---
M gdb/infrun.c
1 file changed, 9 insertions(+), 0 deletions(-)
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 66a066f..01fcbf6 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4383,6 +4383,15 @@
{
/* All resumed threads exited
or one thread/process exited/signalled. */
+ thread_info *t = find_thread_ptid (event_ptid);
+ if (t != nullptr)
+ {
+ t->stop_requested = 0;
+ t->executing = 0;
+ t->resumed = 0;
+ t->control.may_range_step = 0;
+ t->state = THREAD_EXITED;
+ }
}
else
{
^ permalink raw reply [flat|nested] 22+ messages in thread
* [review] infrun: mark an exited thread non-executing when attempting to stop
2019-10-18 13:27 [review] infrun: mark an exited thread non-executing when attempting to stop Tankut Baris Aktemur (Code Review)
@ 2019-11-04 8:36 ` Tankut Baris Aktemur (Code Review)
2019-11-04 10:23 ` [review v2] infrun: handle already-exited threads " Tankut Baris Aktemur (Code Review)
` (19 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Tankut Baris Aktemur (Code Review) @ 2019-11-04 8:36 UTC (permalink / raw)
To: gdb-patches; +Cc: Luis Machado
Tankut Baris Aktemur has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/133
......................................................................
Patch Set 1:
> Patch Set 1: Code-Review+1
>
> This looks like an old oversight when handling the case of exited threads when we're attempting to stop all of them. Looking at older code, we used to have a message saying a thread had exited while we were stopping it, but it was removed by a cleanup.
>
> The change looks good for me.
Thank you. I noticed that the patch has a problem. It leaves the exited inferior in an alive state with no threads. It becomes not possible to re-run the program. I will send a revision, together with updated tests.
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I7cec98f40283773b79255d998511da434e9cd408
Gerrit-Change-Number: 133
Gerrit-PatchSet: 1
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Comment-Date: Mon, 04 Nov 2019 08:35:58 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 22+ messages in thread
* [review v2] infrun: handle already-exited threads when attempting to stop
2019-10-18 13:27 [review] infrun: mark an exited thread non-executing when attempting to stop Tankut Baris Aktemur (Code Review)
2019-11-04 8:36 ` Tankut Baris Aktemur (Code Review)
@ 2019-11-04 10:23 ` Tankut Baris Aktemur (Code Review)
2019-12-04 12:02 ` Tankut Baris Aktemur (Code Review)
` (18 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Tankut Baris Aktemur (Code Review) @ 2019-11-04 10:23 UTC (permalink / raw)
To: Luis Machado, gdb-patches
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/133
......................................................................
infrun: handle already-exited threads when attempting to stop
In stop_all_threads, GDB sends signals to other threads in an attempt
to stop them. While in a typical scenario the expected wait status is
TARGET_WAITKIND_STOPPED, it is possible that the thread GDB attempted
to stop has already terminated. If so, a waitstatus other than
TARGET_WAITKIND_STOPPED would be received. Handle this case
appropriately.
If a wait status that denotes thread termination is ignored, GDB goes
into an infinite loop in stop_all_threads.
E.g.:
~~~
$ gdb ./a.out
(gdb) start
...
(gdb) add-inferior -exec ./a.out
...
(gdb) inferior 2
...
(gdb) start
...
(gdb) set schedule-multiple on
(gdb) set debug infrun 2
(gdb) continue
Continuing.
infrun: clear_proceed_status_thread (process 23419)
infrun: clear_proceed_status_thread (process 23703)
infrun: proceed (addr=0xffffffffffffffff, signal=GDB_SIGNAL_DEFAULT)
infrun: proceed: resuming process 23419
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 23419] at 0x55555555514e
infrun: infrun_async(1)
infrun: prepare_to_wait
infrun: proceed: resuming process 23703
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 23703] at 0x55555555514e
infrun: prepare_to_wait
infrun: target_wait (-1.0.0, status) =
infrun: 23703.23703.0 [process 23703],
infrun: status->kind = exited, status = 0
infrun: handle_inferior_event status->kind = exited, status = 0
[Inferior 2 (process 23703) exited normally]
infrun: stop_waiting
infrun: stop_all_threads
infrun: stop_all_threads, pass=0, iterations=0
infrun: process 23419 executing, need stop
infrun: target_wait (-1.0.0, status) =
infrun: 23419.23419.0 [process 23419],
infrun: status->kind = exited, status = 0
infrun: stop_all_threads status->kind = exited, status = 0 process 23419
infrun: process 23419 executing, already stopping
infrun: target_wait (-1.0.0, status) =
infrun: -1.0.0 [process -1],
infrun: status->kind = no-resumed
infrun: stop_all_threads status->kind = no-resumed process -1
infrun: process 23419 executing, already stopping
infrun: target_wait (-1.0.0, status) =
infrun: -1.0.0 [process -1],
infrun: status->kind = no-resumed
infrun: stop_all_threads status->kind = no-resumed process -1
infrun: process 23419 executing, already stopping
infrun: target_wait (-1.0.0, status) =
infrun: -1.0.0 [process -1],
infrun: status->kind = no-resumed
infrun: stop_all_threads status->kind = no-resumed process -1
infrun: process 23419 executing, already stopping
infrun: target_wait (-1.0.0, status) =
infrun: -1.0.0 [process -1],
infrun: status->kind = no-resumed
infrun: stop_all_threads status->kind = no-resumed process -1
infrun: process 23419 executing, already stopping
~~~
And this polling goes on forever. This patch prevents the infinite
looping behavior. For the same scenario above, we obtain the
following behavior:
~~~
...
(gdb) continue
Continuing.
infrun: clear_proceed_status_thread (process 1635)
infrun: clear_proceed_status_thread (process 1763)
infrun: proceed (addr=0xffffffffffffffff, signal=GDB_SIGNAL_DEFAULT)
infrun: proceed: resuming process 1635
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 1635] at 0x55555555514e
infrun: infrun_async(1)
infrun: prepare_to_wait
infrun: proceed: resuming process 1763
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 1763] at 0x55555555514e
infrun: prepare_to_wait
infrun: target_wait (-1.0.0, status) =
infrun: 1635.1635.0 [process 1635],
infrun: status->kind = exited, status = 0
infrun: handle_inferior_event status->kind = exited, status = 0
[Inferior 1 (process 1635) exited normally]
infrun: stop_waiting
infrun: stop_all_threads
infrun: stop_all_threads, pass=0, iterations=0
infrun: process 1763 executing, need stop
infrun: target_wait (-1.0.0, status) =
infrun: 1763.1763.0 [process 1763],
infrun: status->kind = exited, status = 0
infrun: stop_all_threads status->kind = exited, status = 0 process 1763
[Inferior 2 (process 1763) exited normally]
infrun: stop_all_threads, pass=1, iterations=1
infrun: stop_all_threads done
(gdb) info inferiors
Num Description Executable
* 1 <null> /path/to/a.out
2 <null> /path/to/a.out
(gdb) info threads
No threads.
(gdb)
~~~
gdb/ChangeLog:
2019-11-04 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
* infrun.c (stop_all_threads): Do NOT ignore
TARGET_WAITKIND_NO_RESUMED, TARGET_WAITKIND_THREAD_EXITED,
TARGET_WAITKIND_EXITED, TARGET_WAITKIND_SIGNALLED wait statuses
received from threads we attempt to stop.
Change-Id: I7cec98f40283773b79255d998511da434e9cd408
---
M gdb/infrun.c
1 file changed, 29 insertions(+), 2 deletions(-)
diff --git a/gdb/infrun.c b/gdb/infrun.c
index f628fd1..45aa1d9 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4489,8 +4489,35 @@
|| ws.kind == TARGET_WAITKIND_EXITED
|| ws.kind == TARGET_WAITKIND_SIGNALLED)
{
- /* All resumed threads exited
- or one thread/process exited/signalled. */
+ if (event_ptid != minus_one_ptid)
+ {
+ thread_info *t = find_thread_ptid (event_ptid);
+ if (t == nullptr)
+ {
+ /* This is the first time we see this thread. */
+ t = add_thread (event_ptid);
+ }
+
+ /* Set the threads as non-executing to avoid infinitely
+ waiting for them to stop. */
+ mark_non_executing_threads (event_ptid, ws);
+
+ if (ws.kind == TARGET_WAITKIND_NO_RESUMED)
+ {
+ /* Do nothing. Already marked the threads. */
+ }
+ if (ws.kind == TARGET_WAITKIND_THREAD_EXITED)
+ delete_thread (t);
+ else
+ {
+ /* TARGET_WAITKIND_EXITED or
+ TARGET_WAITKIND_SIGNALLED. */
+ /* Need to restore the context because
+ handle_inferior_exit switches it. */
+ scoped_restore_current_pspace_and_thread restore;
+ handle_inferior_exit (event_ptid, ws);
+ }
+ }
}
else
{
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I7cec98f40283773b79255d998511da434e9cd408
Gerrit-Change-Number: 133
Gerrit-PatchSet: 2
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-MessageType: newpatchset
^ permalink raw reply [flat|nested] 22+ messages in thread
* [review v2] infrun: handle already-exited threads when attempting to stop
2019-10-18 13:27 [review] infrun: mark an exited thread non-executing when attempting to stop Tankut Baris Aktemur (Code Review)
2019-11-04 8:36 ` Tankut Baris Aktemur (Code Review)
2019-11-04 10:23 ` [review v2] infrun: handle already-exited threads " Tankut Baris Aktemur (Code Review)
@ 2019-12-04 12:02 ` Tankut Baris Aktemur (Code Review)
2019-12-05 19:26 ` Pedro Alves (Code Review)
` (17 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Tankut Baris Aktemur (Code Review) @ 2019-12-04 12:02 UTC (permalink / raw)
To: gdb-patches; +Cc: Pedro Alves, Luis Machado
Tankut Baris Aktemur has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/133
......................................................................
Patch Set 2:
Kindly pinging.
Thanks.
-Baris
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I7cec98f40283773b79255d998511da434e9cd408
Gerrit-Change-Number: 133
Gerrit-PatchSet: 2
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Comment-Date: Wed, 04 Dec 2019 12:02:10 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 22+ messages in thread
* [review v2] infrun: handle already-exited threads when attempting to stop
2019-10-18 13:27 [review] infrun: mark an exited thread non-executing when attempting to stop Tankut Baris Aktemur (Code Review)
` (2 preceding siblings ...)
2019-12-04 12:02 ` Tankut Baris Aktemur (Code Review)
@ 2019-12-05 19:26 ` Pedro Alves (Code Review)
2019-12-06 16:19 ` Pedro Alves (Code Review)
` (16 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Pedro Alves (Code Review) @ 2019-12-05 19:26 UTC (permalink / raw)
To: Tankut Baris Aktemur, gdb-patches; +Cc: Luis Machado
Pedro Alves has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/133
......................................................................
Patch Set 2:
> Kindly pinging.
> Thanks.
Thanks. I'm taking a look.
(FWIW, I'm skimmed this before and put it off because it seemingly didn't come with a testcase.
But today, while digging deeper, I see that this is actually part of a patch series, which _does_ include tests. Thanks for that. This is an instance of my annoyance with gerrit's lack of cover letter & threading.)
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I7cec98f40283773b79255d998511da434e9cd408
Gerrit-Change-Number: 133
Gerrit-PatchSet: 2
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Comment-Date: Thu, 05 Dec 2019 19:25:56 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 22+ messages in thread
* [review v2] infrun: handle already-exited threads when attempting to stop
2019-10-18 13:27 [review] infrun: mark an exited thread non-executing when attempting to stop Tankut Baris Aktemur (Code Review)
` (3 preceding siblings ...)
2019-12-05 19:26 ` Pedro Alves (Code Review)
@ 2019-12-06 16:19 ` Pedro Alves (Code Review)
2019-12-06 17:41 ` Tankut Baris Aktemur (Code Review)
` (15 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Pedro Alves (Code Review) @ 2019-12-06 16:19 UTC (permalink / raw)
To: Tankut Baris Aktemur, gdb-patches; +Cc: Luis Machado
Pedro Alves has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/133
......................................................................
Patch Set 2:
(1 comment)
| --- gdb/infrun.c
| +++ gdb/infrun.c
| @@ -4494,7 +4509,19 @@ stop_all_threads (void)
| + if (ws.kind == TARGET_WAITKIND_THREAD_EXITED)
| + delete_thread (t);
| + else
| + {
| + /* TARGET_WAITKIND_EXITED or
| + TARGET_WAITKIND_SIGNALLED. */
| + /* Need to restore the context because
| + handle_inferior_exit switches it. */
| + scoped_restore_current_pspace_and_thread restore;
| + handle_inferior_exit (event_ptid, ws);
PS2, Line 4518:
Sorry, but this doesn't look right.
We're inside stop_all_threads, processing some other event, and a
process exit event for one of the processes we're trying to stop comes
along, and this processes it right away. Once the stop_all_threads
dance is done, we go back to handling the original event, and possibly
reporting a stop to the user. Meanwhile, whatever state that was set
by handle_inferior_exit, like e.g. $_exitcode, is lost, or now
incorrect for the reported stop. We also never present a stop on the
CLI for that "spurious" process exit. Here:
(gdb)
continue
Continuing.
Executing on build: kill -9 29412 29417 (timeout = 300)
spawn -ignore SIGHUP kill -9 29412 29417
Program terminated with signal SIGKILL, Killed.
The program no longer exists.
<<<<<<<<<<< no user-visible stop / prompt here. <<<<<<<<<<<<<<<
Program terminated with signal SIGKILL, Killed.
The program no longer exists.
(gdb) PASS: gdb.multi/multi-kill.exp: iteration 1: back to gdb prompt
The fix for this I think must be around leaving the
TARGET_WAITKIND_EXITED/TARGET_WAITKIND_SIGNALLED
event pending, so that it is processed later when we're out of the
stop_all_threads loop and back to
dequeuing the next event.
gdb/linux-nat.c also has its own "stop all threads temporarily" logic,
and that does that -- leaves process exits pending. See wait_lwp:
/* If this is the leader exiting, it means the whole
process is gone. Store the status to report to the
core. Store it in lp->waitstatus, because lp->status
would be ambiguous (W_EXITCODE(0,0) == 0). */
store_waitstatus (&lp->waitstatus, status);
return 0;
| + }
| + }
| }
| else
| {
| thread_info *t = find_thread_ptid (event_ptid);
| if (t == NULL)
| t = add_thread (event_ptid);
|
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I7cec98f40283773b79255d998511da434e9cd408
Gerrit-Change-Number: 133
Gerrit-PatchSet: 2
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Comment-Date: Fri, 06 Dec 2019 16:19:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 22+ messages in thread
* [review v2] infrun: handle already-exited threads when attempting to stop
2019-10-18 13:27 [review] infrun: mark an exited thread non-executing when attempting to stop Tankut Baris Aktemur (Code Review)
` (4 preceding siblings ...)
2019-12-06 16:19 ` Pedro Alves (Code Review)
@ 2019-12-06 17:41 ` Tankut Baris Aktemur (Code Review)
2019-12-09 15:09 ` Tankut Baris Aktemur (Code Review)
` (14 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Tankut Baris Aktemur (Code Review) @ 2019-12-06 17:41 UTC (permalink / raw)
To: gdb-patches; +Cc: Pedro Alves, Luis Machado
Tankut Baris Aktemur has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/133
......................................................................
Patch Set 2:
(1 comment)
| --- gdb/infrun.c
| +++ gdb/infrun.c
| @@ -4494,7 +4509,19 @@ stop_all_threads (void)
| + if (ws.kind == TARGET_WAITKIND_THREAD_EXITED)
| + delete_thread (t);
| + else
| + {
| + /* TARGET_WAITKIND_EXITED or
| + TARGET_WAITKIND_SIGNALLED. */
| + /* Need to restore the context because
| + handle_inferior_exit switches it. */
| + scoped_restore_current_pspace_and_thread restore;
| + handle_inferior_exit (event_ptid, ws);
PS2, Line 4518:
Thanks for your comment. I'll work on making the event pending and
then submit a revision.
| + }
| + }
| }
| else
| {
| thread_info *t = find_thread_ptid (event_ptid);
| if (t == NULL)
| t = add_thread (event_ptid);
|
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I7cec98f40283773b79255d998511da434e9cd408
Gerrit-Change-Number: 133
Gerrit-PatchSet: 2
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Comment-Date: Fri, 06 Dec 2019 17:41:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Pedro Alves <palves@redhat.com>
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 22+ messages in thread
* [review v2] infrun: handle already-exited threads when attempting to stop
2019-10-18 13:27 [review] infrun: mark an exited thread non-executing when attempting to stop Tankut Baris Aktemur (Code Review)
` (5 preceding siblings ...)
2019-12-06 17:41 ` Tankut Baris Aktemur (Code Review)
@ 2019-12-09 15:09 ` Tankut Baris Aktemur (Code Review)
2020-01-08 15:57 ` Tankut Baris Aktemur (Code Review)
` (13 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Tankut Baris Aktemur (Code Review) @ 2019-12-09 15:09 UTC (permalink / raw)
To: gdb-patches; +Cc: Pedro Alves, Luis Machado
Tankut Baris Aktemur has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/133
......................................................................
Patch Set 2:
(1 comment)
> Patch Set 2:
>
> (1 comment)
| --- gdb/infrun.c
| +++ gdb/infrun.c
| @@ -4494,7 +4509,19 @@ stop_all_threads (void)
| + if (ws.kind == TARGET_WAITKIND_THREAD_EXITED)
| + delete_thread (t);
| + else
| + {
| + /* TARGET_WAITKIND_EXITED or
| + TARGET_WAITKIND_SIGNALLED. */
| + /* Need to restore the context because
| + handle_inferior_exit switches it. */
| + scoped_restore_current_pspace_and_thread restore;
| + handle_inferior_exit (event_ptid, ws);
PS2, Line 4518:
> The fix for this I think must be around leaving the TARGET_WAITKIND_EXITED/TARGET_WAITKIND_SIGNALLED
event pending, so that it is processed later when we're out of the
stop_all_threads loop and back to
dequeuing the next event.
I'd like to ask for your opinion on making the second exit event
pending. One problem is, because the event has not been reported to
the user yet, the user still thinks that the inferior is alive. So,
after getting the prompt because of the first exit event, they may be
tempted to do "info threads" or switch to the not-yet-reported-
inferior and inspect its state. This triggers a query (e.g. of
registers) on the process that is already gone. I tried the following
scenario with the current master branch (the patch that I proposed was
not applied):
~~~
$ gdb ./a.out
(gdb) maint set target-non-stop off
(gdb) start
...
(gdb) add-inferior -exec ./a.out
[New inferior 2]
Added inferior 2
...
(gdb) inferior 2
[Switching to inferior 2 [<null>] (/tmp/a.out)]
(gdb) start
...
(gdb) set schedule-multiple on
(gdb) c
Continuing.
[Inferior 2 (process 16331) exited normally]
(gdb) i inferiors
Num Description Executable
1 process 16137 /tmp/a.out
* 2 <null> /tmp/a.out
(gdb) inferior 1
[Switching to inferior 1 [process 16137] (/tmp/a.out)]
[Switching to thread 1.1 (process 16137)]
Couldn't get registers: No such process.
(gdb) i threads
Id Target Id Frame
Couldn't get registers: No such process.
(gdb) c
Continuing.
Couldn't get registers: No such process.
~~~
If I save the exit event in my patch as a pending event (and omit
'maint set target-non-stop off'), I get essentially the same problem.
What is the expected GDB behavior here? Would it be alright to
actually print both exit events, followed by the gdb-prompt, where the
user can now query $_exitcode or $_exitsignal by switching between
inferiors, assuming those special variables are set correctly per
inferior?
| + }
| + }
| }
| else
| {
| thread_info *t = find_thread_ptid (event_ptid);
| if (t == NULL)
| t = add_thread (event_ptid);
|
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I7cec98f40283773b79255d998511da434e9cd408
Gerrit-Change-Number: 133
Gerrit-PatchSet: 2
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Comment-Date: Mon, 09 Dec 2019 15:09:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Pedro Alves <palves@redhat.com>
Comment-In-Reply-To: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 22+ messages in thread
* [review v2] infrun: handle already-exited threads when attempting to stop
2019-10-18 13:27 [review] infrun: mark an exited thread non-executing when attempting to stop Tankut Baris Aktemur (Code Review)
` (6 preceding siblings ...)
2019-12-09 15:09 ` Tankut Baris Aktemur (Code Review)
@ 2020-01-08 15:57 ` Tankut Baris Aktemur (Code Review)
2020-01-29 17:54 ` Tom de Vries (Code Review)
` (12 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Tankut Baris Aktemur (Code Review) @ 2020-01-08 15:57 UTC (permalink / raw)
To: gdb-patches; +Cc: Pedro Alves, Luis Machado
Tankut Baris Aktemur has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/133
......................................................................
Patch Set 2:
(1 comment)
| --- gdb/infrun.c
| +++ gdb/infrun.c
| @@ -4494,7 +4509,19 @@ stop_all_threads (void)
| + if (ws.kind == TARGET_WAITKIND_THREAD_EXITED)
| + delete_thread (t);
| + else
| + {
| + /* TARGET_WAITKIND_EXITED or
| + TARGET_WAITKIND_SIGNALLED. */
| + /* Need to restore the context because
| + handle_inferior_exit switches it. */
| + scoped_restore_current_pspace_and_thread restore;
| + handle_inferior_exit (event_ptid, ws);
PS2, Line 4518:
> > The fix for this I think must be around leaving the TARGET_WAITKIND_EXITED/TARGET_WAITKIND_SIGNALLED
> event pending, so that it is processed later when we're out of the stop_all_threads loop and back to
> dequeuing the next event.
>
>
> I'd like to ask for your opinion on making the second exit event pending.
> One problem is, because the event has not been reported to the user yet,
> the user still thinks that the inferior is alive. So, after getting the
> prompt because of the first exit event, they may be tempted to do "info
> threads" or switch to the not-yet-reported-inferior and inspect its state.
> This triggers a query (e.g. of registers) on the process that is already
> gone. I tried the following scenario with the current master branch
> (the patch that I proposed was not applied):
Kindly pinging.
Thanks,
-Baris
| + }
| + }
| }
| else
| {
| thread_info *t = find_thread_ptid (event_ptid);
| if (t == NULL)
| t = add_thread (event_ptid);
|
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I7cec98f40283773b79255d998511da434e9cd408
Gerrit-Change-Number: 133
Gerrit-PatchSet: 2
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Comment-Date: Wed, 08 Jan 2020 15:57:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Pedro Alves <palves@redhat.com>
Comment-In-Reply-To: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 22+ messages in thread
* [review v2] infrun: handle already-exited threads when attempting to stop
2019-10-18 13:27 [review] infrun: mark an exited thread non-executing when attempting to stop Tankut Baris Aktemur (Code Review)
` (7 preceding siblings ...)
2020-01-08 15:57 ` Tankut Baris Aktemur (Code Review)
@ 2020-01-29 17:54 ` Tom de Vries (Code Review)
2020-01-30 9:10 ` Tom de Vries (Code Review)
` (11 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Tom de Vries (Code Review) @ 2020-01-29 17:54 UTC (permalink / raw)
To: Tankut Baris Aktemur, gdb-patches; +Cc: Pedro Alves, Luis Machado
Tom de Vries has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/133
......................................................................
Patch Set 2:
> I'd like to ask for your opinion on making the second exit event pending
Discussion continued here: https://sourceware.org/ml/gdb-patches/2020-01/msg00212.html .
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I7cec98f40283773b79255d998511da434e9cd408
Gerrit-Change-Number: 133
Gerrit-PatchSet: 2
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-CC: Tom de Vries <tdevries@suse.de>
Gerrit-Comment-Date: Wed, 29 Jan 2020 16:19:53 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 22+ messages in thread
* [review v2] infrun: handle already-exited threads when attempting to stop
2019-10-18 13:27 [review] infrun: mark an exited thread non-executing when attempting to stop Tankut Baris Aktemur (Code Review)
` (8 preceding siblings ...)
2020-01-29 17:54 ` Tom de Vries (Code Review)
@ 2020-01-30 9:10 ` Tom de Vries (Code Review)
2020-01-30 13:58 ` Tom de Vries (Code Review)
` (10 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Tom de Vries (Code Review) @ 2020-01-30 9:10 UTC (permalink / raw)
To: Tankut Baris Aktemur, gdb-patches; +Cc: Pedro Alves, Luis Machado
Tom de Vries has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/133
......................................................................
Patch Set 2:
FTR, I build patchset 2 and tested the example from PR25478, and hit an assert:
...
Thread 1 "gdb" hit Breakpoint 1, stop_all_threads () at /data/gdb_versions/devel/src/gdb/infrun.c:4395
4395 int iterations = 0;
(master-gdb) c
Continuing.
infrun: stop_all_threads
infrun: stop_all_threads, pass=0, iterations=0
infrun: Thread 0x7ffff7fa6740 (LWP 32692) not executing
infrun: Thread 0x7ffff77fe700 (LWP 32696) executing, need stop
infrun: target_wait (-1.0.0, status) =
infrun: 32692.32696.0 [Thread 0x7ffff77fe700 (LWP 32696)],
infrun: status->kind = signalled, signal = GDB_SIGNAL_KILL
infrun: stop_all_threads status->kind = signalled, signal = GDB_SIGNAL_KILL Thread 0x7ffff77fe700 (LWP 32696)
Program terminated with signal SIGKILL, Killed.
The program no longer exists.
infrun: stop_all_threads, pass=1, iterations=1
infrun: stop_all_threads done
thread.c:95: internal-error: thread_info* inferior_thread(): Assertion `tp' failed.
...
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I7cec98f40283773b79255d998511da434e9cd408
Gerrit-Change-Number: 133
Gerrit-PatchSet: 2
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-CC: Tom de Vries <tdevries@suse.de>
Gerrit-Comment-Date: Thu, 30 Jan 2020 09:01:05 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 22+ messages in thread
* [review v2] infrun: handle already-exited threads when attempting to stop
2019-10-18 13:27 [review] infrun: mark an exited thread non-executing when attempting to stop Tankut Baris Aktemur (Code Review)
` (9 preceding siblings ...)
2020-01-30 9:10 ` Tom de Vries (Code Review)
@ 2020-01-30 13:58 ` Tom de Vries (Code Review)
2020-01-30 16:41 ` Tankut Baris Aktemur (Code Review)
` (9 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Tom de Vries (Code Review) @ 2020-01-30 13:58 UTC (permalink / raw)
To: Tankut Baris Aktemur, gdb-patches; +Cc: Pedro Alves, Luis Machado
Tom de Vries has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/133
......................................................................
Patch Set 2:
(1 comment)
| --- gdb/infrun.c
| +++ gdb/infrun.c
| @@ -4494,0 +4500,19 @@ stop_all_threads (void)
| +
| + /* Set the threads as non-executing to avoid infinitely
| + waiting for them to stop. */
| + mark_non_executing_threads (event_ptid, ws);
| +
| + if (ws.kind == TARGET_WAITKIND_NO_RESUMED)
| + {
| + /* Do nothing. Already marked the threads. */
| + }
| + if (ws.kind == TARGET_WAITKIND_THREAD_EXITED)
PS2, Line 4509:
This doesn't look right. Did you mean to write:
else if (ws.kind == TARGET_WAITKIND_THREAD_EXITED)
?
| + delete_thread (t);
| + else
| + {
| + /* TARGET_WAITKIND_EXITED or
| + TARGET_WAITKIND_SIGNALLED. */
| + /* Need to restore the context because
| + handle_inferior_exit switches it. */
| + scoped_restore_current_pspace_and_thread restore;
| + handle_inferior_exit (event_ptid, ws);
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I7cec98f40283773b79255d998511da434e9cd408
Gerrit-Change-Number: 133
Gerrit-PatchSet: 2
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-CC: Tom de Vries <tdevries@suse.de>
Gerrit-Comment-Date: Thu, 30 Jan 2020 09:10:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 22+ messages in thread
* [review v2] infrun: handle already-exited threads when attempting to stop
2019-10-18 13:27 [review] infrun: mark an exited thread non-executing when attempting to stop Tankut Baris Aktemur (Code Review)
` (10 preceding siblings ...)
2020-01-30 13:58 ` Tom de Vries (Code Review)
@ 2020-01-30 16:41 ` Tankut Baris Aktemur (Code Review)
2020-01-30 21:06 ` Tom de Vries (Code Review)
` (8 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Tankut Baris Aktemur (Code Review) @ 2020-01-30 16:41 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom de Vries, Pedro Alves, Luis Machado
Tankut Baris Aktemur has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/133
......................................................................
Patch Set 2:
(1 comment)
> Patch Set 2:
>
> FTR, I build patchset 2 and tested the example from PR25478, and hit an assert:
> ...
I was not able to repeat the problematic scenario with the current master. The multi-target feature seems to have changed the behavior. I'll need to first look into whether I can reproduce the infinite loop.
| --- gdb/infrun.c
| +++ gdb/infrun.c
| @@ -4494,0 +4500,19 @@ stop_all_threads (void)
| +
| + /* Set the threads as non-executing to avoid infinitely
| + waiting for them to stop. */
| + mark_non_executing_threads (event_ptid, ws);
| +
| + if (ws.kind == TARGET_WAITKIND_NO_RESUMED)
| + {
| + /* Do nothing. Already marked the threads. */
| + }
| + if (ws.kind == TARGET_WAITKIND_THREAD_EXITED)
PS2, Line 4509:
Oops, yes. Thanks for spotting that.
| + delete_thread (t);
| + else
| + {
| + /* TARGET_WAITKIND_EXITED or
| + TARGET_WAITKIND_SIGNALLED. */
| + /* Need to restore the context because
| + handle_inferior_exit switches it. */
| + scoped_restore_current_pspace_and_thread restore;
| + handle_inferior_exit (event_ptid, ws);
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I7cec98f40283773b79255d998511da434e9cd408
Gerrit-Change-Number: 133
Gerrit-PatchSet: 2
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-CC: Tom de Vries <tdevries@suse.de>
Gerrit-Comment-Date: Thu, 30 Jan 2020 16:30:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tom de Vries <tdevries@suse.de>
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 22+ messages in thread
* [review v2] infrun: handle already-exited threads when attempting to stop
2019-10-18 13:27 [review] infrun: mark an exited thread non-executing when attempting to stop Tankut Baris Aktemur (Code Review)
` (11 preceding siblings ...)
2020-01-30 16:41 ` Tankut Baris Aktemur (Code Review)
@ 2020-01-30 21:06 ` Tom de Vries (Code Review)
2020-02-03 15:13 ` Tom de Vries (Code Review)
` (7 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Tom de Vries (Code Review) @ 2020-01-30 21:06 UTC (permalink / raw)
To: Tankut Baris Aktemur, gdb-patches; +Cc: Pedro Alves, Luis Machado
Tom de Vries has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/133
......................................................................
Patch Set 2:
> Patch Set 2:
>
> (1 comment)
>
> > Patch Set 2:
> >
> > FTR, I build patchset 2 and tested the example from PR25478, and hit an assert:
> > ...
>
> I was not able to repeat the problematic scenario with the current master.
Do you mean with 'problematic scenario' PR25478 as such, or the assert?
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I7cec98f40283773b79255d998511da434e9cd408
Gerrit-Change-Number: 133
Gerrit-PatchSet: 2
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-CC: Tom de Vries <tdevries@suse.de>
Gerrit-Comment-Date: Thu, 30 Jan 2020 17:52:47 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 22+ messages in thread
* [review v2] infrun: handle already-exited threads when attempting to stop
2019-10-18 13:27 [review] infrun: mark an exited thread non-executing when attempting to stop Tankut Baris Aktemur (Code Review)
` (12 preceding siblings ...)
2020-01-30 21:06 ` Tom de Vries (Code Review)
@ 2020-02-03 15:13 ` Tom de Vries (Code Review)
2020-02-03 16:02 ` Tankut Baris Aktemur (Code Review)
` (6 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Tom de Vries (Code Review) @ 2020-02-03 15:13 UTC (permalink / raw)
To: Tankut Baris Aktemur, gdb-patches; +Cc: Pedro Alves, Luis Machado
Tom de Vries has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/133
......................................................................
Patch Set 2:
> Patch Set 2:
>
> FTR, I build patchset 2 and tested the example from PR25478, and hit an assert:
> ...
> Thread 1 "gdb" hit Breakpoint 1, stop_all_threads () at /data/gdb_versions/devel/src/gdb/infrun.c:4395
> 4395 int iterations = 0;
> (master-gdb) c
> Continuing.
> infrun: stop_all_threads
> infrun: stop_all_threads, pass=0, iterations=0
> infrun: Thread 0x7ffff7fa6740 (LWP 32692) not executing
> infrun: Thread 0x7ffff77fe700 (LWP 32696) executing, need stop
> infrun: target_wait (-1.0.0, status) =
> infrun: 32692.32696.0 [Thread 0x7ffff77fe700 (LWP 32696)],
> infrun: status->kind = signalled, signal = GDB_SIGNAL_KILL
> infrun: stop_all_threads status->kind = signalled, signal = GDB_SIGNAL_KILL Thread 0x7ffff77fe700 (LWP 32696)
>
> Program terminated with signal SIGKILL, Killed.
> The program no longer exists.
> infrun: stop_all_threads, pass=1, iterations=1
> infrun: stop_all_threads done
> thread.c:95: internal-error: thread_info* inferior_thread(): Assertion `tp' failed.
> ...
I've:
- ported patch set 2 to master
- integrated the fix in patch set 3 of
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/759 such that the assert above is
fixed.
- build and reg-tested on x86_64-linux
Available at https://github.com/vries/gdb/tree/handle-already-exited-threads-when-attempting-to-stop-try-master-2 .
Note: this does not address all the comments on patch set 2.
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I7cec98f40283773b79255d998511da434e9cd408
Gerrit-Change-Number: 133
Gerrit-PatchSet: 2
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-CC: Tom de Vries <tdevries@suse.de>
Gerrit-Comment-Date: Mon, 03 Feb 2020 15:13:02 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 22+ messages in thread
* [review v2] infrun: handle already-exited threads when attempting to stop
2019-10-18 13:27 [review] infrun: mark an exited thread non-executing when attempting to stop Tankut Baris Aktemur (Code Review)
` (13 preceding siblings ...)
2020-02-03 15:13 ` Tom de Vries (Code Review)
@ 2020-02-03 16:02 ` Tankut Baris Aktemur (Code Review)
2020-02-03 16:27 ` Tom de Vries (Code Review)
` (5 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Tankut Baris Aktemur (Code Review) @ 2020-02-03 16:02 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom de Vries, Pedro Alves, Luis Machado
Tankut Baris Aktemur has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/133
......................................................................
Patch Set 2:
> > >
> > > FTR, I build patchset 2 and tested the example from PR25478, and hit an assert:
> > > ...
> >
> > I was not able to repeat the problematic scenario with the current master.
>
> Do you mean with 'problematic scenario' PR25478 as such, or the assert?
Sorry for the vagueness. I was referring to the test scenario I gave in the patch
description. Like, scenarios that can be used for testing as in
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/134
without having to stop GDB from outside at 'stop_all_threads'.
I just saw that you posted a rebased version of this patch. I'll look at that,
thank you.
Btw, I think this patch is kind of obsolete after Pedro's comments in
https://sourceware.org/ml/gdb-patches/2020-01/msg00212.html
because the patch causes two stop event reports at once.
-Baris
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I7cec98f40283773b79255d998511da434e9cd408
Gerrit-Change-Number: 133
Gerrit-PatchSet: 2
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-CC: Tom de Vries <tdevries@suse.de>
Gerrit-Comment-Date: Mon, 03 Feb 2020 16:02:11 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 22+ messages in thread
* [review v2] infrun: handle already-exited threads when attempting to stop
2019-10-18 13:27 [review] infrun: mark an exited thread non-executing when attempting to stop Tankut Baris Aktemur (Code Review)
` (14 preceding siblings ...)
2020-02-03 16:02 ` Tankut Baris Aktemur (Code Review)
@ 2020-02-03 16:27 ` Tom de Vries (Code Review)
2020-02-04 9:06 ` Tankut Baris Aktemur (Code Review)
` (4 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Tom de Vries (Code Review) @ 2020-02-03 16:27 UTC (permalink / raw)
To: Tankut Baris Aktemur, gdb-patches; +Cc: Pedro Alves, Luis Machado
Tom de Vries has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/133
......................................................................
Patch Set 2:
> Patch Set 2:
> I just saw that you posted a rebased version of this patch. I'll look at that,
> thank you.
That would be great, thanks.
BTW, do you happen to have something more up-to-date than patch set 2? If so, could you share this?
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I7cec98f40283773b79255d998511da434e9cd408
Gerrit-Change-Number: 133
Gerrit-PatchSet: 2
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-CC: Tom de Vries <tdevries@suse.de>
Gerrit-Comment-Date: Mon, 03 Feb 2020 16:26:57 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 22+ messages in thread
* [review v2] infrun: handle already-exited threads when attempting to stop
2019-10-18 13:27 [review] infrun: mark an exited thread non-executing when attempting to stop Tankut Baris Aktemur (Code Review)
` (15 preceding siblings ...)
2020-02-03 16:27 ` Tom de Vries (Code Review)
@ 2020-02-04 9:06 ` Tankut Baris Aktemur (Code Review)
2020-02-05 13:19 ` [review v3] " Tankut Baris Aktemur (Code Review)
` (3 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Tankut Baris Aktemur (Code Review) @ 2020-02-04 9:06 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom de Vries, Pedro Alves, Luis Machado
Tankut Baris Aktemur has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/133
......................................................................
Patch Set 2:
> BTW, do you happen to have something more up-to-date than patch set 2? If so, could you share this?
Yes. I need to revise the associated tests, and will post it as soon as I can -- hopefully today.
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I7cec98f40283773b79255d998511da434e9cd408
Gerrit-Change-Number: 133
Gerrit-PatchSet: 2
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-CC: Tom de Vries <tdevries@suse.de>
Gerrit-Comment-Date: Tue, 04 Feb 2020 09:06:35 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 22+ messages in thread
* [review v3] infrun: handle already-exited threads when attempting to stop
2019-10-18 13:27 [review] infrun: mark an exited thread non-executing when attempting to stop Tankut Baris Aktemur (Code Review)
` (16 preceding siblings ...)
2020-02-04 9:06 ` Tankut Baris Aktemur (Code Review)
@ 2020-02-05 13:19 ` Tankut Baris Aktemur (Code Review)
2020-02-05 13:24 ` Tankut Baris Aktemur (Code Review)
` (2 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Tankut Baris Aktemur (Code Review) @ 2020-02-05 13:19 UTC (permalink / raw)
To: Luis Machado, Pedro Alves, gdb-patches; +Cc: Tom de Vries
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/133
......................................................................
infrun: handle already-exited threads when attempting to stop
In stop_all_threads, GDB sends signals to other threads in an attempt
to stop them. While in a typical scenario the expected wait status is
TARGET_WAITKIND_STOPPED, it is possible that the thread GDB attempted
to stop has already terminated. If so, a waitstatus other than
TARGET_WAITKIND_STOPPED would be received. Handle this case
appropriately.
If a wait status that denotes thread termination is ignored, GDB goes
into an infinite loop in stop_all_threads.
E.g.:
~~~
$ gdb ./a.out
(gdb) start
...
(gdb) add-inferior -exec ./a.out
...
(gdb) inferior 2
...
(gdb) start
...
(gdb) set schedule-multiple on
(gdb) set debug infrun 2
(gdb) continue
Continuing.
infrun: clear_proceed_status_thread (process 23419)
infrun: clear_proceed_status_thread (process 23703)
infrun: proceed (addr=0xffffffffffffffff, signal=GDB_SIGNAL_DEFAULT)
infrun: proceed: resuming process 23419
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 23419] at 0x55555555514e
infrun: infrun_async(1)
infrun: prepare_to_wait
infrun: proceed: resuming process 23703
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 23703] at 0x55555555514e
infrun: prepare_to_wait
infrun: target_wait (-1.0.0, status) =
infrun: 23703.23703.0 [process 23703],
infrun: status->kind = exited, status = 0
infrun: handle_inferior_event status->kind = exited, status = 0
[Inferior 2 (process 23703) exited normally]
infrun: stop_waiting
infrun: stop_all_threads
infrun: stop_all_threads, pass=0, iterations=0
infrun: process 23419 executing, need stop
infrun: target_wait (-1.0.0, status) =
infrun: 23419.23419.0 [process 23419],
infrun: status->kind = exited, status = 0
infrun: stop_all_threads status->kind = exited, status = 0 process 23419
infrun: process 23419 executing, already stopping
infrun: target_wait (-1.0.0, status) =
infrun: -1.0.0 [process -1],
infrun: status->kind = no-resumed
infrun: stop_all_threads status->kind = no-resumed process -1
infrun: process 23419 executing, already stopping
infrun: target_wait (-1.0.0, status) =
infrun: -1.0.0 [process -1],
infrun: status->kind = no-resumed
infrun: stop_all_threads status->kind = no-resumed process -1
infrun: process 23419 executing, already stopping
infrun: target_wait (-1.0.0, status) =
infrun: -1.0.0 [process -1],
infrun: status->kind = no-resumed
infrun: stop_all_threads status->kind = no-resumed process -1
infrun: process 23419 executing, already stopping
infrun: target_wait (-1.0.0, status) =
infrun: -1.0.0 [process -1],
infrun: status->kind = no-resumed
infrun: stop_all_threads status->kind = no-resumed process -1
infrun: process 23419 executing, already stopping
~~~
And this polling goes on forever. This patch prevents the infinite
looping behavior. For the same scenario above, we obtain the
following behavior:
~~~
...
(gdb) continue
Continuing.
infrun: clear_proceed_status_thread (process 8564)
infrun: clear_proceed_status_thread (process 8652)
infrun: proceed (addr=0xffffffffffffffff, signal=GDB_SIGNAL_DEFAULT)
infrun: proceed: resuming process 8564
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 8564] at 0x55555555514e
infrun: infrun_async(1)
infrun: prepare_to_wait
infrun: proceed: resuming process 8652
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 8652] at 0x55555555514e
infrun: prepare_to_wait
infrun: Found 2 inferiors, starting at #0
infrun: target_wait (-1.0.0, status) =
infrun: 8564.8564.0 [process 8564],
infrun: status->kind = exited, status = 0
infrun: handle_inferior_event status->kind = exited, status = 0
[Inferior 1 (process 8564) exited normally]
infrun: stop_waiting
infrun: stop_all_threads
infrun: stop_all_threads, pass=0, iterations=0
infrun: process 8652 executing, need stop
infrun: target_wait (-1.0.0, status) =
infrun: 8652.8652.0 [process 8652],
infrun: status->kind = exited, status = 0
infrun: stop_all_threads status->kind = exited, status = 0 process 8652
infrun: saving status status->kind = exited, status = 0 for 8652.8652.0
infrun: process 8652 not executing
infrun: stop_all_threads, pass=1, iterations=1
infrun: process 8652 not executing
infrun: stop_all_threads done
(gdb) infrun: Using pending wait status status->kind = exited, status = 0 for process 8652.
infrun: target_wait (-1.0.0, status) =
infrun: 8652.8652.0 [process 8652],
infrun: status->kind = exited, status = 0
infrun: handle_inferior_event status->kind = exited, status = 0
[Inferior 2 (process 8652) exited normally]
infrun: stop_waiting
info inferiors
Num Description Connection Executable
1 <null> a.out
* 2 <null> a.out
(gdb) info threads
No threads.
(gdb)
~~~
gdb/ChangeLog:
2020-02-05 Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Tom de Vries <tdevries@suse.de>
PR threads/25478
* infrun.c (stop_all_threads): Do NOT ignore
TARGET_WAITKIND_NO_RESUMED, TARGET_WAITKIND_THREAD_EXITED,
TARGET_WAITKIND_EXITED, TARGET_WAITKIND_SIGNALLED wait statuses
received from threads we attempt to stop.
Change-Id: I7cec98f40283773b79255d998511da434e9cd408
---
M gdb/infrun.c
1 file changed, 43 insertions(+), 5 deletions(-)
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 9224376..77c64da 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4799,9 +4799,14 @@
"infrun: %s not executing\n",
target_pid_to_str (t->ptid).c_str ());
- /* The thread may be not executing, but still be
- resumed with a pending status to process. */
- t->resumed = false;
+ if ((t->suspend.waitstatus.kind == TARGET_WAITKIND_SIGNALLED
+ || t->suspend.waitstatus.kind == TARGET_WAITKIND_EXITED)
+ && t->suspend.waitstatus_pending_p)
+ ;
+ else
+ /* The thread may be not executing, but still be
+ resumed with a pending status to process. */
+ t->resumed = false;
}
}
@@ -4829,8 +4834,41 @@
|| event.ws.kind == TARGET_WAITKIND_EXITED
|| event.ws.kind == TARGET_WAITKIND_SIGNALLED)
{
- /* All resumed threads exited
- or one thread/process exited/signalled. */
+ /* Eventing target is null if there were no waitable
+ targets. */
+ if (event.target != nullptr)
+ {
+ thread_info *t = find_thread_ptid (event.target,
+ event.ptid);
+ /* Check if this is the first time we see this
+ thread. Don't bother adding if it exited. */
+ if (t == nullptr
+ && event.ws.kind != TARGET_WAITKIND_THREAD_EXITED)
+ t = add_thread (event.target, event.ptid);
+
+ if (event.ws.kind != TARGET_WAITKIND_THREAD_EXITED)
+ {
+ /* Set the threads as non-executing to avoid
+ another stop attempt on them. */
+ mark_non_executing_threads (event.target, event.ptid,
+ event.ws);
+
+ if (event.ws.kind == TARGET_WAITKIND_NO_RESUMED)
+ {
+ /* Do nothing. Already marked the threads. */
+ }
+ else
+ {
+ /* TARGET_WAITKIND_EXITED or
+ TARGET_WAITKIND_SIGNALLED. */
+ save_waitstatus (t, &event.ws);
+ /* This was cleared in mark_non_executing_threads.
+ Set it so this pending event is considered by
+ do_target_wait. */
+ t->resumed = true;
+ }
+ }
+ }
}
else
{
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I7cec98f40283773b79255d998511da434e9cd408
Gerrit-Change-Number: 133
Gerrit-PatchSet: 3
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-CC: Tom de Vries <tdevries@suse.de>
Gerrit-MessageType: newpatchset
^ permalink raw reply [flat|nested] 22+ messages in thread
* [review v3] infrun: handle already-exited threads when attempting to stop
2019-10-18 13:27 [review] infrun: mark an exited thread non-executing when attempting to stop Tankut Baris Aktemur (Code Review)
` (17 preceding siblings ...)
2020-02-05 13:19 ` [review v3] " Tankut Baris Aktemur (Code Review)
@ 2020-02-05 13:24 ` Tankut Baris Aktemur (Code Review)
2020-02-05 22:59 ` Tom de Vries (Code Review)
2020-02-07 12:11 ` Tankut Baris Aktemur (Code Review)
20 siblings, 0 replies; 22+ messages in thread
From: Tankut Baris Aktemur (Code Review) @ 2020-02-05 13:24 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom de Vries, Pedro Alves, Luis Machado
Tankut Baris Aktemur has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/133
......................................................................
Patch Set 3:
> BTW, do you happen to have something more up-to-date than patch set 2? If so, could you share this?
I've uploaded patchset 3. This version uses pending waitstatuses and also includes fixes from
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/759
I've not fully updated the testcases in
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/143.
There are still unaddressed comments there.
This patchset assumes that an existing problem that is explained in
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/766
is fixed. Otherwise the scenario described in the commit message is not reproduced.
Thanks
-Baris
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I7cec98f40283773b79255d998511da434e9cd408
Gerrit-Change-Number: 133
Gerrit-PatchSet: 3
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-CC: Tom de Vries <tdevries@suse.de>
Gerrit-Comment-Date: Wed, 05 Feb 2020 13:24:15 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 22+ messages in thread
* [review v3] infrun: handle already-exited threads when attempting to stop
2019-10-18 13:27 [review] infrun: mark an exited thread non-executing when attempting to stop Tankut Baris Aktemur (Code Review)
` (18 preceding siblings ...)
2020-02-05 13:24 ` Tankut Baris Aktemur (Code Review)
@ 2020-02-05 22:59 ` Tom de Vries (Code Review)
2020-02-07 12:11 ` Tankut Baris Aktemur (Code Review)
20 siblings, 0 replies; 22+ messages in thread
From: Tom de Vries (Code Review) @ 2020-02-05 22:59 UTC (permalink / raw)
To: Tankut Baris Aktemur, gdb-patches; +Cc: Pedro Alves, Luis Machado
Tom de Vries has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/133
......................................................................
Patch Set 3:
> Patch Set 3:
>
> > BTW, do you happen to have something more up-to-date than patch set 2? If so, could you share this?
>
> I've uploaded patchset 3. This version uses pending waitstatuses and also includes fixes from
> https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/759
>
FTR, this also fixes the gdb/testsuite/gdb.threads/kill-in-stop-all-threads.exp test-case in patch set 4 of https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/759 .
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I7cec98f40283773b79255d998511da434e9cd408
Gerrit-Change-Number: 133
Gerrit-PatchSet: 3
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-CC: Tom de Vries <tdevries@suse.de>
Gerrit-Comment-Date: Wed, 05 Feb 2020 22:59:31 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 22+ messages in thread
* [review v3] infrun: handle already-exited threads when attempting to stop
2019-10-18 13:27 [review] infrun: mark an exited thread non-executing when attempting to stop Tankut Baris Aktemur (Code Review)
` (19 preceding siblings ...)
2020-02-05 22:59 ` Tom de Vries (Code Review)
@ 2020-02-07 12:11 ` Tankut Baris Aktemur (Code Review)
20 siblings, 0 replies; 22+ messages in thread
From: Tankut Baris Aktemur (Code Review) @ 2020-02-07 12:11 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom de Vries, Pedro Alves, Luis Machado
Tankut Baris Aktemur has abandoned this change. ( https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/133 )
Change subject: infrun: handle already-exited threads when attempting to stop
......................................................................
Abandoned
Migrated to https://sourceware.org/ml/gdb-patches/2020-02/msg00153.html
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I7cec98f40283773b79255d998511da434e9cd408
Gerrit-Change-Number: 133
Gerrit-PatchSet: 3
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-CC: Tom de Vries <tdevries@suse.de>
Gerrit-MessageType: abandon
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2020-02-07 12:11 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-18 13:27 [review] infrun: mark an exited thread non-executing when attempting to stop Tankut Baris Aktemur (Code Review)
2019-11-04 8:36 ` Tankut Baris Aktemur (Code Review)
2019-11-04 10:23 ` [review v2] infrun: handle already-exited threads " Tankut Baris Aktemur (Code Review)
2019-12-04 12:02 ` Tankut Baris Aktemur (Code Review)
2019-12-05 19:26 ` Pedro Alves (Code Review)
2019-12-06 16:19 ` Pedro Alves (Code Review)
2019-12-06 17:41 ` Tankut Baris Aktemur (Code Review)
2019-12-09 15:09 ` Tankut Baris Aktemur (Code Review)
2020-01-08 15:57 ` Tankut Baris Aktemur (Code Review)
2020-01-29 17:54 ` Tom de Vries (Code Review)
2020-01-30 9:10 ` Tom de Vries (Code Review)
2020-01-30 13:58 ` Tom de Vries (Code Review)
2020-01-30 16:41 ` Tankut Baris Aktemur (Code Review)
2020-01-30 21:06 ` Tom de Vries (Code Review)
2020-02-03 15:13 ` Tom de Vries (Code Review)
2020-02-03 16:02 ` Tankut Baris Aktemur (Code Review)
2020-02-03 16:27 ` Tom de Vries (Code Review)
2020-02-04 9:06 ` Tankut Baris Aktemur (Code Review)
2020-02-05 13:19 ` [review v3] " Tankut Baris Aktemur (Code Review)
2020-02-05 13:24 ` Tankut Baris Aktemur (Code Review)
2020-02-05 22:59 ` Tom de Vries (Code Review)
2020-02-07 12:11 ` Tankut Baris Aktemur (Code Review)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox