Hi Kevin, For your concern "I'd like to understand the situation better before approving it.", please check attached "The patch for bugzilla 30387.msg". This mail has a debug log with some my analysis. For "format nit", I will work on it again. Best Regards. Zhiyong -----Original Message----- From: Kevin Buettner Sent: Wednesday, July 12, 2023 2:43 AM To: Yan, Zhiyong Cc: gdb-patches@sourceware.org; tom@tromey.com; luis.machado@arm.com Subject: Re: [PATCH] gdbserver: Install single-step breakpoint for a pending thread whose last_resume_kind is resume_step CAUTION: This email comes from a non Wind River email account! Do not click links or open attachments unless you recognize the sender and know the content is safe. Hi Zhiyong, See my comments below; there's still one formatting nit, but I also have a question about whether this is the right place to fix the bug that you're seeing. Also, I've added Luis to the CC list since he knows a lot more about the ARM architecture than I do. On Mon, 3 Jul 2023 11:04:58 +0800 wrote: > From: Zhiyong Yan > > Gdb should not assume pending threads always generate "a non-gdbserver trap event", for example "Signal 17" event could happen. Now that resume_stopped_resumed_lwps() -> may_hw_step() assumes that the break point must already exist, resume_one_thread() should ensure the software breaking point is installed although the thread is pending. > > Signed-off-by: Zhiyong Yan zhiyong.yan@windriver.com > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30387 > --- > gdbserver/linux-low.cc | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc index > e6a39202a98..7d8bbf71ddc 100644 > --- a/gdbserver/linux-low.cc > +++ b/gdbserver/linux-low.cc > @@ -4671,7 +4671,16 @@ linux_process_target::resume_one_thread (thread_info *thread, > proceed_one_lwp (thread, NULL); > } > else > - threads_debug_printf ("leaving LWP %ld stopped", lwpid_of (thread)); > + { > + threads_debug_printf ("leaving LWP %ld stopped", lwpid_of (thread)); > + if (thread->last_resume_kind == resume_step) > + { > + /* If resume_step is required by GDB, > + install single-step breakpoint. */ > + if (supports_software_single_step()) Formatting nit: you need a space between 'supports_software_single_step' and the left paren. > + install_software_single_step_breakpoints (lwp); > + } > + } > > thread->last_status.set_ignore (); > lwp->resume = NULL; With regard to the change itself... If the debugging printf is accurate, the LWP is being left in a stopped state. If that's the case, then why are software single step breakpoints being inserted? It seems to me that you'd only want to insert these breakpoints when the thread is actually about to resume. That said, I have no doubt that this change works for you, so clearly there's something going on which I do not understand. I'd like to understand the situation better before approving it. Also, once you have an explanation, please update the code comments and/or commit log message as appropriate. My personal preference is for a concise explanation to be placed in the code comments with any additional, longer winded explanations or examples being placed in the commit log message. Kevin