search

rust-lang / backtrace-rs

Backtraces in Rust
https://docs.rs/backtrace
Other
516 stars 237 forks source link

Add option to not demangle symbols #607

Open ChrisDenton opened 3 months ago

ChrisDenton commented 3 months ago

I've been playing with various backtrace related things in std, one of which is skipping demangling. And I thought this might be kinda useful more generally? Even if you want symbolization, you may want to bring your own demangler without paying the cost of the built-in one. Maybe this is a too niche scenario though 🤷.

So this is a new API for consideration. It simply adds a new RawSymbolName type. I considered just returning &[u8] but then thought that having some of the convinces of SymbolName is actually useful. And this is Rust so we can never have too many types.

ChrisDenton commented 3 months ago

Oh wow, CI is fully working again! So even if this PR isn't merged it has served a purpose, lol.

workingjubilee commented 3 months ago

So, I'm receptive to the core idea. Haven't evaluated the impl details closely yet. But I'm thinking and... it would be an obvious breaking change but I gotta ask:

Is it actually our business to demangle symbols to begin with? :thinking:

ChrisDenton commented 3 months ago

Well I think not tbh. We also have a weird hack to optionally add C++ demangling too, which seems silly to me. I mean, even if this crate does want to include demangling support, it could just be a separate function rather than baked in.

But yeah, backwards compatibility.

bjorn3 commented 3 months ago

Is it actually our business to demangle symbols to begin with? 🤔

For the Display impls, IMO yes as Display is meant for human consumption and mangled symbol names are hard to read (the v0 mangled symbols even more so than the legacy mangled symbols. the latter I can read with a little bit of effort. the former I pretty much can't.). For programmatically getting the symbol name, IMO no it should give the symbol name that is actually stored in the object file.

ChrisDenton commented 3 months ago

This feels somewhat similar to why Path has a display method rather than implementing the Display trait directly.

ChrisDenton commented 3 months ago

Well different also in that there's lots of different ways to demangle symbols (depending on the language, etc).

Mark-Simulacrum commented 3 months ago

Isn't it a moot point? Even if we don't do it here, it would be a pretty bad user experience regression for rust backtraces to not demangle by default in std. I'm not sure mitigating that with extra tooling is a viable solution.

Splitting that out from this library just means that theres extra cost paid - you need to run a demangling pass over the full backtrace after formatting or otherwise potentially incur extra allocations/time to do the demangling I suspect.

ChrisDenton commented 3 months ago

Well yes, the default to std is unlikely to change. At least not in this way. I would however like to explore giving people more options (via build-std).

And eventually it would be nice if it were possible to compile rust std applications without also having to compile a debugger into every single binary. But that's another issue.

ChrisDenton commented 3 months ago

Oh, hm... this probably also needs some support in BacktraceFrameFmt otherwise there would be no way to skip symbolization when printing via the helpers.

workingjubilee commented 3 months ago

Splitting that out from this library just means that theres extra cost paid - you need to run a demangling pass over the full backtrace after formatting or otherwise potentially incur extra allocations/time to do the demangling I suspect.

Ooh, this is a good point: you want to generate the symbol string once, because generating and regenerating it is going to be nasty. Ideally you'd have a formatter you can just point at a stdout instead of allocating at all.