[lldb][AIX] Taking Linux Host Info header's base for AIX #106910
Conversation
|
@llvm/pr-subscribers-lldb Author: Dhruv Srivastava (DhruvSrivastavaX) ChangesThis PR is in reference to porting LLDB on AIX. Link to discussions on llvm discourse and github: https://discourse.llvm.org/t/port-lldb-to-ibm-aix/80640 Taking Base code for Host Info header files from Linux, and setting up header files for AIX Host Info.
Full diff: https://github.com/llvm/llvm-project/pull/106910.diff 10 Files Affected:
diff --git a/lldb/CMakeLists.txt b/lldb/CMakeLists.txt
index 59cdc4593463c1..f78b7619695c21 100644
--- a/lldb/CMakeLists.txt
+++ b/lldb/CMakeLists.txt
@@ -38,6 +38,12 @@ endif()
include(LLDBConfig)
include(AddLLDB)
+# This has been added to keep the AIX build isolated for now.
+# It will need to be modified later.
+if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX")
+ add_definitions("-D__AIX__")
+endif()
+
# Define the LLDB_CONFIGURATION_xxx matching the build type.
if(uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG" )
add_definitions(-DLLDB_CONFIGURATION_DEBUG)
diff --git a/lldb/include/lldb/Host/HostGetOpt.h b/lldb/include/lldb/Host/HostGetOpt.h
index 52cfdf4dbb89c2..f450e561d6afb1 100644
--- a/lldb/include/lldb/Host/HostGetOpt.h
+++ b/lldb/include/lldb/Host/HostGetOpt.h
@@ -9,7 +9,7 @@
#ifndef LLDB_HOST_HOSTGETOPT_H
#define LLDB_HOST_HOSTGETOPT_H
-#if !defined(_MSC_VER) && !defined(__NetBSD__)
+#if !defined(_MSC_VER) && !defined(__NetBSD__) && !defined(__AIX__)
#include <getopt.h>
#include <unistd.h>
diff --git a/lldb/include/lldb/Host/HostInfo.h b/lldb/include/lldb/Host/HostInfo.h
index b7010d69d88e7f..156df8cf6901df 100644
--- a/lldb/include/lldb/Host/HostInfo.h
+++ b/lldb/include/lldb/Host/HostInfo.h
@@ -55,6 +55,9 @@
#elif defined(__APPLE__)
#include "lldb/Host/macosx/HostInfoMacOSX.h"
#define HOST_INFO_TYPE HostInfoMacOSX
+#elif defined(__AIX__)
+#include "lldb/Host/aix/HostInfoAIX.h"
+#define HOST_INFO_TYPE HostInfoAIX
#else
#include "lldb/Host/posix/HostInfoPosix.h"
#define HOST_INFO_TYPE HostInfoPosix
diff --git a/lldb/include/lldb/Host/aix/AbstractSocket.h b/lldb/include/lldb/Host/aix/AbstractSocket.h
new file mode 100644
index 00000000000000..78a567a6b90953
--- /dev/null
+++ b/lldb/include/lldb/Host/aix/AbstractSocket.h
@@ -0,0 +1,25 @@
+//===-- AbstractSocket.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_AbstractSocket_h_
+#define liblldb_AbstractSocket_h_
+
+#include "lldb/Host/posix/DomainSocket.h"
+
+namespace lldb_private {
+class AbstractSocket : public DomainSocket {
+public:
+ AbstractSocket(bool child_processes_inherit);
+
+protected:
+ size_t GetNameOffset() const override;
+ void DeleteSocketFile(llvm::StringRef name) override;
+};
+}
+
+#endif // ifndef liblldb_AbstractSocket_h_
diff --git a/lldb/include/lldb/Host/aix/Host.h b/lldb/include/lldb/Host/aix/Host.h
new file mode 100644
index 00000000000000..ef1e74cd1d501b
--- /dev/null
+++ b/lldb/include/lldb/Host/aix/Host.h
@@ -0,0 +1,22 @@
+//===-- Host.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_HOST_LINUX_HOST_H
+#define LLDB_HOST_LINUX_HOST_H
+
+#include "lldb/lldb-types.h"
+#include <optional>
+
+namespace lldb_private {
+
+// Get PID (i.e. the primary thread ID) corresponding to the specified TID.
+std::optional<lldb::pid_t> getPIDForTID(lldb::pid_t tid);
+
+} // namespace lldb_private
+
+#endif // #ifndef LLDB_HOST_LINUX_HOST_H
diff --git a/lldb/include/lldb/Host/aix/HostInfoAIX.h b/lldb/include/lldb/Host/aix/HostInfoAIX.h
new file mode 100644
index 00000000000000..2964f3f1dc9893
--- /dev/null
+++ b/lldb/include/lldb/Host/aix/HostInfoAIX.h
@@ -0,0 +1,43 @@
+//===-- HostInfoLinux.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_Host_linux_HostInfoLinux_h_
+#define lldb_Host_linux_HostInfoLinux_h_
+
+#include "lldb/Host/posix/HostInfoPosix.h"
+#include "lldb/Utility/FileSpec.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/VersionTuple.h"
+
+#include <optional>
+#include <string>
+
+namespace lldb_private {
+
+class HostInfoLinux : public HostInfoPosix {
+ friend class HostInfoBase;
+
+public:
+ static void Initialize(SharedLibraryDirectoryHelper *helper = nullptr);
+ static void Terminate();
+
+ static llvm::VersionTuple GetOSVersion();
+ static std::optional<std::string> GetOSBuildString();
+ static llvm::StringRef GetDistributionId();
+ static FileSpec GetProgramFileSpec();
+
+protected:
+ static bool ComputeSupportExeDirectory(FileSpec &file_spec);
+ static bool ComputeSystemPluginsDirectory(FileSpec &file_spec);
+ static bool ComputeUserPluginsDirectory(FileSpec &file_spec);
+ static void ComputeHostArchitectureSupport(ArchSpec &arch_32,
+ ArchSpec &arch_64);
+};
+}
+
+#endif
diff --git a/lldb/include/lldb/Host/aix/Ptrace.h b/lldb/include/lldb/Host/aix/Ptrace.h
new file mode 100644
index 00000000000000..aabd3fd4fc5573
--- /dev/null
+++ b/lldb/include/lldb/Host/aix/Ptrace.h
@@ -0,0 +1,60 @@
+//===-- Ptrace.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
+//
+//===----------------------------------------------------------------------===//
+
+// This file defines ptrace functions & structures
+
+#ifndef liblldb_Host_linux_Ptrace_h_
+#define liblldb_Host_linux_Ptrace_h_
+
+#include <sys/ptrace.h>
+
+#ifndef __GLIBC__
+typedef int __ptrace_request;
+#endif
+
+#define DEBUG_PTRACE_MAXBYTES 20
+
+// Support ptrace extensions even when compiled without required kernel support
+#ifndef PTRACE_GETREGS
+#define PTRACE_GETREGS 12
+#endif
+#ifndef PTRACE_SETREGS
+#define PTRACE_SETREGS 13
+#endif
+#ifndef PTRACE_GETFPREGS
+#define PTRACE_GETFPREGS 14
+#endif
+#ifndef PTRACE_SETFPREGS
+#define PTRACE_SETFPREGS 15
+#endif
+#ifndef PTRACE_GETREGSET
+#define PTRACE_GETREGSET 0x4204
+#endif
+#ifndef PTRACE_SETREGSET
+#define PTRACE_SETREGSET 0x4205
+#endif
+#ifndef PTRACE_GET_THREAD_AREA
+#define PTRACE_GET_THREAD_AREA 25
+#endif
+#ifndef PTRACE_ARCH_PRCTL
+#define PTRACE_ARCH_PRCTL 30
+#endif
+#ifndef ARCH_GET_FS
+#define ARCH_SET_GS 0x1001
+#define ARCH_SET_FS 0x1002
+#define ARCH_GET_FS 0x1003
+#define ARCH_GET_GS 0x1004
+#endif
+#ifndef PTRACE_PEEKMTETAGS
+#define PTRACE_PEEKMTETAGS 33
+#endif
+#ifndef PTRACE_POKEMTETAGS
+#define PTRACE_POKEMTETAGS 34
+#endif
+
+#endif // liblldb_Host_linux_Ptrace_h_
diff --git a/lldb/include/lldb/Host/aix/Support.h b/lldb/include/lldb/Host/aix/Support.h
new file mode 100644
index 00000000000000..d1eb7f83d49161
--- /dev/null
+++ b/lldb/include/lldb/Host/aix/Support.h
@@ -0,0 +1,29 @@
+//===-- Support.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_HOST_LINUX_SUPPORT_H
+#define LLDB_HOST_LINUX_SUPPORT_H
+
+#include "llvm/Support/ErrorOr.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include <memory>
+
+namespace lldb_private {
+
+llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
+getProcFile(::pid_t pid, ::pid_t tid, const llvm::Twine &file);
+
+llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
+getProcFile(::pid_t pid, const llvm::Twine &file);
+
+llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
+getProcFile(const llvm::Twine &file);
+
+} // namespace lldb_private
+
+#endif // #ifndef LLDB_HOST_LINUX_SUPPORT_H
diff --git a/lldb/include/lldb/Host/aix/Uio.h b/lldb/include/lldb/Host/aix/Uio.h
new file mode 100644
index 00000000000000..460bd6c84ca361
--- /dev/null
+++ b/lldb/include/lldb/Host/aix/Uio.h
@@ -0,0 +1,23 @@
+//===-- Uio.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_Host_linux_Uio_h_
+#define liblldb_Host_linux_Uio_h_
+
+#include "lldb/Host/Config.h"
+#include <sys/uio.h>
+
+// We shall provide our own implementation of process_vm_readv if it is not
+// present
+#if !HAVE_PROCESS_VM_READV
+ssize_t process_vm_readv(::pid_t pid, const struct iovec *local_iov,
+ unsigned long liovcnt, const struct iovec *remote_iov,
+ unsigned long riovcnt, unsigned long flags);
+#endif
+
+#endif // liblldb_Host_linux_Uio_h_
diff --git a/lldb/include/lldb/Host/common/GetOptInc.h b/lldb/include/lldb/Host/common/GetOptInc.h
index 3fb9add4795417..7a3c2c9627cc7a 100644
--- a/lldb/include/lldb/Host/common/GetOptInc.h
+++ b/lldb/include/lldb/Host/common/GetOptInc.h
@@ -11,11 +11,11 @@
#include "lldb/lldb-defines.h"
-#if defined(_MSC_VER)
+#if defined(_MSC_VER) || defined(__AIX__)
#define REPLACE_GETOPT
#define REPLACE_GETOPT_LONG
#endif
-#if defined(_MSC_VER) || defined(__NetBSD__)
+#if defined(_MSC_VER) || defined(__NetBSD__) || defined(__AIX__)
#define REPLACE_GETOPT_LONG_ONLY
#endif
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
| # This has been added to keep the AIX build isolated for now. | ||
| # It will need to be modified later. | ||
| if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX") | ||
| add_definitions("-D__AIX__") |
There was a problem hiding this comment.
We shouldn't be defining a reserved identifier. Also, isn't there an existing macro that we could test use to check if we're on AIX?
There was a problem hiding this comment.
It would be really helpful, if we can decide on a better alternative.
There was a problem hiding this comment.
You didn't really answer my question. Are there predefined compiler macros that we could use? I'd be surprised if there weren't any, as every platform I know of has some. FWIW, these are the predefined macros on linux: https://godbolt.org/z/EG9fKaxMv
There was a problem hiding this comment.
At this point we are not aware of any predefined ones for AIX though. It would be great if the community can suggest something in this regard. We will also try to find some alternative accordingly.
There was a problem hiding this comment.
There is _AIX at least in clang: https://godbolt.org/z/95MzjcsKn
And according to this (totally official, legit) doc (https://www.lnf.infn.it/computing/doc/aixcxx/html/language/ref/rnmcradd.htm) they exist for the AIX system compiler as well. I'm sure you can find the proper page :)
If for some reason that's not going to work, then we would use something like LLDB_... as we have done for LLDB_ENABLE_LIBXML2 and friends.
(but I'm yet to look at the whole PR so I don't know yet)
There was a problem hiding this comment.
Ok Thats helpful 😄
I will give it go, trying to integrate it with the overall changes.
Meanwhile, shall I remove __AIX__ from this PR to keep that discussion separate?
There was a problem hiding this comment.
Yes. If we need a cmake generated definition, add that in its own PR along with anything that uses it.
There was a problem hiding this comment.
Well, with something that uses it. I'm not asking you to look into the future and know all possible uses.
AIX is UNIX based but its very different from Linux, so it cannot be treated as a flavour of Linux. |
That's exactly what I'm worried about. I have seen the full PR, and while I haven't compared it side by side with linux, I did have a very strong sense of deja-vu when viewing it. So if like 80% of the code is going to be the same, then we should figure out how to share that code instead of making it a copy. Maybe both systems could be instances of some generic linux-like (whatever you want to call it) system... |
|
Thank you for your feedback; I understand the concern about potential code duplication. Indeed, components like We've also had to exclude certain functionalities that aren't feasible on AIX at this stage. The main intentionn for our current approach is to isolate platform dependencies, thereby avoiding conditional directives like That said, we've made every effort to keep the code as generic as possible, and we’re open to any further suggestions on how to improve this. It will help in refining the changes. |
|
I'm not saying we should merge everything. I'm just talking about the classes you're introducing in this PR (HostInfo, etc.) What are the linux vs. aix differences there? |
|
Ok, besides some minor differences in other files here and there, |
|
A couple of suggestions. Perhaps you could reduce the PRs just to the files with the biggest differences on AIX, and include those changes in the PR. So that it is copying the file + making the AIX changes in one PR. Even if you have to add files that aren't used yet, we could wait to land the PR until we have reviewed the follow ups that do use them. Maybe this is a good tactic for ptrace.h and Host.cpp, going by what you've said. Do one PR for each file so that everything is as clear as possible. For the rest, a way to see whether one file for Linux and AIX would work would be to modify the existing file and add It doesn't even have to be buildable, as long as we get a rough idea of the changes. (I do understand the instinct to split the initial copy-paste step into its own PR, I would have done the same, but in this case I think it makes it harder for us to assess the differences) |
|
Ok sure. For readability and clarity, for the files having comparatively bigger changes, I can drop it as multiple commits in the same PR, so that the changes can be seen as cleanly as possible:
And for each of these bigger files, we can have respective PRs as per your suggestion. And for the rest of the common files, another draft PR should be ok for the readability pov. One question though? For the comparatively smaller changes across multiple files, is it okay to club them in one PR if they are related? |
|
One more thing. For platform differentiating files/paths like HostInfoAIX and future files like PlatformAIX, How do you suggest we add them? To keep the code path more streamlined, shall we keep their code under Linux with #if _AIX, Or shall we use some form of inheritance to keep AIX as its own platform? |
Even better!
Yes, whether that be by theme or because they need to be together to compile.
So this is what my second suggestion addresses. If we could see the #ifdef'd version then we would be able to compare the approaches. This is why Pavel is asking about differences to get an idea of whether it's a few #ifdef or a few 1000 lines scattered all over. We're not against a copy paste and change, or against adding some base class, we're trying to assess where on that spectrum a good solution is. So if you've got the files with #ifdef, upload that as a draft PR. Then we can talk there about how to share, or not share the code. |
|
Ok great! Then we will proceed with these suggestions for upcoming PRs. Thanks! |
|
(+1 to everything that David said) |
This PR is in reference to porting LLDB on AIX.
Link to discussions on llvm discourse and github:
The complete changes for porting are present in this draft PR:
#102601
Taking Base code for Host Info header files from Linux, and setting up header files for AIX Host Info.
-D__AIX__Review Request: @DavidSpickett