Use framehop 0.3.0 for better x86 unwinding.

Framehop 0.3.0 comes with x86_64 instruction analysis for the leaf frame,
more accurate stubs/stub_helper unwinding, supports one global unwinder
cache, and is less confused by unwind info for the text_env section.

Author: mstange

perfrecord/Cargo.lock

@@ -25,7 +25,7 @@ crossbeam-channel = "0.5.1"
 signal-hook = "0.3.9"
 serde_json = "1.0.53"
 gecko_profile = { version = "0.2.0", path = "../gecko_profile" }
-framehop = "0.2.0"
+framehop = "0.3.0"
 
 [profile.release]
 debug = true

perfrecord/Cargo.toml

@@ -52,7 +52,7 @@ pub struct DyldInfo {
     pub file: String,
     pub address: u64,
     pub vmsize: u64,
-    pub sections: framehop::ModuleSectionAddresses,
+    pub sections: framehop::ModuleSectionAddressRanges,
     pub uuid: Option<Uuid>,
     pub arch: Option<&'static str>,
     pub unwind_sections: UnwindSectionInfo,
@@ -65,7 +65,7 @@ pub struct UnwindSectionInfo {
     /// (address, size)
     pub eh_frame_section: Option<(u64, u64)>,
     /// (address, size)
-    pub text_section: Option<(u64, u64)>,
+    pub text_segment: Option<(u64, u64)>,
 }
 
 pub struct DyldInfoManager {
@@ -282,7 +282,11 @@ fn get_dyld_image_info(
     let mut unwind_info_section = None;
     let mut eh_frame_section = None;
     let mut text_section = None;
-    let mut got_section_addr = 0;
+    let mut text_env_section = None;
+    let mut stubs_section = None;
+    let mut stub_helper_section = None;
+    let mut got_section = None;
+    let mut text_segment = None;
     let mut offset = 0;
     for _ in 0..header.ncmds {
         unsafe {
@@ -294,6 +298,7 @@ fn get_dyld_image_info(
                     if (*segcmd).segname[0..7] == [95, 95, 84, 69, 88, 84, 0] {
                         // This is the __TEXT segment.
                         vmsize = (*segcmd).vmsize;
+                        text_segment = Some((image_load_address, vmsize));
                         let textvmaddr = (*segcmd).vmaddr;
 
                         let mut section_offset = offset + mem::size_of::<segment_command_64>();
@@ -310,9 +315,26 @@ fn get_dyld_image_info(
                             if (*section).sectname[0..7] == [95, 95, 116, 101, 120, 116, 0] {
                                 // This is the __text section.
                                 text_section = Some((addr, size));
+                            } else if (*section).sectname[0..9]
+                                == [116, 101, 120, 116, 95, 101, 110, 118, 0]
+                            {
+                                // This is the text_env section.
+                                text_env_section = Some((addr, size));
+                            } else if (*section).sectname[0..8]
+                                == [95, 95, 115, 116, 117, 98, 115, 0]
+                            {
+                                // This is the __stubs section.
+                                stubs_section = Some((addr, size));
+                            } else if (*section).sectname[0..14]
+                                == [
+                                    95, 95, 115, 116, 117, 98, 95, 104, 101, 108, 112, 101, 114, 0,
+                                ]
+                            {
+                                // This is the __stub_helper section.
+                                stub_helper_section = Some((addr, size));
                             } else if (*section).sectname[0..6] == [95, 95, 103, 111, 116, 0] {
                                 // This is the __got section.
-                                got_section_addr = addr;
+                                got_section = Some((addr, size));
                             } else if (*section).sectname[0..14]
                                 == [
                                     95, 95, 117, 110, 119, 105, 110, 100, 95, 105, 110, 102, 111, 0,
@@ -345,19 +367,22 @@ fn get_dyld_image_info(
         file: filename,
         address: image_load_address,
         vmsize,
-        sections: framehop::ModuleSectionAddresses {
-            text: text_section.map(|(addr, _)| addr).unwrap_or(0),
-            eh_frame: eh_frame_section.map(|(addr, _)| addr).unwrap_or(0),
-            eh_frame_hdr: 0,
-            got: got_section_addr,
+        sections: framehop::ModuleSectionAddressRanges {
+            text: text_section.map(|(addr, size)| addr..addr + size),
+            text_env: text_env_section.map(|(addr, size)| addr..addr + size),
+            stubs: stubs_section.map(|(addr, size)| addr..addr + size),
+            stub_helper: stub_helper_section.map(|(addr, size)| addr..addr + size),
+            eh_frame: eh_frame_section.map(|(addr, size)| addr..addr + size),
+            eh_frame_hdr: None,
+            got: got_section.map(|(addr, size)| addr..addr + size),
         },
         uuid,
         arch: get_arch_string(header.cputype as u32, header.cpusubtype as u32),
         is_executable: header.filetype == MH_EXECUTE,
         unwind_sections: UnwindSectionInfo {
             unwind_info_section,
             eh_frame_section,
-            text_section,
+            text_segment,
         },
     })
 }

perfrecord/src/proc_maps.rs

@@ -1,5 +1,6 @@
 use crate::error::SamplingError;
 use crate::task_profiler::TaskProfiler;
+use crate::task_profiler::UnwinderCache;
 use crossbeam_channel::Receiver;
 use gecko_profile::ProfileBuilder;
 use std::mem;
@@ -15,6 +16,7 @@ pub struct Sampler {
     live_other_tasks: Vec<TaskProfiler>,
     dead_root_task: Option<TaskProfiler>,
     dead_other_tasks: Vec<TaskProfiler>,
+    unwinder_cache: UnwinderCache,
 }
 
 impl Sampler {
@@ -32,6 +34,7 @@ impl Sampler {
             live_other_tasks: Vec::new(),
             dead_root_task: None,
             dead_other_tasks: Vec::new(),
+            unwinder_cache: Default::default(),
         }
     }
 
@@ -62,7 +65,7 @@ impl Sampler {
             }
 
             if let Some(task) = &mut self.live_root_task {
-                let still_alive = task.sample(sample_timestamp)?;
+                let still_alive = task.sample(sample_timestamp, &mut self.unwinder_cache)?;
                 if !still_alive {
                     task.notify_dead(sample_timestamp);
                     self.dead_root_task = self.live_root_task.take();
@@ -72,7 +75,7 @@ impl Sampler {
             let mut other_tasks = Vec::with_capacity(self.live_other_tasks.capacity());
             mem::swap(&mut self.live_other_tasks, &mut other_tasks);
             for mut task in other_tasks.into_iter() {
-                let still_alive = task.sample(sample_timestamp)?;
+                let still_alive = task.sample(sample_timestamp, &mut self.unwinder_cache)?;
                 if still_alive {
                     self.live_other_tasks.push(task);
                 } else {

perfrecord/src/sampler.rs

@@ -2,6 +2,10 @@ use crate::error::SamplingError;
 use crate::kernel_error::{IntoResult, KernelError};
 use crate::proc_maps::{DyldInfo, DyldInfoManager, Modification, StackwalkerRef, VmSubData};
 use crate::thread_profiler::ThreadProfiler;
+use framehop::{
+    CacheNative, MayAllocateDuringUnwind, Module, ModuleUnwindData, TextByteData, Unwinder,
+    UnwinderNative,
+};
 use gecko_profile::debugid::DebugId;
 use mach::mach_types::thread_act_port_array_t;
 use mach::mach_types::thread_act_t;
@@ -18,9 +22,10 @@ use std::path::Path;
 use std::sync::Arc;
 use std::time::{Duration, Instant, SystemTime};
 
-use framehop::Unwinder;
 use gecko_profile::ProfileBuilder;
 
+pub type UnwinderCache = CacheNative<VmSubData, MayAllocateDuringUnwind>;
+
 pub struct TaskProfiler {
     task: mach_port_t,
     pid: u32,
@@ -35,8 +40,7 @@ pub struct TaskProfiler {
     executable_lib: Option<DyldInfo>,
     command_name: String,
     ignored_errors: Vec<SamplingError>,
-    unwinder: framehop::UnwinderNative<VmSubData, framehop::MayAllocateDuringUnwind>,
-    unwinder_cache: framehop::CacheNative<VmSubData, framehop::MayAllocateDuringUnwind>,
+    unwinder: UnwinderNative<VmSubData, MayAllocateDuringUnwind>,
 }
 
 impl TaskProfiler {
@@ -71,13 +75,16 @@ impl TaskProfiler {
             command_name: command_name.to_owned(),
             executable_lib: None,
             ignored_errors: Vec::new(),
-            unwinder: framehop::UnwinderNative::new(),
-            unwinder_cache: framehop::CacheNative::new(),
+            unwinder: UnwinderNative::new(),
         })
     }
 
-    pub fn sample(&mut self, now: Instant) -> Result<bool, SamplingError> {
-        let result = self.sample_impl(now);
+    pub fn sample(
+        &mut self,
+        now: Instant,
+        unwinder_cache: &mut UnwinderCache,
+    ) -> Result<bool, SamplingError> {
+        let result = self.sample_impl(now, unwinder_cache);
         match result {
             Ok(()) => Ok(true),
             Err(SamplingError::ProcessTerminated(_, _)) => Ok(false),
@@ -99,7 +106,11 @@ impl TaskProfiler {
         }
     }
 
-    fn sample_impl(&mut self, now: Instant) -> Result<(), SamplingError> {
+    fn sample_impl(
+        &mut self,
+        now: Instant,
+        unwinder_cache: &mut UnwinderCache,
+    ) -> Result<(), SamplingError> {
         // First, check for any newly-loaded libraries.
         let changes = self
             .lib_info_manager
@@ -141,7 +152,7 @@ impl TaskProfiler {
                 }
             };
             // Grab a sample from the thread.
-            let stackwalker = StackwalkerRef::new(&self.unwinder, &mut self.unwinder_cache);
+            let stackwalker = StackwalkerRef::new(&self.unwinder, unwinder_cache);
             let still_alive = thread.sample(stackwalker, now)?;
             if still_alive {
                 now_live_threads.insert(thread_act);
@@ -165,26 +176,21 @@ impl TaskProfiler {
             .unwind_sections
             .eh_frame_section
             .and_then(|(addr, size)| VmSubData::map_from_task(self.task, addr, size).ok());
-        let text_data = lib
-            .unwind_sections
-            .text_section
-            .and_then(|(addr, size)| VmSubData::map_from_task(self.task, addr, size).ok());
+        let text_data = lib.unwind_sections.text_segment.and_then(|(addr, size)| {
+            VmSubData::map_from_task(self.task, addr, size)
+                .ok()
+                .map(|data| TextByteData::new(data, addr..addr + size))
+        });
 
         let unwind_data = match (unwind_info_data, eh_frame_data) {
-            (Some(unwind_info), Some(eh_frame)) => {
-                framehop::ModuleUnwindData::CompactUnwindInfoAndEhFrame(
-                    unwind_info,
-                    Some(Arc::new(eh_frame)),
-                )
-            }
-            (Some(unwind_info), None) => {
-                framehop::ModuleUnwindData::CompactUnwindInfoAndEhFrame(unwind_info, None)
+            (Some(unwind_info), eh_frame) => {
+                ModuleUnwindData::CompactUnwindInfoAndEhFrame(unwind_info, eh_frame.map(Arc::new))
             }
-            (None, Some(eh_frame)) => framehop::ModuleUnwindData::EhFrame(Arc::new(eh_frame)),
-            (None, None) => framehop::ModuleUnwindData::None,
+            (None, Some(eh_frame)) => ModuleUnwindData::EhFrame(Arc::new(eh_frame)),
+            (None, None) => ModuleUnwindData::None,
         };
 
-        let module = framehop::Module::new(
+        let module = Module::new(
             lib.file.clone(),
             lib.address..(lib.address + lib.vmsize),
             lib.address,