[Lldb-commits] [PATCH] D30520: Make LLDB skip server-client roundtrip for signals that don't require any actions

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

I added a few comments.  It doesn't look like you addressed Pavel's idea for testing more of the code path you're introducing.  Do you plan to do that?



================
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:3766-3776
+  llvm::SmallVector<int32_t, 40> signals_to_ignore;
+  int32_t signo = m_unix_signals_sp->GetFirstSignalNumber();
+  while (signo != LLDB_INVALID_SIGNAL_NUMBER) {
+    bool should_ignore = !m_unix_signals_sp->GetShouldStop(signo) &&
+                         !m_unix_signals_sp->GetShouldNotify(signo) &&
+                         !m_unix_signals_sp->GetShouldSuppress(signo);
+
----------------
This code should go into the UnixSignals class.  Any other Process plugin that wanted to implement expedited signal handling would also have to do this computation, and we shouldn't duplicate the code.


================
Comment at: source/Target/Process.cpp:1629-1630
+
+void Process::DidLaunch() { UpdateAutomaticSignalFiltering(); }
+
 Error Process::ResumeSynchronous(Stream *stream) {
----------------
For this to work, you are relying on the Constructor of the signals class to set up the object by calling AddSignal on all the signals.  Otherwise initially m_version in the signals class and m_last_signal_version will start out at 0 and you won't prime the filtering here.

You should probably add a comment to that effect somewhere in UnixSignals so we don't forget this dependency.


https://reviews.llvm.org/D30520