Skip to content

Commit 982b6bd

Browse files
authored
fix(profiling-ffi): Potential panic case (#1955)
# What does this PR do? Fix a potential crash site. # Motivation A crash report gave us this callstack ``` Error UnhandledException: Process was terminated due to an unknown unhandled exception of type STATUS_STACK_BUFFER_OVERRUN (0xC0000409) std::sys::pal::windows::abort_internal() (\rustc\e71f9a9a98b0faf423844bf0ba7438f29dc27d58\library\std\src\sys\pal\windows\mod.rs:337) 0x7FFE738ED69F std::panicking::rust_panic_with_hook() (\rustc\e71f9a9a98b0faf423844bf0ba7438f29dc27d58\library\std\src\panicking.rs:823) 0x7FFE738ED063 std::panicking::begin_panic_handler::closure$0() (\rustc\e71f9a9a98b0faf423844bf0ba7438f29dc27d58\library\std\src\panicking.rs:675) 0x7FFE738ECFEF std::sys::backtrace::__rust_end_short_backtrace<std::panicking::begin_panic_handler::closure_env$0,never$>() (\rustc\e71f9a9a98b0faf423844bf0ba7438f29dc27d58\library\std\src\sys\backtrace.rs:170) 0x7FFE738ECFDE std::panicking::begin_panic_handler() (\rustc\e71f9a9a98b0faf423844bf0ba7438f29dc27d58\library\std\src\panicking.rs:665) core::panicking::panic_nounwind_fmt::runtime() (\rustc\e71f9a9a98b0faf423844bf0ba7438f29dc27d58\library\core\src\panicking.rs:119) 0x7FFE739D99DB core::panicking::panic_nounwind_fmt() (\rustc\e71f9a9a98b0faf423844bf0ba7438f29dc27d58\library\core\src\intrinsics\mod.rs:3535) 0x7FFE739D9A55 core::panicking::panic_nounwind() (\rustc\e71f9a9a98b0faf423844bf0ba7438f29dc27d58\library\core\src\panicking.rs:223) 0x7FFE739D9A03 core::panicking::panic_cannot_unwind() (\rustc\e71f9a9a98b0faf423844bf0ba7438f29dc27d58\library\core\src\panicking.rs:315) 0x7FFE73583A80 datadog_profiling_ffi::profiles::datatypes::ddog_prof_Profile_new() (C:\mnt\libdatadog\libdd-profiling-ffi\src\profiles\datatypes.rs:436) 0x7FFE739C81D0 _CallSettingFrame() (D:\a\_work\1\s\src\vctools\crt\vcruntime\src\eh\amd64\handlers.asm:50) 0x7FFE739C7874 __FrameHandler3::CxxCallCatchBlock(_EXCEPTION_RECORD*) (D:\a\_work\1\s\src\vctools\crt\vcruntime\src\eh\frame.cpp:1521) 0x7FFE8DF31CD6 RcConsolidateFrames 0x7FFE73583A62 datadog_profiling_ffi::profiles::datatypes::ddog_prof_Profile_new() (C:\mnt\libdatadog\libdd-profiling-ffi\src\profiles\datatypes.rs:441) 0x7FFE73C9980D libdatadog::CreateProfile(std::vector<SampleValueType,std::allocator<SampleValueType> > const&, std::basic_string<char,std::char_traits<char>,std::allocator<char> > const&, std::basic_string<char,std::char_traits<char>,std::allocator<char> > const&) (c:\mnt\profiler\src\ProfilerEngine\Datadog.Profiler.Native\Profile.cpp:212) std::exchange(libdatadog::ProfileImpl*&, void*&&) (c:\devtools\vstudio\VC\Tools\MSVC\14.44.35207\include\utility:773) std::unique_ptr<libdatadog::ProfileImpl,std::default_delete<libdatadog::ProfileImpl> >::release() (c:\devtools\vstudio\VC\Tools\MSVC\14.44.35207\include\memory:3465) std::unique_ptr<libdatadog::ProfileImpl,std::default_delete<libdatadog::ProfileImpl> >::operator=(std::unique_ptr<libdatadog::ProfileImpl,std::default_delete<libdatadog::ProfileImpl> >&&) (c:\devtools\vstudio\VC\Tools\MSVC\14.44.35207\include\memory:3414) libdatadog::Profile::{ctor}(IConfiguration*, std::vector<SampleValueType,std::allocator<SampleValueType> > const&, std::basic_string<char,std::char_traits<char>,std::allocator<char> > const&, std::basic_string<char,std::char_traits<char>,std::allocator<char> > const&, std::basic_string<char,std::char_traits<char>,std::allocator<char> >) (c:\mnt\profiler\src\ProfilerEngine\Datadog.Profiler.Native\Profile.cpp:25) std::make_unique(IConfiguration*&, std::vector<SampleValueType,std::allocator<SampleValueType> >&, std::basic_string<char,std::char_traits<char>,std::allocator<char> > const&, std::basic_string<char,std::char_traits<char>,std::allocator<char> > const&, std::basic_string<char,std::char_traits<char>,std::allocator<char> >&&) (c:\devtools\vstudio\VC\Tools\MSVC\14.44.35207\include\memory:3630) 0x7FFE73C60B24 ProfileExporter::CreateProfile(std::basic_string<char,std::char_traits<char>,std::allocator<char> >) (c:\mnt\profiler\src\ProfilerEngine\Datadog.Profiler.Native\ProfileExporter.cpp:165) std::exchange(libdatadog::Profile*&, void*&&) (c:\devtools\vstudio\VC\Tools\MSVC\14.44.35207\include\utility:773) std::unique_ptr<libdatadog::Profile,std::default_delete<libdatadog::Profile> >::release() (c:\devtools\vstudio\VC\Tools\MSVC\14.44.35207\include\memory:3465) std::unique_ptr<libdatadog::Profile,std::default_delete<libdatadog::Profile> >::operator=(std::unique_ptr<libdatadog::Profile,std::default_delete<libdatadog::Profile> >&&) (c:\devtools\vstudio\VC\Tools\MSVC\14.44.35207\include\memory:3414) 0x7FFE73C61C27 ProfileExporter::Add(std::shared_ptr<Sample> const&) (c:\mnt\profiler\src\ProfilerEngine\Datadog.Profiler.Native\ProfileExporter.cpp:402) 0x7FFE73C67F6B SamplesCollector::CollectSamples(std::forward_list<std::pair<ISamplesProvider *,unsigned __int64>,std::allocator<std::pair<ISamplesProvider *,unsigned __int64> > >&) (c:\mnt\profiler\src\ProfilerEngine\Datadog.Profiler.Native\SamplesCollector.cpp:154) 0x7FFE73C67B50 SamplesCollector::SamplesWork() (c:\mnt\profiler\src\ProfilerEngine\Datadog.Profiler.Native\SamplesCollector.cpp:85) 0x7FFE73C683DC std::thread::_Invoke<std::tuple<`SamplesCollector::StartImpl'::`2'::<lambda_1> >,0>(void*) (c:\devtools\vstudio\VC\Tools\MSVC\14.44.35207\include\thread:61) 0x7FFE73CB3DB0 thread_start<unsigned int (__cdecl*)(void *),1>(void* const) (minkernel\crts\ucrt\src\appcrt\startup\thread.cpp:97) 0x7FFE8DD97374 BaseThreadInitThunk 0x7FFE8DEDCC91 RtlUserThreadStart ``` One possible cause is that `into_slice` may have panicked. In any case, we should do the same as what the dictionary-based API is doing. Co-authored-by: gregory.leocadie <gregory.leocadie@datadoghq.com>
1 parent d700bb0 commit 982b6bd

1 file changed

Lines changed: 17 additions & 1 deletion

File tree

libdd-profiling-ffi/src/profiles/datatypes.rs

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -481,7 +481,10 @@ unsafe fn profile_new(
481481
period: Option<&Period>,
482482
string_storage: Option<ManagedStringStorage>,
483483
) -> ProfileNewResult {
484-
let types = sample_types.into_slice();
484+
let types = match sample_types.try_as_slice() {
485+
Ok(s) => s,
486+
Err(e) => return ProfileNewResult::Err(anyhow::Error::from(e).into()),
487+
};
485488
let period = period.copied();
486489

487490
let result = match string_storage {
@@ -913,6 +916,19 @@ mod tests {
913916
}
914917
}
915918

919+
/// Invalid FFI `sample_types` must not panic: `try_as_slice` fails and we return `Err`.
920+
#[test]
921+
fn profile_new_invalid_sample_types_slice_returns_err() {
922+
unsafe {
923+
let bad_slice: Slice<'_, SampleType> = Slice::from_raw_parts(std::ptr::null(), 1);
924+
let result = ddog_prof_Profile_new(bad_slice, None);
925+
assert!(
926+
matches!(result, ProfileNewResult::Err(_)),
927+
"expected Err for null pointer with non-zero length (SliceConversionError::NullPointer)"
928+
);
929+
}
930+
}
931+
916932
#[test]
917933
fn add_failure() -> Result<(), Error> {
918934
unsafe {

0 commit comments

Comments
 (0)