dtolnay / no-panic

Attribute macro to require that the compiler prove a function can't ever panic
Apache License 2.0
1k stars 13 forks source link

Panic detected on a dummy function from a lib #15

Closed RazrFalcon closed 4 years ago

RazrFalcon commented 4 years ago

It's hard to explain what is wrong, so I've created a dedicated repo: https://github.com/RazrFalcon/no-panic-bug

> cd subcrate
> cargo run --release
   Compiling subcrate v0.1.0 (/home/razr/Projects/rust/no-panic-bug/subcrate)
error: linking with `cc` failed: exit code: 1
  |
  = note: "cc" "-Wl,--as-needed" "-Wl,-z,noexecstack" "-m64" "-L" "/home/razr/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "/home/razr/Projects/rust/no-panic-bug/subcrate/target/release/deps/no_panic_bug-cf83053df4e0051c.no_panic_bug.82plb7ue-cgu.0.rcgu.o" "/home/razr/Projects/rust/no-panic-bug/subcrate/target/release/deps/no_panic_bug-cf83053df4e0051c.no_panic_bug.82plb7ue-cgu.1.rcgu.o" "/home/razr/Projects/rust/no-panic-bug/subcrate/target/release/deps/no_panic_bug-cf83053df4e0051c.no_panic_bug.82plb7ue-cgu.2.rcgu.o" "-o" "/home/razr/Projects/rust/no-panic-bug/subcrate/target/release/deps/no_panic_bug-cf83053df4e0051c" "/home/razr/Projects/rust/no-panic-bug/subcrate/target/release/deps/no_panic_bug-cf83053df4e0051c.3j01wz2tg4ra285.rcgu.o" "-Wl,--gc-sections" "-pie" "-Wl,-zrelro" "-Wl,-znow" "-Wl,-O1" "-nodefaultlibs" "-L" "/home/razr/Projects/rust/no-panic-bug/subcrate/target/release/deps" "-L" "/home/razr/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-Wl,-Bstatic" "/home/razr/Projects/rust/no-panic-bug/subcrate/target/release/deps/libno_panic_bug-d051b0da612a7e03.rlib" "-Wl,--start-group" "/home/razr/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-c32b051c3aafd36c.rlib" "/home/razr/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libpanic_unwind-eabf8b29c0a244dd.rlib" "/home/razr/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libhashbrown-5c336cc1b5ec2048.rlib" "/home/razr/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc_std_workspace_alloc-c7631f762b1ba6d9.rlib" "/home/razr/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libbacktrace-db0f6c539591c951.rlib" "/home/razr/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libbacktrace_sys-32c2dc6fbc292c9c.rlib" "/home/razr/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc_demangle-84e9c510dc249620.rlib" "/home/razr/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libunwind-13bc027534de0b4c.rlib" "/home/razr/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcfg_if-b3c13ecda1794c6c.rlib" "/home/razr/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/liblibc-72dc11de859645e9.rlib" "/home/razr/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/liballoc-a78b04f112feb31a.rlib" "/home/razr/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc_std_workspace_core-29469f6c53ac35f8.rlib" "/home/razr/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcore-0eb3c513c640c4a6.rlib" "-Wl,--end-group" "/home/razr/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcompiler_builtins-0b278345638bce90.rlib" "-Wl,-Bdynamic" "-ldl" "-lrt" "-lpthread" "-lgcc_s" "-lc" "-lm" "-lrt" "-lpthread" "-lutil" "-lutil"
  = note: /usr/lib/gcc/x86_64-pc-linux-gnu/9.2.0/../../../../x86_64-pc-linux-gnu/bin/ld: /home/razr/Projects/rust/no-panic-bug/subcrate/target/release/deps/no_panic_bug-cf83053df4e0051c.no_panic_bug.82plb7ue-cgu.1.rcgu.o: in function `<no_panic_bug::dummy::__NoPanic as core::ops::drop::Drop>::drop':
          no_panic_bug.82plb7ue-cgu.1:(.text._ZN72_$LT$no_panic_bug..dummy..__NoPanic$u20$as$u20$core..ops..drop..Drop$GT$4drop17hde01b7f3842a4fd1E+0x3): undefined reference to `

          ERROR[no-panic]: detected panic in function `dummy`
          '
          collect2: error: ld returned 1 exit status

error: aborting due to previous error

error: could not compile `subcrate`.

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

Interestingly, it works fine when the library function is marked as #[inline].

dtolnay commented 4 years ago

I think this is behaving correctly -- the extern "Rust" ABI includes panicking on every extern function call at codegen time, which applies to all non-generic non-inline Rust functions from other crates as well as to all indirect calls made through Rust function pointers or trait objects.

The linker should be able to eliminate those panics at link time if you link with LTO:

[profile.release]
lto = "thin"
RazrFalcon commented 4 years ago

Thanks, it worked. But I'm not sure what is happening here. Does rustc wraps each call in a panic (for whatever reason) or is this a no-panic limitation?

dtolnay commented 4 years ago

Rustc codegen wraps every extern function call with "Rust" ABI (as described above) in panic handling.

In your case it's two different codegen units compiling the function being called and the function making the call, so the earliest place that would have information to know there is no panic is the linker at LTO.

RazrFalcon commented 4 years ago

I see, thanks. I wanted to test my library without adding the no-panic dependency and stumbled on this issue.

RazrFalcon commented 4 years ago

Sorry for bothering again, but I just wanted to clarify the no-panic behavior. Let's say I will create a separate executable that will invoke all library methods wrapped in #[no_panic], like it was done in the linked repo, does it 100% guarantee that it will catch all panics or is it still false-positive?

dtolnay commented 4 years ago

It takes into account all panics.