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

Deriving Deserialize inside macro expansion can lead to crash #1176

Closed
jannschu opened this issue Mar 13, 2018 · 7 comments
Closed

Deriving Deserialize inside macro expansion can lead to crash #1176

jannschu opened this issue Mar 13, 2018 · 7 comments
Labels

Comments

@jannschu
Copy link

jannschu commented Mar 13, 2018

I have encountered a crash of the derive macro.

Using Rust 1.24.1 and the dependencies

[dependencies]
serde = "1.0"
serde_derive = "1.0"

the following lib.rs (in a fresh cargo project) does crash the compiler:

extern crate serde;
#[macro_use]
extern crate serde_derive;

macro_rules! crash_compiler {
    (
    	$(#[ $meta:meta ])*
    	struct $name:ident<'a> {
    		$($body:tt)*
    	}
    ) => {
    	$(#[$meta])*
    	struct $name<'a> {
    		$($body)*
    	}

    	$(#[$meta])*
    	#[derive(Deserialize)]
    	struct Raw<'a> {
    		other_field: &str,
    		$($body)*
    	}
    }
}

crash_compiler! {
	#[derive(Debug)]
	struct A<'a> {
	    #[serde(borrow, default)]
	    field: Option<&'a str>,
	}
}

Removing some lines from the macro reveals a proper error (the #[serde(...)] inside the first struct after macro expansion makes no sense).

Checking with Rust nightly I get the following stack trace:

thread 'rustc' panicked at 'called `Result::unwrap()` on an `Err` value: ()', libcore/result.rs:945:5
note: Run with `RUST_BACKTRACE=1` for a backtrace.
thread 'rustc' panicked at 'forgot to check for errors', /Users/jannschu/.cargo/registry/src/github.com-1ecc6299db9ec823/serde_derive_internals-0.20.0/src/ctxt.rs:52:13
stack backtrace:
   0:        0x11139e883 - std::sys::unix::backtrace::tracing::imp::unwind_backtrace::h9c9ce376793e6730
   1:        0x1113963e1 - std::sys_common::backtrace::_print::h74624edd14ef3f1e
   2:        0x11139b530 - std::panicking::default_hook::{{closure}}::h75896328f4aa59bf
   3:        0x11139b1ae - std::panicking::default_hook::hfd95abc29d3b7f61
   4:        0x11139b9d6 - std::panicking::rust_panic_with_hook::hd0b337c90f237633
   5:        0x11833e457 - std::panicking::begin_panic::hcb10be311556e452
                               at /Users/travis/build/rust-lang/rust/src/libstd/panicking.rs:537
   6:        0x1183457cd - serde_derive_internals::ctxt::Ctxt::error::hf3cdd3c93774596d
                               at /Users/jannschu/.cargo/registry/src/github.com-1ecc6299db9ec823/serde_derive_internals-0.20.0/src/ctxt.rs:52
   7:        0x118276874 - core::ptr::drop_in_place::h82a45f9adce8ec89
                               at /Users/travis/build/rust-lang/rust/src/libcore/ptr.rs:59
   8:        0x118203272 - serde_derive::de::expand_derive_deserialize::he6c6406e03201c14
                               at /Users/jannschu/.cargo/registry/src/github.com-1ecc6299db9ec823/serde_derive-1.0.30/src/de.rs:71
   9:        0x11828022c - serde_derive::derive_deserialize::hae11970e7e265ed3
                               at /Users/jannschu/.cargo/registry/src/github.com-1ecc6299db9ec823/serde_derive-1.0.30/src/lib.rs:64
  10:        0x10f16071a - std::panicking::try::do_call::h90410b4266209460
  11:        0x1113b698e - __rust_maybe_catch_panic
  12:        0x10f1b0c9b - <syntax_ext::deriving::custom::ProcMacroDerive as syntax::ext::base::MultiItemModifier>::expand::ha974d842c5e80d27
  13:        0x110d25764 - syntax::ext::expand::MacroExpander::expand_invoc::h257752422620757d
  14:        0x110d1db28 - syntax::ext::expand::MacroExpander::expand::h9897ef13f3cd3edf
  15:        0x110d1ccb2 - syntax::ext::expand::MacroExpander::expand_crate::h2651a53662ff80f2
  16:        0x10e6f14d5 - rustc_driver::driver::phase_2_configure_and_expand_inner::{{closure}}::ha8fa4f039e3c1d57
  17:        0x10e6ee980 - rustc_driver::driver::phase_2_configure_and_expand_inner::hff9b05c6a140eb7c
  18:        0x10e6e798f - rustc_driver::driver::compile_input::hbebb4a750d86bb48
  19:        0x10e70580f - rustc_driver::run_compiler::h5d4fdc0bc3aef1c2
  20:        0x10e62e605 - std::sys_common::backtrace::__rust_begin_short_backtrace::hd2cdbbc75a404436
  21:        0x1113b698e - __rust_maybe_catch_panic
  22:        0x10e66a615 - <F as alloc::boxed::FnBox<A>>::call_box::h6cb998f8dcd1f621
  23:        0x1113ab7ab - std::sys::unix::thread::Thread::new::thread_start::h9199a8f85f61748b
  24:     0x7fff52eca6c0 - _pthread_body
  25:     0x7fff52eca56c - _pthread_start
thread panicked while panicking. aborting.
@jannschu
Copy link
Author

Just noticed that adding a lifetime to the line with other_field: &str, also prevents the crash.

@Marwes
Copy link
Contributor

Marwes commented Mar 13, 2018

Probably

panic!("forgot to check for errors");
should check for panics and avoid panicing if the code is unwinding due to an earlier panic https://doc.rust-lang.org/std/thread/fn.panicking.html? (Though I suppose the underlying panic should be fixed as well)

@jannschu
Copy link
Author

I am not sure if this is the same problem, but this does also crash the compiler:

extern crate serde;
#[macro_use]
extern crate serde_derive;

macro_rules! deserialize {
    () => {
    	#[derive(Deserialize)]
        struct A {
            field: &str,
        }
    }
}

deserialize! {}
deserialize! {}

@hcpl
Copy link
Contributor

hcpl commented Mar 13, 2018

The last case can be simplified:

extern crate serde;
#[macro_use]
extern crate serde_derive;

#[derive(Deserialize)]
struct A {
    field: &str,
}

But the output is slightly different from the one in OP:

$ RUST_BACKTRACE=1 cargo build
   Compiling serde-issue-1176 v0.1.0 (file:///tmp/rust/serde-issue-1176)
thread panicked while panicking. aborting.
error: Could not compile `serde-issue-1176`.

To learn more, run the command again with --verbose.

Verbose output shows that there is a SIGILL:

thread panicked while panicking. aborting.
error: Could not compile `serde-issue-1176`.

Caused by:
  process didn't exit successfully: `rustc --crate-name serde_issue_1176 src/lib.rs --crate-type lib --emit=dep-info,link -C debuginfo=2 -C metadata=57ddcb1a7d32f7bb -C extra-filename=-57ddcb1a7d32f7bb --out-dir /tmp/rust/serde-issue-1176/target/debug/deps -L dependency=/tmp/rust/serde-issue-1176/target/debug/deps --extern serde=/tmp/rust/serde-issue-1176/target/debug/deps/libserde-17f8368d6e644a9d.rlib --extern serde_derive=/tmp/rust/serde-issue-1176/target/debug/deps/libserde_derive-ebb2bb204993d3ef.so` (signal: 4, SIGILL: illegal instruction)

tail -f /var/log/kern.log shows:

...
Mar 13 16:44:33 hcpl-comp kernel: [768509.773146] traps: rustc[20688] trap invalid opcode ip:7fdc8f51183c sp:7fdc84ff5bb0 error:0 in libstd-58a9e2944951d97f.so[7fdc8f498000+125000]

@dtolnay
Copy link
Member

dtolnay commented Mar 13, 2018

I fixed the double panic as suggested by @Marwes in 5bc8053 and fixed the underlying panic in 56b2e39. I released 1.0.31 with these fixes. Thanks!

@dtolnay dtolnay closed this as completed Mar 13, 2018
@hcpl
Copy link
Contributor

hcpl commented Mar 13, 2018

@dtolnay those fixes only affect serde_derive_internals which you didn't update... and because of that it was pointless to release new serde/serde_derive at all.

Can you release serde_derive_internals 0.21.0? I guess serde/serde_derive will have to be bumped once again.

@dtolnay
Copy link
Member

dtolnay commented Mar 13, 2018

Oops! Hopefully I got it right with 1.0.32. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

4 participants