Add a lint to plug autotrait impl soundness hole (#263)

README.md

@@ -236,7 +236,7 @@ The PL/Rust-specific configuration options, some of which are **required**, are:
 | `plrust.aarch64_linker`            | string | Name of the linker `rustc` should use on for cross-compile.                                                                                                   | no       | `'aarch64_linux_gnu_gcc'`                                                                                            |
 | `plrust.x86_64_pgx_bindings_path`  | string | Path to output from `cargo pgx cross pgx-target` on x86_64.                                                                                                   | no-ish   | <none>                                                                                                               |
 | `plrust.aarch64_pgx_bindings_path` | string | Path to output form `cargo pgx cross pgx-target` on aarch64.                                                                                                  | no-ish   | <none>                                                                                                               |
-| `plrust.compile_lints`             | string | A comma-separated list of Rust lints to apply to every user function.                                                                                         | no       | `'plrust_extern_blocks, plrust_lifetime_parameterized_traits, implied_bounds_entailment, unsafe_code, plrust_filesystem_macros, plrust_env_macros, plrust_external_mod, plrust_fn_pointers, plrust_async, plrust_leaky, plrust_print_macros, plrust_stdio, unknown_lints, deprecated, suspicious_auto_trait_impls, unaligned_references, soft_unstable'` |
+| `plrust.compile_lints`             | string | A comma-separated list of Rust lints to apply to every user function.                                                                                         | no       | `'plrust_extern_blocks, plrust_lifetime_parameterized_traits, implied_bounds_entailment, unsafe_code, plrust_filesystem_macros, plrust_env_macros, plrust_external_mod, plrust_fn_pointers, plrust_async, plrust_leaky, plrust_print_macros, plrust_stdio, unknown_lints, deprecated, suspicious_auto_trait_impls, unaligned_references, soft_unstable, plrust_autotrait_impls'` |
 | `plrust.required_lints`            | string | A comma-separated list of Rust lints that are required to have been applied to a user function before PL/Rust will load the library and execute the function. | no       | defaults to whatever `plrust.compile_lints` happens to be                                                            |            
 | `plrust.trusted_pgx_version`       | string | The version of the [`plrust-trusted-pgx`](https://crates.io/crates/plrust-trusted-pgx) crate from crates.io to use when compiling user functions              |
 

doc/src/config-lints.md

@@ -216,3 +216,17 @@ std::io::stdout().write_all(b"foobar").unwrap();
 std::io::stderr().write_all(b"foobar").unwrap();
 let _stdin_is_forbidden_too = std::io::stdin();
 ```
+
+### `plrust_autotrait_impls`
+
+This lint forbids explicit implementations of the safe auto traits, as a workaround for various soundness holes around these. It may be relaxed in the future if those are fixed.
+
+```rust
+struct Foo(std::cell::Cell<i32>, std::marker::PhantomPinned);
+// Any of the following implementations would be forbidden.
+impl std::panic::UnwindSafe for Foo {}
+impl std::panic::RefUnwindSafe for Foo {}
+impl std::marker::Unpin for Foo {}
+```
+
+As a workaround, in most cases, you should be able to use [`std::panic::AssertUnwindSafe`](https://doc.rust-lang.org/nightly/std/panic/struct.AssertUnwindSafe.html) instead of implementing one of the `UnwindSafe` traits, and Boxing your type can usually work around the need for `Unpin` (which should be rare in non-`async` code anyway).

plrust/src/lib.rs

@@ -69,6 +69,7 @@ const DEFAULT_LINTS: &'static str = "\
     plrust_extern_blocks, \
     plrust_lifetime_parameterized_traits, \
     implied_bounds_entailment, \
+    plrust_autotrait_impls, \
     plrust_fn_pointers, \
     plrust_filesystem_macros, \
     plrust_env_macros, \

plrustc/plrustc/src/lints.rs

@@ -62,6 +62,51 @@ impl<'tcx> LateLintPass<'tcx> for NoExternBlockPass {
     }
 }
 
+declare_plrust_lint!(
+    pub(crate) PLRUST_AUTOTRAIT_IMPLS,
+    "Disallow impls of auto traits",
+);
+
+declare_lint_pass!(PlrustAutoTraitImpls => [PLRUST_AUTOTRAIT_IMPLS]);
+
+impl<'tcx> LateLintPass<'tcx> for PlrustAutoTraitImpls {
+    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) {
+        fn trigger(cx: &LateContext<'_>, span: Span) {
+            cx.lint(
+                PLRUST_AUTOTRAIT_IMPLS,
+                "explicit implementations of auto traits are forbidden in PL/Rust",
+                |b| b.set_span(span),
+            );
+        }
+        let hir::ItemKind::Impl(imp) = &item.kind else {
+            return;
+        };
+        let Some(trait_) = &imp.of_trait else {
+            return;
+        };
+        let Some(did) = trait_.trait_def_id() else {
+            return;
+        };
+        // I think ideally we'd resolve `did` into the actual trait type and
+        // check if it's audo, but I don't know how to do that here, so I'll
+        // just check for UnwindSafe, RefUnwindSafe, and Unpin explicitly (these
+        // are currently the only safe stable auto traits, I believe).
+        //
+        // The former two have diagnostic items, the latter doesn't but is a
+        // lang item.
+        if matches!(cx.tcx.lang_items().unpin_trait(), Some(unpin_did) if unpin_did == did) {
+            trigger(cx, item.span);
+        }
+
+        if let Some(thing) = cx.tcx.get_diagnostic_name(did) {
+            let name = thing.as_str();
+            if name == "unwind_safe_trait" || name == "ref_unwind_safe_trait" {
+                trigger(cx, item.span);
+            }
+        }
+    }
+}
+
 declare_plrust_lint!(
     pub(crate) PLRUST_LIFETIME_PARAMETERIZED_TRAITS,
     "Disallow lifetime parameterized traits"
@@ -489,6 +534,7 @@ static INCLUDE_TEST_ONLY_LINTS: Lazy<bool> =
 static PLRUST_LINTS: Lazy<Vec<&'static Lint>> = Lazy::new(|| {
     let mut v = vec![
         PLRUST_ASYNC,
+        PLRUST_AUTOTRAIT_IMPLS,
         PLRUST_EXTERN_BLOCKS,
         PLRUST_EXTERNAL_MOD,
         PLRUST_FILESYSTEM_MACROS,
@@ -533,6 +579,7 @@ pub fn register(store: &mut LintStore, _sess: &Session) {
     );
     store.register_early_pass(move || Box::new(PlrustAsync));
     store.register_early_pass(move || Box::new(PlrustExternalMod));
+    store.register_late_pass(move |_| Box::new(PlrustAutoTraitImpls));
     store.register_late_pass(move |_| Box::new(PlrustFnPointer));
     store.register_late_pass(move |_| Box::new(PlrustLeaky));
     store.register_late_pass(move |_| Box::new(PlrustBuiltinMacros));

plrustc/plrustc/uitests/impl_auto_trait.rs

@@ -0,0 +1,9 @@
+#![crate_type = "lib"]
+
+pub struct Foo(pub std::cell::Cell<i32>, pub std::marker::PhantomPinned);
+
+impl std::panic::UnwindSafe for Foo {}
+
+impl std::panic::RefUnwindSafe for Foo {}
+
+impl std::marker::Unpin for Foo {}

plrustc/plrustc/uitests/impl_auto_trait.stderr

@@ -0,0 +1,22 @@
+error: explicit implementations of auto traits are forbidden in PL/Rust
+  --> $DIR/impl_auto_trait.rs:5:1
+   |
+LL | impl std::panic::UnwindSafe for Foo {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `-F plrust-autotrait-impls` implied by `-F plrust-lints`
+
+error: explicit implementations of auto traits are forbidden in PL/Rust
+  --> $DIR/impl_auto_trait.rs:7:1
+   |
+LL | impl std::panic::RefUnwindSafe for Foo {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: explicit implementations of auto traits are forbidden in PL/Rust
+  --> $DIR/impl_auto_trait.rs:9:1
+   |
+LL | impl std::marker::Unpin for Foo {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 3 previous errors
+