No problem, I'll ping the list when I have a patch ready.
On Fri, Mar 21, 2014 at 8:11 PM, Todd Fiala <[email protected]> wrote: > Oh yeah sure that's fine. When testing the test to make sure it catches > the failure, you can just essentially reverse out the patch and then > reapply to verify the fix. > > No rush either, it would just be great to get more tests into the system > to catch these things as we discover proper behavior. > > > On Fri, Mar 21, 2014 at 12:09 PM, Andrew MacPherson <[email protected] > > wrote: > >> Hey Todd, sure thing. I haven't written a test before and I'm out of time >> tonight but I can take a look over the weekend or early next week, I'm >> looking into another issue with a SIGSTOP leaking out on detach as well so >> would you be okay if I apply this small patch now and then write a test >> that covers both this case and the detach case for when I submit a patch >> for that? >> >> Cheers, >> Andrew >> >> >> On Fri, Mar 21, 2014 at 8:04 PM, Todd Fiala <[email protected]> wrote: >> >>> Hey Andrew, >>> >>> Any chance you can put a test into lldb to verify the fix? I'm doing >>> some work on (essentially currently) parallel code (implementations of >>> NativeProcessProtocol/NativeThreadProtocol) for Linux that will be used in >>> lldb-gdbserver and I'd love us to have tests that verify we don't gunk that >>> up or regress it in the future. >>> >>> >>> On Fri, Mar 21, 2014 at 11:59 AM, <[email protected]> wrote: >>> >>>> Indeed, setting the resume signal explicitly on the thread on stop was >>>> not the right thing to do. You don't want to clear the thread's resume >>>> signal out in Thread::ShouldStop because somebody might be trying to resume >>>> the thread with a hand-provided signal, and you wouldn't want to interrupt >>>> that. But the thread itself should let the StopInfo carry the stop signal >>>> information. >>>> >>>> Jim >>>> >>>> On Mar 21, 2014, at 11:35 AM, Andrew MacPherson <[email protected]> >>>> wrote: >>>> >>>> > Actually it looks like those calls may be redundant now as it's being >>>> handled by StopInfo. This patch gets attach resume/interrupt working for me >>>> on Linux. >>>> > >>>> > >>>> > On Fri, Mar 21, 2014 at 7:21 PM, Andrew MacPherson < >>>> [email protected]> wrote: >>>> > Thanks Jim, LinuxSignals.cpp had SIGSTOP marked as suppress = false >>>> unlike UnixSignals.cpp which had it correctly marked suppress = true. With >>>> that fixed though the SIGSTOP is still getting set due to the call to >>>> SetResumeSignal() in POSIXThread::SignalDeliveredNotify() it looks like. >>>> Should this code also be using the signals table to decide whether to >>>> suppress it? >>>> > >>>> > Thanks again. >>>> > >>>> > >>>> > >>>> > On Fri, Mar 21, 2014 at 6:58 PM, <[email protected]> wrote: >>>> > Sorry, that's StopInfoSignal::WillResume. >>>> > >>>> > Jim >>>> > >>>> > On Mar 21, 2014, at 10:54 AM, [email protected] wrote: >>>> > >>>> > > Somebody is not paying attention to the "process handle" settings. >>>> Normally SIGSTOP is set not to pass: >>>> > > >>>> > > (lldb) process handle SIGSTOP >>>> > > NAME PASS STOP NOTIFY >>>> > > ========== ===== ===== ====== >>>> > > SIGSTOP false true true >>>> > > >>>> > > This check should be done in Process::WillResume: >>>> > > >>>> > > virtual void >>>> > > WillResume (lldb::StateType resume_state) >>>> > > { >>>> > > ThreadSP thread_sp (m_thread_wp.lock()); >>>> > > if (thread_sp) >>>> > > { >>>> > > if >>>> (thread_sp->GetProcess()->GetUnixSignals().GetShouldSuppress(m_value) == >>>> false) >>>> > > thread_sp->SetResumeSignal(m_value); >>>> > > } >>>> > > } >>>> > > >>>> > > I wonder why this isn't happening in your case? >>>> > > >>>> > > Jim >>>> > > >>>> > > >>>> > > On Mar 21, 2014, at 10:46 AM, Andrew MacPherson < >>>> [email protected]> wrote: >>>> > > >>>> > >> Currently when attaching to a running process on Linux, a SIGSTOP >>>> signal is (incorrectly I think) injected into the inferior on resume. This >>>> can be reproduced by simply launching any process and then in a separate >>>> terminal doing: >>>> > >> >>>> > >> sudo lldb -p <pid> >>>> > >> c >>>> > >> process interrupt >>>> > >> c >>>> > >> >>>> > >> On the second continue the SIGSTOP that was used to stop the >>>> process is injected into the inferior by being passed to PTRACE_CONT, >>>> resulting in the process going into a stop state outside the control of >>>> LLDB. The SIGSTOP comes from the SetResumeSignal() call in POSIXThread and >>>> StopInfo. >>>> > >> >>>> > >> I can't think of any reason why a SIGSTOP should ever be injected >>>> into the inferior so as a test I simply prevented it from ever happening: >>>> > >> >>>> > >> diff --git a/source/Plugins/Process/Linux/ProcessMonitor.cpp >>>> b/source/Plugins/Process/Linux/ProcessMonitor.cpp >>>> > >> index 3dec6de..3079379 100644 >>>> > >> --- a/source/Plugins/Process/Linux/ProcessMonitor.cpp >>>> > >> +++ b/source/Plugins/Process/Linux/ProcessMonitor.cpp >>>> > >> @@ -2209,6 +2209,9 @@ ProcessMonitor::Resume(lldb::tid_t tid, >>>> uint32_t signo) >>>> > >> bool result; >>>> > >> Log *log (ProcessPOSIXLog::GetLogIfAllCategoriesSet >>>> (POSIX_LOG_PROCESS)); >>>> > >> >>>> > >> + if (signo == SIGSTOP) >>>> > >> + signo = eResumeSignalNone; >>>> > >> + >>>> > >> if (log) >>>> > >> log->Printf ("ProcessMonitor::%s() resuming thread = %" >>>> PRIu64 " with signal %s", __FUNCTION__, tid, >>>> > >> >>>> m_process->GetUnixSignals().GetSignalAsCString (signo)); >>>> > >> >>>> > >> This resolves the issue and doesn't cause any other problems that >>>> I can find but almost certainly isn't the proper fix. My main concern is >>>> that all of the resume signal code is shared with other OSes which I'm >>>> guessing treat this differently. >>>> > >> >>>> > >> Any thoughts as to what the proper fix here might be? >>>> > >> >>>> > >> Thanks, >>>> > >> Andrew >>>> > >> _______________________________________________ >>>> > >> lldb-dev mailing list >>>> > >> [email protected] >>>> > >> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev >>>> > > >>>> > > _______________________________________________ >>>> > > lldb-dev mailing list >>>> > > [email protected] >>>> > > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev >>>> > >>>> > >>>> > >>>> > <SIGSTOP-resume.patch> >>>> >>>> _______________________________________________ >>>> lldb-dev mailing list >>>> [email protected] >>>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev >>>> >>> >>> >>> >>> -- >>> Todd Fiala | Software Engineer | [email protected] | 650-943-3180 >>> >> >> > > > -- > Todd Fiala | Software Engineer | [email protected] | 650-943-3180 >
_______________________________________________ lldb-dev mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
