Skip to content

Conversation

@DhruvSrivastavaX
Copy link
Member

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

Added base files for NativeProcess Support for AIX.
Will be adding further support in consequent incremental PR.

Review Request: @labath @DavidSpickett

@llvmbot
Copy link
Member

llvmbot commented Nov 30, 2024

@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

Added base files for NativeProcess Support for AIX.
Will be adding further support in consequent incremental PR.

Review Request: @labath @DavidSpickett


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

4 Files Affected:

  • (added) lldb/source/Plugins/Process/AIX/CMakeLists.txt (+14)
  • (added) lldb/source/Plugins/Process/AIX/NativeProcessAIX.cpp (+257)
  • (added) lldb/source/Plugins/Process/AIX/NativeProcessAIX.h (+134)
  • (modified) lldb/source/Plugins/Process/CMakeLists.txt (+3)
diff --git a/lldb/source/Plugins/Process/AIX/CMakeLists.txt b/lldb/source/Plugins/Process/AIX/CMakeLists.txt
new file mode 100644
index 00000000000000..4fabbe93022870
--- /dev/null
+++ b/lldb/source/Plugins/Process/AIX/CMakeLists.txt
@@ -0,0 +1,14 @@
+add_lldb_library(lldbPluginProcessAIX
+  NativeProcessAIX.cpp
+
+  LINK_LIBS
+    lldbCore
+    lldbHost
+    lldbSymbol
+    lldbTarget
+    lldbUtility
+    lldbPluginProcessPOSIX
+    lldbPluginProcessUtility
+  LINK_COMPONENTS
+    Support
+  )
diff --git a/lldb/source/Plugins/Process/AIX/NativeProcessAIX.cpp b/lldb/source/Plugins/Process/AIX/NativeProcessAIX.cpp
new file mode 100644
index 00000000000000..a8d54483239ad2
--- /dev/null
+++ b/lldb/source/Plugins/Process/AIX/NativeProcessAIX.cpp
@@ -0,0 +1,257 @@
+//===-- NativeProcessAIX.cpp --------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "NativeProcessAIX.h"
+#include <cerrno>
+#include <cstdint>
+#include <cstring>
+#include <unistd.h>
+#include <fstream>
+#include <mutex>
+#include <optional>
+#include <sstream>
+#include <string>
+#include <unordered_map>
+#include "NativeThreadAIX.h"
+#include "Plugins/Process/POSIX/ProcessPOSIXLog.h"
+#include "lldb/Host/Host.h"
+#include "lldb/Host/HostProcess.h"
+#include "lldb/Host/ProcessLaunchInfo.h"
+#include "lldb/Symbol/ObjectFile.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Utility/LLDBAssert.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/State.h"
+#include "lldb/Utility/Status.h"
+#include "lldb/Utility/StringExtractor.h"
+#include "llvm/Support/Errno.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/FileSystem.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::process_aix;
+using namespace llvm;
+
+static constexpr unsigned k_ptrace_word_size = sizeof(void *);
+static_assert(sizeof(long) >= k_ptrace_word_size,
+              "Size of long must be larger than ptrace word size");
+
+// Simple helper function to ensure flags are enabled on the given file
+// descriptor.
+static Status EnsureFDFlags(int fd, int flags) {
+  Status error;
+
+  int status = fcntl(fd, F_GETFL);
+  if (status == -1) {
+    error = Status::FromErrno();
+    return error;
+  }
+
+  if (fcntl(fd, F_SETFL, status | flags) == -1) {
+    error = Status::FromErrno();
+    return error;
+  }
+
+  return error;
+}
+
+NativeProcessAIX::Manager::Manager(MainLoop &mainloop)
+    : NativeProcessProtocol::Manager(mainloop) {
+  Status status;
+  m_sigchld_handle = mainloop.RegisterSignal(
+      SIGCHLD, [this](MainLoopBase &) { SigchldHandler(); }, status);
+  assert(m_sigchld_handle && status.Success());
+}
+
+// Public Static Methods
+
+llvm::Expected<std::unique_ptr<NativeProcessProtocol>>
+NativeProcessAIX::Manager::Launch(ProcessLaunchInfo &launch_info,
+                                    NativeDelegate &native_delegate) {
+  Log *log = GetLog(POSIXLog::Process);
+
+  Status status;
+  ::pid_t pid = ProcessLauncherPosixFork()
+                    .LaunchProcess(launch_info, status)
+                    .GetProcessId();
+  LLDB_LOG(log, "pid = {0:x}", pid);
+  if (status.Fail()) {
+    LLDB_LOG(log, "failed to launch process: {0}", status);
+    return status.ToError();
+  }
+
+  // Wait for the child process to trap on its call to execve.
+  int wstatus = 0;
+  ::pid_t wpid = llvm::sys::RetryAfterSignal(-1, ::waitpid, pid, &wstatus, 0);
+  assert(wpid == pid);
+  UNUSED_IF_ASSERT_DISABLED(wpid);
+  if (!WIFSTOPPED(wstatus)) {
+    LLDB_LOG(log, "Could not sync with inferior process: wstatus={1}",
+             WaitStatus::Decode(wstatus));
+    return llvm::make_error<StringError>("Could not sync with inferior process",
+                                         llvm::inconvertibleErrorCode());
+  }
+  LLDB_LOG(log, "inferior started, now in stopped state");
+
+  ProcessInstanceInfo Info;
+  if (!Host::GetProcessInfo(pid, Info)) {
+      return llvm::make_error<StringError>("Cannot get process architectrue",
+                                            llvm::inconvertibleErrorCode());
+  } 
+
+ // Set the architecture to the exe architecture.
+  LLDB_LOG(log, "pid = {0}, detected architecture {1}", pid, 
+          Info.GetArchitecture().GetArchitectureName());
+
+  return std::unique_ptr<NativeProcessAIX>(new NativeProcessAIX(
+      pid, launch_info.GetPTY().ReleasePrimaryFileDescriptor(), native_delegate,
+      Info.GetArchitecture(), *this, {pid}));
+}
+
+llvm::Expected<std::unique_ptr<NativeProcessProtocol>>
+NativeProcessAIX::Manager::Attach(
+    lldb::pid_t pid, NativeProcessProtocol::NativeDelegate &native_delegate) { 
+  Log *log = GetLog(POSIXLog::Process);
+  LLDB_LOG(log, "pid = {0:x}", pid);
+
+  ProcessInstanceInfo Info;
+  if (!Host::GetProcessInfo(pid, Info)) {
+      return llvm::make_error<StringError>("Cannot get process architectrue",
+                                            llvm::inconvertibleErrorCode());
+  } 
+  auto tids_or = NativeProcessAIX::Attach(pid);
+  if (!tids_or)
+    return tids_or.takeError();
+
+  return std::unique_ptr<NativeProcessAIX>(
+      new NativeProcessAIX(pid, -1, native_delegate, Info.GetArchitecture(), *this, *tids_or));
+}
+
+NativeProcessAIX::Extension
+NativeProcessAIX::Manager::GetSupportedExtensions() const {
+  NativeProcessAIX::Extension supported =
+      Extension::multiprocess | Extension::fork | Extension::vfork |
+      Extension::pass_signals | Extension::auxv | Extension::libraries_svr4 |
+      Extension::siginfo_read;
+
+  return supported;
+}
+
+void NativeProcessAIX::Manager::SigchldHandler() {
+}
+
+void NativeProcessAIX::Manager::CollectThread(::pid_t tid) {
+}
+
+// Public Instance Methods
+
+NativeProcessAIX::NativeProcessAIX(::pid_t pid, int terminal_fd,
+                                       NativeDelegate &delegate,
+                                       const ArchSpec &arch, Manager &manager,
+                                       llvm::ArrayRef<::pid_t> tids)
+    : NativeProcessProtocol(pid, terminal_fd, delegate), m_manager(manager),
+      m_arch(arch) {
+  manager.AddProcess(*this);
+  if (m_terminal_fd != -1) {
+    Status status = EnsureFDFlags(m_terminal_fd, O_NONBLOCK);
+    assert(status.Success());
+  }
+
+  // Let our process instance know the thread has stopped.
+  SetCurrentThreadID(tids[0]);
+  SetState(StateType::eStateStopped, false);
+}
+
+llvm::Expected<std::vector<::pid_t>> NativeProcessAIX::Attach(::pid_t pid) {
+
+  Status status;
+  if ((status = PtraceWrapper(PT_ATTACH, pid)).Fail()) {
+    return status.ToError();
+  }
+
+  std::vector<::pid_t> tids;
+  tids.push_back(pid);
+  return std::move(tids);
+}
+
+void NativeProcessAIX::MonitorSIGTRAP(const WaitStatus status,
+                                        NativeThreadAIX &thread) {
+}
+
+void NativeProcessAIX::MonitorBreakpoint(NativeThreadAIX &thread) {
+}
+
+bool NativeProcessAIX::SupportHardwareSingleStepping() const {
+  return false;
+}
+
+Status NativeProcessAIX::Resume(const ResumeActionList &resume_actions) {
+  return Status();
+}
+
+Status NativeProcessAIX::Halt() {
+  Status error;
+  return error;
+}
+
+Status NativeProcessAIX::Detach() {
+  Status error;
+  return error;
+}
+
+Status NativeProcessAIX::Signal(int signo) {
+  Status error;
+  return error;
+}
+
+Status NativeProcessAIX::Interrupt() {
+  return Status();
+}
+
+Status NativeProcessAIX::Kill() {
+  Status error;
+  return error;
+}
+
+Status NativeProcessAIX::SetBreakpoint(lldb::addr_t addr, uint32_t size,
+                                         bool hardware) {
+  if (hardware)
+    return SetHardwareBreakpoint(addr, size);
+  else
+    return SetSoftwareBreakpoint(addr, size);
+}
+
+Status NativeProcessAIX::RemoveBreakpoint(lldb::addr_t addr, bool hardware) {
+  if (hardware)
+    return RemoveHardwareBreakpoint(addr);
+  else
+    return NativeProcessProtocol::RemoveBreakpoint(addr);
+}
+
+int8_t NativeProcessAIX::GetSignalInfo(WaitStatus wstatus) const {
+  return wstatus.status;
+}
+
+Status NativeProcessAIX::Detach(lldb::tid_t tid) {
+  if (tid == LLDB_INVALID_THREAD_ID)
+    return Status();
+
+  return PtraceWrapper(PT_DETACH, tid);
+}
+
+// Wrapper for ptrace to catch errors and log calls. Note that ptrace sets
+// errno on error because -1 can be a valid result (i.e. for PTRACE_PEEK*)
+Status NativeProcessAIX::PtraceWrapper(int req, lldb::pid_t pid, void *addr,
+                                         void *data, size_t data_size,
+                                         long *result) {
+  Status error;
+  return error;
+}
+
diff --git a/lldb/source/Plugins/Process/AIX/NativeProcessAIX.h b/lldb/source/Plugins/Process/AIX/NativeProcessAIX.h
new file mode 100644
index 00000000000000..2c07ba420d3fee
--- /dev/null
+++ b/lldb/source/Plugins/Process/AIX/NativeProcessAIX.h
@@ -0,0 +1,134 @@
+//===-- NativeProcessAIX.h ---------------------------------- -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef liblldb_NativeProcessAIX_H_
+#define liblldb_NativeProcessAIX_H_
+
+#include <csignal>
+#include <unordered_set>
+
+#include "lldb/Host/Debug.h"
+#include "lldb/Host/HostThread.h"
+#include "lldb/Target/MemoryRegionInfo.h"
+#include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/FileSpec.h"
+#include "lldb/lldb-types.h"
+#include "llvm/ADT/SmallPtrSet.h"
+#include "lldb/Host/aix/Support.h"
+
+#include "NativeThreadAIX.h"
+#include "lldb/Host/common/NativeProcessProtocol.h"
+#include "Plugins/Process/Utility/NativeProcessSoftwareSingleStep.h"
+
+namespace lldb_private {
+class Status;
+class Scalar;
+
+namespace process_aix {
+/// \class NativeProcessAIX
+/// Manages communication with the inferior (debugee) process.
+///
+/// Upon construction, this class prepares and launches an inferior process
+/// for debugging.
+///
+/// Changes in the inferior process state are broadcasted.
+class NativeProcessAIX : public NativeProcessProtocol,
+                           private NativeProcessSoftwareSingleStep {
+public:
+  class Manager : public NativeProcessProtocol::Manager {
+  public:
+    Manager(MainLoop &mainloop);
+
+    llvm::Expected<std::unique_ptr<NativeProcessProtocol>>
+    Launch(ProcessLaunchInfo &launch_info,
+            NativeDelegate &native_delegate) override;
+
+    llvm::Expected<std::unique_ptr<NativeProcessProtocol>>
+    Attach(lldb::pid_t pid, NativeDelegate &native_delegate) override;
+
+    Extension GetSupportedExtensions() const override;
+
+    void AddProcess(NativeProcessAIX &process) {
+      m_processes.insert(&process);
+    }
+
+    void RemoveProcess(NativeProcessAIX &process) {
+      m_processes.erase(&process);
+    }
+
+    // Collect an event for the given tid, waiting for it if necessary.
+    void CollectThread(::pid_t tid);
+
+  private:
+    MainLoop::SignalHandleUP m_sigchld_handle;
+
+    llvm::SmallPtrSet<NativeProcessAIX *, 2> m_processes;
+
+    void SigchldHandler();
+  };
+
+  // NativeProcessProtocol Interface
+
+  ~NativeProcessAIX() override { m_manager.RemoveProcess(*this); }
+
+  Status Resume(const ResumeActionList &resume_actions) override;
+
+  Status Halt() override;
+
+  Status Detach() override;
+
+  Status Signal(int signo) override;
+
+  Status Interrupt() override;
+
+  Status Kill() override;
+
+  const ArchSpec &GetArchitecture() const override { return m_arch; }
+
+  Status SetBreakpoint(lldb::addr_t addr, uint32_t size,
+                       bool hardware) override;
+
+  Status RemoveBreakpoint(lldb::addr_t addr, bool hardware = false) override;
+
+  Status GetFileLoadAddress(const llvm::StringRef &file_name,
+                            lldb::addr_t &load_addr) override;
+
+  static Status PtraceWrapper(int req, lldb::pid_t pid, void *addr = nullptr,
+                              void *data = nullptr, size_t data_size = 0,
+                              long *result = nullptr);
+
+// Wrapper for ptrace to catch errors and log calls. Note that ptrace sets
+
+private:
+  Manager &m_manager;
+  /*MainLoop::SignalHandleUP m_sigchld_handle;*/
+  ArchSpec m_arch;
+  /*MainLoop& m_main_loop;*/
+
+  // Private Instance Methods
+  NativeProcessAIX(::pid_t pid, int terminal_fd, NativeDelegate &delegate,
+                     const ArchSpec &arch, Manager &manager,
+                     llvm::ArrayRef<::pid_t> tids);
+
+  // Returns a list of process threads that we have attached to.
+  static llvm::Expected<std::vector<::pid_t>> Attach(::pid_t pid);
+
+  void MonitorSIGTRAP(const WaitStatus status, NativeThreadAIX &thread);
+
+  void MonitorBreakpoint(NativeThreadAIX &thread);
+
+  Status Detach(lldb::tid_t tid);
+
+  void SigchldHandler();
+
+};
+
+} // namespace process_aix
+} // namespace lldb_private
+
+#endif // #ifndef liblldb_NativeProcessAIX_H_
diff --git a/lldb/source/Plugins/Process/CMakeLists.txt b/lldb/source/Plugins/Process/CMakeLists.txt
index a51d0f7afd1759..01bb5f462eba44 100644
--- a/lldb/source/Plugins/Process/CMakeLists.txt
+++ b/lldb/source/Plugins/Process/CMakeLists.txt
@@ -7,6 +7,9 @@ elseif (CMAKE_SYSTEM_NAME MATCHES "FreeBSD")
 elseif (CMAKE_SYSTEM_NAME MATCHES "NetBSD")
   add_subdirectory(NetBSD)
   add_subdirectory(POSIX)
+elseif (CMAKE_SYSTEM_NAME MATCHES "AIX")
+  add_subdirectory(AIX)
+  add_subdirectory(POSIX)
 elseif (CMAKE_SYSTEM_NAME MATCHES "Windows")
   add_subdirectory(Windows/Common)
 elseif (CMAKE_SYSTEM_NAME MATCHES "Darwin")

@github-actions
Copy link

github-actions bot commented Nov 30, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Collaborator

@labath labath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can make some progress here, but I don't think we'll get far without ProcessLauncher support. At some point, not too far from now, I'm going to start asking questions like "which tests does this fix", and I don't think you'll be able to run any of them without a process launcher.

@DhruvSrivastavaX
Copy link
Member Author

DhruvSrivastavaX commented Dec 9, 2024

I think we can make some progress here, but I don't think we'll get far without ProcessLauncher support. At some point, not too far from now, I'm going to start asking questions like "which tests does this fix", and I don't think you'll be able to run any of them without a process launcher.

Yes, Thats true. I am just adding this as a part of the parallel drops for each of the independent plugins. Will drop ProcessLauncher support too as a separate patch soon. Hope that is okay for now?
But yes, It would be great to get inputs from you regarding adding the new tests on which I can build on as I proceed.

@DhruvSrivastavaX
Copy link
Member Author

Addressed your comments in this commit to keep track of the changes.

@labath
Copy link
Collaborator

labath commented Dec 13, 2024

But yes, It would be great to get inputs from you regarding adding the new tests on which I can build on as I proceed.

For this code, I don't think you need to build new tests. We just need to figure out what it takes to get the existing ones (test/API/tools/lldb-server/) running.

Copy link
Collaborator

@DavidSpickett DavidSpickett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if you have worked on LLVM before but regardless, thank you for making use of the nice error handling patterns we have. Even if sometimes they're a bit too much.

I don't have any major problems with this, just some style points and a couple of things that need better explanations.

@DhruvSrivastavaX
Copy link
Member Author

Okay, let me then focus on other areas till we are actually able to build this PR. Will come back to this once we have some base build-ability on AIX.

@DhruvSrivastavaX
Copy link
Member Author

I have builT the lldb-server on AIX and added the changes accordingly, so that we can restart the review on this.
I will be handling the pending comments on this PR next.

Copy link
Collaborator

@labath labath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I started reviewing this, but I got a feeling that you did not actually push the changes that you made.

Does the PR accurately reflect the latest version of the patch?

@DhruvSrivastavaX
Copy link
Member Author

I started reviewing this, but I got a feeling that you did not actually push the changes that you made.

Does the PR accurately reflect the latest version of the patch?

Hi @labath , I am in the process of making the changes. will push them together and post a comment once I am done

@DhruvSrivastavaX
Copy link
Member Author

DhruvSrivastavaX commented Mar 11, 2025

I don't know if you have worked on LLVM before but regardless, thank you for making use of the nice error handling patterns we have. Even if sometimes they're a bit too much.

I don't have any major problems with this, just some style points and a couple of things that need better explanations.

Thanks @DavidSpickett , Although I am comparatively new to llvm source code, but I have been using the ibm version of clang compiler for quite some time and I've always wanted to get into the code as I am able to do now. 🙂
but as for the error handling, I can see that they are pretty effective, so its good if I can follow it well.

@DhruvSrivastavaX
Copy link
Member Author

@labath @DavidSpickett
I am done with the update of the files, I have compiled and checked the changes based on your mentioned comments.
The only doubt I have is am I required to replace Status with llvm::Error/Expected at any other places too?
Please feel free go through the changes now. Thanks for the insightful review on this component!

Copy link
Collaborator

@DavidSpickett DavidSpickett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Been a bit since I looked at this series, so I think "base files" means enough to get a build and you'll come back and implement the specifics later.

In that context, mostly small comments other than, I think some of these methods should return failure or error values to prevent you forgetting to update them later.

A string error "Implement me!" is fine for the short term.

Copy link
Collaborator

@labath labath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with the included changes.

@DhruvSrivastavaX
Copy link
Member Author

LGTM with the included changes.

Thanks! I have updated. I will merge once the checks complete, hope thats okay?

@DhruvSrivastavaX DhruvSrivastavaX merged commit ec95ce3 into llvm:main Mar 14, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants