Support `-Cpanic=unwind` (on nightly), by just changing documentation.

Crubit is made sound for free by the changes in nightly to abort when panicking through `extern "C"`. These are documented here: https://doc.rust-lang.org/nomicon/ffi.html#ffi-and-unwinding -- however, note that this functionality has not yet hit Rust stable. The documentation change was made too soon, as far as I can tell. Still, this _will_ be the behavior, and when it is, we will be sound.

For `extern "C-unwind"`, I think our docs here were inaccurate, and it's not sound unless the caller enables exceptions. I suspect we should not release this functionality to end-users yet, as it's dangerous. Maybe we should instead default to the sound option of aborting.

We probably shouldn't say the issue of `panic=unwind` support is "solved" until we both remove the `-Cpanic=abort`, and the behavior reaches a stable release of rustc.

PiperOrigin-RevId: 663349913
Change-Id: I1ab8cb1131c038a023fceae123927eb22c1d0a0f
diff --git a/cc_bindings_from_rs/README.md b/cc_bindings_from_rs/README.md
index e94f72c..b2f0c08 100644
--- a/cc_bindings_from_rs/README.md
+++ b/cc_bindings_from_rs/README.md
@@ -21,8 +21,7 @@
     --clang-format-exe-path=<path_of_clang_format_executable> -- \
     --rustfmt-exe-path=<path_of_rustfmt_executable> -- \
     $HOME/scratch/test.rs \
-    --crate-type=lib \
-    --codegen=panic=abort
+    --crate-type=lib
 
 $ cat $HOME/scratch/test.h
 // Automatically @generated C++ bindings for the following Rust crate:
diff --git a/cc_bindings_from_rs/bazel_support/cc_bindings_from_rust_rule.bzl b/cc_bindings_from_rs/bazel_support/cc_bindings_from_rust_rule.bzl
index 3391903..dc545ba 100644
--- a/cc_bindings_from_rs/bazel_support/cc_bindings_from_rust_rule.bzl
+++ b/cc_bindings_from_rs/bazel_support/cc_bindings_from_rust_rule.bzl
@@ -135,7 +135,7 @@
         # can be interleaved with rustc flags in any order, and if we used _cc_bindings_from_rs_tool
         # as the tool_path for construct_arguments, then this could be `args.all` instead.
 
-        # TODO(b/254049425): We shouldn't override the panic arg, and instead work fine in any case.
+        # TODO(b/254049425): Remove `-Cpanic=abort` after crosstool contains cl/657372371.
         arguments = [args.process_wrapper_flags, "--", ctx.executable._cc_bindings_from_rs_tool.path, crubit_args, "--", args.rustc_flags, "-Cpanic=abort"],
     )
 
diff --git a/cc_bindings_from_rs/bindings.rs b/cc_bindings_from_rs/bindings.rs
index f88d573..d566e14 100644
--- a/cc_bindings_from_rs/bindings.rs
+++ b/cc_bindings_from_rs/bindings.rs
@@ -115,10 +115,6 @@
 
 pub fn generate_bindings(db: &Database) -> Result<Output> {
     let tcx = db.tcx();
-    match tcx.sess().panic_strategy() {
-        PanicStrategy::Unwind => bail!("No support for panic=unwind strategy (b/254049425)"),
-        PanicStrategy::Abort => (),
-    };
 
     let top_comment = {
         let crate_name = tcx.crate_name(LOCAL_CRATE);
@@ -1195,25 +1191,20 @@
 /// Otherwise returns an error the describes why the thunk is needed.
 fn is_thunk_required(sig: &ty::FnSig) -> Result<()> {
     match sig.abi {
-        // "C" ABI is okay: Before https://rust-lang.github.io/rfcs/2945-c-unwind-abi.html a
-        // Rust panic that "escapes" a "C" ABI function leads to Undefined Behavior.  This is
-        // unfortunate, but Crubit's `panics_and_exceptions.md` documents that `-Cpanic=abort`
-        // is the only supported configuration.
-        //
-        // After https://rust-lang.github.io/rfcs/2945-c-unwind-abi.html a Rust panic that
-        // tries to "escape" a "C" ABI function will terminate the program.  This is okay.
+        // "C" ABI is okay: since https://rust-lang.github.io/rfcs/2945-c-unwind-abi.html has been
+        // accepted, a Rust panic that "escapes" a "C" ABI function is a defined crash. See
+        // https://doc.rust-lang.org/nomicon/ffi.html#ffi-and-unwinding.
         rustc_target::spec::abi::Abi::C { unwind: false } => (),
 
-        // "C-unwind" ABI is okay: After
-        // https://rust-lang.github.io/rfcs/2945-c-unwind-abi.html a new "C-unwind" ABI may be
-        // used by Rust functions that want to safely propagate Rust panics through frames that
-        // may belong to another language.
+        // This requires a thunk if the calling C++ frames use `-fno-exceptions`, as it is
+        // UB. However, we leave this to the caller: if you use `extern "C-unwind"`, we assume you
+        // know what you are doing and do not block you from integrating with exception-enabled C++.
         rustc_target::spec::abi::Abi::C { unwind: true } => (),
 
         // All other ABIs trigger thunk generation.  This covers Rust ABI functions, but also
         // ABIs that theoretically are understood both by C++ and Rust (e.g. see
         // `format_cc_call_conv_as_clang_attribute` in `rs_bindings_from_cc/src_code_gen.rs`).
-        _ => bail!("Calling convention other than `extern \"C\"` requires a thunk"),
+        _ => bail!("Any calling convention other than `extern \"C\"` requires a thunk"),
     };
 
     ensure!(is_c_abi_compatible_by_value(sig.output()), "Return type requires a thunk");
@@ -7758,7 +7749,7 @@
             (
                 "fn(i32) -> i32", // TyKind::FnPtr (default ABI = "Rust")
                 "Function pointers can't have a thunk: \
-                 Calling convention other than `extern \"C\"` requires a thunk",
+                 Any calling convention other than `extern \"C\"` requires a thunk",
             ),
             (
                 "extern \"C\" fn (SomeStruct, f32) -> f32",
diff --git a/cc_bindings_from_rs/cc_bindings_from_rs.rs b/cc_bindings_from_rs/cc_bindings_from_rs.rs
index 5560d8c..2abae7c 100644
--- a/cc_bindings_from_rs/cc_bindings_from_rs.rs
+++ b/cc_bindings_from_rs/cc_bindings_from_rs.rs
@@ -437,21 +437,27 @@
         Ok(())
     }
 
-    /// `test_rustc_unsupported_panic_mechanism` tests that `panic=unwind`
-    /// results in an error.
+    /// `test_rustc_with_panic_unwind` tests that `panic=unwind`
+    /// is supported at least nominally.
     ///
     /// This is tested at the `cc_bindings_from_rs.rs` level instead of at the
     /// `bindings.rs` level, because `run_compiler_test_support` doesn't
     /// support specifying a custom panic mechanism.
     #[test]
-    fn test_rustc_unsupported_panic_mechanism() -> Result<()> {
-        let err = TestArgs::default_args()?
+    fn test_rustc_with_panic_unwind() -> Result<()> {
+        let _ = TestArgs::default_args()?
             .with_panic_mechanism("unwind")
             .run()
-            .expect_err("panic=unwind should trigger an error");
+            .expect("panic=unwind should not cause an error");
+        Ok(())
+    }
 
-        let msg = format!("{err:#}");
-        assert_eq!("No support for panic=unwind strategy (b/254049425)", msg);
+    #[test]
+    fn test_rustc_with_panic_abort() -> Result<()> {
+        let _ = TestArgs::default_args()?
+            .with_panic_mechanism("abort")
+            .run()
+            .expect("panic=abort should not cause an error");
         Ok(())
     }
 
diff --git a/cc_bindings_from_rs/cc_bindings_from_rs_sh_test.sh b/cc_bindings_from_rs/cc_bindings_from_rs_sh_test.sh
index 620a7fd..c2b8740 100755
--- a/cc_bindings_from_rs/cc_bindings_from_rs_sh_test.sh
+++ b/cc_bindings_from_rs/cc_bindings_from_rs_sh_test.sh
@@ -60,8 +60,7 @@
         \"$RS_INPUT_PATH\" \
         --crate-type=lib \
         --sysroot=$SYSROOT_PATH \
-        $(rustc_target_arg) \
-        --codegen=panic=abort" \
+        $(rustc_target_arg)" \
         "Expecting that this invocation of cc_bindings_from_rs will succeed"
 
   EXPECT_STR_EMPTY "$(cat $STDOUT_PATH)"
@@ -146,8 +145,7 @@
         \"$RS_INPUT_PATH\" \
         --crate-type=lib \
         --sysroot=$SYSROOT_PATH \
-        $(rustc_target_arg) \
-        --codegen=panic=abort" \
+        $(rustc_target_arg)" \
     "Invalid --h-out path should result in non-0 exit code"
 
   EXPECT_STR_EMPTY "$(cat $STDOUT_PATH)"
@@ -192,8 +190,7 @@
         \"$RS_INPUT_PATH\" \
         --crate-type=lib \
         $(rustc_target_arg) \
-        --sysroot=$SYSROOT_PATH \
-        --codegen=panic=abort" \
+        --sysroot=$SYSROOT_PATH" \
     "Expecting that this invocation of cc_bindings_from_rs will succeed"
 
   EXPECT_STR_EMPTY "$(cat $STDOUT_PATH)"
diff --git a/cc_bindings_from_rs/run_compiler_test_support.rs b/cc_bindings_from_rs/run_compiler_test_support.rs
index 1ec8269..adf2ae0 100644
--- a/cc_bindings_from_rs/run_compiler_test_support.rs
+++ b/cc_bindings_from_rs/run_compiler_test_support.rs
@@ -99,12 +99,6 @@
             ("warnings".to_string(), rustc_lint_defs::Level::Deny),
             ("stable_features".to_string(), rustc_lint_defs::Level::Allow),
         ],
-        cg: CodegenOptions {
-            // As pointed out in `panics_and_exceptions.md` the tool only supports `-C
-            // panic=abort` and therefore we explicitly opt into this config for tests.
-            panic: Some(rustc_target::spec::PanicStrategy::Abort),
-            ..Default::default()
-        },
         ..Default::default()
     };
 
diff --git a/cc_bindings_from_rs/test/bazel/unit_tests/generating_files/generating_files_test.bzl b/cc_bindings_from_rs/test/bazel/unit_tests/generating_files/generating_files_test.bzl
index a53a006..3dfe6bc 100644
--- a/cc_bindings_from_rs/test/bazel/unit_tests/generating_files/generating_files_test.bzl
+++ b/cc_bindings_from_rs/test/bazel/unit_tests/generating_files/generating_files_test.bzl
@@ -195,11 +195,6 @@
         "--crate-type=rlib" in cmdline,
         "Expected to find `--crate-type=rlib` on the command line, got {}.".format(cmdline),
     )
-    asserts.true(
-        env,
-        "-Cpanic=abort" in cmdline,
-        "Expected to find `-Cpanic=abort` on the command line, got {}.".format(cmdline),
-    )
 
     # ":emptylib" is a dependency of the crate for which we are generating bindings.
     # Similarly to how we pass `--extern=emptylib` to the command line for the `Rustc`
diff --git a/cc_bindings_from_rs/test/unwinding/BUILD b/cc_bindings_from_rs/test/unwinding/BUILD
new file mode 100644
index 0000000..c56084d
--- /dev/null
+++ b/cc_bindings_from_rs/test/unwinding/BUILD
@@ -0,0 +1,32 @@
+load(
+    "@rules_rust//rust:defs.bzl",
+    "rust_library",
+)
+load(
+    "//cc_bindings_from_rs/bazel_support:cc_bindings_from_rust_rule.bzl",
+    "cc_bindings_from_rust",
+)
+load("//common:crubit_wrapper_macros_oss.bzl", "crubit_cc_test")
+
+package(default_applicable_licenses = ["//:license"])
+
+rust_library(
+    name = "panic_function",
+    testonly = 1,
+    srcs = ["panic_function.rs"],
+)
+
+cc_bindings_from_rust(
+    name = "panic_function_cc_api",
+    testonly = 1,
+    crate = ":panic_function",
+)
+
+crubit_cc_test(
+    name = "unwinding_test",
+    srcs = ["unwinding_test.cc"],
+    deps = [
+        ":panic_function_cc_api",
+        "@com_google_googletest//:gtest_main",
+    ],
+)
diff --git a/cc_bindings_from_rs/test/unwinding/panic_function.rs b/cc_bindings_from_rs/test/unwinding/panic_function.rs
new file mode 100644
index 0000000..2a2585e
--- /dev/null
+++ b/cc_bindings_from_rs/test/unwinding/panic_function.rs
@@ -0,0 +1,11 @@
+// Part of the Crubit project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+pub fn panic_rust() {
+    panic!("this is a panic");
+}
+
+pub extern "C" fn panic_c() {
+    panic!("this is a panic");
+}
diff --git a/cc_bindings_from_rs/test/unwinding/unwinding_test.cc b/cc_bindings_from_rs/test/unwinding/unwinding_test.cc
new file mode 100644
index 0000000..014a734
--- /dev/null
+++ b/cc_bindings_from_rs/test/unwinding/unwinding_test.cc
@@ -0,0 +1,26 @@
+// Part of the Crubit project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include "cc_bindings_from_rs/test/unwinding/panic_function_cc_api.h"
+
+namespace {
+
+using ::testing::AllOf;
+using ::testing::HasSubstr;
+
+TEST(PanicTest, PanicRust) {
+  EXPECT_DEATH(panic_function::panic_rust(),
+               AllOf(HasSubstr("this is a panic"),
+                     HasSubstr("panic in a function that cannot unwind")));
+}
+
+TEST(PanicTest, PanicC) {
+  EXPECT_DEATH(panic_function::panic_c(),
+               AllOf(HasSubstr("this is a panic"),
+                     HasSubstr("panic in a function that cannot unwind")));
+}
+
+}  // namespace
diff --git a/docs/overview/panics_and_exceptions.md b/docs/overview/panics_and_exceptions.md
index 960dd49..4dcffed 100644
--- a/docs/overview/panics_and_exceptions.md
+++ b/docs/overview/panics_and_exceptions.md
@@ -1,82 +1,61 @@
 # Panics and Exceptions in C++/Rust FFI bindings
 
-This document explains what Crubit's `rs_bindings_from_cc` and
-`cc_bindings_from_rs` tools do to handle Rust panics and C++ exceptions.
+SUMMARY: Crubit currently requires `-fno-exceptions`, and converts unwinding
+panics into aborts.
 
-## Aborting on panic
+## Unwinding and Aborting
 
 > "Alright so I'm panicking, what else is there to do?" \
 > -- "The Hitchhiker’s Guide to the Galaxy" by Douglas Adams
 
-Rust libraries built with
-[`-Cpanic=abort`](https://doc.rust-lang.org/rustc/codegen-options/index.html#panic)
-terminate their process upon panic and there isn't much else to do. Similarly,
-C++ libraries built with Clang's
-[`-fno-exceptions`](https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fexceptions)
-flag terminate the process when a C++ exception is thrown.
+Rust and C++ both support a form of unwinding exception/panic, where ordinary
+control flow is terminated, and instead control proceeds up the stack, calling
+destructors along the way, until it is caught, converted to a termination, or
+completely unwinds the stack (which also leads to termination).
 
-In build environments that use the flags above, Rust panics and C++ exceptions
-don't unwind the callstack looking for appropriate handlers. This means that
-bindings generated by Crubit's `rs_bindings_from_cc` and `cc_bindings_from_rs`
-tools don't need to do anything special to handle panics and/or exceptions. In
-particular, Rust calls into C++ (and C++ calls into Rust) can just use the "C"
-ABI and assume that no panics and no exceptions will ever need to unwind across
-the FFI boundary.
+This has a performance cost (in that data must be managed soundly, destroyed if
+appropriate, in case of unwinding), and a code complexity cost (in that unsafe
+code must handle unexpected control flow edges soundly), so implementations of
+both languages allow for unwinding to be disabled, in favor of immediate process
+termination:
 
-Currently Crubit-generated bindings only support `-Cpanic=abort`,
-`-fno-exceptions` environment. See
-[the "Exceptions" section in the Google C++ Style Guide](https://google.github.io/styleguide/cppguide.html#Exceptions)
-for discussion of some of the pros and cons of an `-fno-exceptions` environment.
+*   Rust:
+    [`-Cpanic=abort`](https://doc.rust-lang.org/rustc/codegen-options/index.html#panic)
+*   C++
+    [`-fno-exceptions`](https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fexceptions)
 
-## Cross-language unwinding
+Crubit, and any FFI, will work if unwinding is disabled for both sides of the
+FFI boundary. But if it is enabled for one or both sides, and an exception or
+panic unwinds past an FFI boundary, we need special support to ensure that the
+behavior is defined.
 
-TODO(b/254049425): Add support for cross-FFI unwinding of Rust panics and C++
-exceptions.
+## Supported Configurations
 
 > "Who said anything about panicking?" snapped Arthur. "This is still just the
 > culture shock. You wait till I've settled down into the situation and found my
 > bearings. Then I'll start panicking." \
 > -- "The Hitchhiker’s Guide to the Galaxy" by Douglas Adams
 
-Crubit doesn't currently support unwinding of Rust panics or C++ exceptions
-across the FFI boundary. In other words, the generated bindings are only safe
-when `-Cpanic=abort` and `-fno-exceptions` are used - other configurations may
-lead to Undefined Behavior:
+**Rust:** Rust: Crubit can create bindings for libraries built with either
+`-Cpanic=abort` or `-Cpanic=unwind`.
 
-*   C++ exception unwinding through Rust frames leads to Undefined Behavior
-    (based on
-    [this part](https://rust-lang.github.io/rfcs/2945-c-unwind-abi.html#changes-to-the-behavior-of-existing-abi-strings)
-    of RFC 2945).
+If a panic unwinds past a Crubit FFI boundary, the process will terminate
+(on rustc nightly[^terminate_requirements]), with
+the **sole** exception of `extern "C-unwind"` functions. If you define an
+`extern "C-unwind"` function, you must ensure that it is only called by C++ code
+which enables exceptions. This responsibility is left to the caller.
 
-*   Rust panic unwinding through C++ frames leads to Undefined Behavior. Note
-    that [RFC 2945](https://rust-lang.github.io/rfcs/2945-c-unwind-abi.html)
-    defines some of that behavior: "with the `panic=unwind` runtime, `panic!`
-    will cause an abort if it would otherwise "escape" from a function defined
-    with `extern "C"`."
+**C++:** Crubit can create bindings for libraries built with `-fno-exceptions`.
+We do not generate `extern "C-unwind"` interfaces which could propagate a C++
+exception, and the behavior is undefined if an exception propagates past a
+Crubit FFI boundary. (See b/200067087 for catching this at compile time.)
 
-Below is an incomplete, tentative list of steps needed to add Crubit support for
-cross-language unwinding of panics and exceptions:
+<!-- TODO(b/200067087): fail when exceptions are enabled, document above. -->
 
-*   Support building Crubit's automated tests with `-Cpanic=unwind` and/or
-    `-fexceptions`. Use this support to add test coverage for:
+[^terminate_requirements]: We use the behavior of
+    https://doc.rust-lang.org/nomicon/ffi.html#ffi-and-unwinding
+    to cause a crash. However, this is not yet
+    incorporated into a stable Rust release, and requires
+    nightly.
 
-    -   Propagating a Rust panic across C++ frames and catching it in another
-        Rust frame.
-    -   Propagating a C++ exception across Rust frames and catching it in
-        another C++ frame.
-
-*   If one or both languages don't abort on unwind, then modify the generated
-    Rust code to use the new `"C-unwind"` ABI string proposed by
-    [RFC 2945](https://rust-lang.github.io/rfcs/2945-c-unwind-abi.html) (instead
-    of using the old `"C"` ABI string):
-
-    -   `rs_bindings_from_cc` should use `"C-unwind"` ABI when declaring C++
-        thunks in `mod detail` in the generated `..._rs_api.rs`
-    -   `cc_bindings_from_rs` should use `"C-unwind"` ABI when defining C++
-        thunks in `..._cc_api_impl.rs`
-
-*   Investigate if Crubit needs to modify the generated C++ code. For example:
-
-    -   C++ thunk definitions generated by `rs_bindings_from_cc` in
-        `..._rs_api_impl.cc`.
-    -   C++ declarations generated by `cc_bindings_from_rs` in `..._cc_api.h`.
+<!-- TODO: remove above note once crash reaches stable. -->