Skip to content

fix: keep semicolons inside xctrace symbols - #464

Open
tomatotomata wants to merge 2 commits into
flamegraph-rs:mainfrom
tomatotomata:codex/fix-xctrace-semicolon-demangling
Open

fix: keep semicolons inside xctrace symbols#464
tomatotomata wants to merge 2 commits into
flamegraph-rs:mainfrom
tomatotomata:codex/fix-xctrace-semicolon-demangling

Conversation

@tomatotomata

Copy link
Copy Markdown

Summary

xctrace folded stacks use semicolons to separate frames. Rust demangling can introduce semicolons inside array types such as [u8; 2], so those symbols were being split into multiple frames.

I was thinking of applying the same normalization already used by inferno's DTrace collapser: replace semicolons inside the demangled symbol with colons before the XML reaches the xctrace collapser. The change is limited to the macOS demangling path and includes a regression test for an array type.

Validation

  • cargo fmt -- --check
  • cargo test --all-targets
  • git diff --check

The test suite passed on Windows. I could not run the macOS-only test locally because the Apple target is not installed here.

This follows the maintainer's invitation in #462. Let me know what you think.

Comment thread src/lib.rs Outdated
}

#[cfg(target_os = "macos")]
fn normalize_demangled_symbol(symbol: String) -> String {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just inline this.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inlined the semicolon normalization at the demangling site in 48b191e.

Comment thread src/lib.rs Outdated
}

#[cfg(all(test, target_os = "macos"))]
mod tests {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any tests modules should be at the bottom of the file.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved the test module to the bottom of src/lib.rs. I also changed it to exercise demangle_stream with a real Rust v0 symbol, so it covers the XML boundary instead of the removed helper.

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.

2 participants