From fee79009efdd6200f7d00a6512abf07981f379df Mon Sep 17 00:00:00 2001 From: michael-yuji Date: Sat, 4 Jun 2022 23:10:10 +0800 Subject: [PATCH 01/11] Check if dli_sname and dli_fname are null pointers. On some implementation of `dladdr`, the `dli_sname` and `dli_fname` can be null even if the `dladdr` itself succeed. --- usdt-impl/src/record.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/usdt-impl/src/record.rs b/usdt-impl/src/record.rs index d41c68dd..e3cf0e00 100644 --- a/usdt-impl/src/record.rs +++ b/usdt-impl/src/record.rs @@ -71,10 +71,15 @@ pub(crate) fn addr_to_info(addr: u64) -> (Option, Option) { if libc::dladdr(addr as *const c_void, &mut info as *mut _) == 0 { (None, None) } else { - ( - Some(CStr::from_ptr(info.dli_sname).to_string_lossy().to_string()), - Some(CStr::from_ptr(info.dli_fname).to_string_lossy().to_string()), - ) + unsafe fn to_str_if_not_null(ptr: *const std::os::raw::c_char) -> Option { + if ptr == null() { + None + } else { + Some(CStr::from_ptr(ptr).to_string_lossy().to_string()) + } + } + + (to_str_if_not_null(info.dli_sname), to_str_if_not_null(info.dli_fname)) } } } From 678324e7813a15ace60314f5b2c8ed21ec67775e Mon Sep 17 00:00:00 2001 From: michael-yuji Date: Sun, 5 Jun 2022 00:41:28 +0800 Subject: [PATCH 02/11] Add preliminary FreeBSD support FreeBSD implemented DTrace fairly standardly in compare to Illumos. 1. build.rs This patch enable FreeBSD to be treated as a "standard" implementation with no linker supported in the usdt build.rs. 2. The values to issue ioctl to dtrace helper are different. 2.1. The cmd value on FreeBSD is derived the C macro `_IOWR('z', 3, dot_helper_t)`; which is unfolded and the value is inlined in this patch. 2.2 The struct dof_helper is also different, in FreeBSD there are two extra fields: `dofhp_pid` and `dofhp_gen`. 3. Linker Issue Like Illumos, without `FORCE_LOAD` line the linker will omit the symbols (even with "-C link-dead-code"); therefore the line also needed to activate on FreeBSD. There are however still issues in linker that make the FreeBSD support preliminary, on FreeBSD the crate consumer need to build with "-C link-dead-code", or otherwise the probes will thrown away by the linker. --- dof/src/dof_bindings.rs | 4 ++++ usdt-impl/build.rs | 2 +- usdt-impl/src/no-linker.rs | 16 +++++++++++++++- 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/dof/src/dof_bindings.rs b/dof/src/dof_bindings.rs index 14673d7c..ea4dc48c 100644 --- a/dof/src/dof_bindings.rs +++ b/dof/src/dof_bindings.rs @@ -446,6 +446,10 @@ pub struct dof_helper { pub dofhp_mod: [::std::os::raw::c_char; 64usize], pub dofhp_addr: u64, pub dofhp_dof: u64, + #[cfg(target_os = "freebsd")] + pub dofhp_pid: i32, + #[cfg(target_os = "freebsd")] + pub dofhp_gen: ::std::os::raw::c_int } impl Default for dof_helper { fn default() -> Self { diff --git a/usdt-impl/build.rs b/usdt-impl/build.rs index c27391f7..5d26b71b 100644 --- a/usdt-impl/build.rs +++ b/usdt-impl/build.rs @@ -57,7 +57,7 @@ fn main() { Backend::NoOp } } - Some("illumos") | Some("solaris") if feat_asm => { + Some("illumos") | Some("solaris") | Some("freebsd") if feat_asm => { if have_stable_asm { Backend::Standard } else if feat_strict_asm || is_nightly { diff --git a/usdt-impl/src/no-linker.rs b/usdt-impl/src/no-linker.rs index 53e5bc39..5d9bcd27 100644 --- a/usdt-impl/src/no-linker.rs +++ b/usdt-impl/src/no-linker.rs @@ -130,7 +130,7 @@ fn extract_probe_records_from_section() -> Result { // writable (to implement one-time registration), so an immutable variable here // leads to _two_ sections, one writable and one read-only. A mutable variable // here ensures this ends up in a mutable section, the same as the probe records. - #[cfg(target_os = "illumos")] + #[cfg(any(target_os = "illumos", target_os = "freebsd"))] #[link_section = "set_dtrace_probes"] #[used] static mut FORCE_LOAD: [u64; 0] = []; @@ -170,9 +170,23 @@ fn ioctl_section(buf: &[u8], modname: [std::os::raw::c_char; 64]) -> Result<(), dofhp_mod: modname, dofhp_addr: buf.as_ptr() as u64, dofhp_dof: buf.as_ptr() as u64, + #[cfg(target_os = "freebsd")] + dofhp_pid: std::process::id() as i32, + #[cfg(target_os = "freebsd")] + dofhp_gen: 0 }; let data = &helper as *const _; + #[cfg(target_os = "illumos")] let cmd: i32 = 0x64746803; + #[cfg(target_os = "freebsd")] + let cmd: u64 = { + const DOFHELPER_SIZE: u64 = std::mem::size_of::() as u64; + const IOCPARM_SHIFT: u64 = 13; + const IOCPARM_MASK: u64 = (1 << IOCPARM_SHIFT) - 1; + const IOC_INOUT: u64 = 0xc0000000; + IOC_INOUT | ((DOFHELPER_SIZE & IOCPARM_MASK) << 16) | 0x7a << 8 | 0x3 + }; + let file = OpenOptions::new() .read(true) .write(true) From 700ca3d81ff0dccc8573db7e641449aa12abd930 Mon Sep 17 00:00:00 2001 From: michael-yuji Date: Mon, 6 Jun 2022 18:27:48 +0800 Subject: [PATCH 03/11] FreeBSD: support nightly and stable(link-dead-code) Provide a macro version of register_probes!() enables a explicit reference to the set_dtrace_probes section from the crate consumer, and prevent lld from gc the section. This trick along with the unstable "used(linker)" feature enable use on FreeBSD target with nightly toolchain without explicit "link-dead-code" Since the "used_with_arg" is available only on nightly, a patch is applied on usdt-impl/build.rs such that standard backend is enabled only for FreeBSD targets that are either on nightly or have link-dead-code explicitly passed to the linker. --- usdt-impl/build.rs | 46 +++++++++++++++++++++++++++++++++++++- usdt-impl/src/lib.rs | 1 + usdt-impl/src/no-linker.rs | 3 ++- usdt/src/lib.rs | 46 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 94 insertions(+), 2 deletions(-) diff --git a/usdt-impl/build.rs b/usdt-impl/build.rs index 5d26b71b..038c0fae 100644 --- a/usdt-impl/build.rs +++ b/usdt-impl/build.rs @@ -27,6 +27,23 @@ enum Backend { NoOp, } +// Extract any -C ARGS ... passed to RUSTFLAGS +fn extract_encoded_rustc_flags() -> Vec { + match env::var_os("CARGO_ENCODED_RUSTFLAGS").as_deref() { + Some(rustflags) => { + let mut atoms = rustflags.to_str().unwrap_or("").split(' '); + let mut flags = vec![]; + while let Some(atom) = atoms.next() { + if atom.starts_with("-C") { + flags.push(atom[2..].to_string()); + } + } + flags + } + _ => vec![] + } +} + fn main() { println!("cargo:rerun-if-changed=build.rs"); @@ -34,6 +51,7 @@ fn main() { let have_stable_asm = version_check::is_min_version("1.59").unwrap_or(false); // XXX: `asm_sym` feature is not yet stable let have_stable_asm_sym = false; + let have_stable_used_with_arg = false; // Are we being built with a compiler which allows feature flags (nightly) let is_nightly = version_check::is_feature_flaggable().unwrap_or(false); @@ -41,6 +59,14 @@ fn main() { let feat_asm = env::var_os("CARGO_FEATURE_ASM").is_some(); let feat_strict_asm = env::var_os("CARGO_FEATURE_STRICT_ASM").is_some(); + // Check if upstream have enabled link-dead-code, which in those cases we can + // enable standard backend for FreeBSD. We check this by finding the last + // -C link-dead-code* flag, and check if it is a negation of link-dead-code + let have_link_dead_code = extract_encoded_rustc_flags().iter() + .filter(|flag| flag.contains("link-dead-code")) + .map(|flag| !flag.contains("link-dead-code=n")) + .last().unwrap_or(false); + let backend = match env::var("CARGO_CFG_TARGET_OS").ok().as_deref() { Some("macos") if feat_asm => { if have_stable_asm && have_stable_asm_sym { @@ -57,7 +83,7 @@ fn main() { Backend::NoOp } } - Some("illumos") | Some("solaris") | Some("freebsd") if feat_asm => { + Some("illumos") | Some("solaris") if feat_asm => { if have_stable_asm { Backend::Standard } else if feat_strict_asm || is_nightly { @@ -67,6 +93,24 @@ fn main() { Backend::NoOp } } + Some("freebsd") if feat_asm => { + // FreeBSD require used(linker) to preserve __(start|stop)_set_dtrace_probes + // without explicit "link-dead-code" by consumer + if have_link_dead_code || have_stable_used_with_arg || is_nightly { + if !have_stable_used_with_arg && is_nightly { + println!("cargo:rustc-cfg=usdt_need_used_with_arg"); + } + if have_stable_asm { + Backend::Standard + } else if feat_strict_asm || is_nightly { + Backend::Standard + } else { + Backend::NoOp + } + } else { + Backend::NoOp + } + } _ => Backend::NoOp, }; diff --git a/usdt-impl/src/lib.rs b/usdt-impl/src/lib.rs index 54ba2afc..a47eb038 100644 --- a/usdt-impl/src/lib.rs +++ b/usdt-impl/src/lib.rs @@ -16,6 +16,7 @@ #![cfg_attr(usdt_need_feat_asm, feature(asm))] #![cfg_attr(usdt_need_feat_asm_sym, feature(asm_sym))] +#![cfg_attr(usdt_need_feat_used_with_arg, feature(used_with_arg))] use serde::Deserialize; use std::cell::RefCell; diff --git a/usdt-impl/src/no-linker.rs b/usdt-impl/src/no-linker.rs index 5d9bcd27..cbd1d632 100644 --- a/usdt-impl/src/no-linker.rs +++ b/usdt-impl/src/no-linker.rs @@ -132,7 +132,8 @@ fn extract_probe_records_from_section() -> Result { // here ensures this ends up in a mutable section, the same as the probe records. #[cfg(any(target_os = "illumos", target_os = "freebsd"))] #[link_section = "set_dtrace_probes"] - #[used] + #[cfg_attr(usdt_need_feat_used_with_arg, used(linker))] + #[cfg_attr(not(usdt_need_feat_used_with_arg), used)] static mut FORCE_LOAD: [u64; 0] = []; let data = unsafe { diff --git a/usdt/src/lib.rs b/usdt/src/lib.rs index 9fe20f83..938b5f82 100644 --- a/usdt/src/lib.rs +++ b/usdt/src/lib.rs @@ -447,6 +447,52 @@ impl Builder { /// concurrent situations. Probes will be registered at most once. /// /// [probe_test_macro]: https://github.com/oxidecomputer/usdt/tree/master/probe-test-macro +#[cfg(not(target_os = "freebsd"))] pub fn register_probes() -> Result<(), Error> { usdt_impl::register_probes().map_err(Error::from) } + +// Hide the actual register_probes from FreeBSD consumer to prevent accidental use of the original +// register_probes(), however since the register_probes!() macro still need visibility to a pub fn +// thet perform the same functionality. +pub fn usdt_internal_register_probes() -> Result<(), Error> { + usdt_impl::register_probes().map_err(Error::from) +} + +/// Register an application's probes with DTrace. +/// +/// This function collects the probes defined in an application, and forwards them to the DTrace +/// kernel module. This _must_ be done for the probes to be visible via the `dtrace(1)` tool. See +/// [probe_test_macro] for a detailed example. +/// +/// Notes +/// ----- +/// +/// This function registers all probes in a process's binary image, regardless of which crate +/// actually defines the probes. It's also safe to call this function multiple times, even in +/// concurrent situations. Probes will be registered at most once. +/// +/// [probe_test_macro]: https://github.com/oxidecomputer/usdt/tree/master/probe-test-macro +// The lld linker, which used by FreeBSD implements a mark and sweep gc that wipes out symbols +// that it thinks cannot be reached. This includes the set_dtrace_probes section that stores the +// compiled probes. In order to combat this issue, a macro can be use to "paste" the snipper that +// contains references to the `set_dtrace_probes` section. In order to make the reference not +// throw away by the compiler, we need to make an offer that rustc cannot refuse (to not optmize). +// We accomplish this by using an extern function that likely to always exist (memcmp) and yet +// since rustc do not have visibility to the implementation, it cannot be optimized. +#[cfg(any(target_os = "freebsd", target_os = "illumos"))] +#[macro_export] +macro_rules! register_probes { + () => { + { + extern "C" { + fn memcmp(d: *mut u64, c: *mut u64, n: usize); + } + + #[link_section = "set_dtrace_probes"] + static mut dummy: [u64; 0] = []; + unsafe { memcmp(dummy.as_mut_ptr(), 0 as *mut u64, 0); } + usdt_internal_register_probes() + } + }; +} From d5c1701314cfe64e4f85533cd9e1914dbe891819 Mon Sep 17 00:00:00 2001 From: michael-yuji Date: Tue, 7 Jun 2022 01:54:55 +0800 Subject: [PATCH 04/11] simplify link-dead-code check --- usdt-impl/build.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/usdt-impl/build.rs b/usdt-impl/build.rs index 038c0fae..2e96eac7 100644 --- a/usdt-impl/build.rs +++ b/usdt-impl/build.rs @@ -27,20 +27,20 @@ enum Backend { NoOp, } -// Extract any -C ARGS ... passed to RUSTFLAGS -fn extract_encoded_rustc_flags() -> Vec { +fn have_link_dead_code_check() -> bool { match env::var_os("CARGO_ENCODED_RUSTFLAGS").as_deref() { Some(rustflags) => { let mut atoms = rustflags.to_str().unwrap_or("").split(' '); - let mut flags = vec![]; + let mut link_dead_code = false; + // check if the last link-dead-code is n or no while let Some(atom) = atoms.next() { - if atom.starts_with("-C") { - flags.push(atom[2..].to_string()); + if atom.starts_with("-C") && atom.contains("link-dead-code") { + link_dead_code = !atom.contains("link-dead-code=n") } } - flags + link_dead_code } - _ => vec![] + _ => false } } @@ -62,10 +62,7 @@ fn main() { // Check if upstream have enabled link-dead-code, which in those cases we can // enable standard backend for FreeBSD. We check this by finding the last // -C link-dead-code* flag, and check if it is a negation of link-dead-code - let have_link_dead_code = extract_encoded_rustc_flags().iter() - .filter(|flag| flag.contains("link-dead-code")) - .map(|flag| !flag.contains("link-dead-code=n")) - .last().unwrap_or(false); + let have_link_dead_code = have_link_dead_code_check(); let backend = match env::var("CARGO_CFG_TARGET_OS").ok().as_deref() { Some("macos") if feat_asm => { From b9910f1368192598829a6a1d17dcdaffe7be2ba5 Mon Sep 17 00:00:00 2001 From: michael-yuji Date: Mon, 6 Jun 2022 17:24:00 -0700 Subject: [PATCH 05/11] Remove register_probes! macro, and use R flag for sections instead Turns out we do not need all the hacks to get FreeBSD linker working. GNU and llvm both support the "R" flag for sections, which prevent them from gc. This make the ugly register_probes! hack obsolete so it can be removed. No one deserve to have it in their codebase. https://lists.freebsd.org/archives/freebsd-hackers/2022-June/001191.html --- usdt-impl/src/record.rs | 3 +++ usdt/src/lib.rs | 46 ----------------------------------------- 2 files changed, 3 insertions(+), 46 deletions(-) diff --git a/usdt-impl/src/record.rs b/usdt-impl/src/record.rs index e3cf0e00..103c419d 100644 --- a/usdt-impl/src/record.rs +++ b/usdt-impl/src/record.rs @@ -218,7 +218,10 @@ impl<'a> ReadCstrExt<'a> for &'a [u8] { // Construct the ASM record for a probe. If `types` is `None`, then is is an is-enabled probe. #[allow(dead_code)] pub(crate) fn emit_probe_record(prov: &str, probe: &str, types: Option<&[DataType]>) -> String { + #[cfg(not(target_os = "freebsd"))] let section_ident = r#"set_dtrace_probes,"aw","progbits""#; + #[cfg(target_os = "freebsd")] + let section_ident = r#"set_dtrace_probes,"awR","progbits""#; let is_enabled = types.is_none(); let n_args = types.map_or(0, |typ| typ.len()); let arguments = types.map_or_else(String::new, |types| { diff --git a/usdt/src/lib.rs b/usdt/src/lib.rs index 938b5f82..9fe20f83 100644 --- a/usdt/src/lib.rs +++ b/usdt/src/lib.rs @@ -447,52 +447,6 @@ impl Builder { /// concurrent situations. Probes will be registered at most once. /// /// [probe_test_macro]: https://github.com/oxidecomputer/usdt/tree/master/probe-test-macro -#[cfg(not(target_os = "freebsd"))] pub fn register_probes() -> Result<(), Error> { usdt_impl::register_probes().map_err(Error::from) } - -// Hide the actual register_probes from FreeBSD consumer to prevent accidental use of the original -// register_probes(), however since the register_probes!() macro still need visibility to a pub fn -// thet perform the same functionality. -pub fn usdt_internal_register_probes() -> Result<(), Error> { - usdt_impl::register_probes().map_err(Error::from) -} - -/// Register an application's probes with DTrace. -/// -/// This function collects the probes defined in an application, and forwards them to the DTrace -/// kernel module. This _must_ be done for the probes to be visible via the `dtrace(1)` tool. See -/// [probe_test_macro] for a detailed example. -/// -/// Notes -/// ----- -/// -/// This function registers all probes in a process's binary image, regardless of which crate -/// actually defines the probes. It's also safe to call this function multiple times, even in -/// concurrent situations. Probes will be registered at most once. -/// -/// [probe_test_macro]: https://github.com/oxidecomputer/usdt/tree/master/probe-test-macro -// The lld linker, which used by FreeBSD implements a mark and sweep gc that wipes out symbols -// that it thinks cannot be reached. This includes the set_dtrace_probes section that stores the -// compiled probes. In order to combat this issue, a macro can be use to "paste" the snipper that -// contains references to the `set_dtrace_probes` section. In order to make the reference not -// throw away by the compiler, we need to make an offer that rustc cannot refuse (to not optmize). -// We accomplish this by using an extern function that likely to always exist (memcmp) and yet -// since rustc do not have visibility to the implementation, it cannot be optimized. -#[cfg(any(target_os = "freebsd", target_os = "illumos"))] -#[macro_export] -macro_rules! register_probes { - () => { - { - extern "C" { - fn memcmp(d: *mut u64, c: *mut u64, n: usize); - } - - #[link_section = "set_dtrace_probes"] - static mut dummy: [u64; 0] = []; - unsafe { memcmp(dummy.as_mut_ptr(), 0 as *mut u64, 0); } - usdt_internal_register_probes() - } - }; -} From d9cd365a94856959073a53dd853002268ecf5eee Mon Sep 17 00:00:00 2001 From: michael-yuji Date: Wed, 8 Jun 2022 00:06:57 -0700 Subject: [PATCH 06/11] Tidy up dli_sname NULL check and use constant for FreeBSD ioctl --- usdt-impl/build.rs | 2 +- usdt-impl/src/no-linker.rs | 8 +------- usdt-impl/src/record.rs | 16 +++++++--------- 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/usdt-impl/build.rs b/usdt-impl/build.rs index 2e96eac7..e3bea06a 100644 --- a/usdt-impl/build.rs +++ b/usdt-impl/build.rs @@ -95,7 +95,7 @@ fn main() { // without explicit "link-dead-code" by consumer if have_link_dead_code || have_stable_used_with_arg || is_nightly { if !have_stable_used_with_arg && is_nightly { - println!("cargo:rustc-cfg=usdt_need_used_with_arg"); + println!("cargo:rustc-cfg=usdt_need_feat_used_with_arg"); } if have_stable_asm { Backend::Standard diff --git a/usdt-impl/src/no-linker.rs b/usdt-impl/src/no-linker.rs index cbd1d632..b28570b6 100644 --- a/usdt-impl/src/no-linker.rs +++ b/usdt-impl/src/no-linker.rs @@ -180,13 +180,7 @@ fn ioctl_section(buf: &[u8], modname: [std::os::raw::c_char; 64]) -> Result<(), #[cfg(target_os = "illumos")] let cmd: i32 = 0x64746803; #[cfg(target_os = "freebsd")] - let cmd: u64 = { - const DOFHELPER_SIZE: u64 = std::mem::size_of::() as u64; - const IOCPARM_SHIFT: u64 = 13; - const IOCPARM_MASK: u64 = (1 << IOCPARM_SHIFT) - 1; - const IOC_INOUT: u64 = 0xc0000000; - IOC_INOUT | ((DOFHELPER_SIZE & IOCPARM_MASK) << 16) | 0x7a << 8 | 0x3 - }; + let cmd: u64 = 0xc0587a03; let file = OpenOptions::new() .read(true) diff --git a/usdt-impl/src/record.rs b/usdt-impl/src/record.rs index 103c419d..dfc0a710 100644 --- a/usdt-impl/src/record.rs +++ b/usdt-impl/src/record.rs @@ -71,15 +71,13 @@ pub(crate) fn addr_to_info(addr: u64) -> (Option, Option) { if libc::dladdr(addr as *const c_void, &mut info as *mut _) == 0 { (None, None) } else { - unsafe fn to_str_if_not_null(ptr: *const std::os::raw::c_char) -> Option { - if ptr == null() { - None - } else { - Some(CStr::from_ptr(ptr).to_string_lossy().to_string()) - } - } - - (to_str_if_not_null(info.dli_sname), to_str_if_not_null(info.dli_fname)) + // On some non Illumos platfroms dli_sname can be NULL + let dli_sname = if info.dli_sname == null() { + None + } else { + Some(CStr::from_ptr(info.dli_sname).to_string_lossy().to_string()) + }; + (dli_sname, Some(CStr::from_ptr(info.dli_fname).to_string_lossy().to_string())) } } } From 65f1bfd12d7c73e053d32e9aeddc4f8ea7685f05 Mon Sep 17 00:00:00 2001 From: michael-yuji Date: Wed, 8 Jun 2022 10:10:32 -0700 Subject: [PATCH 07/11] Add the FreeBSD situation to the comment of FORCE_LOAD --- usdt-impl/src/no-linker.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/usdt-impl/src/no-linker.rs b/usdt-impl/src/no-linker.rs index b28570b6..a4021930 100644 --- a/usdt-impl/src/no-linker.rs +++ b/usdt-impl/src/no-linker.rs @@ -123,8 +123,8 @@ fn extract_probe_records_from_section() -> Result { static dtrace_probes_stop: usize; } - // Without this the illumos linker may decide to omit the symbols above that - // denote the start and stop addresses for this section. Note that the variable + // Without this the illumos and FreeBSD linker may decide to omit the symbols above + // that denote the start and stop addresses for this section. Note that the variable // must be mutable, otherwise this will generate a read-only section with the // name `set_dtrace_probes`. The section containing the actual probe records is // writable (to implement one-time registration), so an immutable variable here From f66766b8b907b512bca7806b4e838a81d697e486 Mon Sep 17 00:00:00 2001 From: michael-yuji Date: Thu, 9 Jun 2022 11:41:01 -0700 Subject: [PATCH 08/11] Use libexecinfo to determine dli_sname and dli_fname on FreeBSD FreeBSD dladdr(3M) examine dynamic symbol table only hence never give use the mangled symbol. Instead, we use `backtrace_symbols_fmt` to give us the symbol. Handle backtrace_symbols_fmt and ffi a bit more carefully --- usdt-impl/src/record.rs | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/usdt-impl/src/record.rs b/usdt-impl/src/record.rs index dfc0a710..3eed1891 100644 --- a/usdt-impl/src/record.rs +++ b/usdt-impl/src/record.rs @@ -60,6 +60,7 @@ pub fn process_section(mut data: &[u8]) -> Result { } // Convert an address in an object file into a function and file name, if possible. +#[cfg(not(target_os = "freebsd"))] pub(crate) fn addr_to_info(addr: u64) -> (Option, Option) { unsafe { let mut info = Dl_info { @@ -82,6 +83,38 @@ pub(crate) fn addr_to_info(addr: u64) -> (Option, Option) { } } +// On FreeBSD, dladdr(3M) only examines the dynamic symbol table. Which is pretty useless as it +// will always gives null dli_sname. To workaround this issue, we use `backtrace_symbols_fmt` from +// libexecinfo, which internally lookup in the executable to determine the symbol of the given +// address +#[cfg(target_os = "freebsd")] +pub(crate) fn addr_to_info(addr: u64) -> (Option, Option) { + unsafe { + // The libc crate does not have `backtrace_symbos_fmt` + #[link(name = "execinfo")] + extern "C" { + pub fn backtrace_symbols_fmt(_: *const *mut c_void, _: libc::size_t, _: *const libc::c_char) + -> *mut *mut libc::c_char; + } + + let addrs = [addr].as_ptr() as *const *mut c_void; + + // Use \n as a seperator for dli_sname(%n) and dli_fname(%f), we put one more \n to the end + // to ensure s.lines() (see below) always contains two elements + let format = std::ffi::CString::new("%n\n%f\n").unwrap(); + let symbols = backtrace_symbols_fmt(addrs, 1, format.as_ptr()); + + if symbols == null_mut() { + (None, None) + } else { + let s = CStr::from_ptr(*symbols).to_string_lossy().to_string(); + let lines: Vec<_> = s.lines().collect(); + (Some(lines[0].to_string()), Some(lines[1].to_string())) + } + } +} + + // Limit a string to the DTrace-imposed maxima. Note that this ensures a null-terminated C string // result, i.e., the actual string is of length `limit - 1`. // See dtrace.h, From 0cadb0f3ab92c2e522fb411e32b72371273acd77 Mon Sep 17 00:00:00 2001 From: michael-yuji Date: Fri, 24 Jun 2022 01:48:25 +0800 Subject: [PATCH 09/11] fix formatting --- dof/src/dof_bindings.rs | 2 +- usdt-impl/build.rs | 44 +++++++++++++++++++------------------- usdt-impl/src/no-linker.rs | 2 +- usdt-impl/src/record.rs | 15 ++++++++----- 4 files changed, 34 insertions(+), 29 deletions(-) diff --git a/dof/src/dof_bindings.rs b/dof/src/dof_bindings.rs index ea4dc48c..33846c41 100644 --- a/dof/src/dof_bindings.rs +++ b/dof/src/dof_bindings.rs @@ -449,7 +449,7 @@ pub struct dof_helper { #[cfg(target_os = "freebsd")] pub dofhp_pid: i32, #[cfg(target_os = "freebsd")] - pub dofhp_gen: ::std::os::raw::c_int + pub dofhp_gen: ::std::os::raw::c_int, } impl Default for dof_helper { fn default() -> Self { diff --git a/usdt-impl/build.rs b/usdt-impl/build.rs index e3bea06a..9a438ac4 100644 --- a/usdt-impl/build.rs +++ b/usdt-impl/build.rs @@ -29,18 +29,18 @@ enum Backend { fn have_link_dead_code_check() -> bool { match env::var_os("CARGO_ENCODED_RUSTFLAGS").as_deref() { - Some(rustflags) => { - let mut atoms = rustflags.to_str().unwrap_or("").split(' '); - let mut link_dead_code = false; - // check if the last link-dead-code is n or no - while let Some(atom) = atoms.next() { - if atom.starts_with("-C") && atom.contains("link-dead-code") { - link_dead_code = !atom.contains("link-dead-code=n") - } - } - link_dead_code - } - _ => false + Some(rustflags) => { + let mut atoms = rustflags.to_str().unwrap_or("").split(' '); + let mut link_dead_code = false; + // check if the last link-dead-code is n or no + while let Some(atom) = atoms.next() { + if atom.starts_with("-C") && atom.contains("link-dead-code") { + link_dead_code = !atom.contains("link-dead-code=n") + } + } + link_dead_code + } + _ => false, } } @@ -94,16 +94,16 @@ fn main() { // FreeBSD require used(linker) to preserve __(start|stop)_set_dtrace_probes // without explicit "link-dead-code" by consumer if have_link_dead_code || have_stable_used_with_arg || is_nightly { - if !have_stable_used_with_arg && is_nightly { - println!("cargo:rustc-cfg=usdt_need_feat_used_with_arg"); - } - if have_stable_asm { - Backend::Standard - } else if feat_strict_asm || is_nightly { - Backend::Standard - } else { - Backend::NoOp - } + if !have_stable_used_with_arg && is_nightly { + println!("cargo:rustc-cfg=usdt_need_feat_used_with_arg"); + } + if have_stable_asm { + Backend::Standard + } else if feat_strict_asm || is_nightly { + Backend::Standard + } else { + Backend::NoOp + } } else { Backend::NoOp } diff --git a/usdt-impl/src/no-linker.rs b/usdt-impl/src/no-linker.rs index a4021930..648257b5 100644 --- a/usdt-impl/src/no-linker.rs +++ b/usdt-impl/src/no-linker.rs @@ -174,7 +174,7 @@ fn ioctl_section(buf: &[u8], modname: [std::os::raw::c_char; 64]) -> Result<(), #[cfg(target_os = "freebsd")] dofhp_pid: std::process::id() as i32, #[cfg(target_os = "freebsd")] - dofhp_gen: 0 + dofhp_gen: 0, }; let data = &helper as *const _; #[cfg(target_os = "illumos")] diff --git a/usdt-impl/src/record.rs b/usdt-impl/src/record.rs index 3eed1891..27a7c97e 100644 --- a/usdt-impl/src/record.rs +++ b/usdt-impl/src/record.rs @@ -78,7 +78,10 @@ pub(crate) fn addr_to_info(addr: u64) -> (Option, Option) { } else { Some(CStr::from_ptr(info.dli_sname).to_string_lossy().to_string()) }; - (dli_sname, Some(CStr::from_ptr(info.dli_fname).to_string_lossy().to_string())) + ( + dli_sname, + Some(CStr::from_ptr(info.dli_fname).to_string_lossy().to_string()), + ) } } } @@ -93,15 +96,18 @@ pub(crate) fn addr_to_info(addr: u64) -> (Option, Option) { // The libc crate does not have `backtrace_symbos_fmt` #[link(name = "execinfo")] extern "C" { - pub fn backtrace_symbols_fmt(_: *const *mut c_void, _: libc::size_t, _: *const libc::c_char) - -> *mut *mut libc::c_char; + pub fn backtrace_symbols_fmt( + _: *const *mut c_void, + _: libc::size_t, + _: *const libc::c_char, + ) -> *mut *mut libc::c_char; } let addrs = [addr].as_ptr() as *const *mut c_void; // Use \n as a seperator for dli_sname(%n) and dli_fname(%f), we put one more \n to the end // to ensure s.lines() (see below) always contains two elements - let format = std::ffi::CString::new("%n\n%f\n").unwrap(); + let format = std::ffi::CString::new("%n\n%f\n").unwrap(); let symbols = backtrace_symbols_fmt(addrs, 1, format.as_ptr()); if symbols == null_mut() { @@ -114,7 +120,6 @@ pub(crate) fn addr_to_info(addr: u64) -> (Option, Option) { } } - // Limit a string to the DTrace-imposed maxima. Note that this ensures a null-terminated C string // result, i.e., the actual string is of length `limit - 1`. // See dtrace.h, From 2f0774d1add70573377082996ed19197c9828a6a Mon Sep 17 00:00:00 2001 From: yuuji Date: Sat, 19 Nov 2022 09:54:08 +0000 Subject: [PATCH 10/11] Cleanup typos and nits --- usdt-impl/src/record.rs | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/usdt-impl/src/record.rs b/usdt-impl/src/record.rs index 27a7c97e..b9ad2dcc 100644 --- a/usdt-impl/src/record.rs +++ b/usdt-impl/src/record.rs @@ -18,6 +18,7 @@ use std::{ collections::BTreeMap, ffi::CStr, + ffi::CString, ptr::{null, null_mut}, }; @@ -72,7 +73,7 @@ pub(crate) fn addr_to_info(addr: u64) -> (Option, Option) { if libc::dladdr(addr as *const c_void, &mut info as *mut _) == 0 { (None, None) } else { - // On some non Illumos platfroms dli_sname can be NULL + // On some non Illumos platforms dli_sname can be NULL let dli_sname = if info.dli_sname == null() { None } else { @@ -87,13 +88,12 @@ pub(crate) fn addr_to_info(addr: u64) -> (Option, Option) { } // On FreeBSD, dladdr(3M) only examines the dynamic symbol table. Which is pretty useless as it -// will always gives null dli_sname. To workaround this issue, we use `backtrace_symbols_fmt` from -// libexecinfo, which internally lookup in the executable to determine the symbol of the given +// will always returns a dli_sname. To workaround this issue, we use `backtrace_symbols_fmt` from +// libexecinfo, which internally looks in the executable to determine the symbol of the given // address #[cfg(target_os = "freebsd")] pub(crate) fn addr_to_info(addr: u64) -> (Option, Option) { unsafe { - // The libc crate does not have `backtrace_symbos_fmt` #[link(name = "execinfo")] extern "C" { pub fn backtrace_symbols_fmt( @@ -103,19 +103,22 @@ pub(crate) fn addr_to_info(addr: u64) -> (Option, Option) { ) -> *mut *mut libc::c_char; } - let addrs = [addr].as_ptr() as *const *mut c_void; + let addrs_arr = [addr]; + let addrs = addrs_arr.as_ptr() as *const *mut c_void; // Use \n as a seperator for dli_sname(%n) and dli_fname(%f), we put one more \n to the end // to ensure s.lines() (see below) always contains two elements - let format = std::ffi::CString::new("%n\n%f\n").unwrap(); + let format = CString::new("%n\n%f").unwrap(); let symbols = backtrace_symbols_fmt(addrs, 1, format.as_ptr()); - if symbols == null_mut() { - (None, None) + if symbols != null_mut() { + if let Some((sname, fname)) = CStr::from_ptr(*symbols).to_string_lossy().split_once('\n') { + (Some(sname.to_string()), Some(fname.to_string())) + } else { + (None, None) + } } else { - let s = CStr::from_ptr(*symbols).to_string_lossy().to_string(); - let lines: Vec<_> = s.lines().collect(); - (Some(lines[0].to_string()), Some(lines[1].to_string())) + (None, None) } } } From 3a3f63b612011301b4780c568a65d79a2779a3aa Mon Sep 17 00:00:00 2001 From: "nyan@myuji.xyz" Date: Fri, 3 Mar 2023 08:05:14 -0800 Subject: [PATCH 11/11] style fixes from cargo fmt --- usdt-impl/src/record.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/usdt-impl/src/record.rs b/usdt-impl/src/record.rs index 13ee2e92..c48a9604 100644 --- a/usdt-impl/src/record.rs +++ b/usdt-impl/src/record.rs @@ -109,7 +109,9 @@ pub(crate) fn addr_to_info(addr: u64) -> (Option, Option) { let symbols = backtrace_symbols_fmt(addrs, 1, format.as_ptr()); if symbols != null_mut() { - if let Some((sname, fname)) = CStr::from_ptr(*symbols).to_string_lossy().split_once('\n') { + if let Some((sname, fname)) = + CStr::from_ptr(*symbols).to_string_lossy().split_once('\n') + { (Some(sname.to_string()), Some(fname.to_string())) } else { (None, None)