Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Don’t forward demangling errors in Display impl #296

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

surma
Copy link

@surma surma commented Sep 28, 2024

In the Display implementation of Symbol the potential error after demangling is turned into a std::fmt::Error.

I’d like to propose to not do that, as this will lead to panics in many cases (even just a simple format!("{my_symbol}"). This PR changes the Display::fmt implementation to output "<Demangling failed>" in case where the demanging failed, avoiding the panic scenario.

@khuey
Copy link
Collaborator

khuey commented Sep 29, 2024

Can you provide a testcase that shows the issue you're talking about? I don't think I understand the problem.

@surma
Copy link
Author

surma commented Sep 29, 2024

Yeah, I should have really provided that. Apologies.

I dug through the WebAssembly file that triggered the problem for me, and the problematic function name in this case was "RD4":

pub fn main() {
    let func_name = "RD4";
    let sym = cpp_demangle::Symbol::new(func_name).expect("new");
    // sym.demangle(&Default::default()) // <- would return an error
    println!("{}", sym);
}

The program above will panic with the following message:

❯ cargo run --example testcase
   Compiling cpp_demangle v0.4.4 (/Users/surma/src/github.com/gimli-rs/cpp_demangle)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.54s
     Running `target/debug/examples/testcase`
thread 'main' panicked at library/std/src/io/mod.rs:1836:21:
a formatting trait implementation returned an error when the underlying stream did not
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

As stated in the docs of Display::fmt:

This function should return Err if, and only if, the provided Formatter returns Err. String formatting is considered an infallible operation;

So even if demanging fails, Display::fmt should not return an Error. That’s what I am trying to fix in this PR.

@khuey
Copy link
Collaborator

khuey commented Sep 30, 2024

Ok, thanks. Looks like this assertion was added in rust-lang/rust#125012

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants