[PATCH] Define and use the __ptrace_request enumeration type in ptrace.

[PATCH] Define and use the __ptrace_request enumeration type in ptrace.

[Vasileios Kalintiris]

[-- Attachment #1: Type: text/plain, Size: 2831 bytes --]

[PATCH] Define and use the __ptrace_request enumeration type in ptrace.

Signed-off-by: Vasileios Kalintiris <Vasileios.Kalintiris@imgtec.com>
---
 include/sys/ptrace.h | 70 +++++++++++++++++++++++++++-------------------------
 src/linux/ptrace.c   |  2 +-
 2 files changed, 38 insertions(+), 34 deletions(-)

diff --git a/include/sys/ptrace.h b/include/sys/ptrace.h
index a133e66..7551955 100644
--- a/include/sys/ptrace.h
+++ b/include/sys/ptrace.h
@@ -6,40 +6,44 @@ extern "C" {
 
 #include <stdint.h>
 
-#define PTRACE_TRACEME 0
-#define PT_TRACE_ME PTRACE_TRACEME
+enum __ptrace_request {
+	PTRACE_TRACEME = 0,
+	PTRACE_PEEKTEXT,
+	PTRACE_PEEKDATA,
+	PTRACE_PEEKUSER,
+	PTRACE_POKETEXT,
+	PTRACE_POKEDATA,
+	PTRACE_POKEUSER,
+	PTRACE_CONT,
+	PTRACE_KILL,
+	PTRACE_SINGLESTEP,
+
+	PTRACE_GETREGS = 12,
+	PTRACE_SETREGS,
+	PTRACE_GETFPREGS,
+	PTRACE_SETFPREGS,
+	PTRACE_ATTACH,
+	PTRACE_DETACH,
+	PTRACE_GETFPXREGS,
+	PTRACE_SETFPXREGS,
 
-#define PTRACE_PEEKTEXT 1
-#define PTRACE_PEEKDATA 2
-#define PTRACE_PEEKUSER 3
-#define PTRACE_POKETEXT 4
-#define PTRACE_POKEDATA 5
-#define PTRACE_POKEUSER 6
-#define PTRACE_CONT 7
-#define PTRACE_KILL 8
-#define PTRACE_SINGLESTEP 9
-#define PTRACE_GETREGS 12
-#define PTRACE_SETREGS 13
-#define PTRACE_GETFPREGS 14
-#define PTRACE_SETFPREGS 15
-#define PTRACE_ATTACH 16
-#define PTRACE_DETACH 17
-#define PTRACE_GETFPXREGS 18
-#define PTRACE_SETFPXREGS 19
-#define PTRACE_SYSCALL 24
-#define PTRACE_SETOPTIONS 0x4200
-#define PTRACE_GETEVENTMSG 0x4201
-#define PTRACE_GETSIGINFO 0x4202
-#define PTRACE_SETSIGINFO 0x4203
-#define PTRACE_GETREGSET 0x4204
-#define PTRACE_SETREGSET 0x4205
-#define PTRACE_SEIZE 0x4206
-#define PTRACE_INTERRUPT 0x4207
-#define PTRACE_LISTEN 0x4208
-#define PTRACE_PEEKSIGINFO 0x4209
-#define PTRACE_GETSIGMASK 0x420a
-#define PTRACE_SETSIGMASK 0x420b
+	PTRACE_SYSCALL = 24,
 
+	PTRACE_SETOPTIONS = 0x4200,
+	PTRACE_GETEVENTMSG,
+	PTRACE_GETSIGINFO,
+	PTRACE_SETSIGINFO,
+	PTRACE_GETREGSET,
+	PTRACE_SETREGSET,
+	PTRACE_SEIZE,
+	PTRACE_INTERRUPT,
+	PTRACE_LISTEN,
+	PTRACE_PEEKSIGINFO,
+	PTRACE_GETSIGMASK,
+	PTRACE_SETSIGMASK
+};
+
+#define PT_TRACE_ME PTRACE_TRACEME
 #define PT_READ_I PTRACE_PEEKTEXT
 #define PT_READ_D PTRACE_PEEKDATA
 #define PT_READ_U PTRACE_PEEKUSER
@@ -90,7 +94,7 @@ struct ptrace_peeksiginfo_args {
 	int32_t nr;
 };
 
-long ptrace(int, ...);
+long ptrace(enum __ptrace_request, ...);
 
 #ifdef __cplusplus
 }
diff --git a/src/linux/ptrace.c b/src/linux/ptrace.c
index 83b8022..35ee262 100644
--- a/src/linux/ptrace.c
+++ b/src/linux/ptrace.c
@@ -3,7 +3,7 @@
 #include <unistd.h>
 #include "syscall.h"
 
-long ptrace(int req, ...)
+long ptrace(enum __ptrace_request req, ...)
 {
 	va_list ap;
 	pid_t pid;
-- 
2.5.0

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: __ptrace_request.patch --]
[-- Type: text/x-patch; name="__ptrace_request.patch", Size: 2719 bytes --]

[PATCH] Define and use the __ptrace_request enumeration type in ptrace.

Signed-off-by: Vasileios Kalintiris <Vasileios.Kalintiris@imgtec.com>
---
 include/sys/ptrace.h | 70 +++++++++++++++++++++++++++-------------------------
 src/linux/ptrace.c   |  2 +-
 2 files changed, 38 insertions(+), 34 deletions(-)

diff --git a/include/sys/ptrace.h b/include/sys/ptrace.h
index a133e66..7551955 100644
--- a/include/sys/ptrace.h
+++ b/include/sys/ptrace.h
@@ -6,40 +6,44 @@ extern "C" {
 
 #include <stdint.h>
 
-#define PTRACE_TRACEME 0
-#define PT_TRACE_ME PTRACE_TRACEME
+enum __ptrace_request {
+	PTRACE_TRACEME = 0,
+	PTRACE_PEEKTEXT,
+	PTRACE_PEEKDATA,
+	PTRACE_PEEKUSER,
+	PTRACE_POKETEXT,
+	PTRACE_POKEDATA,
+	PTRACE_POKEUSER,
+	PTRACE_CONT,
+	PTRACE_KILL,
+	PTRACE_SINGLESTEP,
+
+	PTRACE_GETREGS = 12,
+	PTRACE_SETREGS,
+	PTRACE_GETFPREGS,
+	PTRACE_SETFPREGS,
+	PTRACE_ATTACH,
+	PTRACE_DETACH,
+	PTRACE_GETFPXREGS,
+	PTRACE_SETFPXREGS,
 
-#define PTRACE_PEEKTEXT 1
-#define PTRACE_PEEKDATA 2
-#define PTRACE_PEEKUSER 3
-#define PTRACE_POKETEXT 4
-#define PTRACE_POKEDATA 5
-#define PTRACE_POKEUSER 6
-#define PTRACE_CONT 7
-#define PTRACE_KILL 8
-#define PTRACE_SINGLESTEP 9
-#define PTRACE_GETREGS 12
-#define PTRACE_SETREGS 13
-#define PTRACE_GETFPREGS 14
-#define PTRACE_SETFPREGS 15
-#define PTRACE_ATTACH 16
-#define PTRACE_DETACH 17
-#define PTRACE_GETFPXREGS 18
-#define PTRACE_SETFPXREGS 19
-#define PTRACE_SYSCALL 24
-#define PTRACE_SETOPTIONS 0x4200
-#define PTRACE_GETEVENTMSG 0x4201
-#define PTRACE_GETSIGINFO 0x4202
-#define PTRACE_SETSIGINFO 0x4203
-#define PTRACE_GETREGSET 0x4204
-#define PTRACE_SETREGSET 0x4205
-#define PTRACE_SEIZE 0x4206
-#define PTRACE_INTERRUPT 0x4207
-#define PTRACE_LISTEN 0x4208
-#define PTRACE_PEEKSIGINFO 0x4209
-#define PTRACE_GETSIGMASK 0x420a
-#define PTRACE_SETSIGMASK 0x420b
+	PTRACE_SYSCALL = 24,
 
+	PTRACE_SETOPTIONS = 0x4200,
+	PTRACE_GETEVENTMSG,
+	PTRACE_GETSIGINFO,
+	PTRACE_SETSIGINFO,
+	PTRACE_GETREGSET,
+	PTRACE_SETREGSET,
+	PTRACE_SEIZE,
+	PTRACE_INTERRUPT,
+	PTRACE_LISTEN,
+	PTRACE_PEEKSIGINFO,
+	PTRACE_GETSIGMASK,
+	PTRACE_SETSIGMASK
+};
+
+#define PT_TRACE_ME PTRACE_TRACEME
 #define PT_READ_I PTRACE_PEEKTEXT
 #define PT_READ_D PTRACE_PEEKDATA
 #define PT_READ_U PTRACE_PEEKUSER
@@ -90,7 +94,7 @@ struct ptrace_peeksiginfo_args {
 	int32_t nr;
 };
 
-long ptrace(int, ...);
+long ptrace(enum __ptrace_request, ...);
 
 #ifdef __cplusplus
 }
diff --git a/src/linux/ptrace.c b/src/linux/ptrace.c
index 83b8022..35ee262 100644
--- a/src/linux/ptrace.c
+++ b/src/linux/ptrace.c
@@ -3,7 +3,7 @@
 #include <unistd.h>
 #include "syscall.h"
 
-long ptrace(int req, ...)
+long ptrace(enum __ptrace_request req, ...)
 {
 	va_list ap;
 	pid_t pid;
-- 
2.5.0

Re: [PATCH] Define and use the __ptrace_request enumeration type in ptrace.

[Rich Felker]

On Thu, Sep 17, 2015 at 02:20:00PM +0000, Vasileios Kalintiris wrote:
> [PATCH] Define and use the __ptrace_request enumeration type in ptrace.

Sorry nobody replied to your post earlier. Is there a particular
problem you're trying to solve here? Generally enum is avoided in musl
unless it's mandated as part of an interface definition. As written, I
think the patch would also be a regression that would break apps which
test for header/library support of a particular PTRACE_* command using
#ifdef.

Rich

RE: [PATCH] Define and use the __ptrace_request enumeration type in ptrace.

[Vasileios Kalintiris]

Hi Rich,

Thank you for taking the time to look into this.

> Is there a particular problem you're trying to solve here?

I tried to compile LLDB with musl and I found two places where
the __ptrace_request type is being used:

http://reviews.llvm.org/diffusion/L/browse/lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp;248212$3161

> Generally enum is avoided in musl unless it's mandated as part of an
> interface definition.

I based my patch on the definition provided by ptrace(2) on my Linux distro,
thinking that this is the "correct" function prototype that we should provide.

> As written, I think the patch would also be a regression that would break
> apps which test for header/library support of a particular PTRACE_* command
> using #ifdef.

I do understand your concerns, I suppose that fixing this from the LLDB side
will solve the mis-compilation problem and will keep us from introducing a
regression to other apps from musl.

Out of curiosity, what is the general implementation choice in this case in
other C libraries? I'm asking because discovering GLIBC and handing it as a
special case would be easy through the __GLIBC__ macro. However, musl doesn't
provide any such macro (and other C libraries probably).

Thanks,
Vasileios

RE: [PATCH] Define and use the __ptrace_request enumeration type in ptrace.

[Alexander Monakov]

On Mon, 21 Sep 2015, Vasileios Kalintiris wrote:
> Out of curiosity, what is the general implementation choice in this case in
> other C libraries? I'm asking because discovering GLIBC and handing it as a
> special case would be easy through the __GLIBC__ macro. However, musl doesn't
> provide any such macro (and other C libraries probably).

Such unfortunate exposure of __ptrace_request appears to be specific to glibc
and uclibc (which wants to mirror glibc interfaces in other ways also).  BSD
libcs and Android's Bionic libc have a plain 'int' there.

As a result, LLDB already had to workaround __ptrace_request on Android:
https://github.com/llvm-mirror/lldb/blob/303c5382c20618cd07944023b259d138fc07c822/include/lldb/Host/linux/Ptrace.h#L19

It would make sense to 'typedef int __ptrace_request' in LLDB under '#ifndef
__GLIBC__' instead of '#ifdef __ANDROID_NDK__'.

(likewise it makes sense to guard '#define PT_DETACH PTRACE_DETACH' with
'#ifndef PT_DETACH')

If a more fancy fix is desired, it is also possible to hide the variations in
the first argument type via a bit of preprocessor and C++ duct tape:

#include <sys/ptrace.h>

#define ptrace(arg1, ...) ptrace(_pcast(PTRACE_TRACEME, arg1), __VA_ARGS__)

template<class T, class V>
T _pcast(T, V v)
{
  return static_cast<T>(v);
}


The above would allow to eliminate explicit typecasts where LLDB uses ptrace.

Or alternatively if GNU features are acceptable, one can simply use
__typeof__(PTRACE_TRACEME) to retrieve the underlying type.

Alexander