fix(profiling-ffi): Potential panic case

[gleocadie]

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.

[github-actions]

Clippy Allow Annotation Report

Comparing clippy allow annotations between branches:

• Base Branch: origin/main
• PR Branch: origin/gleocadie/fix-potential-crash-site

Summary by Rule

Rule	Base Branch	PR Branch	Change

Annotation Counts by File

File	Base Branch	PR Branch	Change

Annotation Stats by Crate

Crate	Base Branch	PR Branch	Change
clippy-annotation-reporter	5	5	No change (0%)
datadog-ffe-ffi	1	1	No change (0%)
datadog-ipc	21	21	No change (0%)
datadog-live-debugger	6	6	No change (0%)
datadog-live-debugger-ffi	10	10	No change (0%)
datadog-profiling-replayer	4	4	No change (0%)
datadog-remote-config	3	3	No change (0%)
datadog-sidecar	57	57	No change (0%)
libdd-common	10	10	No change (0%)
libdd-common-ffi	12	12	No change (0%)
libdd-data-pipeline	5	5	No change (0%)
libdd-ddsketch	2	2	No change (0%)
libdd-dogstatsd-client	1	1	No change (0%)
libdd-profiling	13	13	No change (0%)
libdd-telemetry	20	20	No change (0%)
libdd-tinybytes	4	4	No change (0%)
libdd-trace-normalization	2	2	No change (0%)
libdd-trace-obfuscation	8	8	No change (0%)
libdd-trace-stats	1	1	No change (0%)
libdd-trace-utils	15	15	No change (0%)
Total	200	200	No change (0%)

About This Report

This report tracks Clippy allow annotations for specific rules, showing how they've changed in this PR. Decreasing the number of these annotations generally improves code quality.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.85%. Comparing base (fff4fd3) to head (f9980e1).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1955      +/-   ##
==========================================
+ Coverage   71.80%   71.85%   +0.04%     
==========================================
  Files         434      434              
  Lines       70607    70615       +8     
==========================================
+ Hits        50697    50738      +41     
+ Misses      19910    19877      -33     

Components	Coverage Δ
libdd-crashtracker	65.94% <ø> (-0.03%)	⬇️
libdd-crashtracker-ffi	34.09% <ø> (ø)
libdd-alloc	98.77% <ø> (ø)
libdd-data-pipeline	86.24% <ø> (ø)
libdd-data-pipeline-ffi	73.93% <ø> (ø)
libdd-common	79.73% <ø> (+0.15%)	⬆️
libdd-common-ffi	74.41% <ø> (+0.54%)	⬆️
libdd-telemetry	69.29% <ø> (+0.02%)	⬆️
libdd-telemetry-ffi	19.37% <ø> (ø)
libdd-dogstatsd-client	82.64% <ø> (ø)
datadog-ipc	76.22% <ø> (+1.46%)	⬆️
libdd-profiling	81.62% <100.00%> (+0.01%)	⬆️
libdd-profiling-ffi	64.51% <100.00%> (+0.15%)	⬆️
datadog-sidecar	28.61% <ø> (-0.04%)	⬇️
datdog-sidecar-ffi	7.31% <ø> (ø)
spawn-worker	54.69% <ø> (ø)
libdd-tinybytes	93.16% <ø> (ø)
libdd-trace-normalization	81.71% <ø> (ø)
libdd-trace-obfuscation	87.26% <ø> (ø)
libdd-trace-protobuf	68.25% <ø> (ø)
libdd-trace-utils	89.27% <ø> (ø)
libdd-tracer-flare	86.88% <ø> (ø)
libdd-log	74.83% <ø> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[datadog-prod-us1-6]

🎉 All green!

❄️ No new flaky tests detected
🧪 All tests passed

🎯 Code Coverage (details)
• Patch Coverage: 100.00%
• Overall Coverage: 71.85% (+0.05%)

_{This comment will be updated automatically if new data arrives.
🔗 Commit SHA: f9980e1 | Docs | Datadog PR Page | Give us feedback!}

[yannham]

Shouldn't we use catch_panic! instead? This is one possible panic, but there are just so many possibilities given this function calls to a bunch of subfunctions as well...

[gleocadie]

Shouldn't we use catch_panic! instead? This is one possible panic, but there are just so many possibilities given this function calls to a bunch of subfunctions as well...

@yannham yes, this has never been decided for profiling-ffi

If we go that way we should do it for all the APIs and in another review

[dd-octo-sts]

Artifact Size Benchmark Report

aarch64-alpine-linux-musl

Artifact	Baseline	Commit	Change
/aarch64-alpine-linux-musl/lib/libdatadog_profiling.so	7.51 MB	7.51 MB	0% (0 B) 👌
/aarch64-alpine-linux-musl/lib/libdatadog_profiling.a	81.49 MB	81.49 MB	-0% (-856 B) 👌

aarch64-unknown-linux-gnu

Artifact	Baseline	Commit	Change
/aarch64-unknown-linux-gnu/lib/libdatadog_profiling.a	97.63 MB	97.63 MB	-0% (-984 B) 👌
/aarch64-unknown-linux-gnu/lib/libdatadog_profiling.so	9.93 MB	9.93 MB	0% (0 B) 👌

libdatadog-x64-windows

Artifact	Baseline	Commit	Change
/libdatadog-x64-windows/debug/dynamic/datadog_profiling_ffi.dll	24.38 MB	24.38 MB	-0% (-512 B) 👌
/libdatadog-x64-windows/debug/dynamic/datadog_profiling_ffi.lib	79.87 KB	79.87 KB	0% (0 B) 👌
/libdatadog-x64-windows/debug/dynamic/datadog_profiling_ffi.pdb	179.53 MB	179.52 MB	-0% (-8.00 KB) 👌
/libdatadog-x64-windows/debug/static/datadog_profiling_ffi.lib	910.43 MB	910.43 MB	-0% (-3.60 KB) 👌
/libdatadog-x64-windows/release/dynamic/datadog_profiling_ffi.dll	7.71 MB	7.71 MB	0% (0 B) 👌
/libdatadog-x64-windows/release/dynamic/datadog_profiling_ffi.lib	79.87 KB	79.87 KB	0% (0 B) 👌
/libdatadog-x64-windows/release/dynamic/datadog_profiling_ffi.pdb	23.07 MB	23.07 MB	0% (0 B) 👌
/libdatadog-x64-windows/release/static/datadog_profiling_ffi.lib	45.18 MB	45.18 MB	-0% (-178 B) 👌

libdatadog-x86-windows

Artifact	Baseline	Commit	Change
/libdatadog-x86-windows/debug/dynamic/datadog_profiling_ffi.dll	21.00 MB	21.00 MB	0% (0 B) 👌
/libdatadog-x86-windows/debug/dynamic/datadog_profiling_ffi.lib	81.11 KB	81.11 KB	0% (0 B) 👌
/libdatadog-x86-windows/debug/dynamic/datadog_profiling_ffi.pdb	183.67 MB	183.66 MB	-0% (-8.00 KB) 👌
/libdatadog-x86-windows/debug/static/datadog_profiling_ffi.lib	896.47 MB	896.46 MB	-0% (-3.50 KB) 👌
/libdatadog-x86-windows/release/dynamic/datadog_profiling_ffi.dll	5.97 MB	5.97 MB	0% (0 B) 👌
/libdatadog-x86-windows/release/dynamic/datadog_profiling_ffi.lib	81.11 KB	81.11 KB	0% (0 B) 👌
/libdatadog-x86-windows/release/dynamic/datadog_profiling_ffi.pdb	24.70 MB	24.70 MB	0% (0 B) 👌
/libdatadog-x86-windows/release/static/datadog_profiling_ffi.lib	42.69 MB	42.69 MB	-0% (-164 B) 👌

x86_64-alpine-linux-musl

Artifact	Baseline	Commit	Change
/x86_64-alpine-linux-musl/lib/libdatadog_profiling.a	72.61 MB	72.61 MB	-0% (-640 B) 👌
/x86_64-alpine-linux-musl/lib/libdatadog_profiling.so	8.38 MB	8.38 MB	0% (0 B) 👌

x86_64-unknown-linux-gnu

Artifact	Baseline	Commit	Change
/x86_64-unknown-linux-gnu/lib/libdatadog_profiling.a	90.35 MB	90.35 MB	-0% (-840 B) 👌
/x86_64-unknown-linux-gnu/lib/libdatadog_profiling.so	9.99 MB	9.99 MB	0% (0 B) 👌

[yannham]

I'm fine either way, if you need a stamp as hotfix for this bug 👍 It's just that if you were to go the catch_panic! route, this change should probably be reverted, because it would be both unneeded and meaningless to give this very specific panic a special treatment.