Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions .github/composite/godot-itest/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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: ''
Expand Down Expand Up @@ -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
Expand All @@ -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

Expand Down
32 changes: 27 additions & 5 deletions .github/workflows/full-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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 }}
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
26 changes: 24 additions & 2 deletions .github/workflows/minimal-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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 }}
Expand Down
4 changes: 4 additions & 0 deletions godot-bindings/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }

Expand Down
2 changes: 1 addition & 1 deletion godot-bindings/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down
26 changes: 26 additions & 0 deletions godot-bindings/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"#);
}
}
24 changes: 12 additions & 12 deletions godot-codegen/src/generator/classes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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") {
Expand All @@ -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,
)
};
Expand All @@ -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
)
Expand Down
1 change: 1 addition & 0 deletions godot-codegen/src/generator/native_structures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 4 additions & 0 deletions godot-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,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.1" }

Expand Down
1 change: 1 addition & 0 deletions godot-core/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,5 @@ fn main() {

godot_bindings::emit_godot_version_cfg();
godot_bindings::emit_wasm_nothreads_cfg();
godot_bindings::emit_safeguard_levels();
}
11 changes: 5 additions & 6 deletions godot-core/src/builtin/collections/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -968,7 +968,7 @@ impl<T: ArrayElement> Array<T> {
}

/// Validates that all elements in this array can be converted to integers of type `T`.
#[cfg(debug_assertions)]
#[cfg(safeguards_strict)]
pub(crate) fn debug_validate_int_elements(&self) -> Result<(), ConvertError> {
// SAFETY: every element is internally represented as Variant.
let canonical_array = unsafe { self.assume_type_ref::<Variant>() };
Expand All @@ -990,7 +990,7 @@ impl<T: ArrayElement> Array<T> {
}

// No-op in Release. Avoids O(n) conversion checks, but still panics on access.
#[cfg(not(debug_assertions))]
#[cfg(not(safeguards_strict))]
pub(crate) fn debug_validate_int_elements(&self) -> Result<(), ConvertError> {
Ok(())
}
Expand Down Expand Up @@ -1233,10 +1233,9 @@ impl<T: ArrayElement> Clone for Array<T> {
let copy = unsafe { self.clone_unchecked() };

// Double-check copy's runtime type in Debug mode.
if cfg!(debug_assertions) {
copy.with_checked_type().unwrap_or_else(|e| {
panic!("copied array should have same type as original array: {e}")
})
if cfg!(safeguards_strict) {
copy.with_checked_type()
.expect("copied array should have same type as original array")
} else {
copy
}
Expand Down
22 changes: 16 additions & 6 deletions godot-core/src/classes/class_runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,15 @@
//! Runtime checks and inspection of Godot classes.

use crate::builtin::{GString, StringName, Variant, VariantType};
#[cfg(debug_assertions)]
#[cfg(safeguards_strict)]
use crate::classes::{ClassDb, Object};
#[cfg(safeguards_balanced)]
use crate::meta::CallContext;
#[cfg(debug_assertions)]
#[cfg(safeguards_strict)]
use crate::meta::ClassId;
use crate::obj::{bounds, Bounds, Gd, GodotClass, InstanceId, RawGd, Singleton};
#[cfg(safeguards_strict)]
use crate::obj::Singleton;
use crate::obj::{bounds, Bounds, Gd, GodotClass, InstanceId, RawGd};
use crate::sys;

pub(crate) fn debug_string<T: GodotClass>(
Expand Down Expand Up @@ -191,6 +194,13 @@ where
Gd::<T>::from_obj_sys(object_ptr)
}

/// Checks that the object with the given instance ID is still alive and that the pointer is valid.
///
/// This does **not** perform type checking — use `ensure_object_type()` for that.
///
/// # Panics (balanced+strict safeguards)
/// If the object has been freed or the instance ID points to a different object.
#[cfg(safeguards_balanced)]
pub(crate) fn ensure_object_alive(
instance_id: InstanceId,
old_object_ptr: sys::GDExtensionObjectPtr,
Expand All @@ -211,7 +221,7 @@ pub(crate) fn ensure_object_alive(
);
}

#[cfg(debug_assertions)]
#[cfg(safeguards_strict)]
pub(crate) fn ensure_object_inherits(derived: ClassId, base: ClassId, instance_id: InstanceId) {
if derived == base
|| base == Object::class_id() // for Object base, anything inherits by definition
Expand All @@ -226,7 +236,7 @@ pub(crate) fn ensure_object_inherits(derived: ClassId, base: ClassId, instance_i
)
}

#[cfg(debug_assertions)]
#[cfg(safeguards_strict)]
pub(crate) fn ensure_binding_not_null<T>(binding: sys::GDExtensionClassInstancePtr)
where
T: GodotClass + Bounds<Declarer = bounds::DeclUser>,
Expand Down Expand Up @@ -254,7 +264,7 @@ where
// Implementation of this file

/// Checks if `derived` inherits from `base`, using a cache for _successful_ queries.
#[cfg(debug_assertions)]
#[cfg(safeguards_strict)]
fn is_derived_base_cached(derived: ClassId, base: ClassId) -> bool {
use std::collections::HashSet;

Expand Down
Loading
Loading