Update itests to conform with new safeguard levels

Author: Bromeon

itest/rust/build.rs

@@ -272,6 +272,7 @@ fn main() {
     rustfmt_if_needed(vec![rust_file]);
 
     godot_bindings::emit_godot_version_cfg();
+    godot_bindings::emit_safeguard_levels();
 
     // The godot crate has a __codegen-full default feature that enables the godot-codegen/codegen-full feature. When compiling the entire
     // workspace itest also gets compiled with full codegen due to feature unification. This causes compiler errors since the

itest/rust/src/builtin_tests/containers/variant_test.rs

@@ -21,7 +21,7 @@ use godot::obj::{Gd, InstanceId, NewAlloc, NewGd};
 use godot::sys::GodotFfi;
 
 use crate::common::roundtrip;
-use crate::framework::{expect_panic, itest, runs_release};
+use crate::framework::{expect_panic, expect_panic_or_ub, itest, runs_release};
 
 const TEST_BASIS: Basis = Basis::from_rows(
     Vector3::new(1.0, 2.0, 3.0),
@@ -272,7 +272,7 @@ fn variant_dead_object_conversions() {
     );
 
     // Variant::to().
-    expect_panic("Variant::to() with dead object should panic", || {
+    expect_panic_or_ub("Variant::to() with dead object should panic", || {
         let _: Gd<Node> = variant.to();
     });
 
@@ -361,15 +361,15 @@ fn variant_array_from_untyped_conversions() {
     )
 }
 
-// Same builtin type INT, but incompatible integers (Debug-only).
+// Same builtin type INT, but incompatible integers (strict-safeguards only).
 #[itest]
 fn variant_array_bad_integer_conversions() {
     let i32_array: Array<i32> = array![1, 2, 160, -40];
     let i32_variant = i32_array.to_variant();
     let i8_back = i32_variant.try_to::<Array<i8>>();
 
-    // In Debug mode, we expect an error upon conversion.
-    #[cfg(debug_assertions)]
+    // In strict safeguard mode, we expect an error upon conversion.
+    #[cfg(safeguards_strict)]
     {
         let err = i8_back.expect_err("Array<i32> -> Array<i8> conversion should fail");
         assert_eq!(
@@ -378,8 +378,8 @@ fn variant_array_bad_integer_conversions() {
         )
     }
 
-    // In Release mode, we expect the conversion to succeed, but a panic to occur on element access.
-    #[cfg(not(debug_assertions))]
+    // In balanced/disengaged modes, we expect the conversion to succeed, but a panic to occur on element access.
+    #[cfg(not(safeguards_strict))]
     {
         let i8_array = i8_back.expect("Array<i32> -> Array<i8> conversion should succeed");
         expect_panic("accessing element 160 as i8 should panic", || {

itest/rust/src/framework/mod.rs

@@ -200,6 +200,14 @@ pub fn expect_panic(context: &str, code: impl FnOnce()) {
     );
 }
 
+/// Run for code that panics in *strict* and *balanced* safeguard levels, but cause UB in *disengaged* level.
+///
+/// The code is not executed for the latter.
+pub fn expect_panic_or_ub(_context: &str, _code: impl FnOnce()) {
+    #[cfg(safeguards_balanced)]
+    expect_panic(_context, _code);
+}
+
 pin_project_lite::pin_project! {
     pub struct ExpectPanicFuture<T: std::future::Future> {
         context: &'static str,

itest/rust/src/object_tests/base_init_test.rs

@@ -10,7 +10,7 @@ use godot::builtin::Vector2;
 use godot::classes::notify::ObjectNotification;
 use godot::classes::{ClassDb, IRefCounted, RefCounted};
 use godot::meta::ToGodot;
-use godot::obj::{Base, Gd, InstanceId, NewAlloc, NewGd, Singleton, WithBaseField};
+use godot::obj::{Base, Gd, InstanceId, NewGd, Singleton, WithBaseField};
 use godot::register::{godot_api, GodotClass};
 use godot::task::TaskHandle;
 
@@ -65,13 +65,14 @@ fn base_init_extracted_gd() {
 
 // Checks bad practice of rug-pulling the base pointer.
 #[itest]
+#[cfg(safeguards_balanced)]
 fn base_init_freed_gd() {
     let mut free_executed = false;
 
     expect_panic("base object is destroyed", || {
         let _obj = Gd::<Based>::from_init_fn(|base| {
             let obj = base.to_init_gd();
-            obj.free(); // Causes the problem, but doesn't panic yet.
+            obj.free(); // Causes the problem, but doesn't panic yet. UB in safeguards-disengaged.
             free_executed = true;
 
             Based { base, i: 456 }
@@ -144,27 +145,28 @@ fn verify_complex_init((obj, base): (Gd<RefcBased>, Gd<RefCounted>)) -> Instance
     id
 }
 
-#[cfg(debug_assertions)]
+#[cfg(safeguards_strict)]
 #[itest]
 fn base_init_outside_init() {
+    use godot::obj::NewAlloc;
     let mut obj = Based::new_alloc();
 
     expect_panic("to_init_gd() outside init() function", || {
         let guard = obj.bind_mut();
-        let _gd = guard.base.to_init_gd(); // Panics in Debug builds.
+        let _gd = guard.base.to_init_gd(); // Panics in strict safeguard mode.
     });
 
     obj.free();
 }
 
-#[cfg(debug_assertions)]
+#[cfg(safeguards_strict)]
 #[itest]
 fn base_init_to_gd() {
     expect_panic("WithBaseField::to_gd() inside init() function", || {
         let _obj = Gd::<Based>::from_init_fn(|base| {
             let temp_obj = Based { base, i: 999 };
 
-            // Call to self.to_gd() during initialization should panic in Debug builds.
+            // Call to self.to_gd() during initialization should panic in strict safeguard mode.
             let _gd = godot::obj::WithBaseField::to_gd(&temp_obj);
 
             temp_obj

itest/rust/src/object_tests/base_test.rs

@@ -7,7 +7,7 @@
 
 use godot::prelude::*;
 
-use crate::framework::{expect_panic, itest};
+use crate::framework::{expect_panic_or_ub, itest};
 
 #[itest(skip)]
 fn base_test_is_weak() {
@@ -82,6 +82,8 @@ fn base_gd_self() {
 }
 
 // Hardening against https://github.com/godot-rust/gdext/issues/711.
+// Furthermore, this now keeps expect_panic_or_ub() instead of expect_panic(), although it's questionable if much remains with all these checks
+// disabled. There is still some logic remaining, and an alternative would be to #[cfg(safeguards_balanced)] this.
 #[itest]
 fn base_smuggling() {
     let (mut obj, extracted_base) = create_object_with_extracted_base();
@@ -98,27 +100,27 @@ fn base_smuggling() {
     extracted_base_obj.free();
 
     // Access to object should now fail.
-    expect_panic("object with dead base: calling base methods", || {
+    expect_panic_or_ub("object with dead base: calling base methods", || {
         obj.get_position();
     });
-    expect_panic("object with dead base: bind()", || {
+    expect_panic_or_ub("object with dead base: bind()", || {
         obj.bind();
     });
-    expect_panic("object with dead base: instance_id()", || {
+    expect_panic_or_ub("object with dead base: instance_id()", || {
         obj.instance_id();
     });
-    expect_panic("object with dead base: clone()", || {
+    expect_panic_or_ub("object with dead base: clone()", || {
         let _ = obj.clone();
     });
-    expect_panic("object with dead base: upcast()", || {
+    expect_panic_or_ub("object with dead base: upcast()", || {
         obj.upcast::<Object>();
     });
 
     // Now vice versa: destroy object, access base.
     let (obj, extracted_base) = create_object_with_extracted_base();
     obj.free();
 
-    expect_panic("accessing extracted base of dead object", || {
+    expect_panic_or_ub("accessing extracted base of dead object", || {
         extracted_base.__constructed_gd().get_position();
     });
 }

itest/rust/src/object_tests/object_swap_test.rs

@@ -10,8 +10,8 @@
 // A lot these tests also exist in the `object_test` module, where they test object lifetime rather than type swapping.
 // TODO consolidate them, so that it's less likely to forget edge cases.
 
-// Disabled in Release mode, since we don't perform the subtype check there.
-#![cfg(debug_assertions)]
+// Disabled in balanced/disengaged levels, since we don't perform the subtype check there.
+#![cfg(safeguards_strict)]
 
 use godot::builtin::GString;
 use godot::classes::{Node, Node3D, Object};

itest/rust/src/object_tests/object_test.rs

@@ -22,7 +22,7 @@ use godot::obj::{Base, Gd, Inherits, InstanceId, NewAlloc, NewGd, RawGd, Singlet
 use godot::register::{godot_api, GodotClass};
 use godot::sys::{self, interface_fn, GodotFfi};
 
-use crate::framework::{expect_panic, itest, TestContext};
+use crate::framework::{expect_panic, expect_panic_or_ub, itest, TestContext};
 
 // TODO:
 // * make sure that ptrcalls are used when possible (i.e. when type info available; maybe GDScript integration test)
@@ -171,7 +171,7 @@ fn object_instance_id_when_freed() {
     node.clone().free(); // destroys object without moving out of reference
     assert!(!node.is_instance_valid());
 
-    expect_panic("instance_id() on dead object", move || {
+    expect_panic_or_ub("instance_id() on dead object", move || {
         node.instance_id();
     });
 }
@@ -235,7 +235,7 @@ fn object_user_bind_after_free() {
     let copy = obj.clone();
     obj.free();
 
-    expect_panic("bind() on dead user object", move || {
+    expect_panic_or_ub("bind() on dead user object", move || {
         let _ = copy.bind();
     });
 }
@@ -247,7 +247,7 @@ fn object_user_free_during_bind() {
 
     let copy = obj.clone(); // TODO clone allowed while bound?
 
-    expect_panic("direct free() on user while it's bound", move || {
+    expect_panic_or_ub("direct free() on user while it's bound", move || {
         copy.free();
     });
 
@@ -275,7 +275,7 @@ fn object_engine_freed_argument_passing(ctx: &TestContext) {
 
     // Destroy object and then pass it to a Godot engine API.
     node.free();
-    expect_panic("pass freed Gd<T> to Godot engine API (T=Node)", || {
+    expect_panic_or_ub("pass freed Gd<T> to Godot engine API (T=Node)", || {
         tree.add_child(&node2);
     });
 }
@@ -288,10 +288,10 @@ fn object_user_freed_casts() {
 
     // Destroy object and then pass it to a Godot engine API (upcast itself works, see other tests).
     obj.free();
-    expect_panic("Gd<T>::upcast() on dead object (T=user)", || {
+    expect_panic_or_ub("Gd<T>::upcast() on dead object (T=user)", || {
         let _ = obj2.upcast::<Object>();
     });
-    expect_panic("Gd<T>::cast() on dead object (T=user)", || {
+    expect_panic_or_ub("Gd<T>::cast() on dead object (T=user)", || {
         let _ = base_obj.cast::<ObjPayload>();
     });
 }
@@ -306,7 +306,7 @@ fn object_user_freed_argument_passing() {
 
     // Destroy object and then pass it to a Godot engine API (upcast itself works, see other tests).
     obj.free();
-    expect_panic("pass freed Gd<T> to Godot engine API (T=user)", || {
+    expect_panic_or_ub("pass freed Gd<T> to Godot engine API (T=user)", || {
         engine.register_singleton("NeverRegistered", &obj2);
     });
 }
@@ -344,7 +344,7 @@ fn object_user_call_after_free() {
     let mut copy = obj.clone();
     obj.free();
 
-    expect_panic("call() on dead user object", move || {
+    expect_panic_or_ub("call() on dead user object", move || {
         let _ = copy.call("get_instance_id", &[]);
     });
 }
@@ -355,7 +355,7 @@ fn object_engine_use_after_free() {
     let copy = node.clone();
     node.free();
 
-    expect_panic("call method on dead engine object", move || {
+    expect_panic_or_ub("call method on dead engine object", move || {
         copy.get_position();
     });
 }
@@ -366,7 +366,7 @@ fn object_engine_use_after_free_varcall() {
     let mut copy = node.clone();
     node.free();
 
-    expect_panic("call method on dead engine object", move || {
+    expect_panic_or_ub("call method on dead engine object", move || {
         copy.call_deferred("get_position", &[]);
     });
 }
@@ -411,13 +411,13 @@ fn object_dead_eq() {
 
     {
         let lhs = a.clone();
-        expect_panic("Gd::eq() panics when one operand is dead", move || {
+        expect_panic_or_ub("Gd::eq() panics when one operand is dead", move || {
             let _ = lhs == b;
         });
     }
     {
         let rhs = a.clone();
-        expect_panic("Gd::ne() panics when one operand is dead", move || {
+        expect_panic_or_ub("Gd::ne() panics when one operand is dead", move || {
             let _ = b2 != rhs;
         });
     }
@@ -826,7 +826,7 @@ fn object_engine_manual_double_free() {
     let node2 = node.clone();
     node.free();
 
-    expect_panic("double free()", move || {
+    expect_panic_or_ub("double free()", move || {
         node2.free();
     });
 }
@@ -845,7 +845,7 @@ fn object_user_double_free() {
     let obj2 = obj.clone();
     obj.call("free", &[]);
 
-    expect_panic("double free()", move || {
+    expect_panic_or_ub("double free()", move || {
         obj2.free();
     });
 }