Godot crash in ResourceSaver after 1073741824 bytes

[lyuma]

Godot version

4.0.alpha c41e4b1

System information

Windows 10, 64-bit

Issue description

I ran into this when saving a large scn with lots of textures, and used "Make Local" so all textures got embedded.

I tried saving this scene, and it gave an error in resize

ERROR: Condition "p_size < 0" is true. Returning: ERR_INVALID_PARAMETER
   at: CowData<unsigned char>::resize (S:\repo\godot-fire\core/templates/cowdata.h:261)

After this error, it proceeded to crash.

 	godot.windows.opt.tools.64.exe!FileAccessCompressed::store_8(unsigned char p_dest) Line 339	C++
>	godot.windows.opt.tools.64.exe!FileAccess::store_buffer(const unsigned char * p_src, unsigned __int64 p_length) Line 552	C++
 	godot.windows.opt.tools.64.exe!ResourceFormatSaverBinaryInstance::write_variant(Ref<FileAccess> f, const Variant & p_property, HashMap<Ref<Resource>,int,HashMapHasherDefault,HashMapComparatorDefault<Ref<Resource> >,DefaultTypedAllocator<HashMapElement<Ref<Resource>,int> > > & resource_map, HashMap<Ref<Resource>,int,HashMapHasherDefault,HashMapComparatorDefault<Ref<Resource> >,DefaultTypedAllocator<HashMapElement<Ref<Resource>,int> > > & external_resources, HashMap<StringName,int,HashMapHasherDefault,HashMapComparatorDefault<StringName>,DefaultTypedAllocator<HashMapElement<StringName,int> > > & string_map, const PropertyInfo & p_hint) Line 1657	C++
 	godot.windows.opt.tools.64.exe!ResourceFormatSaverBinaryInstance::write_variant(Ref<FileAccess> f, const Variant & p_property, HashMap<Ref<Resource>,int,HashMapHasherDefault,HashMapComparatorDefault<Ref<Resource> >,DefaultTypedAllocator<HashMapElement<Ref<Resource>,int> > > & resource_map, HashMap<Ref<Resource>,int,HashMapHasherDefault,HashMapComparatorDefault<Ref<Resource> >,DefaultTypedAllocator<HashMapElement<Ref<Resource>,int> > > & external_resources, HashMap<StringName,int,HashMapHasherDefault,HashMapComparatorDefault<StringName>,DefaultTypedAllocator<HashMapElement<StringName,int> > > & string_map, const PropertyInfo & p_hint) Line 1637	C++
 	godot.windows.opt.tools.64.exe!ResourceFormatSaverBinaryInstance::save(const String & p_path, const Ref<Resource> & p_resource, unsigned int p_flags) Line 2081	C++
 	godot.windows.opt.tools.64.exe!ResourceFormatSaverBinary::save(const String & p_path, const Ref<Resource> & p_resource, unsigned int p_flags) Line 2106	C++
 	godot.windows.opt.tools.64.exe!ResourceSaver::save(const String & p_path, const Ref<Resource> & p_resource, unsigned int p_flags) Line 110	C++

CowData resizes to the next power of two, which is 2147483648, and overflows a signed int.

I think everything here should be made less sloppy. Array operations should always use unsigned, not signed values. Integers should be 64-bit where possible: GDScript already uses 64-bit, so why are they downgraded to 32-bit in C++? Finally, FileAccess should check the return value of resize() and fail gracefully.

Steps to reproduce

1. Import a very heavy scene from glTF, such that it has a few GB of assets.
2. Use "Make local" or do a compessed export using a script.

Sorry I don't have more specific steps. The asset I used is not redistributable, and the save was automated by a script in order to enable ZSTD compression.

Minimal reproduction project

N/A

[Calinou]

Related to #45296 and #54679.

Making CowData use 64-bit unsigned integers is welcome, but it's a significant undertaking as it requires replacing the type in all places where relevant.

[akien-mga]

Is this still reproducible in the latest betas?

[RedworkDE]

Still can happen on 4.0.0 stable.

MRP that just generates a few GB of random mesh data and saves it:

extends Node

func _ready():
	const size = 50000000
	
	var mesh = ArrayMesh.new()
	var arrays = []
	var vertices = PackedVector3Array()
	vertices.resize(size)
	for j in range(vertices.size()):
		vertices[j] = Vector3(randf(), randf(), randf())
	var uvs = PackedVector2Array()
	uvs.resize(size)
	for j in range(uvs.size()):
		uvs[j] = Vector2(randf(), randf())
	var uvs2 = PackedVector2Array()
	uvs2.resize(size)
	for j in range(uvs2.size()):
		uvs2[j] = Vector2(randf(), randf())
	var colors = PackedColorArray()
	colors.resize(size)
	for j in range(colors.size()):
		colors[j] = Color(randf(), randf(), randf(), randf())
	arrays.resize(ArrayMesh.ARRAY_MAX)
	arrays[ArrayMesh.ARRAY_VERTEX] = vertices
	arrays[ArrayMesh.ARRAY_TEX_UV] = uvs
	arrays[ArrayMesh.ARRAY_TEX_UV2] = uvs2
	arrays[ArrayMesh.ARRAY_COLOR] = colors
	mesh.add_surface_from_arrays(Mesh.PRIMITIVE_TRIANGLES, arrays);
	
	ResourceSaver.save(mesh, "res://test.res", ResourceSaver.FLAG_COMPRESS)

Also since I didn't see it in any of the linked issues: The MRP for just any of the CowData crashes is just [].resize(100000000)

[Regrad]

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

when saving big map from glb to SCN. TSCN saving normally.

Godot 4.0 Release.

[Calinou]

I can confirm this on 4.1.dev 33957ae (Linux) using this MRP. Note that while I see this error message:

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

I don't get a crash though.

[TokisanGames]

This is now causing a big problem for us and all users of Terrain3D. It does cause Godot to crash and does not save data.
4.1.1-stable, Windows 11/64.

We can cause it by importing 2 8k maps into our 16k world space and using the ResourceSaver to save. Godot can handle a full 16k map in memory, but it only takes half, 8k by 16k to crash Godot when saving. The crash occurs when we call ResourceSaver.save() or by clicking the arrow in the inspector on the .res and selecting save.

The resource is tied to a binary .res file in the inspector. Pre-crash it specifically has:

• a handful of bools, floats and vector2s
• 4 arrays
  • [128] vector2i
  • [128] Image 1024x1024 RFloat (4mb)
  • [128] Image 1024x1024 RGB8 (3mb)
  • [128] Image 1024x1024 RGBA8 (4mb)

So call it 1.4gb of memory. Upon saving godot crashes with:

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

Our issue TokisanGames/Terrain3D#159

[TokisanGames]

@akien-mga The new PR didn't fix this issue. It may have fixed part of it, but I attempted to save a 1.2gb resource file and saved it and it crashed.

I just built Godot 0bcc0e9 with godot-cpp godotengine/godot-cpp@0145e90 (though it's not relevant). I loaded up a terrain in 8k x 8k segments. First segment saved at 413mb fine. Two segments at 800mb. Three segments crashed.

This has nothing to do with our code. Our code loaded the data into a resource, then I manually used the inspector, right-click, and save the resource as a binary .res (contents listed above, defaults to compressed).

Crash without any console error messages. Debugging, I found it crashed in https://github.com/godotengine/godot/blob/master/core/io/file_access_compressed.cpp#L344-L350

PR #77306 was intended to be part of the solution for this issue. When I apply that PR on this commit Godot doesn't crash. However it doesn't save the resource either. It writes 11.7mb and has a popup,

[Rubonnek]

By modifying the MRP at #62585 (comment) to save roughly 2.8 GB through ResourceSaver on a dev build from 0bcc0e9 I'm able to reproduce a crash at the same line as @TokisanGames.

Click to show gdb backtrace

#0  0x000055555a4fa8aa in FileAccessCompressed::store_8 (this=0x55555f465320, p_dest=222 '\336') at core/io/file_access_compressed.cpp:349
#1  0x000055555a4ed346 in FileAccess::store_buffer (this=0x55555f465320, p_src=0x7ffc0f600030 "\021\vj*\276C\030>\307\327\177?\017\223\256>2\273", 
    p_length=2000000000) at core/io/file_access.cpp:711
#2  0x000055555a573aa0 in ResourceFormatSaverBinaryInstance::write_variant (f=..., p_property=..., resource_map=..., external_resources=..., string_map=..., 
    p_hint=...) at core/io/resource_format_binary.cpp:1841
#3  0x000055555a573907 in ResourceFormatSaverBinaryInstance::write_variant (f=..., p_property=..., resource_map=..., external_resources=..., string_map=..., 
    p_hint=...) at core/io/resource_format_binary.cpp:1822
#4  0x000055555a573a17 in ResourceFormatSaverBinaryInstance::write_variant (f=..., p_property=..., resource_map=..., external_resources=..., string_map=..., 
    p_hint=...) at core/io/resource_format_binary.cpp:1831
#5  0x000055555a575c7e in ResourceFormatSaverBinaryInstance::save (this=this@entry=0x7fffffffcc70, p_path=..., p_resource=..., p_flags=p_flags@entry=32)
    at core/io/resource_format_binary.cpp:2288
#6  0x000055555a575f79 in ResourceFormatSaverBinary::save (this=<optimized out>, p_resource=..., p_path=..., p_flags=32)
    at core/io/resource_format_binary.cpp:2427
#7  0x000055555a588062 in ResourceSaver::save (p_resource=..., p_path=..., p_flags=32) at core/io/resource_saver.cpp:127
#8  0x000055555a87986e in core_bind::ResourceSaver::save (this=0x55555f465528, p_resource=..., p_path=..., p_flags=...) at ./core/variant/type_info.h:301
#9  0x000055555a8a998d in call_with_variant_args_ret_helper<__UnexistingClass, Error, Ref<Resource> const&, String const&, BitField<core_bind::ResourceSaver::SaverFlags>, 0ul, 1ul, 2ul> (p_instance=p_instance@entry=0x55555d8c56f0, p_method=<optimized out>, p_args=p_args@entry=0x7fffffffced0, r_ret=..., 
    r_error=...) at ./core/variant/binder_common.h:755
#10 0x000055555a8a9ae9 in call_with_variant_args_ret_dv<__UnexistingClass, Error, Ref<Resource> const&, String const&, BitField<core_bind::ResourceSaver::SaverFlags> > (p_instance=0x55555d8c56f0, p_method=<optimized out>, p_args=0x7fffffffd160, p_argcount=3, r_ret=..., r_error=..., default_values=...)
    at ./core/variant/binder_common.h:534
#11 0x000055555a8a9b46 in MethodBindTR<Error, Ref<Resource> const&, String const&, BitField<core_bind::ResourceSaver::SaverFlags> >::call (
    this=<optimized out>, p_object=<optimized out>, p_args=<optimized out>, p_arg_count=<optimized out>, r_error=...) at ./core/object/method_bind.h:496
#12 0x0000555557eee847 in GDScriptFunction::call (this=<optimized out>, p_instance=<optimized out>, p_instance@entry=0x55555f4626d0, 
    p_args=p_args@entry=0x0, p_argcount=p_argcount@entry=0, r_err=..., p_state=<optimized out>) at modules/gdscript/gdscript_vm.cpp:1830
#13 0x0000555557e13bce in GDScript::_create_instance (this=this@entry=0x55555f4442e0, p_args=p_args@entry=0x0, p_argcount=p_argcount@entry=0, 
    p_owner=<optimized out>, p_owner@entry=0x55555f46dd50, p_is_ref_counted=<optimized out>, r_error=...) at modules/gdscript/gdscript.cpp:181
#14 0x0000555557e14089 in GDScript::instance_create (this=0x55555f4442e0, p_this=0x55555f46dd50) at modules/gdscript/gdscript.cpp:396
#15 0x000055555a7e7d19 in Object::set_script (this=this@entry=0x55555f46dd50, p_script=...) at core/object/object.cpp:907
#16 0x000055555784eb4e in Main::start () at main/main.cpp:3233

Click to show MRP

Run with godot -s:

extends SceneTree

func _init():
	const size = 100000000
	var mesh = ArrayMesh.new()
	var arrays = []
	var vertices = PackedVector3Array()
	vertices.resize(size)
	for j in range(vertices.size()):
		vertices[j] = Vector3(randf(), randf(), randf())
	var uvs = PackedVector2Array()
	uvs.resize(size)
	for j in range(uvs.size()):
		uvs[j] = Vector2(randf(), randf())
	var uvs2 = PackedVector2Array()
	uvs2.resize(size)
	for j in range(uvs2.size()):
		uvs2[j] = Vector2(randf(), randf())
	var colors = PackedColorArray()
	colors.resize(size)
	for j in range(colors.size()):
		colors[j] = Color(randf(), randf(), randf(), randf())
	arrays.resize(ArrayMesh.ARRAY_MAX)
	arrays[ArrayMesh.ARRAY_VERTEX] = vertices
	arrays[ArrayMesh.ARRAY_TEX_UV] = uvs
	arrays[ArrayMesh.ARRAY_TEX_UV2] = uvs2
	arrays[ArrayMesh.ARRAY_COLOR] = colors
	mesh.add_surface_from_arrays(Mesh.PRIMITIVE_TRIANGLES, arrays);

	ResourceSaver.save(mesh, "res://test.res", ResourceSaver.FLAG_COMPRESS)
	quit()

Reopening issue.

[BlueCube3310]

Likely caused by https://github.com/godotengine/godot/blob/master/core/io/file_access_compressed.cpp#L49 in

write_buffer_size = next_power_of_2(write_max);

since next_power_of_2 returns a uint32.

[TokisanGames]

f317cc7

• I tested converting next_power_of_2 to uint64_t, but it's not enough.

• This is needed:

file_access_compressed.h
-       uint32_t write_buffer_size = 0;
+       uint64_t write_buffer_size = 0;

• In store_8, I expanded the WRITE(1) macro and replaced next_power_of_2. The last working power of 2 is 1<<31. So once 2GB is full, the old algorithm starts allocating a 4gb block, 1<<32. Below changes this to allocate in 512mb blocks, which had no different result to allocating 4gb next.

void FileAccessCompressed::store_8(uint8_t p_dest) {
	ERR_FAIL_COND_MSG(f.is_null(), "File must be opened before use.");
	ERR_FAIL_COND_MSG(!writing, "File has not been opened in write mode.");

	if (write_pos + (1) > write_max) {
		write_max = write_pos + (1);
	}
	if (write_max > write_buffer_size) {
		//write_buffer_size = next_power_of_2(write_max);
		uint64_t value = 1;
		while (value <= write_max) {
			value = value << 1;
		}
		write_buffer_size += MIN(value, 1<<29);

		buffer.resize(write_buffer_size);
		write_ptr = buffer.ptrw();
	}
	write_ptr[write_pos++] = p_dest;
}

These changes get this function working beyond 2gb and no longer crashes, however it never returns from the save. The engine is hung. The msvc debugger seems to think it's running. CPU usage is minimal. Temporary files exist, but files are 0.

I tested importing terrain maps and saving to a resource file:

• 8k terrain -> 413mb res file.
• 2x 8k -> 827mb.
• 3x 8k ->
  • master commit - crashed on the same line
  • master + converting next power of 2, crashed because after 1<31, write_buffer_size overflows and buffer is null
  • master + new power of 2 + uint64 write_buffer_size & allocating 4gb next: this function doesn't crash, however Godot never returns from the save. It just runs endlessly.
  • Same as above, but allocating in 512mb blocks. Same result.

[fire]

@lyuma this may be fixed with the cow size increase to 64 bit