Missing max length checks accessing some properties with foreach_get/set #109033

New Issue

Thomas Barlow · 2023-06-15T21:39:02+02:00

Thomas Barlow commented

2023-06-15 21:39:02 +02:00

System Information
Operating system: Windows-10-10.0.19045-SP0 64 Bits
Graphics card: NVIDIA GeForce GTX 1070 Ti/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 531.41

Blender Version
Broken: version: 4.0.0 Alpha, branch: main, commit date: 2023-06-03 14:21, hash: ab6860f3de0f
Worked: varies per property:
For example:
MeshVertex.co worked in 3.4, but no longer works in 3.5 (presumably because it's more of a wrapper around the 'position' attribute as of 3.5)
MeshEdge.crease has not worked since before even 2.79 (the new attribute replacing it in 4.0 does not have the issue)
ShapeKey.value has not worked since before even 2.79

Short description of error
Max length checks are missing from some properties when accessing them with foreach_get/set. This means that when the list/array argument to foreach_get/set is longer than the property being accessed it can read/write to other memory and may crash Blender.

I don't know what exactly determines if a property has this issue, but if you open the System Console and then in the Python Console run my_collection.foreach_get(property_name, [None]):

The property appears to be affected by this issue if the error printed to the System Console is Error: Array length mismatch (got 1, expected more).
The property does not appear to be affected by this issue if the error printed to the System Console is like Error: Array length mismatch (expected 24, got 1).

For example with a default cube as the active object:
bpy.context.object.data.edges.foreach_get("crease", [None]) -> "got 1, expected more"
bpy.context.object.data.vertices.foreach_get("co", [None]) -> "got 1, expected more" (before Blender 3.5 this would instead be "expected 24, got 1")
bpy.context.object.data.attributes['position'].data.foreach_get("vector", [None]) -> "expected 24, got 1"
bpy.data.objects.foreach_get("add_rest_position_attribute", [None]) -> "got 1, expected more"
bpy.context.object.data.attributes.new("my_edge_float", type='FLOAT', domain='EDGE').data.foreach_get("value", [None]) -> "expected 12 got 1"
If at least two shape keys are added:
bpy.context.object.data.shape_keys.key_blocks.foreach_get("value", [None]) -> "got 1, expected more"

Exact steps for others to reproduce the error
foreach_get

Select a default cube as active
Add a few shape keys
Open the Python Console and run:
1. my_list = [None]*100
2. C.object.data.shape_keys.key_blocks.foreach_get("value", my_list) (might crash)
3. my_list
Observe that after the first few values in my_list that correspond to the shape keys that were added to the default cube, the rest are garbage that have been read from memory.

foreach_set
It's difficult to show that it's writing to memory it shouldn't be, but we can write to memory and then read it back into a different list with the same length:

Select a default cube as active
Add a few shape keys
Open the Python Console and run:
1. my_other_list = [0.123]*100
2. my_list = [None] * len(my_other_list)
3. C.object.data.shape_keys.key_blocks.foreach_set("value", my_other_list) (might crash)
4. C.object.data.shape_keys.key_blocks.foreach_get("value", my_list) (might crash)
5. my_list
Observe that most or maybe all of the values in my_list are 0.123, even those beyond the first few that correspond to the shape keys of the default cube.

**System Information** Operating system: Windows-10-10.0.19045-SP0 64 Bits Graphics card: NVIDIA GeForce GTX 1070 Ti/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 531.41 **Blender Version** Broken: version: 4.0.0 Alpha, branch: main, commit date: 2023-06-03 14:21, hash: `ab6860f3de0f` Worked: varies per property: For example: MeshVertex.co worked in 3.4, but no longer works in 3.5 (presumably because it's more of a wrapper around the 'position' attribute as of 3.5) MeshEdge.crease has not worked since before even 2.79 (the new attribute replacing it in 4.0 does not have the issue) ShapeKey.value has not worked since before even 2.79 **Short description of error** Max length checks are missing from some properties when accessing them with foreach_get/set. This means that when the list/array argument to foreach_get/set is longer than the property being accessed it can read/write to other memory and may crash Blender. I don't know what exactly determines if a property has this issue, but if you open the System Console and then in the Python Console run `my_collection.foreach_get(property_name, [None])`: 1) The property appears to be affected by this issue if the error printed to the System Console is `Error: Array length mismatch (got 1, expected more)`. 1) The property does not appear to be affected by this issue if the error printed to the System Console is like `Error: Array length mismatch (expected 24, got 1)`. For example with a default cube as the active object: `bpy.context.object.data.edges.foreach_get("crease", [None])` -> "got 1, expected more" `bpy.context.object.data.vertices.foreach_get("co", [None])` -> "got 1, expected more" (before Blender 3.5 this would instead be "expected 24, got 1") `bpy.context.object.data.attributes['position'].data.foreach_get("vector", [None])` -> "expected 24, got 1" `bpy.data.objects.foreach_get("add_rest_position_attribute", [None])` -> "got 1, expected more" `bpy.context.object.data.attributes.new("my_edge_float", type='FLOAT', domain='EDGE').data.foreach_get("value", [None])` -> "expected 12 got 1" If at least two shape keys are added: `bpy.context.object.data.shape_keys.key_blocks.foreach_get("value", [None])` -> "got 1, expected more" **Exact steps for others to reproduce the error** `foreach_get` 1) Select a default cube as active 1) Add a few shape keys 1) Open the Python Console and run: 1) `my_list = [None]*100` 1) `C.object.data.shape_keys.key_blocks.foreach_get("value", my_list)` (might crash) 1) `my_list` 1) Observe that after the first few values in `my_list` that correspond to the shape keys that were added to the default cube, the rest are garbage that have been read from memory. `foreach_set` It's difficult to show that it's writing to memory it shouldn't be, but we can write to memory and then read it back into a different list with the same length: 1) Select a default cube as active 1) Add a few shape keys 1) Open the Python Console and run: 1) `my_other_list = [0.123]*100` 1) `my_list = [None] * len(my_other_list)` 1) `C.object.data.shape_keys.key_blocks.foreach_set("value", my_other_list)` (might crash) 1) `C.object.data.shape_keys.key_blocks.foreach_get("value", my_list)` (might crash) 1) `my_list` 1) Observe that most or maybe all of the values in `my_list` are `0.123`, even those beyond the first few that correspond to the shape keys of the default cube.

Thomas Barlow added the

labels 2023-06-15 21:39:02 +02:00

Jesse Yurkovich commented

2023-06-15 22:19:08 +02:00

Can you redo the repro steps/description since "creases" have been moved to attributes a few days ago in e5ec04d73c.

Can you redo the repro steps/description since "creases" have been moved to attributes a few days ago in e5ec04d73c7873498f4052cbb9f58acfdaf4b7b0.

Thomas Barlow commented

2023-06-15 22:41:45 +02:00

Can you redo the repro steps/description since "creases" have been moved to attributes a few days ago in e5ec04d73c.

I have updated the reproduction steps and description to use ShapeKey.value.

> Can you redo the repro steps/description since "creases" have been moved to attributes a few days ago in e5ec04d73c7873498f4052cbb9f58acfdaf4b7b0. I have updated the reproduction steps and description to use ShapeKey.value.

Jesse Yurkovich commented

2023-06-16 04:18:12 +02:00

From debugging it seems that both the get and set cases are using a locally allocated array of appropriate size: 100 elements in both the get and set cases. So a crash should not occur, and an ASAN build does not complain about any buffer overruns etc. Are you seeing a crash?

However, the array that's allocated is not initialized to, say, 0 or similar. So what's being mapped back into Python is whatever values happened to be lying in memory at the time of that allocation.

Debugging also shows that the proper number of properties are being iterated in both cases (e.g. 4 in the case of the .blend file from your other bug for example). So during a get you'll get back 4 proper values and then 96 values of whatever happened to be in memory at the time.

Unsure what the API should do in this case. Should it zero out its local array memory? Should it raise an Error if there are fewer property values than the length of the provided array?

From debugging it seems that both the `get` and `set` cases are using a locally allocated array of appropriate size: 100 elements in both the `get` and `set` cases. So a crash should not occur, and an ASAN build does not complain about any buffer overruns etc. Are you seeing a crash? However, the array that's allocated is not initialized to, say, 0 or similar. So what's being mapped back into Python is whatever values happened to be lying in memory at the time of that allocation. Debugging also shows that the proper number of properties are being iterated in both cases (e.g. 4 in the case of the .blend file from your other bug for example). So during a `get` you'll get back 4 proper values and then 96 values of whatever happened to be in memory at the time. Unsure what the API should do in this case. Should it zero out its local array memory? Should it raise an Error if there are fewer property values than the length of the provided array?

Thomas Barlow commented

2023-06-16 05:57:25 +02:00

Ah, I guess what's happening when writing an oversize list with set and then getting the data into a separate oversize list with get is that the locally allocated array during the get is being allocated roughly in the same place as the locally allocated array in the set. That would explain why it sometimes comes back with all the same values and sometimes comes back with only a few different.

I did get some crashes, though at most only one per instance of a different version of Blender I had open for testing and the crashes were all in quick succession. Unfortunately I was messing with #109024 at the time and had addons loaded. I'm pretty sure the line I put in the Python Console was the same as I'd done many times before without crashing, so the crashes may have just been coincidence. And if I push the oversized lists in this issue to much larger numbers, I don't get an almost immediate crash like I would expect if it actually was reading from/writing to memory it shouldn't be.

API-wise, I would prefer raising an error if the list length does not match the number of property elements, so that it's consistent with the properties that already do raise an error when given an oversized list. For example, 'BOOLEAN' and 'INT8' attributes allow oversized lists, but 'FLOAT' and 'INT' attributes do not. Unless there's some way to tell through the Python API how individual properties behave in advance, I would prefer them to behave the same. Unfortunately, I don't know enough about the Blender internals to have any idea as to why the behaviour would differ in the first place. I didn't even know that some properties allowed oversized lists in foreach_get/set until today.

import bpy

me = bpy.context.object.data
attributes = me.attributes

num_faces = len(me.polygons)

face_int_data = attributes.new("", 'INT', 'FACE').data
face_float_data = attributes.new("", 'FLOAT', 'FACE').data
face_int8_data = attributes.new("", 'INT8', 'FACE').data
face_bool_data = attributes.new("", 'BOOLEAN', 'FACE').data

oversized_list = [None] * (num_faces + 10)

try:
    face_int_data.foreach_get("value", oversized_list)
    raise AssertionError("Unreachable")
except RuntimeError:
    print("OK: Got expected error")
    pass

try:
    face_float_data.foreach_get("value", oversized_list)
    raise AssertionError("Unreachable")
except RuntimeError:
    print("OK: Got expected error")
    pass


face_int8_data.foreach_get("value", oversized_list)
print("BUG: No error thrown")
face_bool_data.foreach_get("value", oversized_list)
print("BUG: No error thrown")

Ah, I guess what's happening when writing an oversize list with `set` and then getting the data into a separate oversize list with `get` is that the locally allocated array during the `get` is being allocated roughly in the same place as the locally allocated array in the `set`. That would explain why it sometimes comes back with all the same values and sometimes comes back with only a few different. I did get some crashes, though at most only one per instance of a different version of Blender I had open for testing and the crashes were all in quick succession. Unfortunately I was messing with #109024 at the time and had addons loaded. I'm pretty sure the line I put in the Python Console was the same as I'd done many times before without crashing, so the crashes may have just been coincidence. And if I push the oversized lists in this issue to much larger numbers, I don't get an almost immediate crash like I would expect if it actually was reading from/writing to memory it shouldn't be. API-wise, I would prefer raising an error if the list length does not match the number of property elements, so that it's consistent with the properties that already do raise an error when given an oversized list. For example, 'BOOLEAN' and 'INT8' attributes allow oversized lists, but 'FLOAT' and 'INT' attributes do not. Unless there's some way to tell through the Python API how individual properties behave in advance, I would prefer them to behave the same. Unfortunately, I don't know enough about the Blender internals to have any idea as to why the behaviour would differ in the first place. I didn't even know that some properties allowed oversized lists in foreach_get/set until today. ```py import bpy me = bpy.context.object.data attributes = me.attributes num_faces = len(me.polygons) face_int_data = attributes.new("", 'INT', 'FACE').data face_float_data = attributes.new("", 'FLOAT', 'FACE').data face_int8_data = attributes.new("", 'INT8', 'FACE').data face_bool_data = attributes.new("", 'BOOLEAN', 'FACE').data oversized_list = [None] * (num_faces + 10) try: face_int_data.foreach_get("value", oversized_list) raise AssertionError("Unreachable") except RuntimeError: print("OK: Got expected error") pass try: face_float_data.foreach_get("value", oversized_list) raise AssertionError("Unreachable") except RuntimeError: print("OK: Got expected error") pass face_int8_data.foreach_get("value", oversized_list) print("BUG: No error thrown") face_bool_data.foreach_get("value", oversized_list) print("BUG: No error thrown") ```

Jesse Yurkovich added

and removed

labels 2023-06-19 00:38:07 +02:00

Bart van der Braak added

Type

Bug