[lldb] [MainLoop] Initialize empty sigset_t correctly

[mgorny]

Summary

Fix MainLoop::RunImpl::get_sigmask() to correctly return empty sigset_t
when SIGNAL_POLLING_UNSUPPORTED is true. On NetBSD (and probably
on some other platforms), integers are not implicitly convertible to
sigset_t, so 'return 0' is erraneous. Instead, sigset_t should be reset
through sigemptyset().

While at it, move common parts out of the #ifdef.

// FTR: I don't know if this wouldn't break Windows. I'd appreciate if somebody could test it. Alternatively, I can watch buildbots after merging.

[krytarowski]

I think that sigemptyset(2) is unsupported on Windows.

lldb/source/Host/common/MainLoop.cpp

149 ↗

(On Diff #185992)

Shouldn't we initialize sigmask always before adding/deleting particular signals?

149 ↗

(On Diff #185992)

Ah, I can see it now. It's passed to pthread_sigmask(3) so it's fine.

[mgorny]

Ok, I see that _WIN32 actually redefines sigset_t, so I've added a separate branch for it.

[krytarowski]

In D57959#1390929, @mgorny wrote:

Ok, I see that _WIN32 actually redefines sigset_t, so I've added a separate branch for it.

Maybe #ifdef LLVM_ON_UNIX?

[mgorny]

But to what purpose? I think it's better to use consistent macros to refer to the same scenario.

[krytarowski]

In D57959#1390992, @mgorny wrote:

But to what purpose? I think it's better to use consistent macros to refer to the same scenario.

I had an impression that LLVM_ON_UNIX is LLVM homegrown symbol for this exact purpose to differentiate Windows (out of UNIX [and probably next to Fuchsia today, but it's not supported in LLDB].

[labath]

Lgtm, though I strongly suspect you will need to make sure NetBSD falls into the ! SIGNAL_POLLING_UNSUPPORTED category to have even basic lldb-server functionality working. Have you looked at why that isn't happening?

[mgorny]

I was experimenting with disabling kevent, and apparently no other NetBSD mechanism is supported by LLDB. Noticed this issue, so figured I'll fix it while at it.

[labath]

In D57959#1392394, @mgorny wrote:

I was experimenting with disabling kevent, and apparently no other NetBSD mechanism is supported by LLDB. Noticed this issue, so figured I'll fix it while at it.

Ok, thanks. Just FYI: I think you should at least have pselect, but that one is currently not selected by cmake, but it's hardcoded for android. Fixing that should be easy, if you ever need it.

source/Host/common/MainLoop.cpp

 #else
 MainLoop::RunImpl::RunImpl(MainLoop &loop) : loop(loop) {
 #ifndef __ANDROID__
   read_fds.reserve(loop.m_read_fds.size());
 #endif
 }
 
 sigset_t MainLoop::RunImpl::get_sigmask() {
-#if SIGNAL_POLLING_UNSUPPORTED
-  return 0;
-#else
   sigset_t sigmask;
+#if defined(_WIN32)
+  sigmask = 0;
+#elif SIGNAL_POLLING_UNSUPPORTED
+  sigemptyset(&sigmask);
+#else
   int ret = pthread_sigmask(SIG_SETMASK, nullptr, &sigmask);
   assert(ret == 0);
   (void) ret;
 
   for (const auto &sig : loop.m_signals)
     sigdelset(&sigmask, sig.first);
-  return sigmask;
 #endif
+  return sigmask;
 }
 
 #ifdef __ANDROID__
 Status MainLoop::RunImpl::Poll() {
   // ppoll(2) is not supported on older all android versions. Also, older
   // versions android (API <= 19) implemented pselect in a non-atomic way, as a
   // combination of pthread_sigmask and select. This is not sufficient for us,
   // as we rely on the atomicity to correctly implement signal polling, so we