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

Reply via email to