| Message ID | 1455150506-12760-1-git-send-email-donb@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
Ping Thanks, --Don On 2/10/2016 4:28 PM, Don Breazeal wrote: > On 2/1/2016 12:09 PM, Pedro Alves wrote: >> On 02/01/2016 07:29 PM, Don Breazeal wrote: >>> On 2/1/2016 4:05 AM, Pedro Alves wrote: >> >>> Hi Pedro, > ---snip--- >>> A fork event was reported to GDB before GDB knew about the parent thread, >>> followed immediately by a breakpoint event in a different thread. The >>> parent thread was subsequently added via remote_notice_new_inferior in >>> process_stop_reply, but when the thread was added the thread_info.state >>> was set to THREAD_STOPPED. The fork event was then handled correctly, >>> but when the fork parent was resumed via a call to keep_going, the state >>> was unchanged. >> >> Since this is non-stop, then it sounds to me like the bug is that the >> thread should have been added in THREAD_RUNNING state. >> >> Consider that infrun may be pulling target events out of the target_ops >> backend into its own event queue, but, not process them immediately. >> >> E.g., infrun may be stopping all threads temporarily for a step-over-breakpoint >> operation for thread A (stop_all_threads). The waitstatus of all threads >> is thus left pending in the thread structure (save_status), including the >> fork event of thread B. Right at this point, if the user >> does "info threads", that should show thread B (the fork parent) running, >> not stopped, even if internally, gdb is holding it paused for a little bit. >> > > Hi Pedro, > Here is a new patch that adds the threads with the state set to > THREAD_RUNNING for fork events. > Thanks! > --Don > > This patch addresses a failure in > gdb.threads/forking-threads-plus-breakpoint.exp: > > FAIL: gdb.threads/forking-threads-plus-breakpoint.exp: cond_bp_target=1: > detach_on_fork=on: inferior 1 exited (timeout) > > Cause: > A fork event was reported to GDB before GDB knew about the parent thread, > followed immediately by a breakpoint event in a different thread. The > parent thread was subsequently added via remote_notice_new_inferior in > process_stop_reply, but when the thread was added the thread_info.state > was set to THREAD_STOPPED. The fork event was then handled correctly, > but when the fork parent was resumed via a call to keep_going, the state > was unchanged. > > The breakpoint event was then handled, which caused all the non-breakpoint > threads to be stopped. When the breakpoint thread was resumed, all the > non-breakpoint threads were resumed via infrun.c:restart_threads. Our old > fork parent wasn't restarted, because it still had thread_info.state set to > THREAD_STOPPED. Ultimately the program under debug hung waiting for a > pthread_join while the old fork parent was stopped forever by GDB. > > Fix: > Make sure to add the fork parent thread in the THREAD_RUNNING state by > calling remote_notice_new_inferior with RUNNING set to 1 when processing > a fork stop reply. > > Tested on x86_64 Linux and Nios II Linux target with x86 Linux host. > > gdb/ChangeLog: > 2016-02-10 Don Breazeal <donb@codesourcery.com> > > * remote.c (process_stop_reply): Call remote_notice_new_inferior > with RUNNING set to 1 when handling fork events. > > --- > gdb/remote.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/gdb/remote.c b/gdb/remote.c > index f09a06e..ab750a7 100644 > --- a/gdb/remote.c > +++ b/gdb/remote.c > @@ -6818,7 +6818,14 @@ process_stop_reply (struct stop_reply *stop_reply, > VEC_free (cached_reg_t, stop_reply->regcache); > } > > - remote_notice_new_inferior (ptid, 0); > + /* If a fork event arrived before we knew about the parent thread, > + make sure to mark it as running when it is created. */ > + if (status->kind == TARGET_WAITKIND_FORKED > + || status->kind == TARGET_WAITKIND_VFORKED) > + remote_notice_new_inferior (ptid, 1); > + else > + remote_notice_new_inferior (ptid, 0); > + > remote_thr = demand_private_info (ptid); > remote_thr->core = stop_reply->core; > remote_thr->stop_reason = stop_reply->stop_reason; >
Ping. I checked, the patch still applies cleanly to mainline. Thanks --Don On 2/25/2016 9:29 AM, Don Breazeal wrote: > Ping > Thanks, > --Don > > On 2/10/2016 4:28 PM, Don Breazeal wrote: >> On 2/1/2016 12:09 PM, Pedro Alves wrote: >>> On 02/01/2016 07:29 PM, Don Breazeal wrote: >>>> On 2/1/2016 4:05 AM, Pedro Alves wrote: >>> >>>> Hi Pedro, >> ---snip--- >>>> A fork event was reported to GDB before GDB knew about the parent thread, >>>> followed immediately by a breakpoint event in a different thread. The >>>> parent thread was subsequently added via remote_notice_new_inferior in >>>> process_stop_reply, but when the thread was added the thread_info.state >>>> was set to THREAD_STOPPED. The fork event was then handled correctly, >>>> but when the fork parent was resumed via a call to keep_going, the state >>>> was unchanged. >>> >>> Since this is non-stop, then it sounds to me like the bug is that the >>> thread should have been added in THREAD_RUNNING state. >>> >>> Consider that infrun may be pulling target events out of the target_ops >>> backend into its own event queue, but, not process them immediately. >>> >>> E.g., infrun may be stopping all threads temporarily for a step-over-breakpoint >>> operation for thread A (stop_all_threads). The waitstatus of all threads >>> is thus left pending in the thread structure (save_status), including the >>> fork event of thread B. Right at this point, if the user >>> does "info threads", that should show thread B (the fork parent) running, >>> not stopped, even if internally, gdb is holding it paused for a little bit. >>> >> >> Hi Pedro, >> Here is a new patch that adds the threads with the state set to >> THREAD_RUNNING for fork events. >> Thanks! >> --Don >> >> This patch addresses a failure in >> gdb.threads/forking-threads-plus-breakpoint.exp: >> >> FAIL: gdb.threads/forking-threads-plus-breakpoint.exp: cond_bp_target=1: >> detach_on_fork=on: inferior 1 exited (timeout) >> >> Cause: >> A fork event was reported to GDB before GDB knew about the parent thread, >> followed immediately by a breakpoint event in a different thread. The >> parent thread was subsequently added via remote_notice_new_inferior in >> process_stop_reply, but when the thread was added the thread_info.state >> was set to THREAD_STOPPED. The fork event was then handled correctly, >> but when the fork parent was resumed via a call to keep_going, the state >> was unchanged. >> >> The breakpoint event was then handled, which caused all the non-breakpoint >> threads to be stopped. When the breakpoint thread was resumed, all the >> non-breakpoint threads were resumed via infrun.c:restart_threads. Our old >> fork parent wasn't restarted, because it still had thread_info.state set to >> THREAD_STOPPED. Ultimately the program under debug hung waiting for a >> pthread_join while the old fork parent was stopped forever by GDB. >> >> Fix: >> Make sure to add the fork parent thread in the THREAD_RUNNING state by >> calling remote_notice_new_inferior with RUNNING set to 1 when processing >> a fork stop reply. >> >> Tested on x86_64 Linux and Nios II Linux target with x86 Linux host. >> >> gdb/ChangeLog: >> 2016-02-10 Don Breazeal <donb@codesourcery.com> >> >> * remote.c (process_stop_reply): Call remote_notice_new_inferior >> with RUNNING set to 1 when handling fork events. >> >> --- >> gdb/remote.c | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/gdb/remote.c b/gdb/remote.c >> index f09a06e..ab750a7 100644 >> --- a/gdb/remote.c >> +++ b/gdb/remote.c >> @@ -6818,7 +6818,14 @@ process_stop_reply (struct stop_reply *stop_reply, >> VEC_free (cached_reg_t, stop_reply->regcache); >> } >> >> - remote_notice_new_inferior (ptid, 0); >> + /* If a fork event arrived before we knew about the parent thread, >> + make sure to mark it as running when it is created. */ >> + if (status->kind == TARGET_WAITKIND_FORKED >> + || status->kind == TARGET_WAITKIND_VFORKED) >> + remote_notice_new_inferior (ptid, 1); >> + else >> + remote_notice_new_inferior (ptid, 0); >> + >> remote_thr = demand_private_info (ptid); >> remote_thr->core = stop_reply->core; >> remote_thr->stop_reason = stop_reply->stop_reason; >> >
Ping. Thanks, --Don On 3/3/2016 10:20 AM, Don Breazeal wrote: > Ping. > I checked, the patch still applies cleanly to mainline. > Thanks > --Don > > On 2/25/2016 9:29 AM, Don Breazeal wrote: >> Ping >> Thanks, >> --Don >> >> On 2/10/2016 4:28 PM, Don Breazeal wrote: >>> On 2/1/2016 12:09 PM, Pedro Alves wrote: >>>> On 02/01/2016 07:29 PM, Don Breazeal wrote: >>>>> On 2/1/2016 4:05 AM, Pedro Alves wrote: >>>> >>>>> Hi Pedro, >>> ---snip--- >>>>> A fork event was reported to GDB before GDB knew about the parent thread, >>>>> followed immediately by a breakpoint event in a different thread. The >>>>> parent thread was subsequently added via remote_notice_new_inferior in >>>>> process_stop_reply, but when the thread was added the thread_info.state >>>>> was set to THREAD_STOPPED. The fork event was then handled correctly, >>>>> but when the fork parent was resumed via a call to keep_going, the state >>>>> was unchanged. >>>> >>>> Since this is non-stop, then it sounds to me like the bug is that the >>>> thread should have been added in THREAD_RUNNING state. >>>> >>>> Consider that infrun may be pulling target events out of the target_ops >>>> backend into its own event queue, but, not process them immediately. >>>> >>>> E.g., infrun may be stopping all threads temporarily for a step-over-breakpoint >>>> operation for thread A (stop_all_threads). The waitstatus of all threads >>>> is thus left pending in the thread structure (save_status), including the >>>> fork event of thread B. Right at this point, if the user >>>> does "info threads", that should show thread B (the fork parent) running, >>>> not stopped, even if internally, gdb is holding it paused for a little bit. >>>> >>> >>> Hi Pedro, >>> Here is a new patch that adds the threads with the state set to >>> THREAD_RUNNING for fork events. >>> Thanks! >>> --Don >>> >>> This patch addresses a failure in >>> gdb.threads/forking-threads-plus-breakpoint.exp: >>> >>> FAIL: gdb.threads/forking-threads-plus-breakpoint.exp: cond_bp_target=1: >>> detach_on_fork=on: inferior 1 exited (timeout) >>> >>> Cause: >>> A fork event was reported to GDB before GDB knew about the parent thread, >>> followed immediately by a breakpoint event in a different thread. The >>> parent thread was subsequently added via remote_notice_new_inferior in >>> process_stop_reply, but when the thread was added the thread_info.state >>> was set to THREAD_STOPPED. The fork event was then handled correctly, >>> but when the fork parent was resumed via a call to keep_going, the state >>> was unchanged. >>> >>> The breakpoint event was then handled, which caused all the non-breakpoint >>> threads to be stopped. When the breakpoint thread was resumed, all the >>> non-breakpoint threads were resumed via infrun.c:restart_threads. Our old >>> fork parent wasn't restarted, because it still had thread_info.state set to >>> THREAD_STOPPED. Ultimately the program under debug hung waiting for a >>> pthread_join while the old fork parent was stopped forever by GDB. >>> >>> Fix: >>> Make sure to add the fork parent thread in the THREAD_RUNNING state by >>> calling remote_notice_new_inferior with RUNNING set to 1 when processing >>> a fork stop reply. >>> >>> Tested on x86_64 Linux and Nios II Linux target with x86 Linux host. >>> >>> gdb/ChangeLog: >>> 2016-02-10 Don Breazeal <donb@codesourcery.com> >>> >>> * remote.c (process_stop_reply): Call remote_notice_new_inferior >>> with RUNNING set to 1 when handling fork events. >>> >>> --- >>> gdb/remote.c | 9 ++++++++- >>> 1 file changed, 8 insertions(+), 1 deletion(-) >>> >>> diff --git a/gdb/remote.c b/gdb/remote.c >>> index f09a06e..ab750a7 100644 >>> --- a/gdb/remote.c >>> +++ b/gdb/remote.c >>> @@ -6818,7 +6818,14 @@ process_stop_reply (struct stop_reply *stop_reply, >>> VEC_free (cached_reg_t, stop_reply->regcache); >>> } >>> >>> - remote_notice_new_inferior (ptid, 0); >>> + /* If a fork event arrived before we knew about the parent thread, >>> + make sure to mark it as running when it is created. */ >>> + if (status->kind == TARGET_WAITKIND_FORKED >>> + || status->kind == TARGET_WAITKIND_VFORKED) >>> + remote_notice_new_inferior (ptid, 1); >>> + else >>> + remote_notice_new_inferior (ptid, 0); >>> + >>> remote_thr = demand_private_info (ptid); >>> remote_thr->core = stop_reply->core; >>> remote_thr->stop_reason = stop_reply->stop_reason; >>> >> >
diff --git a/gdb/remote.c b/gdb/remote.c index f09a06e..ab750a7 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -6818,7 +6818,14 @@ process_stop_reply (struct stop_reply *stop_reply, VEC_free (cached_reg_t, stop_reply->regcache); } - remote_notice_new_inferior (ptid, 0); + /* If a fork event arrived before we knew about the parent thread, + make sure to mark it as running when it is created. */ + if (status->kind == TARGET_WAITKIND_FORKED + || status->kind == TARGET_WAITKIND_VFORKED) + remote_notice_new_inferior (ptid, 1); + else + remote_notice_new_inferior (ptid, 0); + remote_thr = demand_private_info (ptid); remote_thr->core = stop_reply->core; remote_thr->stop_reason = stop_reply->stop_reason;