immunant / c2rust

Migrate C code to Rust
https://c2rust.com/
Other
3.93k stars 230 forks source link

Add error messages instead of unwrap #1123

Open ChienKaiMa opened 1 week ago

ChienKaiMa commented 1 week ago

I'm trying c2rust on an open source project https://github.com/berkeley-abc/abc I followed the instructions in README.md, installed c2rust from git (0.19.0), and generated compile_commands.json. c2rust panicked and I could not figure out which part of the C code caused the problem. Part of the command output of RUST_BACKTRACE=full c2rust transpile compile_commands.json

Transpiling abcDfs.c
thread 'main' panicked at c2rust-transpile/src/translator/mod.rs:233:47:
called `Option::unwrap()` on a `None` value

I read the code https://github.com/ChienKaiMa/c2rust/blob/9eaf8a15ede349a3ed9bff4af4638a80c88b8b65/c2rust-transpile/src/translator/mod.rs#L233

pub fn get_va_list_arg_name(&self) -> &str {
    return self.va_list_arg_name.as_ref().unwrap();
}

I assumed that it is abnormal that this unwrap would fail, but it just happened to me. My idea is to return a error message containing self.name so that users are able to identify which C function caused the panic. I'm fairly new to Rust, so I'm not sure if the program behavior is as intended, or it is an opportunity for me to work on a patch.

Full command output of RUST_BACKTRACE=full c2rust transpile compile_commands.json

warning: Skipping duplicate compilation cmd ...
warning: Skipping existing file ...
Transpiling abcDfs.c
thread 'main' panicked at c2rust-transpile/src/translator/mod.rs:233:47:
called `Option::unwrap()` on a `None` value
stack backtrace:
   0:     0x5af2f1943b95 - <std::sys::backtrace::BacktraceLock::print::DisplayBacktrace as core::fmt::Display>::fmt::h1b9dad2a88e955ff
   1:     0x5af2f1969bcb - core::fmt::write::h4b5a1270214bc4a7
   2:     0x5af2f1940e6f - std::io::Write::write_fmt::hd04af345a50c312d
   3:     0x5af2f1944ce1 - std::panicking::default_hook::{{closure}}::h96ab15e9936be7ed
   4:     0x5af2f19449bc - std::panicking::default_hook::h3cacb9c27561ad33
   5:     0x5af2f1945341 - std::panicking::rust_panic_with_hook::hfe205f6954b2c97b
   6:     0x5af2f1945173 - std::panicking::begin_panic_handler::{{closure}}::h6cb44b3a50f28c44
   7:     0x5af2f1944059 - std::sys::backtrace::__rust_end_short_backtrace::hf1c1f2a92799bb0e
   8:     0x5af2f1944e34 - rust_begin_unwind
   9:     0x5af2f13a91d3 - core::panicking::panic_fmt::h3d8fc78294164da7
  10:     0x5af2f13a925c - core::panicking::panic::hec978767ec2d35ff
  11:     0x5af2f13a9139 - core::option::unwrap_failed::hba6b08832f9ce30b
  12:     0x5af2f14d95dd - c2rust_transpile::translator::builtins::<impl c2rust_transpile::translator::Translation>::convert_builtin::hcc0514c63f59a6c3
  13:     0x5af2f1516fb1 - c2rust_transpile::translator::Translation::convert_expr::hd84ff785e5a3f9c2
  14:     0x5af2f14b07f9 - c2rust_transpile::cfg::CfgBuilder::convert_stmt_help::haa7683f6d52f5d2c
  15:     0x5af2f14ae0fc - c2rust_transpile::cfg::CfgBuilder::convert_stmts_help::hd439114e6c00fb51
  16:     0x5af2f151d989 - c2rust_transpile::translator::Translation::with_scope::h97ff826ad07da194
  17:     0x5af2f14a8f0c - c2rust_transpile::cfg::Cfg<c2rust_transpile::cfg::Label,c2rust_transpile::cfg::StmtOrDecl>::from_stmts::h3d700be0329c931c
  18:     0x5af2f150b9a7 - c2rust_transpile::translator::Translation::convert_function_body::hbec7894c1b727df1
  19:     0x5af2f150989e - c2rust_transpile::translator::Translation::convert_function::ha31b975ec6a38d75
  20:     0x5af2f1506d69 - c2rust_transpile::translator::Translation::convert_decl::hba428743f2f2181e
  21:     0x5af2f14f77b0 - c2rust_transpile::translator::translate::hf7235cc10c254ed3
  22:     0x5af2f13dfa7a - c2rust_transpile::transpile_single::hf06722b638c4d155
  23:     0x5af2f156da59 - <alloc::vec::Vec<T> as alloc::vec::spec_from_iter::SpecFromIter<T,I>>::from_iter::h39e510847d8c0a6b
  24:     0x5af2f13dc5bc - c2rust_transpile::transpile::h07c91dcbe877401c
  25:     0x5af2f13ae961 - c2rust_transpile::main::ha9f31e3816346ab2
  26:     0x5af2f13c03c3 - std::sys::backtrace::__rust_begin_short_backtrace::he2871ddce4c3574a
  27:     0x5af2f13c03b9 - std::rt::lang_start::{{closure}}::h29a6ce09ac4504ad
  28:     0x5af2f193bc00 - std::rt::lang_start_internal::h5e7c81cecd7f0954
  29:     0x5af2f13b99c5 - main
  30:     0x751cf0e29d90 - __libc_start_call_main
                               at ./csu/../sysdeps/nptl/libc_start_call_main.h:58:16
  31:     0x751cf0e29e40 - __libc_start_main_impl
                               at ./csu/../csu/libc-start.c:392:3
  32:     0x5af2f13a9ce5 - _start
  33:                0x0 - <unknown>
kkysen commented 1 week ago

Hi! Yes, this would be a fine place to help add some better error messages. The error handling in the transpiler is not very robust at all and mostly done by panicking, so adding better error messages would definitely help. Something like .unwrap_or_else(|| panic!("better error message")) would definitely be appreciated. Thanks!