[lldb][AIX] AIX Changes for MainLoop polling

[DhruvSrivastavaX]

This PR is in reference to porting LLDB on AIX.

Link to discussions on llvm discourse and github:

1. https://discourse.llvm.org/t/port-lldb-to-ibm-aix/80640
2. Extending LLDB to work on AIX #101657
   The complete changes for porting are present in this draft PR:
   Extending LLDB to work on AIX #102601

Dropping changes for MainLoop polling in AIX, as ppoll is not supported in AIX currently.
This change is part of the couple of minimal changes required to build a minimal lldb binary on AIX

Review Request: @labath @DavidSpickett

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Dhruv Srivastava (DhruvSrivastavaX)

Changes

This PR is in reference to porting LLDB on AIX.

Link to discussions on llvm discourse and github:

1. https://discourse.llvm.org/t/port-lldb-to-ibm-aix/80640
2. Extending LLDB to work on AIX #101657
   The complete changes for porting are present in this draft PR:
   Extending LLDB to work on AIX #102601

Dropping changes for MainLoop polling in AIX, as ppoll is not supported in AIX currently.
This change is part of the couple of minimal changes required to build a minimal lldb biniry on AIX

Review Request: @labath @DavidSpickett

---

Full diff: https://github.com/llvm/llvm-project/pull/120378.diff

1 Files Affected:

• (modified) lldb/source/Host/posix/MainLoopPosix.cpp (+14-2)

diff --git a/lldb/source/Host/posix/MainLoopPosix.cpp b/lldb/source/Host/posix/MainLoopPosix.cpp
index 1715610e0f84f1..a1d697e10a04ed 100644
--- a/lldb/source/Host/posix/MainLoopPosix.cpp
+++ b/lldb/source/Host/posix/MainLoopPosix.cpp
@@ -170,11 +170,23 @@ Status MainLoopPosix::RunImpl::Poll() {
     read_fds.push_back(pfd);
   }
 
+#if defined(_AIX)
+  sigset_t origmask;
+  int timeout;
+
+  timeout = -1;
+  pthread_sigmask(SIG_SETMASK, nullptr, &origmask);
+  int ready = poll(read_fds.data(), read_fds.size(), timeout);
+  pthread_sigmask(SIG_SETMASK, &origmask, nullptr);
+  if (ready == -1 && errno != EINTR)
+    return Status(errno, eErrorTypePOSIX);
+#else
   if (ppoll(read_fds.data(), read_fds.size(),
             ToTimeSpec(loop.GetNextWakeupTime()),
             /*sigmask=*/nullptr) == -1 &&
       errno != EINTR)
     return Status(errno, eErrorTypePOSIX);
+#endif
 
   return Status();
 }
@@ -226,13 +238,13 @@ MainLoopPosix::~MainLoopPosix() {
 #endif
   m_read_fds.erase(m_interrupt_pipe.GetReadFileDescriptor());
   m_interrupt_pipe.Close();
-  assert(m_read_fds.size() == 0); 
+  assert(m_read_fds.size() == 0);
   assert(m_signals.size() == 0);
 }
 
 MainLoopPosix::ReadHandleUP
 MainLoopPosix::RegisterReadObject(const IOObjectSP &object_sp,
-                                 const Callback &callback, Status &error) {
+                                  const Callback &callback, Status &error) {
   if (!object_sp || !object_sp->IsValid()) {
     error = Status::FromErrorString("IO object is not valid.");
     return nullptr;

[DavidSpickett]

I just pushed the clang-format changes for you, rebase to include them.

[DavidSpickett]

You can send a formatting PR then one for actual changes, where the first commit is the formatting. You'll just need to rebase the second one once the first one lands. This will save us some back and forth.

You can use one of those stacked PR manager tools but for <= 2 changes I would just do it manually.

[labath]

Small patch, but a lot of comments:

• with the last round of MainLoop changes, the sigmask changes aren't necessary anymore -- just drop them (this also sort of means we could just use poll everywhere, but I kinda like it because of the increased precision.
• you need to used the timeout field -- required due the last set of changes
• let's make this conditional on HAVE_PPOLL instead of !_AIX
• let's move the code into a helper function

Putting everything together, I think the code should be something like this:

// If nfds_t doesn't exist on AIX, just use size_t
static int Poll(ArrayRef<struct pollfd> fds, std::optional<MainLoopPosix::TimePoint> point) {
#ifdef HAVE_PPOLL
  return ppoll(fds.data(), fds.size(),
            ToTimeSpec(loop.GetNextWakeupTime()),
            /*sigmask=*/nullptr);
#else
  using namespace std::chrono;
  int timeout = -1;
  if (point) {
    nanosecond dur = std::max(*point - steady_clock::now(), nanoseconds(0));
    timeout = ceil<milliseconds>(dur).count();
  }
  return poll(fds.data(), fds.size(), timeout);
#endif
}

Please also make sure all of the MainLoop unit tests pass with this change. Ideally on AIX, but if you still can't get even the unit test binary to build then at least by "simulating" it by undefining the HAVE_PPOLL macro.

[DhruvSrivastavaX]

Okay sure. Thats a great suggestion.
Updated my branch for this one. @DavidSpickett

[DhruvSrivastavaX]

I noticed that HAVE_PPOLL is always defined but only its value will either be 0 or 1, so added accordingly.

[DhruvSrivastavaX]

Putting everything together, I think the code should be something like this:

Thanks for the suggestions. I have modified the implementation.

[labath]

For me personally this would look better as thin syscall wrapper -- a static function with all arguments passed explicitly (since the arguments are modified, it would have to be MutableArrayRef, and not ArrayRef as I originally wrote), rather than a method, but this will work too.

[DhruvSrivastavaX]

For me personally this would look better as thin syscall wrapper -- a static function with all arguments passed explicitly (since the arguments are modified, it would have to be MutableArrayRef, and not ArrayRef as I originally wrote), rather than a method, but this will work too.

Okay sure. That will increase the utility of the API. Done!

[DhruvSrivastavaX]

Request you to merge it as well, if all is good. Thanks!