PR Feedback: make PTrace a non-copyable struct

Author: andrurogerz

tools/swift-inspect/Sources/SwiftInspectLinux/PTrace.swift

@@ -13,7 +13,13 @@
 import Foundation
 import LinuxSystemHeaders
 
-public class PTrace {
+// Provides scoped access to a PTrace object.
+public func withPTracedProcess(pid: pid_t, _ closure: (consuming PTrace) throws -> Void) throws {
+  let ptrace = try PTrace(pid)
+  try closure(ptrace)
+}
+
+public struct PTrace: ~Copyable {
   enum PTraceError: Error {
     case operationFailure(_ command: CInt, pid: pid_t, errno: CInt = get_errno())
     case waitFailure(pid: pid_t, errno: CInt = get_errno())
@@ -22,15 +28,10 @@ public class PTrace {
 
   let pid: pid_t
 
-  // Provides scoped access to a PTrace object.
-  public static func withAttachedProcess(pid: pid_t, _ closure: (PTrace) throws -> Void) throws {
-    let ptrace = try PTrace(pid);
-    try closure(ptrace)
-  }
-
   // Initializing a PTrace instance attaches to the target process, waits for
   // it to stop, and leaves it in a stopped state. The caller may resume the
   // process by calling cont().
+  // NOTE: clients must use withPTracedProcess instead of direct initialization.
   init(_ pid: pid_t) throws {
     guard ptrace_attach(pid) != -1 else {
       throw PTraceError.operationFailure(PTRACE_ATTACH, pid: pid)
@@ -108,7 +109,8 @@ public class PTrace {
   // is stopped with a SIGTRAP signal. In this case, the caller is responsible
   // for taking action on the signal.
   public func jump(
-    to address: UInt64, with args: [UInt64] = [], _ callback: (() throws -> Void)? = nil
+    to address: UInt64, with args: [UInt64] = [],
+    _ callback: ((borrowing PTrace) throws -> Void)? = nil
   ) throws -> UInt64 {
     let origRegs = try self.getRegSet()
     defer { try? self.setRegSet(regSet: origRegs) }
@@ -140,7 +142,7 @@ public class PTrace {
       guard wStopSig(status) == SIGTRAP, let callback = callback else { break }
 
       // give the caller the opportunity to handle SIGTRAP
-      try callback()
+      try callback(self)
     }
 
     let sigInfo = try self.getSigInfo()

tools/swift-inspect/Sources/SwiftInspectLinux/RegisterSet.swift

@@ -19,7 +19,10 @@ public typealias RegisterSet = user_pt_regs
 extension RegisterSet {
   public static var trapInstructionSize: UInt { return 4 }  // brk #0x0
 
-  public func setupCall(_ ptrace: PTrace, to funcAddr: UInt64, with args: [UInt64], returnTo returnAddr: UInt64) throws -> RegisterSet {
+  public func setupCall(
+    _ ptrace: borrowing PTrace, to funcAddr: UInt64, with args: [UInt64],
+    returnTo returnAddr: UInt64
+  ) throws -> RegisterSet {
     // The first 8 arguments are passed in regsters. Any additional arguments
     // must be pushed on the stack, which is not implemented.
     precondition(args.count <= 8)
@@ -53,7 +56,10 @@ public typealias RegisterSet = pt_regs
 extension RegisterSet {
   public static var trapInstructionSize: UInt { return 1 }  // int3
 
-  public func setupCall(_ ptrace: PTrace, to funcAddr: UInt64, with args: [UInt64], returnTo returnAddr: UInt64) throws -> RegisterSet {
+  public func setupCall(
+    _ ptrace: borrowing PTrace, to funcAddr: UInt64, with args: [UInt64],
+    returnTo returnAddr: UInt64
+  ) throws -> RegisterSet {
     // The first 6 arguments are passed in registers. Any additional arguments
     // must be pushed on the stack, which is not implemented.
     precondition(args.count <= 6)

tools/swift-inspect/Sources/swift-inspect/AndroidRemoteProcess.swift

@@ -70,7 +70,7 @@ internal final class AndroidRemoteProcess: LinuxRemoteProcess {
     var regionCount = 0
     var allocCount = 0
     do {
-      try PTrace.withAttachedProcess(pid: self.processIdentifier) { ptrace in
+      try withPTracedProcess(pid: self.processIdentifier) { ptrace in
         for entry in self.memoryMap.entries {
           // Limiting malloc_iterate calls to only memory regions that are known
           // to contain heap allocations is not strictly necessary but it does
@@ -105,7 +105,7 @@ internal final class AndroidRemoteProcess: LinuxRemoteProcess {
 
   // Iterate a single heap region in the remote process and return an array
   // of (base, len) pairs describing each heap allocation in the region.
-  internal func iterateHeapRegion(_ ptrace: PTrace, region: MemoryMap.Entry) throws -> [(
+  internal func iterateHeapRegion(_ ptrace: borrowing PTrace, region: MemoryMap.Entry) throws -> [(
     base: swift_addr_t, len: UInt64
   )] {
     // Allocate a page-sized buffer in the remote process that malloc_iterate
@@ -158,7 +158,7 @@ internal final class AndroidRemoteProcess: LinuxRemoteProcess {
 
     let regionLen = region.endAddr - region.startAddr
     let args = [region.startAddr, regionLen, remoteCodeAddr, remoteDataAddr]
-    _ = try ptrace.jump(to: mallocIterateAddr, with: args) {
+    _ = try ptrace.jump(to: mallocIterateAddr, with: args) { ptrace in
       // This callback is invoked when a SIGTRAP is encountered in the remote
       // process. In this context, this signal indicates there is no more room
       // in the allocated metadata region (see AndroidCLib/heap.c).
@@ -208,7 +208,9 @@ internal final class AndroidRemoteProcess: LinuxRemoteProcess {
   }
 
   // call mmap in the remote process with the provided arguments
-  internal func mmapRemote(_ ptrace: PTrace, len: Int, prot: Int32, flags: Int32) throws -> UInt64 {
+  internal func mmapRemote(_ ptrace: borrowing PTrace, len: Int, prot: Int32, flags: Int32) throws
+    -> UInt64
+  {
     guard let sym = self.mmapSymbol.addr else {
       throw RemoteProcessError.missingSymbol(self.mmapSymbol.name)
     }
@@ -217,7 +219,7 @@ internal final class AndroidRemoteProcess: LinuxRemoteProcess {
   }
 
   // call munmap in the remote process with the provdied arguments
-  internal func munmapRemote(_ ptrace: PTrace, addr: UInt64, len: Int) throws -> UInt64 {
+  internal func munmapRemote(_ ptrace: borrowing PTrace, addr: UInt64, len: Int) throws -> UInt64 {
     guard let sym = self.munmapSymbol.addr else {
       throw RemoteProcessError.missingSymbol(self.munmapSymbol.name)
     }
@@ -226,15 +228,15 @@ internal final class AndroidRemoteProcess: LinuxRemoteProcess {
   }
 
   // call malloc_disable in the remote process
-  internal func mallocDisableRemote(_ ptrace: PTrace) throws {
+  internal func mallocDisableRemote(_ ptrace: borrowing PTrace) throws {
     guard let sym = self.mallocDisableSymbol.addr else {
       throw RemoteProcessError.missingSymbol(self.mallocDisableSymbol.name)
     }
     _ = try ptrace.jump(to: sym)
   }
 
   // call malloc_enable in the remote process
-  internal func mallocEnableRemote(_ ptrace: PTrace) throws {
+  internal func mallocEnableRemote(_ ptrace: borrowing PTrace) throws {
     guard let sym = self.mallocEnableSymbol.addr else {
       throw RemoteProcessError.missingSymbol(self.mallocEnableSymbol.name)
     }