Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trying to access an out of bound index of an array using the get method, crashes the game #103328

Closed
JuanFdS opened this issue Feb 26, 2025 · 2 comments · Fixed by #103362
Closed

Comments

@JuanFdS
Copy link

JuanFdS commented Feb 26, 2025

Tested versions

  • Reproducible in v4.4.rc1.official [8ed125b]
  • Couldn't reproduce in 4.3 because the get method doesn't exist in that version.

System information

Godot v4.4.rc1 - Windows 11 (build 26100) - Single-window, 1 monitor - OpenGL 3 (Compatibility) - NVIDIA GeForce RTX 4060 Laptop GPU (NVIDIA; 32.0.15.6094) - 13th Gen Intel(R) Core(TM) i7-13700H (20 threads)

Issue description

The documentations says that get

Returns the element at the given index in the array. This is the same as using the [] operator (array[index]).

However, if I use [] with an array and access an out of bounds position, I get a debugger error. If I do it with get, the game crashes:

[1].get(3) # this crashes without prompting any error
[1][3] # this opens up the debugger

I'd expect get to work exactly as [] in that case.

Steps to reproduce

This code reproduces the error:

[1].get(3)

I also tested with an array that wasn't a literal and the same thing happened:

var my_array = [1,2,3,4,5,6,7]
my_array.clear()
return my_array.get(1)

Minimal reproduction project (MRP)

godot-crash-get-main.zip

@aaronp64
Copy link
Contributor

Looks like the Packed*Array types have the same behavior, stack traces for each below when calling get:

Array:

[0] Array::operator[] (C:\dev\godot\core\variant\array.cpp:111)
[1] `_register_variant_builtin_methods_array'::`2'::Method_Array_get::validated_call (C:\dev\godot\core\variant\variant_call.cpp:2357)
[2] GDScriptFunction::call (C:\dev\godot\modules\gdscript\gdscript_vm.cpp:2327)
[3] GDScriptInstance::callp (C:\dev\godot\modules\gdscript\gdscript.cpp:2053)
[4] Node::_notification (C:\dev\godot\scene\main\node.cpp:227)
[5] Node3D::_notificationv (C:\dev\godot\scene\3d\node_3d.h:52)
[6] Object::notification (C:\dev\godot\core\object\object.cpp:914)
[7] Node::_propagate_ready (C:\dev\godot\scene\main\node.cpp:283)
[8] Node::_propagate_ready (C:\dev\godot\scene\main\node.cpp:272)
[9] Node::_set_tree (C:\dev\godot\scene\main\node.cpp:3292)
[10] OS_Windows::run (C:\dev\godot\platform\windows\os_windows.cpp:2058)
[11] widechar_main (C:\dev\godot\platform\windows\godot_windows.cpp:97)
[12] _main (C:\dev\godot\platform\windows\godot_windows.cpp:124)
[13] main (C:\dev\godot\platform\windows\godot_windows.cpp:136)
[14] __scrt_common_main_seh (D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
[15] <couldn't map PC to fn name>

PackedFloat32Array:

[0] _VariantCall::func_PackedFloat32Array_get (C:\dev\godot\core\variant\variant_call.cpp:668)
[1] `_register_variant_builtin_methods_array'::`2'::Method_PackedFloat32Array_get::validated_call (C:\dev\godot\core\variant\variant_call.cpp:2406)
[2] GDScriptFunction::call (C:\dev\godot\modules\gdscript\gdscript_vm.cpp:2327)
[3] GDScriptInstance::callp (C:\dev\godot\modules\gdscript\gdscript.cpp:2053)
[4] Node::_notification (C:\dev\godot\scene\main\node.cpp:227)
[5] Node3D::_notificationv (C:\dev\godot\scene\3d\node_3d.h:52)
[6] Object::notification (C:\dev\godot\core\object\object.cpp:914)
[7] Node::_propagate_ready (C:\dev\godot\scene\main\node.cpp:283)
[8] Node::_propagate_ready (C:\dev\godot\scene\main\node.cpp:272)
[9] Node::_set_tree (C:\dev\godot\scene\main\node.cpp:3292)
[10] OS_Windows::run (C:\dev\godot\platform\windows\os_windows.cpp:2058)
[11] widechar_main (C:\dev\godot\platform\windows\godot_windows.cpp:97)
[12] _main (C:\dev\godot\platform\windows\godot_windows.cpp:124)
[13] main (C:\dev\godot\platform\windows\godot_windows.cpp:136)
[14] __scrt_common_main_seh (D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
[15] <couldn't map PC to fn name>

@Calinou Calinou added this to the 4.4 milestone Feb 26, 2025
@Calinou Calinou moved this from Unassessed to Very Bad in 4.x Release Blockers Feb 26, 2025
@Ivorforce
Copy link
Member

Ivorforce commented Feb 27, 2025

These functions have newly been bound for 4.4 (#95930, cc @aaronfranke). I've only just realized they haven't been around longer.

operator[] subscript in GDScript normally goes through variant_setget.cpp, which makes bounds checks. The way set and get were bound currently does not do bounds checks (or rather, does it at the last possible moment, leading to a crash rather than a graceful failure).
set and get should probably defer to the existing bindings to get aligned behavior. If that's hard to achieve, they could at least do bounds checks in VARCALL_PACKED_GETTER.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Very Bad
4 participants