-
Notifications
You must be signed in to change notification settings - Fork 14
test(crashtracker): fix segfault tests for 1.86.0 #999
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
Conversation
BenchmarksComparisonBenchmark execution time: 2025-04-09 17:12:27 Comparing candidate commit 1b7f4d9 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 52 metrics, 2 unstable metrics. CandidateCandidate benchmark detailsGroup 1
Group 2
Group 3
Group 4
Group 5
Group 6
Group 7
Group 8
Group 9
Group 10
Group 11
Group 12
Group 13
BaselineOmitted due to size. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #999 +/- ##
=======================================
Coverage 71.51% 71.52%
=======================================
Files 337 337
Lines 50511 50516 +5
=======================================
+ Hits 36124 36132 +8
+ Misses 14387 14384 -3
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
bin_tests/tests/crashtracker_bin_test.rs:336
- The telemetry test now only supports SIGSEGV, which might cause test failures if any platform still produces SIGABRT. Verify that inline assembly reliably triggers SIGSEGV across all intended environments.
assert!(tags.contains("si_signo_human_readable:SIGSEGV"), "{tags:?}");
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you for quickly updating this!
Seems the assembly for ARM gets rejected:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my previous comment, meant it as my review.
Artifact Size Benchmark Reportaarch64-alpine-linux-musl
aarch64-unknown-linux-gnu
libdatadog-x64-windows
libdatadog-x86-windows
x86_64-alpine-linux-musl
x86_64-unknown-linux-gnu
|
I'm going to force merge because this is taking over 30 minutes again:
This takes like 6 minutes when it works, this is happening a lot, we should figure it out. |
What does this PR do?
Rust 1.86 changed code emission so more operations are handled safely. Unfortunately, this made it a little harder to trigger true segmentation faults within the language.
This PR uses some inline assembly to make this situation occur on supported platforms.
Motivation
Make tests more correct
Additional Notes
Anything else we should know when reviewing?
How to test the change?
Describe here in detail how the change can be validated.