Merge pull request #166 from benfred/fixes

 Fix failing to load large line number tables

remoteprocess/src/linux/libunwind/mod.rs

@@ -152,13 +152,15 @@ extern {
     // functions in libunwind-ptrace.so
     fn _UPT_create(pid: pid_t) -> *mut c_void;
     fn _UPT_destroy(p: *mut c_void) -> c_void;
+    #[allow(improper_ctypes)]
     static _UPT_accessors: unw_accessors_t;
 }
 
 #[cfg(target_pointer_width="64")]
 extern {
     // functions in libunwind-x86_64.so (TODO: define similar for 32bit)
      #[link_name="_Ux86_64_create_addr_space"]
+    #[allow(improper_ctypes)]
     fn create_addr_space(acc: *mut unw_accessors_t, byteorder: c_int) -> unw_addr_space_t;
     #[link_name="_Ux86_64_destroy_addr_space"]
     fn destroy_addr_space(addr: unw_addr_space_t) -> c_void;
@@ -214,7 +216,7 @@ impl std::error::Error for Error {
         "LibunwindErrror"
     }
 
-    fn cause(&self) -> Option<&std::error::Error> {
+    fn cause(&self) -> Option<&dyn std::error::Error> {
         None
     }
 }

remoteprocess/src/linux/mod.rs

@@ -169,7 +169,7 @@ impl ThreadLock {
 impl Drop for ThreadLock {
     fn drop(&mut self) {
         if let Err(e) = ptrace::detach(self.tid) {
-            error!("Failed to detach from thread {} : {}", self.tid, e);
+            warn!("Failed to detach from thread {} : {}", self.tid, e);
         }
         debug!("detached from thread {}", self.tid);
     }

remoteprocess/src/linux/symbolication.rs

@@ -113,7 +113,7 @@ impl Symbolicator {
         Ok(())
     }
 
-    pub fn symbolicate(&self, addr: u64, line_info: bool, callback: &mut FnMut(&StackFrame)) -> Result<(), Error> {
+    pub fn symbolicate(&self, addr: u64, line_info: bool, callback: &mut dyn FnMut(&StackFrame)) -> Result<(), Error> {
         let binary = match self.get_binary(addr) {
             Some(binary) => binary,
             None => {
@@ -196,7 +196,7 @@ impl SymbolData {
         Ok(SymbolData{ctx, offset, dynamic_symbols, symbols, filename: filename.to_owned()})
     }
 
-    pub fn symbolicate(&self, addr: u64, line_info: bool, callback: &mut FnMut(&StackFrame)) -> Result<(), Error> {
+    pub fn symbolicate(&self, addr: u64, line_info: bool, callback: &mut dyn FnMut(&StackFrame)) -> Result<(), Error> {
         let mut ret = StackFrame{line:None, filename: None, function: None, addr, module: self.filename.clone()};
 
         // get the address before relocations

src/cython.rs

@@ -125,9 +125,9 @@ pub fn ignore_frame(name: &str) -> bool {
 }
 
 pub fn demangle(name: &str) -> &str {
-    // slice off any leading cython prefix
-    let prefixes = ["__pyx_fuse_1_0__pyx_pw",  "__pyx_pf", "__pyx_pw", "__pyx_f", "___pyx_f", "___pyx_pw",
-                    "use_0__pyx_f", "use_1__pyx_f"];
+    // slice off any leading cython prefix.
+    let prefixes = ["__pyx_fuse_1_0__pyx_pw", "__pyx_fuse_0__pyx_f", "__pyx_fuse_1__pyx_f",
+                    "__pyx_pf", "__pyx_pw", "__pyx_f", "___pyx_f", "___pyx_pw"];
     let mut current = match prefixes.iter().find(|&prefix| name.starts_with(prefix)) {
         Some(prefix) => &name[prefix.len()..],
         None => return name
@@ -181,7 +181,10 @@ mod tests {
         assert_eq!(demangle("__pyx_pw_8implicit_4_als_5least_squares_cg"), "least_squares_cg");
         assert_eq!(demangle("__pyx_fuse_1_0__pyx_pw_8implicit_4_als_31_least_squares_cg"), "_least_squares_cg");
         assert_eq!(demangle("__pyx_f_6mtrand_cont0_array"), "mtrand_cont0_array");
-        assert_eq!(demangle("use_1__pyx_f_8implicit_3bpr_has_non_zero"), "bpr_has_non_zero");
+        // in both of these cases we should ideally slice off the module (_als/bpr), but it gets tricky
+        // implementation wise
+        assert_eq!(demangle("__pyx_fuse_0__pyx_f_8implicit_4_als_axpy"), "_als_axpy");
+        assert_eq!(demangle("__pyx_fuse_1__pyx_f_8implicit_3bpr_has_non_zero"), "bpr_has_non_zero");
     }
 
     #[test]

src/stack_trace.rs

@@ -66,7 +66,19 @@ pub fn get_stack_trace<T, P >(thread: &T, process: &P) -> Result<StackTrace, Err
 
         let filename = copy_string(code.filename(), process).context("Failed to copy filename")?;
         let name = copy_string(code.name(), process).context("Failed to copy function name")?;
-        let line = get_line_number(&code, frame.lasti(), process).context("Failed to get line number")?;
+
+
+        let line = match get_line_number(&code, frame.lasti(), process) {
+            Ok(line) => line,
+            Err(e) => {
+                // Failling to get the line number really shouldn't be fatal here, but
+                // can happen in extreme cases (https://github.com/benfred/py-spy/issues/164)
+                // Rather than fail set the linenumber to 0. This is used by the native extensions
+                // to indicate that we can't load a line number and it should be handled gracefully
+                warn!("Failed to get line number from {}.{}: {}", filename, name, e);
+                0
+            }
+        };
 
         frames.push(Frame{name, filename, line, short_filename: None, module: None});
         if frames.len() > 4096 {
@@ -145,7 +157,7 @@ pub fn copy_string<T: StringObject, P: ProcessMemory>(ptr: * const T, process: &
 pub fn copy_bytes<T: BytesObject, P: ProcessMemory>(ptr: * const T, process: &P) -> Result<Vec<u8>, Error> {
     let obj = process.copy_pointer(ptr)?;
     let size = obj.size();
-    if size >= 8192 {
+    if size >= 65536 {
         return Err(format_err!("Refusing to copy {} bytes", size));
     }
     Ok(process.copy(obj.address(ptr as usize), size as usize)?)