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

Details:

Adding PlatformAIX plugin for a basic lldb build support. The 1st commit is the original version as in the draft PR which is a PlatformLinux copy. I have removed some of the code in the next commits.
Please let me know all the other changes required to push the PlatformAIX changes and avoid any duplication.

Review Request: @labath @DavidSpickett

@llvmbot
Copy link
Member

llvmbot commented Dec 28, 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

Details:

Adding PlatformAIX plugin for a basic lldb build support. The 1st commit is the original version as in the draft PR which is a PlatformLinux copy. I have removed some of the code in the next commits.
Please let me know all the other changes required to push the PlatformAIX changes and avoid any duplication.

Review Request: @labath @DavidSpickett


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

4 Files Affected:

  • (added) lldb/source/Plugins/Platform/AIX/CMakeLists.txt (+13)
  • (added) lldb/source/Plugins/Platform/AIX/PlatformAIX.cpp (+368)
  • (added) lldb/source/Plugins/Platform/AIX/PlatformAIX.h (+74)
  • (modified) lldb/source/Plugins/Platform/CMakeLists.txt (+1)
diff --git a/lldb/source/Plugins/Platform/AIX/CMakeLists.txt b/lldb/source/Plugins/Platform/AIX/CMakeLists.txt
new file mode 100644
index 00000000000000..85ff0a315eabd5
--- /dev/null
+++ b/lldb/source/Plugins/Platform/AIX/CMakeLists.txt
@@ -0,0 +1,13 @@
+add_definitions("-D_ALL_SOURCE")
+
+add_lldb_library(lldbPluginPlatformAIX PLUGIN
+  PlatformAIX.cpp
+
+   LINK_LIBS
+    lldbBreakpoint
+    lldbCore
+    lldbHost
+    lldbInterpreter
+    lldbTarget
+    lldbPluginPlatformPOSIX
+  )
diff --git a/lldb/source/Plugins/Platform/AIX/PlatformAIX.cpp b/lldb/source/Plugins/Platform/AIX/PlatformAIX.cpp
new file mode 100644
index 00000000000000..36d02be5642293
--- /dev/null
+++ b/lldb/source/Plugins/Platform/AIX/PlatformAIX.cpp
@@ -0,0 +1,368 @@
+//===-- PlatformAIX.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 "PlatformAIX.h"
+#include "lldb/Host/Config.h"
+#include <cstdio>
+#if LLDB_ENABLE_POSIX
+#include <sys/utsname.h>
+#endif
+#include "Utility/ARM64_DWARF_Registers.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/PluginManager.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Symbol/UnwindPlan.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
+#include "lldb/Utility/State.h"
+#include "lldb/Utility/Status.h"
+#include "lldb/Utility/StreamString.h"
+
+// Use defined constants from AIX mman.h for use when targeting remote aix
+// systems even when host has different values.
+
+#if defined(_AIX)
+#include <sys/mman.h>
+#endif
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::platform_aix;
+
+LLDB_PLUGIN_DEFINE(PlatformAIX)
+
+static uint32_t g_initialize_count = 0;
+
+PlatformSP PlatformAIX::CreateInstance(bool force, const ArchSpec *arch) {
+  Log *log = GetLog(LLDBLog::Platform);
+  LLDB_LOG(log, "force = {0}, arch=({1}, {2})", force,
+           arch ? arch->GetArchitectureName() : "<null>",
+           arch ? arch->GetTriple().getTriple() : "<null>");
+
+  bool create = force;
+  if (!create && arch && arch->IsValid()) {
+    const llvm::Triple &triple = arch->GetTriple();
+    switch (triple.getOS()) {
+    case llvm::Triple::AIX:
+      create = true;
+      break;
+
+    default:
+      break;
+    }
+  }
+
+  LLDB_LOG(log, "create = {0}", create);
+  if (create) {
+    return PlatformSP(new PlatformAIX(false));
+  }
+  return PlatformSP();
+}
+
+llvm::StringRef PlatformAIX::GetPluginDescriptionStatic(bool is_host) {
+  if (is_host)
+    return "Local AIX user platform plug-in.";
+  return "Remote AIX user platform plug-in.";
+}
+
+void PlatformAIX::Initialize() {
+  PlatformPOSIX::Initialize();
+
+  if (g_initialize_count++ == 0) {
+    PlatformSP default_platform_sp(new PlatformAIX(true));
+    default_platform_sp->SetSystemArchitecture(HostInfo::GetArchitecture());
+    Platform::SetHostPlatform(default_platform_sp);
+    PluginManager::RegisterPlugin(
+        PlatformAIX::GetPluginNameStatic(false),
+        PlatformAIX::GetPluginDescriptionStatic(false),
+        PlatformAIX::CreateInstance, nullptr);
+  }
+}
+
+void PlatformAIX::Terminate() {
+  if (g_initialize_count > 0) {
+    if (--g_initialize_count == 0) {
+      PluginManager::UnregisterPlugin(PlatformAIX::CreateInstance);
+    }
+  }
+
+  PlatformPOSIX::Terminate();
+}
+
+/// Default Constructor
+PlatformAIX::PlatformAIX(bool is_host)
+    : PlatformPOSIX(is_host) // This is the local host platform
+{
+  if (is_host) {
+    ArchSpec hostArch = HostInfo::GetArchitecture(HostInfo::eArchKindDefault);
+    m_supported_architectures.push_back(hostArch);
+    if (hostArch.GetTriple().isArch64Bit()) {
+      m_supported_architectures.push_back(
+          HostInfo::GetArchitecture(HostInfo::eArchKind32));
+    }
+  } else {
+    m_supported_architectures =
+        CreateArchList({llvm::Triple::ppc64}, llvm::Triple::AIX);
+  }
+}
+
+std::vector<ArchSpec>
+PlatformAIX::GetSupportedArchitectures(const ArchSpec &process_host_arch) {
+  if (m_remote_platform_sp)
+    return m_remote_platform_sp->GetSupportedArchitectures(process_host_arch);
+  return m_supported_architectures;
+}
+
+void PlatformAIX::GetStatus(Stream &strm) {
+  Platform::GetStatus(strm);
+
+#if LLDB_ENABLE_POSIX
+  // Display local kernel information only when we are running in host mode.
+  // Otherwise, we would end up printing non-AIX information (when running on
+  // Mac OS for example).
+  if (IsHost()) {
+    struct utsname un;
+
+    if (uname(&un))
+      return;
+
+    strm.Printf("    Kernel: %s\n", un.sysname);
+    strm.Printf("   Release: %s\n", un.release);
+    strm.Printf("   Version: %s\n", un.version);
+  }
+#endif
+}
+
+uint32_t
+PlatformAIX::GetResumeCountForLaunchInfo(ProcessLaunchInfo &launch_info) {
+  uint32_t resume_count = 0;
+
+  // Always resume past the initial stop when we use eLaunchFlagDebug
+  if (launch_info.GetFlags().Test(eLaunchFlagDebug)) {
+    // Resume past the stop for the final exec into the true inferior.
+    ++resume_count;
+  }
+
+  // If we're not launching a shell, we're done.
+  const FileSpec &shell = launch_info.GetShell();
+  if (!shell)
+    return resume_count;
+
+  std::string shell_string = shell.GetPath();
+  // We're in a shell, so for sure we have to resume past the shell exec.
+  ++resume_count;
+
+  // Figure out what shell we're planning on using.
+  const char *shell_name = strrchr(shell_string.c_str(), '/');
+  if (shell_name == nullptr)
+    shell_name = shell_string.c_str();
+  else
+    shell_name++;
+
+  if (strcmp(shell_name, "csh") == 0 || strcmp(shell_name, "tcsh") == 0 ||
+      strcmp(shell_name, "zsh") == 0 || strcmp(shell_name, "sh") == 0) {
+    // These shells seem to re-exec themselves.  Add another resume.
+    ++resume_count;
+  }
+
+  return resume_count;
+}
+
+bool PlatformAIX::CanDebugProcess() {
+  if (IsHost()) {
+    return true;
+  } else {
+    // If we're connected, we can debug.
+    return IsConnected();
+  }
+}
+
+void PlatformAIX::CalculateTrapHandlerSymbolNames() {
+  m_trap_handlers.push_back(ConstString("_sigtramp"));
+  m_trap_handlers.push_back(ConstString("__kernel_rt_sigreturn"));
+  m_trap_handlers.push_back(ConstString("__restore_rt"));
+}
+
+lldb::UnwindPlanSP
+PlatformAIX::GetTrapHandlerUnwindPlan(const llvm::Triple &triple,
+                                      ConstString name) {
+  return {};
+}
+
+MmapArgList PlatformAIX::GetMmapArgumentList(const ArchSpec &arch, addr_t addr,
+                                             addr_t length, unsigned prot,
+                                             unsigned flags, addr_t fd,
+                                             addr_t offset) {
+#if defined(_AIX)
+  unsigned flags_platform = MAP_VARIABLE | MAP_PRIVATE | MAP_ANONYMOUS;
+#else
+  unsigned flags_platform = 0;
+#endif
+  MmapArgList args({addr, length, prot, flags_platform, fd, offset});
+  return args;
+}
+
+CompilerType PlatformAIX::GetSiginfoType(const llvm::Triple &triple) {
+  if (!m_type_system_up)
+    m_type_system_up.reset(new TypeSystemClang("siginfo", triple));
+  TypeSystemClang *ast = m_type_system_up.get();
+
+  bool si_errno_then_code = true;
+
+  switch (triple.getArch()) {
+  case llvm::Triple::mips:
+  case llvm::Triple::mipsel:
+  case llvm::Triple::mips64:
+  case llvm::Triple::mips64el:
+    // mips has si_code and si_errno swapped
+    si_errno_then_code = false;
+    break;
+  default:
+    break;
+  }
+
+  // generic types
+  CompilerType int_type = ast->GetBasicType(eBasicTypeInt);
+  CompilerType uint_type = ast->GetBasicType(eBasicTypeUnsignedInt);
+  CompilerType short_type = ast->GetBasicType(eBasicTypeShort);
+  CompilerType long_type = ast->GetBasicType(eBasicTypeLong);
+  CompilerType voidp_type = ast->GetBasicType(eBasicTypeVoid).GetPointerType();
+
+  // platform-specific types
+  CompilerType &pid_type = int_type;
+  CompilerType &uid_type = uint_type;
+  CompilerType &clock_type = long_type;
+  CompilerType &band_type = long_type;
+
+  CompilerType sigval_type = ast->CreateRecordType(
+      nullptr, OptionalClangModuleID(), lldb::eAccessPublic, "__lldb_sigval_t",
+      llvm::to_underlying(clang::TagTypeKind::Union), lldb::eLanguageTypeC);
+  ast->StartTagDeclarationDefinition(sigval_type);
+  ast->AddFieldToRecordType(sigval_type, "sival_int", int_type,
+                            lldb::eAccessPublic, 0);
+  ast->AddFieldToRecordType(sigval_type, "sival_ptr", voidp_type,
+                            lldb::eAccessPublic, 0);
+  ast->CompleteTagDeclarationDefinition(sigval_type);
+
+  CompilerType sigfault_bounds_type = ast->CreateRecordType(
+      nullptr, OptionalClangModuleID(), lldb::eAccessPublic, "",
+      llvm::to_underlying(clang::TagTypeKind::Union), lldb::eLanguageTypeC);
+  ast->StartTagDeclarationDefinition(sigfault_bounds_type);
+  ast->AddFieldToRecordType(
+      sigfault_bounds_type, "_addr_bnd",
+      ast->CreateStructForIdentifier(ConstString(),
+                                     {
+                                         {"_lower", voidp_type},
+                                         {"_upper", voidp_type},
+                                     }),
+      lldb::eAccessPublic, 0);
+  ast->AddFieldToRecordType(sigfault_bounds_type, "_pkey", uint_type,
+                            lldb::eAccessPublic, 0);
+  ast->CompleteTagDeclarationDefinition(sigfault_bounds_type);
+
+  // siginfo_t
+  CompilerType siginfo_type = ast->CreateRecordType(
+      nullptr, OptionalClangModuleID(), lldb::eAccessPublic, "__lldb_siginfo_t",
+      llvm::to_underlying(clang::TagTypeKind::Struct), lldb::eLanguageTypeC);
+  ast->StartTagDeclarationDefinition(siginfo_type);
+  ast->AddFieldToRecordType(siginfo_type, "si_signo", int_type,
+                            lldb::eAccessPublic, 0);
+
+  if (si_errno_then_code) {
+    ast->AddFieldToRecordType(siginfo_type, "si_errno", int_type,
+                              lldb::eAccessPublic, 0);
+    ast->AddFieldToRecordType(siginfo_type, "si_code", int_type,
+                              lldb::eAccessPublic, 0);
+  } else {
+    ast->AddFieldToRecordType(siginfo_type, "si_code", int_type,
+                              lldb::eAccessPublic, 0);
+    ast->AddFieldToRecordType(siginfo_type, "si_errno", int_type,
+                              lldb::eAccessPublic, 0);
+  }
+
+  // the structure is padded on 64-bit arches to fix alignment
+  if (triple.isArch64Bit())
+    ast->AddFieldToRecordType(siginfo_type, "__pad0", int_type,
+                              lldb::eAccessPublic, 0);
+
+  // union used to hold the signal data
+  CompilerType union_type = ast->CreateRecordType(
+      nullptr, OptionalClangModuleID(), lldb::eAccessPublic, "",
+      llvm::to_underlying(clang::TagTypeKind::Union), lldb::eLanguageTypeC);
+  ast->StartTagDeclarationDefinition(union_type);
+
+  ast->AddFieldToRecordType(
+      union_type, "_kill",
+      ast->CreateStructForIdentifier(ConstString(),
+                                     {
+                                         {"si_pid", pid_type},
+                                         {"si_uid", uid_type},
+                                     }),
+      lldb::eAccessPublic, 0);
+
+  ast->AddFieldToRecordType(
+      union_type, "_timer",
+      ast->CreateStructForIdentifier(ConstString(),
+                                     {
+                                         {"si_tid", int_type},
+                                         {"si_overrun", int_type},
+                                         {"si_sigval", sigval_type},
+                                     }),
+      lldb::eAccessPublic, 0);
+
+  ast->AddFieldToRecordType(
+      union_type, "_rt",
+      ast->CreateStructForIdentifier(ConstString(),
+                                     {
+                                         {"si_pid", pid_type},
+                                         {"si_uid", uid_type},
+                                         {"si_sigval", sigval_type},
+                                     }),
+      lldb::eAccessPublic, 0);
+
+  ast->AddFieldToRecordType(
+      union_type, "_sigchld",
+      ast->CreateStructForIdentifier(ConstString(),
+                                     {
+                                         {"si_pid", pid_type},
+                                         {"si_uid", uid_type},
+                                         {"si_status", int_type},
+                                         {"si_utime", clock_type},
+                                         {"si_stime", clock_type},
+                                     }),
+      lldb::eAccessPublic, 0);
+
+  ast->AddFieldToRecordType(
+      union_type, "_sigfault",
+      ast->CreateStructForIdentifier(ConstString(),
+                                     {
+                                         {"si_addr", voidp_type},
+                                         {"si_addr_lsb", short_type},
+                                         {"_bounds", sigfault_bounds_type},
+                                     }),
+      lldb::eAccessPublic, 0);
+
+  ast->AddFieldToRecordType(
+      union_type, "_sigpoll",
+      ast->CreateStructForIdentifier(ConstString(),
+                                     {
+                                         {"si_band", band_type},
+                                         {"si_fd", int_type},
+                                     }),
+      lldb::eAccessPublic, 0);
+
+  ast->CompleteTagDeclarationDefinition(union_type);
+  ast->AddFieldToRecordType(siginfo_type, "_sifields", union_type,
+                            lldb::eAccessPublic, 0);
+
+  ast->CompleteTagDeclarationDefinition(siginfo_type);
+  return siginfo_type;
+}
diff --git a/lldb/source/Plugins/Platform/AIX/PlatformAIX.h b/lldb/source/Plugins/Platform/AIX/PlatformAIX.h
new file mode 100644
index 00000000000000..fe183e6bce0e47
--- /dev/null
+++ b/lldb/source/Plugins/Platform/AIX/PlatformAIX.h
@@ -0,0 +1,74 @@
+//===-- PlatformAIX.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 LLDB_SOURCE_PLUGINS_PLATFORM_AIX_PLATFORMAIX_H
+#define LLDB_SOURCE_PLUGINS_PLATFORM_AIX_PLATFORMAIX_H
+
+#include "Plugins/Platform/POSIX/PlatformPOSIX.h"
+#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
+
+namespace lldb_private {
+namespace platform_aix {
+
+class PlatformAIX : public PlatformPOSIX {
+public:
+  PlatformAIX(bool is_host);
+
+  static void Initialize();
+
+  static void Terminate();
+
+  // lldb_private::PluginInterface functions
+  static lldb::PlatformSP CreateInstance(bool force, const ArchSpec *arch);
+
+  static llvm::StringRef GetPluginNameStatic(bool is_host) {
+    return is_host ? Platform::GetHostPlatformName() : "remote-AIX";
+  }
+
+  static llvm::StringRef GetPluginDescriptionStatic(bool is_host);
+
+  llvm::StringRef GetPluginName() override {
+    return GetPluginNameStatic(IsHost());
+  }
+
+  // lldb_private::Platform functions
+  llvm::StringRef GetDescription() override {
+    return GetPluginDescriptionStatic(IsHost());
+  }
+
+  void GetStatus(Stream &strm) override;
+
+  std::vector<ArchSpec>
+  GetSupportedArchitectures(const ArchSpec &process_host_arch) override;
+
+  uint32_t GetResumeCountForLaunchInfo(ProcessLaunchInfo &launch_info) override;
+
+  bool CanDebugProcess() override;
+
+  void CalculateTrapHandlerSymbolNames() override;
+
+  lldb::UnwindPlanSP GetTrapHandlerUnwindPlan(const llvm::Triple &triple,
+                                              ConstString name) override;
+
+  MmapArgList GetMmapArgumentList(const ArchSpec &arch, lldb::addr_t addr,
+                                  lldb::addr_t length, unsigned prot,
+                                  unsigned flags, lldb::addr_t fd,
+                                  lldb::addr_t offset) override;
+
+  CompilerType GetSiginfoType(const llvm::Triple &triple) override;
+
+  std::vector<ArchSpec> m_supported_architectures;
+
+private:
+  std::unique_ptr<TypeSystemClang> m_type_system_up;
+};
+
+} // namespace platform_aix
+} // namespace lldb_private
+
+#endif // LLDB_SOURCE_PLUGINS_PLATFORM_AIX_PLATFORMAIX_H
diff --git a/lldb/source/Plugins/Platform/CMakeLists.txt b/lldb/source/Plugins/Platform/CMakeLists.txt
index 6869587f917eba..9d0afd97cff853 100644
--- a/lldb/source/Plugins/Platform/CMakeLists.txt
+++ b/lldb/source/Plugins/Platform/CMakeLists.txt
@@ -8,3 +8,4 @@ add_subdirectory(OpenBSD)
 add_subdirectory(POSIX)
 add_subdirectory(QemuUser)
 add_subdirectory(Windows)
+add_subdirectory(AIX)

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'm not clear what you're asking for feedback on here.

Starting with a copy of the Linux platform, I understand that. However the final changes don't look that different so I assume you have:

  • Copied the Linux platform.
  • Updated it until it builds on AIX.
  • And now want some feedback for the next part?

If so, I think the first thing to do is work out if any of these Linux constructs apply to AIX at all. For example does uname exist and work? Does the siginfo type match up (certainly the MIPS code is not needed for AIX)?

Then we'll be down to what you actually want to keep for AIX, and from there we can think about whether it's worth sharing any of it.

My guess is that a lot of this won't apply to AIX or be different enough to warrant it being its own copy of the code. For example we already have different versions of GetSigInfoType for NetBSD/FreeBSD.

@DhruvSrivastavaX
Copy link
Member Author

Yes, you are right. I should have been more clear in my query.

So as a first step, Removing all other Archs except ppc64::AIX, removing GetAArch64TrapHanlderUnwindPlan and consequently reducing GetTrapHandlerUnwindPlan seemed to be the obvious changes. Hope thats okay.

Other than that, I just needed to get some clarity on GetSiginfoType, for which I have some idea now. Will implement for AIX and update before further discussions.

For example does uname exist and work?

Yes it is supported in AIX.

@DhruvSrivastavaX
Copy link
Member Author

Hi @DavidSpickett @labath , Do let me know if this can be merged now

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'm assuming you're at least building this on a non-AIX system.

Depending on how much of LLDB actually works, you can follow https://lldb.llvm.org/use/remote.html to see if remote debugging works. Requires a working lldb-server though.

@@ -0,0 +1,13 @@
add_definitions("-D_ALL_SOURCE")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remind me please, what symbol is this needed for?

Copy link
Member Author

Choose a reason for hiding this comment

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

For this plugin, it is there for MAP_ANONYMOUS

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay, that makes sense. I'd just suggest using the slightly newer form:

target_compile_definitions(lldbPluginPlatformAIX PRIVATE "-D_ALL_SOURCE")

Copy link
Member Author

Choose a reason for hiding this comment

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

Done, Thanks!

@DhruvSrivastavaX
Copy link
Member Author

DhruvSrivastavaX commented Feb 21, 2025

I'm assuming you're at least building this on a non-AIX system.

Depending on how much of LLDB actually works, you can follow https://lldb.llvm.org/use/remote.html to see if remote debugging works. Requires a working lldb-server though.

Yes, once these base changes are merged, lldb-server changes are next in line (Already have a PR on hold for that).
And yes, I am building these on a Linux system as well before pushing.
We can add modifications needed for remote debugging as well once we have a base.
I will check that out separately with our complete changes though, for the time being.

@@ -0,0 +1,13 @@
add_definitions("-D_ALL_SOURCE")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay, that makes sense. I'd just suggest using the slightly newer form:

target_compile_definitions(lldbPluginPlatformAIX PRIVATE "-D_ALL_SOURCE")

Comment on lines 145 to 148
unsigned flags_platform = 0;
#if defined(_AIX)
flags_platform = MapPrivate | MapVariable | MapAnon;
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this actually be translating the lldb generic flags into the platform-specific ones, like e.g. the linux code does:

  if (flags & eMmapFlagsPrivate)
    flags_platform |= MapPrivate;
  if (flags & eMmapFlagsAnon)
    flags_platform |= MapAnon;

Copy link
Member Author

Choose a reason for hiding this comment

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

Ohh, yes I will do that, but should I still keep the _AIX flag or not? based on the remote debugging conversation?

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, drop the ifdef. Thanks.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hope I have covered everything and its okay now?

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.

Looks good. Thanks.

@DhruvSrivastavaX DhruvSrivastavaX merged commit a5f759e into llvm:main Feb 21, 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