Skip to content
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

Moved ptrace constants into enum types added minor functionality. #734

Closed
wants to merge 0 commits into from
Closed

Conversation

xd009642
Copy link
Contributor

@Susurrus, just creating the initial pull request it's not yet ready to merge but it's currently underway!

Used the libc_enum! macro to create enums for the ptrace event, request, and options constants defined in libc. Also, replicated functionality to move from c_int to PtraceEvent enum in PR #728 as it appears to be abandoned.

Proper effort should be taken to ascertain the ptrace functionality available on the host system and to advertise this in some manner.

@xd009642
Copy link
Contributor Author

Just updating with the latest changes from upstream then it'll be reviewable-ish

@xd009642
Copy link
Contributor Author

Okay I think this is largely a fair implementation. PtraceRequest is kept as a constant instead of enum, I found trying to get it to use different types for different systems with the libc_enum macro an unnecessary hassle which resulted in a bunch of casts. PtraceOptions is a bitflag as that makes most sense and PtraceEvent is a libc_enum.

I've also updated the tests as I realised I missed them off first time.

@Susurrus
Copy link
Contributor

@xd009642 #728 was merged, so I'm not certain what that comment was referring to.

Also, PtraceRequest should be an enum. Yes it will require casts, but it will be nicer for the users of these functions, which is what we should be optimizing for primarily.

Additionally, let's get rid of the ptrace::ptrace module and just have everyone in one module here.

@xd009642
Copy link
Contributor Author

I meant #461 got a bit mixed up. Agreed with the ptrace::ptrace module, I think renaming PtraceRequest to just Request etc would also be an improvement, are you okay with this or are there semver/backwards-compatibility/usability issues?

For the PtraceRequest being an enum I don't suppose you can suggest a nice way of getting the type selection done by cfg_if with the enum and repr()? Given c_int can be either 32 or 64 bits depending on word size and PtraceRequest can be signed or unsigned I can't see a way that doesn't involve 4x cfg_attrs. I have to admit I am rather unexperienced with conditional compilation and a lot of macro stuff in rust so there may well be a more elegant solution

@Susurrus
Copy link
Contributor

We're pre-1.0, so we're not beholding to backwards compatibility. That being said we're not trying to break things just to break things, but we're trying to get to our stable interfaces for 1.0, so there will be a little churn now.

So, let's rename PtraceRequest to Request. Might want to rename the other enum/bitflags as well. We want to avoid having "ptrace" twice in a path for a function or constant if possible, but it depends on how things are actually used. Let's go ahead and try this and see what it looks like!

@Susurrus
Copy link
Contributor

As for the cfg, I'll take a look later, but there's likely a similar example somewhere else in the codebase.

@xd009642
Copy link
Contributor Author

All done apart from Request being an enum. Have a look and see if you prefer the new naming convention. I have to say I do prefer ptrace::Event and WaitStatus::PtraceEvent instead of having PtraceEvent showing up as a type name and in a different file a field in the WaitStatus enum

@Susurrus
Copy link
Contributor

What's the use case for Event::from_c_int()? I don't see any examples listed nor any test code.

I have to say I do prefer ptrace::Event and WaitStatus::PtraceEvent instead of having PtraceEvent showing up as a type name and in a different file a field in the WaitStatus enum.

I didn't quite understand what you mean here.

@xd009642
Copy link
Contributor Author

Event::from_c_int was from #461 I just ported all the functionality over.

And my comment was because PtraceEvent is a field in the WaitStatus enum but also the name of a type in ptrace.rs, I just prefer not using the same name for an enum element but also an enum itself. I feel ptrace::Event is a cleaner solution. Just personal preference of course I don't mind changing it if you disagree :)

@Susurrus
Copy link
Contributor

For the PtraceRequest being an enum I don't suppose you can suggest a nice way of getting the type selection done by cfg_if with the enum and repr()? Given c_int can be either 32 or 64 bits depending on word size and PtraceRequest can be signed or unsigned I can't see a way that doesn't involve 4x cfg_attrs. I have to admit I am rather unexperienced with conditional compilation and a lot of macro stuff in rust so there may well be a more elegant solution

You should just be able to do:

// request_type is either c_int or c_uint depending on platform

libc_enum!{
    #[repr(request_type)]
    PTRACE_TRACEME as request_type,
    #[cfg(target_os = "linux")]
    PTRACE_GETFPXREGS,
    ...
}

Can you modify this PR to do that? I can help you debug it once you post it as part of this PR.

@Susurrus
Copy link
Contributor

Event::from_c_int was from #461 I just ported all the functionality over.

I don't see a use case for this, so let's remove it. If it needs to be included we need example code to demonstrate the need for it and test cases to validate its implementation.

And my comment was because PtraceEvent is a field in the WaitStatus enum but also the name of a type in ptrace.rs, I just prefer not using the same name for an enum element but also an enum itself. I feel ptrace::Event is a cleaner solution. Just personal preference of course I don't mind changing it if you disagree :)

Okay, cool. I think this naming makes sense. The ptrace tests seem to read fine with the name change.

@Susurrus
Copy link
Contributor

@xd009642 Tests are failing on this as well because constants are not properly cfg'd to appropriate platforms.

@xd009642
Copy link
Contributor Author

@Susurrus okay, I should get to sorting this on Saturday, I've been away so haven't got access to my pc

@Susurrus
Copy link
Contributor

@xd009642 No worries, just update and ping me on a new review once you'd done those changes.

@xd009642
Copy link
Contributor Author

As a note I was looking for libc_enum! examples in the code and I reckon a lot of the enums in event.rs and probably elsewhere should make usage of the macro instead of what they currently do. Should I raise an issue regarding this as a future improvement?

@xd009642
Copy link
Contributor Author

Right some of the configs are probably wrong, but it's now in the enum so I'm pushing it so I can see what works and doesn't in travis then fix those issues.

@xd009642
Copy link
Contributor Author

@Susurrus a lot of travis checks fail because "error: no rules expected the token flags" when using the libc_bitflags macro. Any idea on what causes this and how I can remedy it?

@Susurrus
Copy link
Contributor

@xd009642 The syntax for libc_bitflags! now follows the syntax from the 0.9 version of bitflags!. This is described in CONVENTIONS.md, but you probably need to rebase since #725 landed. When you rebase, squash all your commits in to one while your add it.

@xd009642
Copy link
Contributor Author

Okay it's now nearly there, libc_bitflags seems to word just need to cfg out ptrace requests that aren't available on certain platforms!

@Susurrus
Copy link
Contributor

Cool!

Also I'd like to request a few things:

  1. Please make this two commits: the first changes the naming and moves things to the enum and the second adds the detach() function.
  2. Add a commit at the end adding an entry to the changelog for the addition of ptrace::detach().
  3. All these constants needs documentation for them. We've been having people add that when they do this refactoring, so I wanted to ask if you could do it as well. The enum itself needs doccomments as well as all of their variants.

@xd009642
Copy link
Contributor Author

Okay, I was going to add all the comments at the end as given last time I did a PR it was squashed down to one commit I figured the same would happen again. Not too sure how to split out my previous squash back into the commits to branch them out... But I guess if all else fails I can revert to upstream master and add the changes back in that way

@Susurrus
Copy link
Contributor

Yeah, this will be easiest if you do everything and just use git add -p to add the various pieces and generated appropriate commits. Then you can just git push origin -f to override the ones you have on GitHub.


cfg_if! {
if #[cfg(any(all(target_os = "linux", arch = "s390x"),
all(target_os = "linux", target_env = "gnu")))] {
Copy link
Contributor

Choose a reason for hiding this comment

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

You need another space before the all( on this line.

}

libc_enum!{
#[cfg_attr(all(any(all(target_os = "linux", arch = "s390x"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Ca you format these a little better so they're more readable? Do like what you did above.

pub type PtraceRequest = c_int;
}

cfg_if! {
Copy link
Contributor

Choose a reason for hiding this comment

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

RequestType needs to have #[doc(hidden)].

pub const PTRACE_O_TRACEEXIT: PtraceOptions = (1 << PTRACE_EVENT_EXIT);
pub const PTRACE_O_TRACESECCOMP: PtraceOptions = (1 << PTRACE_EVENT_SECCOMP);
libc_bitflags! {
pub struct Options: libc::c_int {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add docs for each variant here? They can be summarized from the man pages.

libc_enum!{
#[repr(i32)]
pub enum Event {
PTRACE_EVENT_FORK,
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add docs for each variant here? They can be summarized from the man pages.

@xd009642
Copy link
Contributor Author

I attempted to use the following method to cast enum fields and found it didn't compile.

libc_enum!{
    #[repr(request_type)]
    PTRACE_TRACEME as request_type,

I'm currently blocked by a weird consistency in libc which I've raised an issue on rust-lang/libc#744 because without knowing a way to cast the fields I can't get it to compile for musl.

I'll start addressing the comments so things move along quicker once the libc issue is resolved. Just keeping you in the loop 😄

@Susurrus Susurrus closed this Aug 27, 2017
@Susurrus
Copy link
Contributor

Hmm, looks like I accidentally closed this trying to update your PR from my end. If you push more commits on to your master branch (you should really create feature branches for all your PRs) I think we can reopen this. I also rebased it so it should be down to 2 commits:

From bd10f83cb17bd36fd213c609d48d4b2c1240a137 Mon Sep 17 00:00:00 2001
From: xd009642 <danielmckenna93@gmail.com>
Date: Sat, 12 Aug 2017 16:49:08 +0100
Subject: [PATCH] Moved ptrace constants into enum types added minor
 functionality.

Used the libc_enum! macro to create enums for the ptrace event, request, and libc_bitflags for options constants defined in libc.
Also, replicated functionality to move from c_int to PtraceEvent enum in PR #728 as it appears to be abandoned.

Added utility function for detaching from tracee. Updated names and removed ptrace::ptrace namespace
---
 src/sys/ptrace.rs       | 235 +++++++++++++++++++++++++++++++-----------------
 test/sys/test_ptrace.rs |   3 +-
 test/sys/test_wait.rs   |   4 +-
 3 files changed, 156 insertions(+), 86 deletions(-)

diff --git a/src/sys/ptrace.rs b/src/sys/ptrace.rs
index 5f0391d4..c028848c 100644
--- a/src/sys/ptrace.rs
+++ b/src/sys/ptrace.rs
@@ -6,68 +6,117 @@ use libc::{self, c_void, c_long, siginfo_t};
 use ::unistd::Pid;
 use sys::signal::Signal;
 
-pub mod ptrace {
-    use libc::c_int;
-
-    cfg_if! {
-        if #[cfg(any(all(target_os = "linux", arch = "s390x"),
-                     all(target_os = "linux", target_env = "gnu")))] {
-            pub type PtraceRequest = ::libc::c_uint;
-        } else {
-            pub type PtraceRequest = c_int;
-        }
+
+cfg_if! {
+    if #[cfg(any(all(target_os = "linux", arch = "s390x"),
+                 all(target_os = "linux", target_env = "gnu")))] {
+        #[doc(hidden)]
+        pub type RequestType = ::libc::c_uint;
+    } else {
+        #[doc(hidden)]
+        pub type RequestType = ::libc::c_int;
+    }
+}
+
+libc_enum!{
+    #[cfg_attr(not(any(target_env = "musl", target_os = "android")), repr(u32))]
+    #[cfg_attr(any(target_env = "musl", target_os = "android"), repr(i32))]
+    /// Ptrace Request enum defining the action to be taken.
+    pub enum Request {
+        PTRACE_TRACEME,
+        PTRACE_PEEKTEXT,
+        PTRACE_PEEKDATA,
+        PTRACE_PEEKUSER,
+        PTRACE_POKETEXT,
+        PTRACE_POKEDATA,
+        PTRACE_POKEUSER,
+        PTRACE_CONT,
+        PTRACE_KILL,
+        PTRACE_SINGLESTEP,
+        #[cfg(all(any(target_env = "musl", target_arch ="x86_64", target_arch = "s390x"), not(target_os = "android")))]
+        PTRACE_GETREGS as RequestType,
+        #[cfg(all(any(target_env = "musl", target_arch ="x86_64", target_arch = "s390x"), not(target_os = "android")))]
+        PTRACE_SETREGS as RequestType,
+        #[cfg(all(any(target_env = "musl", target_arch ="x86_64", target_arch = "s390x"), not(target_os = "android")))]
+        PTRACE_GETFPREGS as RequestType,
+        #[cfg(all(any(target_env = "musl", target_arch ="x86_64", target_arch = "s390x"), not(target_os = "android")))]
+        PTRACE_SETFPREGS as RequestType,
+        PTRACE_ATTACH,
+        PTRACE_DETACH,
+        #[cfg(all(any(target_env = "musl", target_arch ="x86_64"), not(target_os = "android")))]
+        PTRACE_GETFPXREGS as RequestType,
+        #[cfg(all(any(target_env = "musl", target_arch ="x86_64"), not(target_os = "android")))]
+        PTRACE_SETFPXREGS as RequestType,
+        PTRACE_SYSCALL,
+        PTRACE_SETOPTIONS,
+        PTRACE_GETEVENTMSG,
+        PTRACE_GETSIGINFO,
+        PTRACE_SETSIGINFO,
+        #[cfg(all(any(target_env = "musl", target_arch ="x86_64", target_arch = "s390x"), not(target_os = "android")))]
+        PTRACE_GETREGSET,
+        #[cfg(all(any(target_env = "musl", target_arch ="x86_64", target_arch = "s390x"), not(target_os = "android")))]
+        PTRACE_SETREGSET,
+        #[cfg(not(any(target_os = "android", target_arch = "mips", target_arch = "mips64")))]
+        PTRACE_SEIZE,
+        #[cfg(not(any(target_os = "android", target_arch = "mips", target_arch = "mips64")))]
+        PTRACE_INTERRUPT,
+        #[cfg(not(any(target_os = "android", target_arch = "mips", target_arch = "mips64")))]
+        PTRACE_LISTEN,
+        #[cfg(not(any(target_os = "android", target_arch = "mips", target_arch = "mips64")))]
+        PTRACE_PEEKSIGINFO,
+    }
+}
+
+libc_enum!{
+    #[repr(i32)]
+    /// Using the ptrace options the tracer can configure the tracee to stop
+    /// at certain events. This enum is used to define those events as defined
+    /// in `man ptrace`.
+    pub enum Event {
+        /// Event that stops before a return from fork or clone.
+        PTRACE_EVENT_FORK,
+        /// Event that stops before a return from vfork or clone.
+        PTRACE_EVENT_VFORK,
+        /// Event that stops before a return from clone.
+        PTRACE_EVENT_CLONE,
+        /// Event that stops before a return from execve.
+        PTRACE_EVENT_EXEC,
+        /// Event for a return from vfork.
+        PTRACE_EVENT_VFORK_DONE,
+        /// Event for a stop before an exit. Unlike the waitpid Exit status program.
+        /// registers can still be examined
+        PTRACE_EVENT_EXIT,
+        /// STop triggered by a seccomp rule on a tracee.
+        PTRACE_EVENT_SECCOMP,
+        // PTRACE_EVENT_STOP not provided by libc because it's defined in glibc 2.26
     }
+}
 
-    pub const PTRACE_TRACEME:     PtraceRequest = 0;
-    pub const PTRACE_PEEKTEXT:    PtraceRequest = 1;
-    pub const PTRACE_PEEKDATA:    PtraceRequest = 2;
-    pub const PTRACE_PEEKUSER:    PtraceRequest = 3;
-    pub const PTRACE_POKETEXT:    PtraceRequest = 4;
-    pub const PTRACE_POKEDATA:    PtraceRequest = 5;
-    pub const PTRACE_POKEUSER:    PtraceRequest = 6;
-    pub const PTRACE_CONT:        PtraceRequest = 7;
-    pub const PTRACE_KILL:        PtraceRequest = 8;
-    pub const PTRACE_SINGLESTEP:  PtraceRequest = 9;
-    pub const PTRACE_GETREGS:     PtraceRequest = 12;
-    pub const PTRACE_SETREGS:     PtraceRequest = 13;
-    pub const PTRACE_GETFPREGS:   PtraceRequest = 14;
-    pub const PTRACE_SETFPREGS:   PtraceRequest = 15;
-    pub const PTRACE_ATTACH:      PtraceRequest = 16;
-    pub const PTRACE_DETACH:      PtraceRequest = 17;
-    pub const PTRACE_GETFPXREGS:  PtraceRequest = 18;
-    pub const PTRACE_SETFPXREGS:  PtraceRequest = 19;
-    pub const PTRACE_SYSCALL:     PtraceRequest = 24;
-    pub const PTRACE_SETOPTIONS:  PtraceRequest = 0x4200;
-    pub const PTRACE_GETEVENTMSG: PtraceRequest = 0x4201;
-    pub const PTRACE_GETSIGINFO:  PtraceRequest = 0x4202;
-    pub const PTRACE_SETSIGINFO:  PtraceRequest = 0x4203;
-    pub const PTRACE_GETREGSET:   PtraceRequest = 0x4204;
-    pub const PTRACE_SETREGSET:   PtraceRequest = 0x4205;
-    pub const PTRACE_SEIZE:       PtraceRequest = 0x4206;
-    pub const PTRACE_INTERRUPT:   PtraceRequest = 0x4207;
-    pub const PTRACE_LISTEN:      PtraceRequest = 0x4208;
-    pub const PTRACE_PEEKSIGINFO: PtraceRequest = 0x4209;
-
-    pub type PtraceEvent = c_int;
-
-    pub const PTRACE_EVENT_FORK:       PtraceEvent = 1;
-    pub const PTRACE_EVENT_VFORK:      PtraceEvent = 2;
-    pub const PTRACE_EVENT_CLONE:      PtraceEvent = 3;
-    pub const PTRACE_EVENT_EXEC:       PtraceEvent = 4;
-    pub const PTRACE_EVENT_VFORK_DONE: PtraceEvent = 5;
-    pub const PTRACE_EVENT_EXIT:       PtraceEvent = 6;
-    pub const PTRACE_EVENT_SECCOMP:    PtraceEvent = 6;
-    pub const PTRACE_EVENT_STOP:       PtraceEvent = 128;
-
-    pub type PtraceOptions = c_int;
-    pub const PTRACE_O_TRACESYSGOOD: PtraceOptions   = 1;
-    pub const PTRACE_O_TRACEFORK: PtraceOptions      = (1 << PTRACE_EVENT_FORK);
-    pub const PTRACE_O_TRACEVFORK: PtraceOptions     = (1 << PTRACE_EVENT_VFORK);
-    pub const PTRACE_O_TRACECLONE: PtraceOptions     = (1 << PTRACE_EVENT_CLONE);
-    pub const PTRACE_O_TRACEEXEC: PtraceOptions      = (1 << PTRACE_EVENT_EXEC);
-    pub const PTRACE_O_TRACEVFORKDONE: PtraceOptions = (1 << PTRACE_EVENT_VFORK_DONE);
-    pub const PTRACE_O_TRACEEXIT: PtraceOptions      = (1 << PTRACE_EVENT_EXIT);
-    pub const PTRACE_O_TRACESECCOMP: PtraceOptions   = (1 << PTRACE_EVENT_SECCOMP);
+libc_bitflags! {
+    /// Ptrace options used in conjunction with the PTRACE_SETOPTIONS request.
+    /// See `man ptrace` for more details.
+    pub struct Options: libc::c_int {
+        /// When delivering system call traps set a bit to allow tracer to
+        /// distinguish between normal stops or syscall stops. May not work on
+        /// all systems.
+        PTRACE_O_TRACESYSGOOD;
+        /// Stop tracee at next fork and start tracing the forked process.
+        PTRACE_O_TRACEFORK;
+        /// Stop tracee at next vfork call and trace the vforked process.
+        PTRACE_O_TRACEVFORK;
+        /// Stop tracee at next clone call and trace the cloned process.
+        PTRACE_O_TRACECLONE;
+        /// Stop tracee at next execve call.
+        PTRACE_O_TRACEEXEC;
+        /// Stop tracee at vfork completion.
+        PTRACE_O_TRACEVFORKDONE;
+        /// Stop tracee at next exit call. Stops before exit commences allowing
+        /// tracer to see location of exit and register states.
+        PTRACE_O_TRACEEXIT;
+        /// Stop tracee when a SECCOMP_RET_TRACE rule is triggered. See `man seccomp` for more
+        /// details.
+        PTRACE_O_TRACESECCOMP;
+    }
 }
 
 /// Performs a ptrace request. If the request in question is provided by a specialised function
@@ -76,9 +125,8 @@ pub mod ptrace {
     since="0.10.0",
     note="usages of `ptrace()` should be replaced with the specialized helper functions instead"
 )]
-pub unsafe fn ptrace(request: ptrace::PtraceRequest, pid: Pid, addr: *mut c_void, data: *mut c_void) -> Result<c_long> {
-    use self::ptrace::*;
-
+pub unsafe fn ptrace(request: Request, pid: Pid, addr: *mut c_void, data: *mut c_void) -> Result<c_long> {
+    use self::Request::*;
     match request {
         PTRACE_PEEKTEXT | PTRACE_PEEKDATA | PTRACE_PEEKUSER => ptrace_peek(request, pid, addr, data),
         PTRACE_GETSIGINFO | PTRACE_GETEVENTMSG | PTRACE_SETSIGINFO | PTRACE_SETOPTIONS => Err(Error::UnsupportedOperation),
@@ -86,10 +134,10 @@ pub unsafe fn ptrace(request: ptrace::PtraceRequest, pid: Pid, addr: *mut c_void
     }
 }
 
-fn ptrace_peek(request: ptrace::PtraceRequest, pid: Pid, addr: *mut c_void, data: *mut c_void) -> Result<c_long> {
+fn ptrace_peek(request: Request, pid: Pid, addr: *mut c_void, data: *mut c_void) -> Result<c_long> {
     let ret = unsafe {
         Errno::clear();
-        libc::ptrace(request, libc::pid_t::from(pid), addr, data)
+        libc::ptrace(request as RequestType, libc::pid_t::from(pid), addr, data)
     };
     match Errno::result(ret) {
         Ok(..) | Err(Error::Sys(Errno::UnknownErrno)) => Ok(ret),
@@ -101,45 +149,54 @@ fn ptrace_peek(request: ptrace::PtraceRequest, pid: Pid, addr: *mut c_void, data
 /// Some ptrace get requests populate structs or larger elements than c_long
 /// and therefore use the data field to return values. This function handles these
 /// requests.
-fn ptrace_get_data<T>(request: ptrace::PtraceRequest, pid: Pid) -> Result<T> {
+fn ptrace_get_data<T>(request: Request, pid: Pid) -> Result<T> {
     // Creates an uninitialized pointer to store result in
     let data: T = unsafe { mem::uninitialized() };
-    let res = unsafe { libc::ptrace(request, libc::pid_t::from(pid), ptr::null_mut::<T>(), &data as *const _ as *const c_void) };
+    let res = unsafe {
+        libc::ptrace(request as RequestType,
+                     libc::pid_t::from(pid),
+                     ptr::null_mut::<T>(),
+                     &data as *const _ as *const c_void)
+    };
     Errno::result(res)?;
     Ok(data)
 }
 
-unsafe fn ptrace_other(request: ptrace::PtraceRequest, pid: Pid, addr: *mut c_void, data: *mut c_void) -> Result<c_long> {
-    Errno::result(libc::ptrace(request, libc::pid_t::from(pid), addr, data)).map(|_| 0)
+unsafe fn ptrace_other(request: Request, pid: Pid, addr: *mut c_void, data: *mut c_void) -> Result<c_long> {
+    Errno::result(libc::ptrace(request as RequestType, libc::pid_t::from(pid), addr, data)).map(|_| 0)
 }
 
 /// Set options, as with `ptrace(PTRACE_SETOPTIONS,...)`.
-pub fn setoptions(pid: Pid, options: ptrace::PtraceOptions) -> Result<()> {
-    use self::ptrace::*;
+pub fn setoptions(pid: Pid, options: Options) -> Result<()> {
     use std::ptr;
 
-    let res = unsafe { libc::ptrace(PTRACE_SETOPTIONS, libc::pid_t::from(pid), ptr::null_mut::<libc::c_void>(), options as *mut c_void) };
+    let res = unsafe {
+        libc::ptrace(Request::PTRACE_SETOPTIONS as RequestType,
+                     libc::pid_t::from(pid),
+                     ptr::null_mut::<libc::c_void>(),
+                     options.bits() as *mut c_void)
+    };
     Errno::result(res).map(|_| ())
 }
 
 /// Gets a ptrace event as described by `ptrace(PTRACE_GETEVENTMSG,...)`
 pub fn getevent(pid: Pid) -> Result<c_long> {
-    use self::ptrace::*;
-    ptrace_get_data::<c_long>(PTRACE_GETEVENTMSG, pid)
+    ptrace_get_data::<c_long>(Request::PTRACE_GETEVENTMSG, pid)
 }
 
 /// Get siginfo as with `ptrace(PTRACE_GETSIGINFO,...)`
 pub fn getsiginfo(pid: Pid) -> Result<siginfo_t> {
-    use self::ptrace::*;
-    ptrace_get_data::<siginfo_t>(PTRACE_GETSIGINFO, pid)
+    ptrace_get_data::<siginfo_t>(Request::PTRACE_GETSIGINFO, pid)
 }
 
 /// Set siginfo as with `ptrace(PTRACE_SETSIGINFO,...)`
 pub fn setsiginfo(pid: Pid, sig: &siginfo_t) -> Result<()> {
-    use self::ptrace::*;
     let ret = unsafe{
         Errno::clear();
-        libc::ptrace(PTRACE_SETSIGINFO, libc::pid_t::from(pid), ptr::null_mut::<libc::c_void>(), sig as *const _ as *const c_void)
+        libc::ptrace(Request::PTRACE_SETSIGINFO as RequestType,
+                     libc::pid_t::from(pid),
+                     ptr::null_mut::<libc::c_void>(),
+                     sig as *const _ as *const c_void)
     };
     match Errno::result(ret) {
         Ok(_) => Ok(()),
@@ -154,7 +211,7 @@ pub fn setsiginfo(pid: Pid, sig: &siginfo_t) -> Result<()> {
 pub fn traceme() -> Result<()> {
     unsafe {
         ptrace_other(
-            ptrace::PTRACE_TRACEME,
+            Request::PTRACE_TRACEME,
             Pid::from_raw(0),
             ptr::null_mut(),
             ptr::null_mut(),
@@ -168,7 +225,7 @@ pub fn traceme() -> Result<()> {
 pub fn syscall(pid: Pid) -> Result<()> {
     unsafe {
         ptrace_other(
-            ptrace::PTRACE_SYSCALL,
+            Request::PTRACE_SYSCALL,
             pid,
             ptr::null_mut(),
             ptr::null_mut(),
@@ -182,7 +239,7 @@ pub fn syscall(pid: Pid) -> Result<()> {
 pub fn attach(pid: Pid) -> Result<()> {
     unsafe {
         ptrace_other(
-            ptrace::PTRACE_ATTACH,
+            Request::PTRACE_ATTACH,
             pid,
             ptr::null_mut(),
             ptr::null_mut(),
@@ -190,6 +247,20 @@ pub fn attach(pid: Pid) -> Result<()> {
     }
 }
 
+/// Detaches the current running process, as with `ptrace(PTRACE_DETACH, ...)`
+///
+/// Detaches from the process specified in pid allowing it to run freely
+pub fn detach(pid: Pid) -> Result<()> {
+    unsafe {
+        ptrace_other(
+            Request::PTRACE_DETACH,
+            pid,
+            ptr::null_mut(),
+            ptr::null_mut()
+            ).map(|_| ())
+    }
+}
+
 /// Restart the stopped tracee process, as with `ptrace(PTRACE_CONT, ...)`
 ///
 /// Continues the execution of the process with PID `pid`, optionally
@@ -200,7 +271,7 @@ pub fn cont<T: Into<Option<Signal>>>(pid: Pid, sig: T) -> Result<()> {
         None => ptr::null_mut(),
     };
     unsafe {
-        ptrace_other(ptrace::PTRACE_CONT, pid, ptr::null_mut(), data).map(|_| ()) // ignore the useless return value
+        ptrace_other(Request::PTRACE_CONT, pid, ptr::null_mut(), data).map(|_| ()) // ignore the useless return value
     }
 }
 
diff --git a/test/sys/test_ptrace.rs b/test/sys/test_ptrace.rs
index 16b24110..20cde1aa 100644
--- a/test/sys/test_ptrace.rs
+++ b/test/sys/test_ptrace.rs
@@ -16,8 +16,7 @@ fn test_ptrace() {
 // Just make sure ptrace_setoptions can be called at all, for now.
 #[test]
 fn test_ptrace_setoptions() {
-    use nix::sys::ptrace::ptrace::PTRACE_O_TRACESYSGOOD;
-    let err = ptrace::setoptions(getpid(), PTRACE_O_TRACESYSGOOD).unwrap_err();
+    let err = ptrace::setoptions(getpid(), ptrace::PTRACE_O_TRACESYSGOOD).unwrap_err();
     assert!(err != Error::UnsupportedOperation);
 }
 
diff --git a/test/sys/test_wait.rs b/test/sys/test_wait.rs
index 0193e262..0fcaa19c 100644
--- a/test/sys/test_wait.rs
+++ b/test/sys/test_wait.rs
@@ -55,7 +55,7 @@ fn test_waitstatus_pid() {
 #[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
 mod ptrace {
     use nix::sys::ptrace;
-    use nix::sys::ptrace::ptrace::*;
+    use nix::sys::ptrace::*;
     use nix::sys::signal::*;
     use nix::sys::wait::*;
     use nix::unistd::*;
@@ -81,7 +81,7 @@ mod ptrace {
         assert_eq!(waitpid(child, None), Ok(WaitStatus::PtraceSyscall(child)));
         // Then get the ptrace event for the process exiting
         assert!(ptrace::cont(child, None).is_ok());
-        assert_eq!(waitpid(child, None), Ok(WaitStatus::PtraceEvent(child, SIGTRAP, PTRACE_EVENT_EXIT)));
+        assert_eq!(waitpid(child, None), Ok(WaitStatus::PtraceEvent(child, SIGTRAP, Event::PTRACE_EVENT_EXIT as i32)));
         // Finally get the normal wait() result, now that the process has exited
         assert!(ptrace::cont(child, None).is_ok());
         assert_eq!(waitpid(child, None), Ok(WaitStatus::Exited(child, 0)));
-- 
2.13.5

and

From b569cfc1e9bb3b665fb0d4c4fe1084bfa3feb6ba Mon Sep 17 00:00:00 2001
From: Bryant Mairs <bryant@mai.rs>
Date: Sun, 27 Aug 2017 09:02:02 -0700
Subject: [PATCH 1/2] Support casting values in libc_enum!

---
 src/macros.rs | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/src/macros.rs b/src/macros.rs
index 7a0eb07c..39bc466d 100644
--- a/src/macros.rs
+++ b/src/macros.rs
@@ -379,6 +379,23 @@ macro_rules! libc_enum {
         }
     };
 
+    // Munch an ident and cast it to the given type; covers terminating comma.
+    (@accumulate_entries
+        $prefix:tt,
+        [$($entries:tt)*];
+        $entry:ident as $ty:ty, $($tail:tt)*
+    ) => {
+        libc_enum! {
+            @accumulate_entries
+            $prefix,
+            [
+                $($entries)*
+                $entry = libc::$entry as $ty,
+            ];
+            $($tail)*
+        }
+    };
+
     // (non-pub) Entry rule.
     (
         $(#[$attr:meta])*
-- 
2.13.5

@Susurrus
Copy link
Contributor

You can actually just cherry-pick or merge in my commits from https://github.com/Susurrus/nix/tree/734 which includes both of these commits and then we can continue to iterate on them here.

@xd009642
Copy link
Contributor Author

@Susurrus yeah sorry about that. Also, I've submitted a pr to libc RE the musl types rust-lang/libc#745 so I was hoping the musl cast wouldn't be an issue.

@Susurrus
Copy link
Contributor

No worries, why don't you fetch the 734 branch from my nix repo and submit a new PR for it because this seems to have gotten into a weird state.

As for your libc PR, it's a breaking change so it won't be merged for a while. Best to work around it locally. I fixed libc_enum! To do just that iny 734 branch.

@xd009642
Copy link
Contributor Author

Yeah I'm doing that now 👍

bors bot added a commit that referenced this pull request Aug 28, 2017
749: Moved ptrace constants into enum types plus minor additions r=Susurrus a=xd009642

Reopening of #734 hence the branch name, changelog will be updated once the pr number of this new pull request is known.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants