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

Add FreeBSD support #63

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions dof/src/dof_bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
bnaecker marked this conversation as resolved.
Show resolved Hide resolved
#[cfg(target_os = "freebsd")]
pub dofhp_gen: ::std::os::raw::c_int,
}
impl Default for dof_helper {
fn default() -> Self {
Expand Down
41 changes: 41 additions & 0 deletions usdt-impl/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,29 @@ enum Backend {
NoOp,
}

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,
}
}

fn main() {
println!("cargo:rerun-if-changed=build.rs");

// `asm` feature was stabilized in 1.59
let have_stable_asm = version_check::is_min_version("1.59").unwrap_or(false);
let have_stable_used_with_arg = false;
// `asm_sym` feature was stabilized in 1.66
let have_stable_asm_sym = version_check::is_min_version("1.66").unwrap_or(false);

Expand All @@ -41,6 +59,11 @@ 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 = have_link_dead_code_check();

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 {
Expand All @@ -67,6 +90,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_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
}
}
_ => {
if !have_stable_asm {
println!("cargo:rustc-cfg=usdt_need_feat_asm");
Expand Down
1 change: 1 addition & 0 deletions usdt-impl/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm. I don't know if I realized it before, but this feature is not stable. We've recently been able to support a stable compiler on all platforms, and it would be very nice to keep that. I'm not sure that's possible, so FreeBSD might be the hold-out requiring nightly.

Are you certain that the link-dead-code linker-flag is not sufficient by itself to keep the FreeBSD linker from removing the start/stop symbols?

Copy link
Author

@michael-yuji michael-yuji Mar 3, 2023

Choose a reason for hiding this comment

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

With stable compiler and link-dead-code, it kinda works empirically under the limited cases I have tried but not that I can comfortably conclude it works (especially when it's failing tests on wrong mangled function name).

Apart from the correctness issue, It also require consumer of the crate to add the linker flag to make it useful.

😞

(edited: attaching the failed test case)


     Running unittests src/main.rs (target/debug/deps/does_it_work-d0fe3ba398374246)

running 1 test
test tests::test_does_it_work ... FAILED

failures:

---- tests::test_does_it_work stdout ----
   ID   PROVIDER            MODULE                          FUNCTION NAME
89108 doesit2045 does_it_work-d0fe3ba398374246 _ZN12does_it_work4main17hfd4a5f4c02c325b6E work

	Probe Description Attributes
		Identifier Names: Internal
		Data Semantics:   Internal
		Dependency Class: Unknown

	Argument Attributes
		Identifier Names: Internal
		Data Semantics:   Internal
		Dependency Class: Unknown

	Argument Types
		args[0]: uint8_t
		args[1]: char *


thread 'tests::test_does_it_work' panicked at 'Mangled function name appears incorrect: _ZN12does_it_work4main17hfd4a5f4c02c325b6E', tests/does-it-work/src/main.rs:94:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    tests::test_does_it_work

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.26s

error: test failed, to rerun pass `-p does-it-work --bin does-it-work`


use serde::Deserialize;
use std::cell::RefCell;
Expand Down
17 changes: 13 additions & 4 deletions usdt-impl/src/no-linker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,16 +123,17 @@ fn extract_probe_records_from_section() -> Result<Section, crate::Error> {
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
bnaecker marked this conversation as resolved.
Show resolved Hide resolved
// 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"]
bnaecker marked this conversation as resolved.
Show resolved Hide resolved
#[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 {
Expand Down Expand Up @@ -170,9 +171,17 @@ 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 = 0xc0587a03;

let file = OpenOptions::new()
.read(true)
.write(true)
Expand Down
51 changes: 50 additions & 1 deletion usdt-impl/src/record.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use std::sync::atomic::Ordering;
use std::{
collections::BTreeMap,
ffi::CStr,
ffi::CString,
ptr::{null, null_mut},
};

Expand Down Expand Up @@ -57,6 +58,7 @@ pub fn process_section(mut data: &mut [u8], register: bool) -> Result<Section, c
}

// 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<String>, Option<String>) {
unsafe {
let mut info = Dl_info {
Expand All @@ -68,14 +70,58 @@ pub(crate) fn addr_to_info(addr: u64) -> (Option<String>, Option<String>) {
if libc::dladdr(addr as *const c_void, &mut info as *mut _) == 0 {
(None, None)
} else {
// On some non Illumos platforms 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())
};
(
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()),
)
}
}
}

// On FreeBSD, dladdr(3M) only examines the dynamic symbol table. Which is pretty useless as it
// 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<String>, Option<String>) {
unsafe {
#[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_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
bnaecker marked this conversation as resolved.
Show resolved Hide resolved
let format = CString::new("%n\n%f").unwrap();
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')
{
(Some(sname.to_string()), Some(fname.to_string()))
} else {
(None, None)
}
} else {
(None, None)
}
}
}

// 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,
Expand Down Expand Up @@ -212,7 +258,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""#;
bnaecker marked this conversation as resolved.
Show resolved Hide resolved
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| {
Expand Down