Stream: git-wasmtime

Topic: wasmtime / issue #9753 use of cpp_demangle misses check i...

Wasmtime GitHub notifications bot (Dec 06 2024 at 15:50):

chobermaier added the bug label to Issue #9753.

Wasmtime GitHub notifications bot (Dec 06 2024 at 15:50):

chobermaier opened issue #9753:

Probably related: #4162

I came across this issue while trying to use _perf_ with wasmtime following the documentation.
When I used the param --profile=perfmap or --profile=jitdump, I got the following error message:

thread 'main' panicked at crates/wasmtime/src/runtime/instantiate.rs:73:76:
called `Result::unwrap()` on an `Err` value: Error
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I tried using a simple hello-world example wasm file and found that the issue was not present there.
After some trying around, I think, I might have found the cause.

The wasm file I tried to use with perf initially has a symbol named "RC4". When the _demangle_function_name_ function is called in instantiate.rs:73 unwrapping fails, because the if-let statement that passes the function name to cpp_demangle crate only creates the Symbol, which apparently is not safe to demangle actually. Even if its Result is Ok.
I'm not quite sure if this is their intended behavior. What do you think?

This gist shows the isolation of the problem: https://gist.github.com/chobermaier/3adf8f2a004ccbe17865bfc20df5c6d3

I would propose to change _demangle_function_name_ this way (commit with diff):

pub fn demangle_function_name(writer: &mut impl core::fmt::Write, name: &str) -> core::fmt::Result {
    #[cfg(feature = "demangle")]
    if let Ok(demangled) = rustc_demangle::try_demangle(name) {
        return write!(writer, "{demangled}");
    } else if let Ok(symbol) = cpp_demangle::Symbol::new(name) {
        let options = cpp_demangle::DemangleOptions::default();
        if let Ok(demangled) = symbol.demangle(&options) {
            return write!(writer, "{demangled}");
        }
    }

    write!(writer, "{name}")
}

This would call demangle explicitly and provide the information if it's possible through the Result returned. Instead of the implicit call through the write macro. So this should not introduce any performance overhead.

Versions and Environment

Wasmtime version or commit: wasmtime 27.0.0 (8eefa236f 2024-11-20)

Operating system: Linux 6.12.1-arch1-1

Architecture: amd64

Wasmtime GitHub notifications bot (Dec 06 2024 at 17:32):

alexcrichton commented on issue #9753:

That looks like a reasonable fix to me, thanks for this! Would you be interested in sending a PR with your change?

Wasmtime GitHub notifications bot (Dec 06 2024 at 18:40):

chobermaier commented on issue #9753:

I can try to send a PR.

I'm glad if I could help :big_smile:

Wasmtime GitHub notifications bot (Dec 06 2024 at 19:46):

alexcrichton closed issue #9753:

Probably related: #4162

I came across this issue while trying to use _perf_ with wasmtime following the documentation.
When I used the param --profile=perfmap or --profile=jitdump, I got the following error message:

thread 'main' panicked at crates/wasmtime/src/runtime/instantiate.rs:73:76:
called `Result::unwrap()` on an `Err` value: Error
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I tried using a simple hello-world example wasm file and found that the issue was not present there.
After some trying around, I think, I might have found the cause.

The wasm file I tried to use with perf initially has a symbol named "RC4". When the _demangle_function_name_ function is called in instantiate.rs:73 unwrapping fails, because the if-let statement that passes the function name to cpp_demangle crate only creates the Symbol, which apparently is not safe to demangle actually. Even if its Result is Ok.
I'm not quite sure if this is their intended behavior. What do you think?

This gist shows the isolation of the problem: https://gist.github.com/chobermaier/3adf8f2a004ccbe17865bfc20df5c6d3

I would propose to change _demangle_function_name_ this way (commit with diff):

pub fn demangle_function_name(writer: &mut impl core::fmt::Write, name: &str) -> core::fmt::Result {
    #[cfg(feature = "demangle")]
    if let Ok(demangled) = rustc_demangle::try_demangle(name) {
        return write!(writer, "{demangled}");
    } else if let Ok(symbol) = cpp_demangle::Symbol::new(name) {
        let options = cpp_demangle::DemangleOptions::default();
        if let Ok(demangled) = symbol.demangle(&options) {
            return write!(writer, "{demangled}");
        }
    }

    write!(writer, "{name}")
}

This would call demangle explicitly and provide the information if it's possible through the Result returned. Instead of the implicit call through the write macro. So this should not introduce any performance overhead.

Versions and Environment

Wasmtime version or commit: wasmtime 27.0.0 (8eefa236f 2024-11-20)

Operating system: Linux 6.12.1-arch1-1

Architecture: amd64

Last updated: Dec 23 2024 at 12:05 UTC