Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Full build machine paths in panic messages #1918

Open
2 tasks
webmaster128 opened this issue Oct 16, 2023 · 4 comments
Open
2 tasks

Full build machine paths in panic messages #1918

webmaster128 opened this issue Oct 16, 2023 · 4 comments
Labels
dev-experience improvements to DevX

Comments

@webmaster128
Copy link
Member

webmaster128 commented Oct 16, 2023

When a Wasm contract panicks, the full file path and line number is visible in the panic message. If a user does not use workspace-optimizer/rust-optimizer, this exposes personal file system infomation

Query failed with (6): rpc error: code = Unknown desc = failed to execute message; message index: 0: Error calling the VM: Error executing Wasm: Wasmer runtime error: RuntimeError: Aborted: panicked at 'Denominator must not be zero', /Users/username/.cargo/registry/src/github.com-AABBCCDDEEAABBCCDD/cosmwasm-std-1.2.6/src/math/decimal.rs:136:17: execute wasm contract failed [!cosm!wasm/wasmd@v_X.Y.Z_/x/wasm/keeper/keeper.go:389] With gas wanted: '100000000' and gas used: 'SOME_VALUE' : unknown request

The contents of the Aborted error is coming from Wasm:

panicked at 'Denominator must not be zero', /Users/username/.cargo/registry/src/github.com-AABBCCDDEEAABBCCDD/cosmwasm-std-1.2.6/src/math/decimal.rs:136:17

The source of this is https://github.com/CosmWasm/cosmwasm/blob/v1.4.1/packages/std/src/panic.rs#L10-L12. If I recall correctly, this used to be a relative path.

It seems like more recent versions of Rust use a line break between location and message.
Bildschirmfoto 2023-10-16 um 09 03 02

So to me there are two open questions:

  • Can we ensure there are no full file paths in the Wasm file or do we just need to accept that and point to containerized build systems?
  • Should we customize how we build up the panic message let full_message = info.to_string();?
@AmitPr
Copy link
Contributor

AmitPr commented Oct 16, 2023

the --remap-path-prefix rustc flag might be relevant here

@chipshort
Copy link
Collaborator

This seems more like something that should be fixed in the Rust compiler.
I did a bit of testing around this and here is what I found:

  • Removing the info.to_string() will obviously hide the paths from the panic messages, but it did not remove them from the wasm file. We could probably go through the result of info.to_string() and remove all absolute paths, but it feels a bit like a hack.
  • Adding something like RUSTFLAGS="--remap-path-prefix /Users/christoph=/" works, but is obviously very user-specific, so I don't see a way to add it for example to cw-template

@webmaster128
Copy link
Member Author

Yeah, my 2 cents:

  • We should communicate it as a known limitation that a user's paths end up in the Wasm if no rust-optimizer/workspace-optimizer is used
  • Instead of info.to_string() we can build the panic message manually for stability. E.g. in the past it was only a relative file path. Not it's the full path. And in the latest Rust source code there is a line break between location and message, which we don't really want.
  • --remap-path-prefix might be useful inside of rust-optimizer/workspace-optimizer to reduce the size of the Wasm and keep error messages short

@chipshort
Copy link
Collaborator

chipshort commented Nov 28, 2023

Looks like we cannot build the panic message ourselves, since access to PanicInfo::message is currently not stabilized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev-experience improvements to DevX
Projects
None yet
Development

No branches or pull requests

3 participants