flamegraph-rs / flamegraph

Easy flamegraphs for Rust projects and everything else, without Perl or pipes <3
Apache License 2.0
4.61k stars 141 forks source link

On MacOS, always wrap "dtrace" invocation in "arch -arch $native_arch" #304

Closed zbentley closed 7 months ago

zbentley commented 7 months ago

Fixes #302 by always running dtrace as the native architecture. This is accomplished by adding another layer of wrapping in addition to sudo: the arch command forces its spawned children to run as the specified architecture.

Initially I was doing a lot more in this PR (parsing out the architecture of the target binary to be traced and making dtrace run as that), but it turned out to be unnecessary since is prevented from tracing cross-architecture binaries on MacOS in all circumstances.

Since the dtrace binary itself is multi-arch, spawning it inside the arch -arch arm64 command will never fail, but if users do something like cargo flamegraph --root - --target x86_64-apple-darwin they'll get an error from dtrace itself saying "no" regardless of whether this PR has been applied or not.

zbentley commented 7 months ago

There's an option to simplify this code and remove the uname dependency by running arch -64 or arch -32 which prefers the native 64- or 32-bit architecture. However, that would necessarily encode a preference for a specific bit width, since probing the program being profiled for its architecture would be a fair amount more code (see diff as of aa9be61 for an example of how that might look, it's a lot).

Is saying "always invoke native 64-bit dtrace" or "always invoke native dtrace at a bit architecture matching the flamegraph binary" an acceptable tradeoff? If so, I can make that diff.

djc commented 7 months ago

"always invoke native dtrace at a bit architecture matching the flamegraph binary" an acceptable tradeoff? If so, I can make that diff.

This seems like a reasonable option to me?

zbentley commented 7 months ago

This seems like a reasonable option to me?

Done!

djc commented 7 months ago

Thanks!

zbentley commented 7 months ago

Thanks!

Thanks for reviewing!