Skip to content

[lldb][AIX] Adding AIX version of ptrace64 #120390

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Dec 20, 2024

Conversation

DhruvSrivastavaX
Copy link
Contributor

@DhruvSrivastavaX DhruvSrivastavaX commented Dec 18, 2024

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

Adding changes for minimal build for lldb binary on AIX. ptrace64 is needed to debug 64-bit AIX debuggee, and its format is different than the traditional ptrace on other platforms: AIX ptrace

Review Request: @labath @DavidSpickett

@llvmbot
Copy link
Member

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

Adding changes for minimal build for lldb binary on AIX.
ptrace64 is needed to debug 64-bit AIX debuggee, and its format is different than the traditional ptrace on other platforms:
https://www.ibm.com/docs/fr/aix/7.3?topic=p-ptrace-ptracex-ptrace64-subroutine

Review Request: @labath @DavidSpickett


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

1 Files Affected:

  • (modified) lldb/source/Host/posix/ProcessLauncherPosixFork.cpp (+7-3)
diff --git a/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp b/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
index 4a9469bde2f186..0e162d04c35837 100644
--- a/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
+++ b/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
@@ -21,8 +21,8 @@
 #include <sys/wait.h>
 #include <unistd.h>
 
-#include <sstream>
 #include <csignal>
+#include <sstream>
 
 #ifdef __ANDROID__
 #include <android/api-level.h>
@@ -47,8 +47,7 @@ static void write_string(int error_fd, const char *str) {
   (void)r;
 }
 
-[[noreturn]] static void ExitWithError(int error_fd,
-                                       const char *operation) {
+[[noreturn]] static void ExitWithError(int error_fd, const char *operation) {
   int err = errno;
   write_string(error_fd, operation);
   write_string(error_fd, " failed: ");
@@ -193,8 +192,13 @@ struct ForkLaunchInfo {
     }
 
     // Start tracing this child that is about to exec.
+#if !defined(_AIX)
     if (ptrace(PT_TRACE_ME, 0, nullptr, 0) == -1)
       ExitWithError(error_fd, "ptrace");
+#else
+    if (ptrace64(PT_TRACE_ME, 0, 0, 0, nullptr) == -1)
+      ExitWithError(error_fd, "ptrace");
+#endif
   }
 
   // Execute.  We should never return...

@DavidSpickett
Copy link
Collaborator

Please link to the English docs - https://www.ibm.com/docs/en/aix/7.3?topic=p-ptrace-ptracex-ptrace64-subroutine instead.

@DhruvSrivastavaX
Copy link
Contributor Author

Please link to the English docs - https://www.ibm.com/docs/en/aix/7.3?topic=p-ptrace-ptracex-ptrace64-subroutine instead.

Apologies for the blunder, I myself dont speak French by the way. 😀 Corrected

@DavidSpickett
Copy link
Collaborator

Can you summarise the need for ptrace64? Mainly, will this only apply to launching a process or does it apply to any ptrace call?

I wonder:

  1. How many #ifdef we're going to end up with
  2. Whether AIX should use ptrace64 everywhere if your intention is to only support AIX 64 bit (but if it's only this one use of ptrace that needs ptrace64, that's not necessary)

@DavidSpickett
Copy link
Collaborator

Apologies for the blunder, I myself dont speak French by the way. 😀 Corrected

I am kinda impressed how many languages they have available, but the downside is that Google seems to love choosing a non-English one for me.

@DhruvSrivastavaX
Copy link
Contributor Author

DhruvSrivastavaX commented Dec 18, 2024

I am kinda impressed how many languages they have available, but the downside is that Google seems to love choosing a non-English one for me.

True. Pretty much same for me. It will always be either Japanese or Korean. French is a new addition perhaps. 😄

@DhruvSrivastavaX
Copy link
Contributor Author

Can you summarise the need for ptrace64? Mainly, will this only apply to launching a process or does it apply to any ptrace call?

I wonder:

  1. How many #ifdef we're going to end up with
  2. Whether AIX should use ptrace64 everywhere if your intention is to only support AIX 64 bit (but if it's only this one use of ptrace that needs ptrace64, that's not necessary)

Yes, to handle the 64-bit debuggees in AIX, we would need the ptrace64 call. It will apply to all the ptrace calls in general.
It is used in the NativeProcess/Thread plugins, but since those are new plugins only for AIX they dont need an #if AIX.
One other place that will need an #if AIX besides the one in this PR is GDBRemoteCommunicationServerLLGS.cpp and that is pretty much it for now.

@DhruvSrivastavaX
Copy link
Contributor Author

AIX ptrace (ptrace,ptracex,ptrace64) has a different format, so we would need an AIX specific case anyway.

@DavidSpickett
Copy link
Collaborator

Yes, to handle the 64-bit debuggees in AIX, we would need the ptrace64 call. It will apply to all the ptrace calls in general.
It is used in the NativeProcess/Thread plugins, but since those are new plugins only for AIX they dont need an #if AIX.

Thanks. Yes I see that the majority of existing calls are in, for example, lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp. And a lot are going via a Ptracewrapper that is also per platform.

This PosixFork is one of the few doing a direct call in generic code.

I am wondering if it should be #if there is ptrace64 but I don't think so because this seems to be AIX specific. Even if it existed elsewhere I'd rather not silently use it without knowing how it behaves.

@DavidSpickett DavidSpickett requested a review from labath December 18, 2024 12:55
@DavidSpickett
Copy link
Collaborator

@labath what do you think?

DavidSpickett pushed a commit that referenced this pull request Dec 19, 2024
…20459)

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. #101657
The complete changes for porting are present in this draft PR:
#102601

Added clang-format changes for ProcessLauncherPosixFork.cpp which will
be followed by ptrace changes in:
- #120390
@labath
Copy link
Collaborator

labath commented Dec 20, 2024

I think this is fine. We could come up with an abstraction for this, but I don't think it's necessary for a simple change like this. Like @DhruvSrivastavaX said, the rest of the ptrace calls are going to be in platform-specific code.

@DavidSpickett DavidSpickett merged commit cf0bc8d into llvm:main Dec 20, 2024
7 checks passed
github-actions bot pushed a commit to arm/arm-toolchain that referenced this pull request Jan 10, 2025
…ork.cpp (#120459)

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. llvm/llvm-project#101657
The complete changes for porting are present in this draft PR:
llvm/llvm-project#102601

Added clang-format changes for ProcessLauncherPosixFork.cpp which will
be followed by ptrace changes in:
- llvm/llvm-project#120390
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