Skip to content

Commit f85a1a0

Browse files
committed
dlsym to load sidecar symbols not necessary any+
1 parent 3fbbd84 commit f85a1a0

File tree

18 files changed

+292
-587
lines changed

18 files changed

+292
-587
lines changed

Makefile

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ RUN_TESTS_CMD := DD_SERVICE= DD_ENV= REPORT_EXIT_STATUS=1 TEST_PHP_SRCDIR=$(PROJ
5151

5252
C_FILES = $(shell find components components-rs ext src/dogstatsd zend_abstract_interface -name '*.c' -o -name '*.h' | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' )
5353
TEST_FILES = $(shell find tests/ext -name '*.php*' -o -name '*.inc' -o -name '*.json' -o -name '*.yaml' -o -name 'CONFLICTS' | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' )
54-
RUST_FILES = $(BUILD_DIR)/Cargo.toml $(BUILD_DIR)/Cargo.lock $(shell find components-rs -name '*.c' -o -name '*.rs' -o -name 'Cargo.toml' | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' ) $(shell find libdatadog/{build-common,datadog-ffe,datadog-ipc,datadog-ipc-macros,datadog-live-debugger,datadog-live-debugger-ffi,datadog-remote-config,datadog-sidecar,datadog-sidecar-ffi,datadog-sidecar-macros,libdd-alloc,libdd-common,libdd-common-ffi,libdd-crashtracker,libdd-crashtracker-ffi,libdd-data-pipeline,libdd-ddsketch,libdd-dogstatsd-client,libdd-library-config,libdd-library-config-ffi,libdd-log,libdd-telemetry,libdd-telemetry-ffi,libdd-tinybytes,libdd-trace-*,spawn_worker,tools/cc_utils,libdd-trace-*,Cargo.toml} \( -type l -o -type f \) \( -path "*/src*" -o -path "*/examples*" -o -path "*Cargo.toml" -o -path "*/build.rs" -o -path "*/tests/dataservice.rs" -o -path "*/tests/service_functional.rs" \) -not -path "*/datadog-ipc/build.rs" -not -path "*/datadog-sidecar-ffi/build.rs")
54+
RUST_FILES = $(BUILD_DIR)/Cargo.toml $(BUILD_DIR)/Cargo.lock $(shell find components-rs -name '*.c' -o -name '*.rs' -o -name 'Cargo.toml' | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' ) $(shell find libdatadog/{build-common,datadog-ffe,datadog-ipc,datadog-ipc-macros,datadog-live-debugger,datadog-live-debugger-ffi,datadog-remote-config,datadog-sidecar,datadog-sidecar-appsec-ffi,datadog-sidecar-ffi,datadog-sidecar-macros,libdd-alloc,libdd-common,libdd-common-ffi,libdd-crashtracker,libdd-crashtracker-ffi,libdd-data-pipeline,libdd-ddsketch,libdd-dogstatsd-client,libdd-library-config,libdd-library-config-ffi,libdd-log,libdd-telemetry,libdd-telemetry-ffi,libdd-tinybytes,libdd-trace-*,spawn_worker,tools/cc_utils,libdd-trace-*,Cargo.toml} \( -type l -o -type f \) \( -path "*/src*" -o -path "*/examples*" -o -path "*Cargo.toml" -o -path "*/build.rs" -o -path "*/tests/dataservice.rs" -o -path "*/tests/service_functional.rs" \) -not -path "*/datadog-ipc/build.rs" -not -path "*/datadog-sidecar-ffi/build.rs")
5555
ALL_OBJECT_FILES = $(C_FILES) $(RUST_FILES) $(BUILD_DIR)/Makefile
5656
TEST_OPCACHE_FILES = $(shell find tests/opcache -name '*.php*' -o -name '.gitkeep' | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' )
5757
TEST_STUB_FILES = $(shell find tests/ext -type d -name 'stubs' -exec find '{}' -type f \; | awk '{ printf "$(BUILD_DIR)/%s\n", $$1 }' )
@@ -426,7 +426,7 @@ clang_format_fix:
426426
cbindgen: remove_cbindgen generate_cbindgen
427427

428428
remove_cbindgen:
429-
rm -f components-rs/ddtrace.h components-rs/live-debugger.h components-rs/telemetry.h components-rs/sidecar.h components-rs/common.h components-rs/crashtracker.h components-rs/library-config.h
429+
rm -f components-rs/ddtrace.h components-rs/live-debugger.h components-rs/telemetry.h components-rs/sidecar.h components-rs/sidecar-appsec.h components-rs/common.h components-rs/crashtracker.h components-rs/library-config.h
430430

431431
generate_cbindgen: cbindgen_binary # Regenerate components-rs/ddtrace.h components-rs/live-debugger.h components-rs/telemetry.h components-rs/sidecar.h components-rs/common.h components-rs/crashtracker.h components-rs/library-config.h
432432
( \
@@ -446,6 +446,9 @@ generate_cbindgen: cbindgen_binary # Regenerate components-rs/ddtrace.h componen
446446
$(command rustup && echo run nightly --) cbindgen --crate datadog-sidecar-ffi \
447447
--config datadog-sidecar-ffi/cbindgen.toml \
448448
--output $(PROJECT_ROOT)/components-rs/sidecar.h; \
449+
$(command rustup && echo run nightly --) cbindgen --crate datadog-sidecar-appsec-ffi \
450+
--config datadog-sidecar-appsec-ffi/cbindgen.toml \
451+
--output $(PROJECT_ROOT)/components-rs/sidecar-appsec.h; \
449452
$(command rustup && echo run nightly --) cbindgen --crate libdd-crashtracker-ffi \
450453
--config libdd-crashtracker-ffi/cbindgen.toml \
451454
--output $(PROJECT_ROOT)/components-rs/crashtracker.h; \
@@ -456,7 +459,7 @@ generate_cbindgen: cbindgen_binary # Regenerate components-rs/ddtrace.h componen
456459
mkdir -pv "$(BUILD_DIR)"; \
457460
export CARGO_TARGET_DIR="$(BUILD_DIR)/target"; \
458461
fi; \
459-
cargo run -p tools --bin dedup_headers -- $(PROJECT_ROOT)/components-rs/common.h $(PROJECT_ROOT)/components-rs/ddtrace.h $(PROJECT_ROOT)/components-rs/live-debugger.h $(PROJECT_ROOT)/components-rs/telemetry.h $(PROJECT_ROOT)/components-rs/sidecar.h $(PROJECT_ROOT)/components-rs/crashtracker.h $(PROJECT_ROOT)/components-rs/library-config.h \
462+
cargo run -p tools --bin dedup_headers -- $(PROJECT_ROOT)/components-rs/common.h $(PROJECT_ROOT)/components-rs/ddtrace.h $(PROJECT_ROOT)/components-rs/live-debugger.h $(PROJECT_ROOT)/components-rs/telemetry.h $(PROJECT_ROOT)/components-rs/sidecar.h $(PROJECT_ROOT)/components-rs/sidecar-appsec.h $(PROJECT_ROOT)/components-rs/crashtracker.h $(PROJECT_ROOT)/components-rs/library-config.h \
460463
)
461464

462465
cbindgen_binary:

appsec/helper-rust/build.rs

Lines changed: 5 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,13 @@ fn main() {
5252
// in the same directory as the binary/library
5353
if target.contains("linux") {
5454
println!("cargo::rustc-link-arg=-Wl,-rpath,$ORIGIN");
55+
// Allow symbols to be resolved from the parent process at dlopen time.
56+
println!("cargo::rustc-link-arg=-Wl,--allow-shlib-undefined");
5557
} else if target.contains("darwin") || target.contains("apple") {
5658
println!("cargo::rustc-link-arg=-Wl,-rpath,@loader_path");
59+
// Allow undefined symbols to be resolved at dlopen time from the parent process.
60+
println!("cargo::rustc-link-arg=-undefined");
61+
println!("cargo::rustc-link-arg=dynamic_lookup");
5762
}
5863

5964
// If LIBDDWAF_PREFIX is set, add that library path to rpath as well
@@ -66,7 +71,6 @@ fn main() {
6671
println!("cargo::rerun-if-env-changed=LIBDDWAF_PREFIX");
6772

6873
set_ddappsec_version();
69-
build_test_sidecar_lib();
7074
}
7175

7276
fn set_ddappsec_version() {
@@ -81,32 +85,3 @@ fn set_ddappsec_version() {
8185
println!("cargo::rustc-env=DDAPPSEC_VERSION={}", version);
8286
println!("cargo::rerun-if-changed={}", version_path.display());
8387
}
84-
85-
fn build_test_sidecar_lib() {
86-
let out_dir = env::var("OUT_DIR").expect("OUT_DIR not set");
87-
let target = env::var("TARGET").expect("TARGET not set");
88-
let host = env::var("HOST").expect("HOST not set");
89-
90-
let (lib_name, shared_flag) = if target.contains("darwin") || target.contains("apple") {
91-
("libtest_sidecar.dylib", "-dynamiclib")
92-
} else {
93-
("libtest_sidecar.so", "-shared")
94-
};
95-
96-
let src = "test_sidecar_lib.c";
97-
let out = PathBuf::from(&out_dir).join(lib_name);
98-
99-
let status = cc::Build::new()
100-
.target(&target)
101-
.host(&host)
102-
.get_compiler()
103-
.to_command()
104-
.args([shared_flag, "-fPIC", "-o"])
105-
.arg(&out)
106-
.arg(src)
107-
.status()
108-
.expect("Failed to run C compiler for test fixture");
109-
110-
assert!(status.success(), "Failed to compile test sidecar library");
111-
println!("cargo::rerun-if-changed={}", src);
112-
}

appsec/helper-rust/scripts/generate-sidecar-ffi.sh

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ OUTPUT_FILE="$PROJECT_DIR/src/ffi/sidecar_ffi.rs"
1414

1515
cd "$COMPONENTS_RS_DIR"
1616

17-
bindgen sidecar.h \
17+
bindgen sidecar-appsec.h \
1818
--allowlist-function 'ddog_sidecar_enqueue_telemetry_log' \
1919
--allowlist-function 'ddog_sidecar_enqueue_telemetry_point' \
2020
--allowlist-function 'ddog_sidecar_enqueue_telemetry_metric' \
@@ -24,6 +24,9 @@ bindgen sidecar.h \
2424
--allowlist-function 'ddog_Error_drop' \
2525
--allowlist-function 'ddog_Error_message' \
2626
--allowlist-function 'ddog_MaybeError_drop' \
27+
--allowlist-function 'ddog_remote_config_path' \
28+
--allowlist-function 'ddog_remote_config_path_free' \
29+
--allowlist-function 'ddog_set_rc_notify_fn' \
2730
--allowlist-type 'ddog_SidecarTransport' \
2831
--allowlist-type 'ddog_LogLevel' \
2932
--allowlist-type 'ddog_CharSlice' \
@@ -34,14 +37,15 @@ bindgen sidecar.h \
3437
--allowlist-type 'ddog_Error' \
3538
--allowlist-type 'ddog_Vec_U8' \
3639
--allowlist-type 'ddog_MetricNamespace' \
40+
--allowlist-type 'ddog_MetricType' \
3741
--no-layout-tests \
3842
--no-doc-comments \
3943
--use-core \
40-
--raw-line '//! Auto-generated FFI bindings from components-rs/sidecar.h' \
44+
--raw-line '//! Auto-generated FFI bindings from components-rs/sidecar-appsec.h' \
4145
--raw-line '//!' \
4246
--raw-line '//! Regenerate with: ./scripts/generate-sidecar-ffi.sh' \
4347
--raw-line '//!' \
44-
--raw-line '//! Only includes types/functions needed for helper-rust as telemetry sender.' \
48+
--raw-line '//! Only includes types/functions needed by helper-rust.' \
4549
--raw-line '#![allow(non_camel_case_types, non_upper_case_globals, dead_code)]' \
4650
--output "$OUTPUT_FILE" \
4751
-- -I.

appsec/helper-rust/src/ffi.rs

Lines changed: 68 additions & 183 deletions
Original file line numberDiff line numberDiff line change
@@ -1,198 +1,83 @@
1-
use std::{
2-
cell::UnsafeCell,
3-
ffi::CStr,
4-
marker::PhantomData,
5-
ops::Deref,
6-
sync::atomic::{AtomicBool, Ordering},
7-
};
8-
9-
use crate::client::log::error;
10-
111
pub mod sidecar_ffi;
122

13-
#[macro_export]
14-
macro_rules! sidecar_symbol {
15-
// form 1: inline function signature
16-
(
17-
static $static:ident =
18-
fn($($arg:ty),* $(, ...)? $(,)?) $(-> $ret:ty)? :
19-
$name:ident
20-
) => {
21-
type $name = unsafe extern "C" fn(
22-
$($arg),*
23-
$(, ...)?
24-
) $(-> $ret)?;
25-
26-
static $static: SidecarSymbol<$name> =
27-
SidecarSymbol::new(::std::concat!(::std::stringify!($name), "\0"));
28-
29-
const _: () = {
30-
let _: $name = $name;
31-
};
32-
};
33-
34-
// form 2: type alias
35-
(
36-
static $static:ident =
37-
$ty:ty :
38-
$name:ident
39-
) => {
40-
static $static: SidecarSymbol<$ty> =
41-
SidecarSymbol::new(unsafe {::std::ffi::CStr::from_bytes_with_nul_unchecked(::std::concat!(::std::stringify!($name), "\0").as_bytes())});
42-
43-
const _: () = {
44-
let _: $ty = $name;
45-
};
46-
};
47-
}
48-
49-
pub struct SidecarSymbol<Func> {
50-
// In order to implement Deref, we need to have a sort of rvalue for the function
51-
// So do not use an AtomicPtr that we check for null to determine if we're initialized
52-
func_ptr: UnsafeCell<*mut libc::c_void>,
53-
initialized: AtomicBool,
54-
name: &'static CStr,
55-
_phantom: PhantomData<Func>,
56-
}
57-
impl<Func> SidecarSymbol<Func> {
58-
pub const fn new(name: &'static CStr) -> Self {
59-
assert!(
60-
std::mem::size_of::<Func>() == std::mem::size_of::<*mut libc::c_void>(),
61-
"Func must be pointer-sized"
62-
);
63-
Self {
64-
func_ptr: UnsafeCell::new(std::ptr::null_mut()),
65-
initialized: AtomicBool::new(false),
66-
name,
67-
_phantom: PhantomData,
68-
}
3+
// Stub implementations of the sidecar symbols for `cargo test`.
4+
// In production the real symbols are provided by datadog-ipc-helper at
5+
// dlopen time. The cdylib has no stubs (--allow-shlib-undefined covers it);
6+
// the test executable needs concrete definitions at link time because lld on
7+
// musl does not allow undefined symbols in executables.
8+
#[cfg(test)]
9+
mod test_stubs {
10+
use super::sidecar_ffi::*;
11+
12+
#[no_mangle]
13+
extern "C" fn ddog_Error_drop(_: *mut ddog_Error) {}
14+
#[no_mangle]
15+
extern "C" fn ddog_Error_message(_: *const ddog_Error) -> ddog_CharSlice {
16+
ddog_CharSlice { ptr: std::ptr::null(), len: 0 }
6917
}
70-
71-
pub fn resolve(&self) -> anyhow::Result<()> {
72-
if self.initialized.load(Ordering::Acquire) {
73-
return Ok(());
74-
}
75-
76-
let func_ptr = unsafe { libc::dlsym(libc::RTLD_DEFAULT, self.name.as_ptr()) };
77-
if func_ptr.is_null() {
78-
return Err(anyhow::anyhow!("Failed to resolve symbol: {:?}", self.name));
79-
}
80-
unsafe { std::ptr::write(self.func_ptr.get(), func_ptr) };
81-
self.initialized.store(true, Ordering::Release);
82-
Ok(())
18+
#[no_mangle]
19+
extern "C" fn ddog_MaybeError_drop(_: ddog_MaybeError) {}
20+
#[no_mangle]
21+
extern "C" fn ddog_set_rc_notify_fn(_: ddog_InProcNotifyFn) {}
22+
#[no_mangle]
23+
unsafe extern "C" fn ddog_remote_config_path(
24+
_: *const ddog_ConfigInvariants,
25+
_: *const ddog_Arc_Target,
26+
) -> *mut std::ffi::c_char {
27+
std::ptr::null_mut()
8328
}
84-
85-
fn get(&self) -> Option<&Func> {
86-
if !self.initialized.load(Ordering::Acquire) {
87-
None
88-
} else {
89-
Some(unsafe { &*self.func_ptr.get().cast() })
29+
#[no_mangle]
30+
extern "C" fn ddog_remote_config_path_free(_: *mut std::ffi::c_char) {}
31+
#[no_mangle]
32+
unsafe extern "C" fn ddog_sidecar_connect(
33+
_: *mut *mut ddog_SidecarTransport,
34+
) -> ddog_MaybeError {
35+
ddog_MaybeError {
36+
tag: ddog_Option_Error_Tag_DDOG_OPTION_ERROR_NONE_ERROR,
37+
__bindgen_anon_1: unsafe { std::mem::zeroed() },
9038
}
9139
}
92-
}
93-
unsafe impl<Func> Sync for SidecarSymbol<Func> {}
94-
impl<Func> Deref for SidecarSymbol<Func> {
95-
type Target = Func;
96-
97-
fn deref(&self) -> &Self::Target {
98-
match self.get() {
99-
Some(func) => func,
100-
None => {
101-
error!("Symbol is not resolved, will panic: {:?}", self);
102-
panic!("Symbol is not resolved: {:?}", self.name);
103-
}
40+
#[no_mangle]
41+
extern "C" fn ddog_sidecar_transport_drop(_: *mut ddog_SidecarTransport) {}
42+
#[no_mangle]
43+
unsafe extern "C" fn ddog_sidecar_ping(
44+
_: *mut *mut ddog_SidecarTransport,
45+
) -> ddog_MaybeError {
46+
ddog_MaybeError {
47+
tag: ddog_Option_Error_Tag_DDOG_OPTION_ERROR_NONE_ERROR,
48+
__bindgen_anon_1: unsafe { std::mem::zeroed() },
10449
}
10550
}
106-
}
107-
impl<Func> std::fmt::Debug for SidecarSymbol<Func> {
108-
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
109-
let initialized = self.initialized.load(Ordering::Acquire);
110-
let mut ds = f.debug_struct("SidecarSymbol");
111-
ds.field("name", &self.name)
112-
.field("name", &self.name)
113-
.field("initialized", &initialized);
114-
if initialized {
115-
ds.field("func_ptr", &self.func_ptr.get());
51+
#[no_mangle]
52+
unsafe extern "C" fn ddog_sidecar_enqueue_telemetry_log(
53+
_: ddog_CharSlice, _: ddog_CharSlice, _: ddog_CharSlice, _: ddog_CharSlice,
54+
_: ddog_CharSlice, _: ddog_LogLevel, _: ddog_CharSlice,
55+
_: *mut ddog_CharSlice, _: *mut ddog_CharSlice, _: bool,
56+
) -> ddog_MaybeError {
57+
ddog_MaybeError {
58+
tag: ddog_Option_Error_Tag_DDOG_OPTION_ERROR_NONE_ERROR,
59+
__bindgen_anon_1: unsafe { std::mem::zeroed() },
11660
}
117-
ds.finish()
11861
}
119-
}
120-
121-
#[cfg(test)]
122-
mod tests {
123-
use super::*;
124-
use std::ffi::CStr;
125-
use std::path::PathBuf;
126-
127-
fn load_library(path: &std::path::Path) -> *mut libc::c_void {
128-
let path_cstr = std::ffi::CString::new(path.to_str().unwrap()).expect("Invalid path");
129-
130-
let handle =
131-
unsafe { libc::dlopen(path_cstr.as_ptr(), libc::RTLD_NOW | libc::RTLD_GLOBAL) };
132-
133-
if handle.is_null() {
134-
let error = unsafe { libc::dlerror() };
135-
if !error.is_null() {
136-
let error_str = unsafe { CStr::from_ptr(error) };
137-
panic!("dlopen failed: {:?}", error_str);
138-
}
139-
panic!("dlopen failed with unknown error");
62+
#[no_mangle]
63+
unsafe extern "C" fn ddog_sidecar_enqueue_telemetry_point(
64+
_: ddog_CharSlice, _: ddog_CharSlice, _: ddog_CharSlice, _: ddog_CharSlice,
65+
_: ddog_CharSlice, _: f64, _: *mut ddog_CharSlice,
66+
) -> ddog_MaybeError {
67+
ddog_MaybeError {
68+
tag: ddog_Option_Error_Tag_DDOG_OPTION_ERROR_NONE_ERROR,
69+
__bindgen_anon_1: unsafe { std::mem::zeroed() },
14070
}
141-
142-
handle
14371
}
144-
145-
fn unload_library(handle: *mut libc::c_void) {
146-
if !handle.is_null() {
147-
unsafe {
148-
libc::dlclose(handle);
149-
}
72+
#[no_mangle]
73+
unsafe extern "C" fn ddog_sidecar_enqueue_telemetry_metric(
74+
_: ddog_CharSlice, _: ddog_CharSlice, _: ddog_CharSlice, _: ddog_CharSlice,
75+
_: ddog_CharSlice, _: ddog_MetricType, _: ddog_MetricNamespace,
76+
) -> ddog_MaybeError {
77+
ddog_MaybeError {
78+
tag: ddog_Option_Error_Tag_DDOG_OPTION_ERROR_NONE_ERROR,
79+
__bindgen_anon_1: unsafe { std::mem::zeroed() },
15080
}
15181
}
152-
153-
type TestAddFn = unsafe extern "C" fn(i32, i32) -> i32;
154-
155-
#[test]
156-
fn test_sidecar_symbol_resolve_and_call() {
157-
#[cfg(target_os = "macos")]
158-
let lib_name = "libtest_sidecar.dylib";
159-
#[cfg(target_os = "linux")]
160-
let lib_name = "libtest_sidecar.so";
161-
let lib_path = PathBuf::from(env!("OUT_DIR")).join(lib_name);
162-
let handle = load_library(&lib_path);
163-
164-
static TEST_ADD_SYMBOL: SidecarSymbol<TestAddFn> = SidecarSymbol::new(c"test_add");
165-
TEST_ADD_SYMBOL
166-
.resolve()
167-
.expect("Failed to resolve test_add symbol");
168-
169-
let result = unsafe { TEST_ADD_SYMBOL(3, 4) };
170-
assert_eq!(result, 7, "test_add(3, 4) should return 7");
171-
172-
unload_library(handle);
173-
}
174-
175-
#[test]
176-
fn test_sidecar_symbol_unresolved_symbol_fails() {
177-
static NONEXISTENT_SYMBOL: SidecarSymbol<TestAddFn> =
178-
SidecarSymbol::new(c"nonexistent_function_12345");
179-
180-
let result = NONEXISTENT_SYMBOL.resolve();
181-
assert!(result.is_err(), "Resolving nonexistent symbol should fail");
182-
}
183-
184-
#[test]
185-
fn test_sidecar_symbol_debug_format() {
186-
static DEBUG_TEST_SYMBOL: SidecarSymbol<TestAddFn> = SidecarSymbol::new(c"debug_test_func");
187-
188-
let debug_str = format!("{:?}", DEBUG_TEST_SYMBOL);
189-
assert!(
190-
debug_str.contains("initialized: false"),
191-
"Unresolved symbol should show initialized: false"
192-
);
193-
assert!(
194-
debug_str.contains("debug_test_func"),
195-
"Debug output should contain the symbol name"
196-
);
197-
}
19882
}
83+

0 commit comments

Comments
 (0)