[Lldb-commits] [PATCH] D64647: [lldb] [Process/NetBSD] Implement per-thread execution control

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:354
+      if (signal != LLDB_INVALID_SIGNAL_NUMBER && signal != action->signal)
+        return Status("NetBSD does not support passing multiple signals simultaneously");
 
----------------
mgorny wrote:
> labath wrote:
> > krytarowski wrote:
> > > mgorny wrote:
> > > > labath wrote:
> > > > > Is this "passing multiple signals simultaneously", or "passing multiple *distinct* signals simultaneously". (E.g,. thread 1 gets a SIGALRM, thread 2 gets SIGIO, etc.).
> > > > The former. Basically there's one siginfo slot, so you can either put a signal for whole process, or for one LWP.
> > > Once we emit a single signal to all threads, it's still technically a single signal that hits the process.
> > Ok, that makes sense. But I don't think that's what the code does right now (IIUC, this line will only fire if the current signal is different that the signal sent to the previous thread).
> There's a second `if` on line 400 that verifies the number of threads signaled.
> 
> The idea is that we have either:
> * exactly one thread signaled,
> * all threads signaled with the same signal.
> 
> Here we check for the 'same signal' condition.
Ah, I see. That makes sense.

It might be more readable to move all the checks up front, perhaps into some function like `Expected< ptrace_siginfo_t> ComputeSignalInfo(const ResumeActionList&)`. Apart from readability, doing this will make sure you don't issue ptrace commands before you before realize that  the resume packet was bogus (and leave the inferior in a weird state in the process).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64647/new/

https://reviews.llvm.org/D64647