Differential D9260
Defer ptracestop signals that cannot be immediately delivered Authored by badger on Jan 20 2017, 4:51 PM. Tags None Referenced Files
Details Related to: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=212607 ptracestop() calls other than the one in kern_sig.c discard any signal Gdb should not hang when a PT_KILL is requested from a ptracestop() call $ gdb712 /bin/ls ... Reading symbols from /bin/ls...Reading symbols from /usr/lib/debug//bin/ls.debug...done. done. (gdb) catch syscall read Catchpoint 1 (syscall 'read' [3]) (gdb) r Starting program: /bin/ls Catchpoint 1 (call to syscall read), 0x000000080061e3fa in _read () from /libexec/ld-elf.so.1 (gdb) q A debugging session is active. Inferior 1 [process 2981] will be killed. Quit anyway? (y or n) y $
Diff Detail
Event TimelineComment Actions Mmm, I had instead been toying with a version that fixed the other callers to post the signal instead to ignoring the return value. (I had also written up some PT_KILL tests). I'm not tied to that solution (though if the tests are correct I'd like to add those). One other area that is "wrong" with PT_KILL is that we can continue to stop the process to report events that occur before the process is truly killed. That is why I haven't finalized my patch as I am still trying to convince myself the tests are correct and that PT_KILL is fully fixed now. In particular, I wouldn't want PT_KILL of a multithreaded application to report thread exit events for the other LWPs as they all exit as each of those need a PT_CONTINUE. Instead, I think PT_KILL probably needs to clear the ptrace() event mask. I also wasn't sure if we needed some of the PT_KILL cleanup to happen as a result of posting the signal so that, e.g. 'kill -9 <pid>' doesn't get hung if there are suspended threads. My WIP is here: https://github.com/freebsd/freebsd/compare/master...bsdjhb:PT_KILL I guess I hadn't actually started posting the signal yet, but instead was trying to figure out how the logic should work (e.g. does it need to explicitly ignore SIGTRAP and not just 0?) Comment Actions So in the places where there's a printf now ("Dropping signal from ptracestop") you'd tdsignal() (or some other variant) the return value of ptracestop()? Won't that cause a second, unexpected stop when the time comes to actually act on the signal? (the ptracestop() in kern_sig.c, which, in this review's patch, I skip if there's already a pending signal from the debugger) Comment Actions I think that I have some preference to the patch posted in this review, over the solution where callers of ptracestop() handle further signals. But then, shouldn't ptracestop() clear p_xsig is the signal is posted ? Also, what is the reason to return td_xsig() from ptracestop() after the patch ? Comment Actions As I said previously, I'm not strongly tied to any particular solution (and was still thinking about what the right solution is). I'm quite happy to defer to kib@'s judgement on not losing signals from ptracestop(). That said, I do think the PT_KILL tests I've added are still relevant and it would be good to verify that those are fixed by this change. (I have some other tests that I need to add to ensure we don't get extra stops for thread exit events if we PT_KILL a multithreaded process as well, but that is orthogonal to this change) Comment Actions Hmm, because the signal has been deferred, but someone might examine p_xsig before it is delivered and act as if it had already been delivered? I'd thought that p_xsig would be overwritten before anything would use it to make decisions, but I may be wrong. I will look further.
td_xsig being set is used from issignal() to determine if ptracestop() previously wanted to deliver a signal but couldn't; we want to skip the stop in this case since the signal originated from the debugger. (Did I understand that question correctly?) Comment Actions In testing, I find that sending a signal from multiple ptracestop() calls before having a chance to deliver any signals does not work as I'd expect. I now think it would be best, if a ptracestop() call wants to send a new signal to the process but there is already a deferred signal, it should be replaced by the new one. E.g., if you
I expect only the signal from step 4 to sent to the process when it is continued. Any thoughts otherwise? Comment Actions I used bad wording in my questions, it seems. I mean, as a vague idea, to move the post-processing after ptracestop() in issignal() into ptracestop() itself, repeating it until td_xsig is cleared. The code which adds signal to the queue, and possibly calling tdnotify(). If ptracestop() enqueues the signals, then different signal requests from different ptrace(2) requests will be queued as well, answering your later question. I did not experimented with this, so it may very well be not possible to do. Comment Actions Take a different direction
new flag KSI_PTRACE to indicate they originated from ptrace. We won't
Comment Actions John - your PT_KILL tests are in this diff since I've been running with them, but I'll remove them before commit to preserve provenance (unless you ask me otherwise). Comment Actions Please include the tests in your change. The extra case I am referring to is if the debugger sends a PT_KILL while other events like a thread birth event is still pending. Also, another instance of that is if you PT_KILL a process with two threads, you shouldn't get a stopevent for the first thread exiting before the final thread really exits due to sigexit(). I think we can add tests for that and fix those as a followup though.
Comment Actions
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
You can just use #ifdef __SSE4_2__. -msse42 (or -msse4) will define that via the compiler driver. That is what controls 'popcnt' (in the case of popcnt we use it if you set CPUTYPE in your make.conf to specify a CPU that includes popcnt).