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

Skip the demangle test under miri #589

Merged
merged 1 commit into from
Aug 16, 2024
Merged

Conversation

sanchda
Copy link
Contributor

@sanchda sanchda commented Aug 16, 2024

What does this PR do?

Skips the demangling test under miri. It appears that miri changes the behavior of symbolic demangle() in a way that breaks the placement of the * character in the demangled function name. Strictly speaking, the test is reflecting a novel incorrectness introduced in demangle() as the produced name is not just syntactically, but also semantically distinct from the original.

For discussion, here's the original prototype of the function as specified in the test. It comes from a real internal function in C++.

bool
_M_futex_wait_until_steady(
  unsigned *__addr,
  unsigned __val,
  bool __has_timeout,
  chrono::seconds __s,
  chrono::nanoseconds __ns);

And then here's the result:

bool
_M_futex_wait_until_steady(
  unsigned __addr,                           // Hey, this is a value type now???
  unsigned __val,
  bool __has_timeout,
  chrono::seconds *__s,                  // ... and THIS is a pointer???
  chrono::nanoseconds __ns)

And then here's the derived result from demangling under miri--as you can see, this is a completely different signature.

Motivation

To get CI to pass

Additional Notes

This probably deserves more scrutiny,

How to test the change?

The point is that we can't, right?

@sanchda sanchda requested a review from a team as a code owner August 16, 2024 13:36
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.17%. Comparing base (16528ff) to head (c5858e4).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #589      +/-   ##
==========================================
+ Coverage   71.13%   71.17%   +0.04%     
==========================================
  Files         220      220              
  Lines       30015    30000      -15     
==========================================
+ Hits        21350    21352       +2     
+ Misses       8665     8648      -17     
Components Coverage Δ
crashtracker 21.25% <100.00%> (+0.08%) ⬆️
datadog-alloc 98.73% <ø> (ø)
data-pipeline 50.00% <ø> (ø)
data-pipeline-ffi 0.00% <ø> (ø)
ddcommon 82.78% <ø> (ø)
ddcommon-ffi 69.72% <ø> (ø)
ddtelemetry 59.02% <ø> (ø)
ipc 84.18% <ø> (ø)
profiling 84.26% <ø> (ø)
profiling-ffi 77.42% <ø> (ø)
serverless 0.00% <ø> (ø)
sidecar 34.03% <ø> (ø)
sidecar-ffi 0.00% <ø> (ø)
spawn-worker 54.87% <ø> (ø)
trace-mini-agent 70.88% <ø> (ø)
trace-normalization 98.25% <ø> (ø)
trace-obfuscation 95.73% <ø> (ø)
trace-protobuf 77.16% <ø> (ø)
trace-utils 93.39% <ø> (+0.42%) ⬆️

@bwoebi bwoebi merged commit dd36a81 into main Aug 16, 2024
34 checks passed
@bwoebi bwoebi deleted the sanchda/skip_demangle_test_miri branch August 16, 2024 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants