[lldb-dev] Resuming traced process on Linux and SIGSTOP
Todd Fiala
tfiala at google.com
Fri Mar 21 12:16:18 PDT 2014
Right - basically whatever didn't fail in your attach scenario we should
test and make sure that it fails without your patch, and (ideally *grin*)
passes with your patch.
On Fri, Mar 21, 2014 at 12:14 PM, <jingham at apple.com> wrote:
> I'm pretty sure we have some test cases that test attach. If we don't,
> then it would be great to add one. If we do, then it would be interesting
> to see why they didn't fail. For instance, maybe they just didn't bother
> to try "continue" after the attach. Just adding that would be good too.
>
> Jim
>
> On Mar 21, 2014, at 12:11 PM, Andrew MacPherson <andrew.macp at gmail.com>
> wrote:
>
> > No problem, I'll ping the list when I have a patch ready.
> >
> >
> > On Fri, Mar 21, 2014 at 8:11 PM, Todd Fiala <tfiala at google.com> 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 <
> andrew.macp at gmail.com> 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 <tfiala at google.com> 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, <jingham at apple.com> 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 <andrew.macp at gmail.com>
> 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 <
> andrew.macp at gmail.com> 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, <jingham at apple.com> wrote:
> > > Sorry, that's StopInfoSignal::WillResume.
> > >
> > > Jim
> > >
> > > On Mar 21, 2014, at 10:54 AM, jingham at apple.com 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 <
> andrew.macp at gmail.com> 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
> > > >> lldb-dev at cs.uiuc.edu
> > > >> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
> > > >
> > > > _______________________________________________
> > > > lldb-dev mailing list
> > > > lldb-dev at cs.uiuc.edu
> > > > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
> > >
> > >
> > >
> > > <SIGSTOP-resume.patch>
> >
> > _______________________________________________
> > lldb-dev mailing list
> > lldb-dev at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
> >
> >
> >
> > --
> > Todd Fiala | Software Engineer | tfiala at google.com |
> 650-943-3180
> >
> >
> >
> >
> >
> > --
> > Todd Fiala | Software Engineer | tfiala at google.com |
> 650-943-3180
> >
> >
>
>
--
Todd Fiala | Software Engineer | tfiala at google.com | 650-943-3180
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20140321/b0e9d52f/attachment.html>
More information about the lldb-dev
mailing list