Serde: Deriving Deserialize inside macro expansion can lead to crash

[jannschu]

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]

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

[Marwes]

Probably https://github.com/serde-rs/serde/blob/ee75e6c0e9e5cc359163202554feb00a28b62d0d/serde_derive_internals/src/ctxt.rs#L52 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]

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]

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]

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

[hcpl]

@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]

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