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 create a large Array causes a silent crash in CowData #46842

Closed
Jayman2000 opened this issue Mar 9, 2021 · 8 comments
Closed

Trying to create a large Array causes a silent crash in CowData #46842

Jayman2000 opened this issue Mar 9, 2021 · 8 comments

Comments

@Jayman2000
Copy link
Contributor

Jayman2000 commented Mar 9, 2021

Godot version:

v3.2.3.stable.official

OS/device including version:

OS: Arch Linux (I don't know how to give a version for this)
CPU: x86_64 AMD Ryzen 5 3600 6-Core Processor
GPU: AMD Radeon RX 580
RAM: 15 GB + 8 GB swap

Issue description:

extends Node


const ARRAY_SIZE_LIMIT = 89478485


func _ready():
	[].resize(ARRAY_SIZE_LIMIT)  # This doesn't crash
	[].resize(ARRAY_SIZE_LIMIT + 1)  # This crashes

When the game crashes, there's no error message. Even if this should have caused a crash, there should at least be an error message.

Steps to reproduce:

  1. Create a new scene.
  2. Attach the above script to a Node.
  3. Run the scene.

Minimal reproduction project:

array_size_limit.zip

@Calinou
Copy link
Member

Calinou commented Mar 9, 2021

I can confirm this on 3.2 Git 08898e9. Backtrace:

handle_crash: Program crashed with signal 11
Dumping the backtrace. Please include this when reporting the bug on https://github.com/godotengine/godot/issues
[1] /lib64/libc.so.6(+0x3da60) [0x7fafcd59aa60] (??:0)
[2] CowData<Variant>::resize(int) (/home/hugo/Documents/Git/godotengine/godot/./core/cowdata.h:308)
[3] Vector<Variant>::resize(int) (/home/hugo/Documents/Git/godotengine/godot/./core/vector.h:84)
[4] Array::resize(int) (/home/hugo/Documents/Git/godotengine/godot/core/array.cpp:127)
[5] _VariantCall::_call_Array_resize(Variant&, Variant&, Variant const**) (/home/hugo/Documents/Git/godotengine/godot/core/variant_call.cpp:557)
[6] _VariantCall::FuncData::call(Variant&, Variant&, Variant const**, int, Variant::CallError&) (/home/hugo/Documents/Git/godotengine/godot/core/variant_call.cpp:?)
[7] Variant::call_ptr(StringName const&, Variant const**, int, Variant*, Variant::CallError&) (/home/hugo/Documents/Git/godotengine/godot/core/variant_call.cpp:1164)
[8] GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Variant::CallError&, GDScriptFunction::CallState*) (/home/hugo/Documents/Git/godotengine/godot/modules/gdscript/gdscript_function.cpp:1086)
[9] GDScriptInstance::_ml_call_reversed(GDScript*, StringName const&, Variant const**, int) (/home/hugo/Documents/Git/godotengine/godot/modules/gdscript/gdscript.cpp:1239)
[10] GDScriptInstance::call_multilevel_reversed(StringName const&, Variant const**, int) (/home/hugo/Documents/Git/godotengine/godot/modules/gdscript/gdscript.cpp:1248)
[11] Node::_notification(int) (/home/hugo/Documents/Git/godotengine/godot/scene/main/node.cpp:152)
[12] Node::_notificationv(int, bool) (/home/hugo/Documents/Git/godotengine/godot/./scene/main/node.h:46)
[13] Object::notification(int, bool) (/home/hugo/Documents/Git/godotengine/godot/core/object.cpp:929)
[14] Node::_propagate_ready() (/home/hugo/Documents/Git/godotengine/godot/scene/main/node.cpp:196)
[15] Node::_propagate_ready() (/home/hugo/Documents/Git/godotengine/godot/scene/main/node.cpp:186)
[16] Node::_set_tree(SceneTree*) (/home/hugo/Documents/Git/godotengine/godot/scene/main/node.cpp:?)
[17] SceneTree::init() (/home/hugo/Documents/Git/godotengine/godot/scene/main/scene_tree.cpp:464)
[18] OS_X11::run() (/home/hugo/Documents/Git/godotengine/godot/platform/x11/os_x11.cpp:?)
[19] /home/hugo/Documents/Git/godotengine/godot/bin/godot.x11.tools.64.llvm(main+0x181) [0x2b490b1] (/home/hugo/Documents/Git/godotengine/godot/platform/x11/godot_x11.cpp:56)
[20] /lib64/libc.so.6(__libc_start_main+0xf2) [0x7fafcd5851e2] (??:0)
[21] /home/hugo/Documents/Git/godotengine/godot/bin/godot.x11.tools.64.llvm(_start+0x2e) [0x2b48e6e] (??:?)
-- END OF BACKTRACE --

@Zylann
Copy link
Contributor

Zylann commented Mar 10, 2021

Turns out 89,478,485 x 24 = 2,147,483,640
Where 24 is roughly the size of Variant I guess (maybe not correct but it's close to that I think)
This is close to the maximum value of int, so it's possible it's an underflow due to a wrap to negatives.
Other possibility, is that 2.1 gigabytes could make Godot run out of memory on computers/devices with low amounts of RAM.

@Bhu1-V
Copy link
Contributor

Bhu1-V commented Mar 10, 2021

In the 3.2 stable build I tested with a named array

func _ready():
	var a = [];
	a.resize(89478485)  # This doesn't crash
	a.resize(89478486)  # This crashes

and it crashed with error:

ERROR: resize: Condition "!_ptrnew" is true. Returned: ERR_OUT_OF_MEMORY
At: ./core/cowdata.h:288

so max size must be 0x5555555 and somehow unnamed array caused the crash with NO ERROR

for the current master branch there was a game-crash with stack trace : (with and without variable name) maybe DEBUG mode forcefully generating BACKTRACE.

[0] CowData<Variant>::resize (D:\godot\core\templates\cowdata.h:307)
[1] Vector<Variant>::resize (D:\godot\core\templates\vector.h:88)
[2] Array::resize (D:\godot\core\variant\array.cpp:203)
[3] call_with_validated_variant_args_ret_helper<Array,enum Error,int,0> (D:\godot\core\variant\binder_common.h:253)
[4] call_with_validated_variant_args_ret<Array,enum Error,int> (D:\godot\core\variant\binder_common.h:466)
[5] vc_validated_call<enum Error,Array,int> (D:\godot\core\variant\variant_call.cpp:67)
[6] `_register_variant_builtin_methods'::`2'::Method_Array_resize::validated_call (D:\godot\core\variant\variant_call.cpp:1288)
[7] GDScriptFunction::call (D:\godot\modules\gdscript\gdscript_vm.cpp:1737)
[8] GDScriptInstance::call (D:\godot\modules\gdscript\gdscript.cpp:1544)
[9] ScriptInstance::call (D:\godot\core\object\script_language.cpp:322)
[10] Node::_notification (D:\godot\scene\main\node.cpp:147)
[11] Node::_notificationv (D:\godot\scene\main\node.h:45)
[12] Object::notification (D:\godot\core\object\object.cpp:795)
[13] Node::_propagate_ready (D:\godot\scene\main\node.cpp:189)
[14] Node::_propagate_ready (D:\godot\scene\main\node.cpp:181)
[15] Node::_set_tree (D:\godot\scene\main\node.cpp:2531)
[16] SceneTree::initialize (D:\godot\scene\main\scene_tree.cpp:394)
[17] OS_Windows::run (D:\godot\platform\windows\os_windows.cpp:620)
[18] widechar_main (D:\godot\platform\windows\godot_windows.cpp:163)
[19] _main (D:\godot\platform\windows\godot_windows.cpp:185)
[20] main (D:\godot\platform\windows\godot_windows.cpp:199)
[21] __scrt_common_main_seh (d:\agent\_work\57\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
[22] BaseThreadInitThunk
[23] RtlGetAppContainerNamedObjectPath
-- END OF BACKTRACE --

@Jayman2000
Copy link
Contributor Author

Jayman2000 commented Mar 10, 2021

In the 3.2 stable build I tested with a named array

func _ready():
	var a = [];
	a.resize(89478485)  # This doesn't crash
	a.resize(89478486)  # This crashes

and it crashed with error:

ERROR: resize: Condition "!_ptrnew" is true. Returned: ERR_OUT_OF_MEMORY
At: ./core/cowdata.h:288

When I run your version of the script, it still crashes without an error.

@fabriceci
Copy link
Contributor

I was able to reproduce the error on the master, I will make a PR.

@MrRevington
Copy link
Contributor

MrRevington commented Mar 10, 2021

Isn't it a problem too that on 64 bit systems _get_alloc_size_checked and _get_alloc_size are limited to an unsigend int value by next_power_of_2 which is only handles unsigned int?

see:

static _FORCE_INLINE_ unsigned int next_power_of_2(unsigned int x) {

return next_power_of_2(p_elements * sizeof(T));

*out = next_power_of_2(o);

@akien-mga
Copy link
Member

I ran into this too, here's my reproduction project for master:

	var INT32_MAX = 2147483647 # 2^31 - 1
	
	var byte_arr = PackedByteArray()
	byte_arr.resize(INT32_MAX) # OK
	byte_arr.resize(INT32_MAX + 1) # error
	
	var int32_arr = PackedInt32Array()
	#int32_arr.resize(INT32_MAX) # crash
	int32_arr.resize(INT32_MAX + 1) # error
	int32_arr.resize(INT32_MAX / 4) # OK, int32 is 4 bytes so alloc == INT32_MAX - 3 (due to truncation)
	int32_arr.resize(INT32_MAX / 4 + 1) # OK, not sure why (alloc INT32_MAX + 1)
	#int32_arr.resize(INT32_MAX / 4 + 2) # crash, alloc INT32_MAX + 5

I'm not sure #46868 is the right solution, it seems to me that we should be able to fix CowData to support allocating arrays of more than 2147483647 bytes.

The error mentioned in my script is:

ERROR: Condition "p_size < 0" is true. Returning: ERR_INVALID_PARAMETER
   at: resize (./core/templates/cowdata.h:260)

@akien-mga akien-mga changed the title Trying to create a large Array causes a silent crash Trying to create a large Array causes a silent crash in CowData May 17, 2021
@akien-mga
Copy link
Member

This is fixed in 4.0 RC 1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants