Merge pull request #1278 from beicause/safety-level-config

Safeguard levels: fine-tune the amount of runtime validations

Author: Bromeon

.github/composite/godot-itest/action.yml

@@ -56,6 +56,11 @@ inputs:
     default: ''
     description: "If provided, acts as an argument for '--target', and results in output files written to ./target/{target}"
 
+  rust-target-dir:
+    required: false
+    default: ''
+    description: "If provided, determines where to find the generated dylib. Example: 'release' if Godot editor build would look for debug otherwise."
+
   rust-cache-key:
     required: false
     default: ''
@@ -151,6 +156,7 @@ runs:
       env:
         RUSTFLAGS: ${{ inputs.rust-env-rustflags }}
         TARGET: ${{ inputs.rust-target }}
+        OUTPUT_DIR: ${{ inputs.rust-target-dir || 'debug' }}
       run: |
         targetArgs=""
         if [[ -n "$TARGET" ]]; then
@@ -162,8 +168,23 @@ runs:
         
         # Instead of modifying .gdextension, rename the output directory.
         if [[ -n "$TARGET" ]]; then
-          rm -rf target/debug
-          mv target/$TARGET/debug target
+          rm -rf target/debug target/release
+          echo "Build output (tree -L 2 target/$TARGET/$OUTPUT_DIR):"
+          tree -L 2 target/$TARGET/$OUTPUT_DIR || echo "(tree not installed)"
+          mv target/$TARGET/$OUTPUT_DIR target/ || {
+            echo "::error::Output dir $TARGET/$OUTPUT_DIR not found"
+            exit 1
+          }
+        #  echo "Intermediate dir (tree -L 2 target):"
+        #  tree -L 2 target || echo "(tree not installed)"
+          if [[ "$OUTPUT_DIR" != "debug" ]]; then
+            mv target/$OUTPUT_DIR target/debug
+          fi
+          echo "Resulting dir (tree -L 2 target):"
+          tree -L 2 target || echo "(tree not installed)"
+        #  echo "itest.gdextension contents:"
+        #  cat itest/godot/itest.gdextension
+        #  echo "----------------------------------------------------"
         fi
       shell: bash
 

.github/workflows/full-ci.yml

@@ -378,14 +378,14 @@ jobs:
             godot-prebuilt-patch: '4.2.2'
             hot-reload: api-4-2
 
-          # Memory checks: special Godot binaries compiled with AddressSanitizer/LeakSanitizer to detect UB/leaks.
+          # Memory checks: special Godot binaries compiled with AddressSanitizer/LeakSanitizer to detect UB/leaks. Always Linux.
           # See also https://rustc-dev-guide.rust-lang.org/sanitizers.html.
           #
           # Additionally, the Godot source is patched to make dlclose() a no-op, as unloading dynamic libraries loses stacktrace and
           # cause false positives like println!. See https://github.com/google/sanitizers/issues/89.
           #
           # There is also a gcc variant besides clang, which is currently not used.
-          - name: linux-memcheck-nightly
+          - name: memcheck-nightly
             os: ubuntu-22.04
             artifact-name: linux-memcheck-nightly
             godot-binary: godot.linuxbsd.editor.dev.x86_64.llvm.san
@@ -395,7 +395,28 @@ jobs:
             # Sanitizers can't build proc-macros and build scripts; with --target, cargo ignores RUSTFLAGS for those two.
             rust-target: x86_64-unknown-linux-gnu
 
-          - name: linux-memcheck-4.2
+          - name: memcheck-release-disengaged
+            os: ubuntu-22.04
+            artifact-name: linux-memcheck-nightly
+            godot-binary: godot.linuxbsd.editor.dev.x86_64.llvm.san
+            rust-toolchain: nightly
+            rust-env-rustflags: -Zrandomize-layout -Zsanitizer=address
+            # Currently without itest/codegen-full-experimental.
+            rust-extra-args: --release --features godot/safeguards-release-disengaged
+            rust-target: x86_64-unknown-linux-gnu
+            rust-target-dir: release # We're running Godot debug build with Rust release dylib.
+
+          - name: memcheck-dev-balanced
+            os: ubuntu-22.04
+            artifact-name: linux-memcheck-nightly
+            godot-binary: godot.linuxbsd.editor.dev.x86_64.llvm.san
+            rust-toolchain: nightly
+            rust-env-rustflags: -Zrandomize-layout -Zsanitizer=address
+            # Currently without itest/codegen-full-experimental.
+            rust-extra-args: --features godot/safeguards-dev-balanced
+            rust-target: x86_64-unknown-linux-gnu
+
+          - name: memcheck-4.2
             os: ubuntu-22.04
             artifact-name: linux-memcheck-4.2
             godot-binary: godot.linuxbsd.editor.dev.x86_64.llvm.san
@@ -419,6 +440,7 @@ jobs:
           rust-toolchain: ${{ matrix.rust-toolchain || 'stable' }}
           rust-env-rustflags: ${{ matrix.rust-env-rustflags }}
           rust-target: ${{ matrix.rust-target }}
+          rust-target-dir: ${{ matrix.rust-target-dir }}
           rust-cache-key: ${{ matrix.rust-cache-key }}
           with-llvm: ${{ contains(matrix.name, 'macos') && contains(matrix.rust-extra-args, 'api-custom') }}
           godot-check-header: ${{ matrix.godot-check-header }}
@@ -500,7 +522,7 @@ jobs:
       - name: "Determine success or failure"
         run: |
           DEPENDENCIES='${{ toJson(needs) }}'
-          
+
           echo "Dependency jobs:"
           all_success=true
           for job in $(echo "$DEPENDENCIES" | jq -r 'keys[]'); do
@@ -510,7 +532,7 @@ jobs:
                   all_success=false
               fi
           done
-          
+
           if [[ "$all_success" == "false" ]]; then
               echo "One or more dependency jobs failed or were cancelled."
               exit 1

.github/workflows/minimal-ci.yml

@@ -210,9 +210,9 @@ jobs:
             godot-binary: godot.linuxbsd.editor.dev.x86_64
             godot-prebuilt-patch: '4.2.2'
 
-          # Memory checkers
+          # Memory checkers (always Linux).
 
-          - name: linux-memcheck
+          - name: memcheck
             os: ubuntu-22.04
             artifact-name: linux-memcheck-nightly
             godot-binary: godot.linuxbsd.editor.dev.x86_64.llvm.san
@@ -222,6 +222,27 @@ jobs:
             # Sanitizers can't build proc-macros and build scripts; with --target, cargo ignores RUSTFLAGS for those two.
             rust-target: x86_64-unknown-linux-gnu
 
+          - name: memcheck-release-disengaged
+            os: ubuntu-22.04
+            artifact-name: linux-memcheck-nightly
+            godot-binary: godot.linuxbsd.editor.dev.x86_64.llvm.san
+            rust-toolchain: nightly
+            rust-env-rustflags: -Zrandomize-layout -Zsanitizer=address
+            # Currently without itest/codegen-full-experimental.
+            rust-extra-args: --release --features godot/safeguards-release-disengaged
+            rust-target: x86_64-unknown-linux-gnu
+            rust-target-dir: release # We're running Godot debug build with Rust release dylib.
+
+          - name: memcheck-dev-balanced
+            os: ubuntu-22.04
+            artifact-name: linux-memcheck-nightly
+            godot-binary: godot.linuxbsd.editor.dev.x86_64.llvm.san
+            rust-toolchain: nightly
+            rust-env-rustflags: -Zrandomize-layout -Zsanitizer=address
+            # Currently without itest/codegen-full-experimental.
+            rust-extra-args: --features godot/safeguards-dev-balanced
+            rust-target: x86_64-unknown-linux-gnu
+
     steps:
       - uses: actions/checkout@v4
 
@@ -236,6 +257,7 @@ jobs:
           rust-toolchain: ${{ matrix.rust-toolchain || 'stable' }}
           rust-env-rustflags: ${{ matrix.rust-env-rustflags }}
           rust-target: ${{ matrix.rust-target }}
+          rust-target-dir: ${{ matrix.rust-target-dir }}
           with-llvm: ${{ contains(matrix.name, 'macos') && contains(matrix.rust-extra-args, 'api-custom') }}
           godot-check-header: ${{ matrix.godot-check-header }}
           godot-indirect-json: ${{ matrix.godot-indirect-json }}

godot-bindings/Cargo.toml

@@ -33,6 +33,10 @@ api-custom = ["dep:bindgen", "dep:regex", "dep:which"]
 api-custom-json = ["dep:nanoserde", "dep:bindgen", "dep:regex", "dep:which"]
 api-custom-extheader = []
 
+# Safeguard levels (see godot/lib.rs for detailed documentation).
+safeguards-dev-balanced = []
+safeguards-release-disengaged = []
+
 [dependencies]
 gdextension-api = { workspace = true }
 

godot-bindings/build.rs

@@ -10,7 +10,7 @@
 // It's the only purpose of this build.rs file. If a better solution is found, this file can be removed.
 
 #[rustfmt::skip]
-fn main() {    
+fn main() {
     let mut count = 0;
     if cfg!(feature = "api-custom") { count += 1; }
     if cfg!(feature = "api-custom-json") { count += 1; }

godot-bindings/src/lib.rs

@@ -267,3 +267,29 @@ pub fn before_api(major_minor: &str) -> bool {
 pub fn since_api(major_minor: &str) -> bool {
     !before_api(major_minor)
 }
+
+pub fn emit_safeguard_levels() {
+    // Levels: disengaged (0), balanced (1), strict (2)
+    let mut safeguards_level = if cfg!(debug_assertions) { 2 } else { 1 };
+
+    // Override default level with Cargo feature, in dev/release profiles.
+    #[cfg(debug_assertions)]
+    if cfg!(feature = "safeguards-dev-balanced") {
+        safeguards_level = 1;
+    }
+    #[cfg(not(debug_assertions))]
+    if cfg!(feature = "safeguards-release-disengaged") {
+        safeguards_level = 0;
+    }
+
+    println!(r#"cargo:rustc-check-cfg=cfg(safeguards_balanced)"#);
+    println!(r#"cargo:rustc-check-cfg=cfg(safeguards_strict)"#);
+
+    // Emit #[cfg]s cumulatively: strict builds get both balanced and strict.
+    if safeguards_level >= 1 {
+        println!(r#"cargo:rustc-cfg=safeguards_balanced"#);
+    }
+    if safeguards_level >= 2 {
+        println!(r#"cargo:rustc-cfg=safeguards_strict"#);
+    }
+}

godot-codegen/src/generator/classes.rs

@@ -192,11 +192,14 @@ fn make_class(class: &Class, ctx: &mut Context, view: &ApiView) -> GeneratedClas
     let (assoc_memory, assoc_dyn_memory, is_exportable) = make_bounds(class, ctx);
 
     let internal_methods = quote! {
-        fn __checked_id(&self) -> Option<crate::obj::InstanceId> {
-            // SAFETY: only Option due to layout-compatibility with RawGd<T>; it is always Some because stored in Gd<T> which is non-null.
-            let rtti = unsafe { self.rtti.as_ref().unwrap_unchecked() };
-            let instance_id = rtti.check_type::<Self>();
-            Some(instance_id)
+        /// Creates a validated object for FFI boundary crossing.
+        ///
+        /// Low-level internal method. Validation (liveness/type checks) depend on safeguard level.
+        fn __validated_obj(&self) -> crate::obj::ValidatedObject {
+            // SAFETY: Self has the same layout as RawGd<Self> (object_ptr + rtti fields in same order).
+            let raw_gd = unsafe { std::mem::transmute::<&Self, &crate::obj::RawGd<Self>>(self) };
+
+            raw_gd.validated_object()
         }
 
         #[doc(hidden)]
@@ -579,10 +582,10 @@ fn make_class_method_definition(
 
     let table_index = ctx.get_table_index(&MethodTableKey::from_class(class, method));
 
-    let maybe_instance_id = if method.qualifier() == FnQualifier::Static {
+    let validated_obj = if method.qualifier() == FnQualifier::Static {
         quote! { None }
     } else {
-        quote! { self.__checked_id() }
+        quote! { Some(self.__validated_obj()) }
     };
 
     let fptr_access = if cfg!(feature = "codegen-lazy-fptrs") {
@@ -598,16 +601,14 @@ fn make_class_method_definition(
         quote! { fptr_by_index(#table_index) }
     };
 
-    let object_ptr = &receiver.ffi_arg;
     let ptrcall_invocation = quote! {
         let method_bind = sys::#get_method_table().#fptr_access;
 
         Signature::<CallParams, CallRet>::out_class_ptrcall(
             method_bind,
             #rust_class_name,
             #rust_method_name,
-            #object_ptr,
-            #maybe_instance_id,
+            #validated_obj,
             args,
         )
     };
@@ -619,8 +620,7 @@ fn make_class_method_definition(
             method_bind,
             #rust_class_name,
             #rust_method_name,
-            #object_ptr,
-            #maybe_instance_id,
+            #validated_obj,
             args,
             varargs
         )

godot-codegen/src/generator/native_structures.rs

@@ -219,6 +219,7 @@ fn make_native_structure_field_and_accessor(
 
                 let obj = #snake_field_name.upcast();
 
+                #[cfg(safeguards_balanced)]
                 assert!(obj.is_instance_valid(), "provided node is dead");
 
                 let id = obj.instance_id().to_u64();

godot-core/Cargo.toml

@@ -39,6 +39,10 @@ api-4-4 = ["godot-ffi/api-4-4"]
 api-4-5 = ["godot-ffi/api-4-5"]
 # ]]
 
+# Safeguard levels (see godot/lib.rs for detailed documentation).
+safeguards-dev-balanced = ["godot-ffi/safeguards-dev-balanced"]
+safeguards-release-disengaged = ["godot-ffi/safeguards-release-disengaged"]
+
 [dependencies]
 godot-ffi = { path = "../godot-ffi", version = "=0.4.2" }
 

godot-core/build.rs

@@ -18,4 +18,5 @@ fn main() {
 
     godot_bindings::emit_godot_version_cfg();
     godot_bindings::emit_wasm_nothreads_cfg();
+    godot_bindings::emit_safeguard_levels();
 }