GPv3: src_frame might point to invalid memory in insert_duplicate_frame() #114352

New Issue

Chao Li · 2023-11-01T04:55:31+01:00

Chao Li commented

2023-11-01 04:55:31 +01:00

System Information
Operating system: Windows 11
Graphics card: 3080

Blender Version
Broken: (example: 2.80, edbf15d3c0, master, 2018-11-28, as found on the splash screen) 4.1.0.alpha
Worked: (newest version of Blender that worked as expected) N/A

Short description of error

  const GreasePencilFrame &src_frame = layer.frames().lookup(src_frame_number);

  /* Create the new frame structure, with the same duration.
   * If we want to make an instance of the source frame, the drawing index gets copied from the
   * source frame. Otherwise, we set the drawing index to the size of the drawings array, since we
   * are going to add a new drawing copied from the source drawing. */
  const int duration = src_frame.is_implicit_hold() ?
                           0 :
                           layer.get_frame_duration_at(src_frame_number);
  const int drawing_index = do_instance ? src_frame.drawing_index : int(this->drawings().size());
  GreasePencilFrame *dst_frame = layer.add_frame(dst_frame_number, drawing_index, duration);

  if (dst_frame == nullptr) {
    return false;
  }

  dst_frame->type = src_frame.type;

  const GreasePencilDrawingBase *src_drawing_base = this->drawing(src_frame.drawing_index);

add_frame() might move the frames(), leaving src_frame pointing to invalid address. Blender will crash at the last line when that happens.

Exact steps for others to reproduce the error
GP_TOOL_FLAG_RETAIN_LAST should be on in order to reproduce.
I don't know how to change this settings so I changed this programmatically (purely for invoking this function).
Then keep inserting frames. When the keyframe number exceeds 8, this will happen.
Please see the videos as demonstration.

**System Information** Operating system: Windows 11 Graphics card: 3080 **Blender Version** Broken: (example: 2.80, edbf15d3c044, master, 2018-11-28, as found on the splash screen) 4.1.0.alpha Worked: (newest version of Blender that worked as expected) N/A **Short description of error** ``` const GreasePencilFrame &src_frame = layer.frames().lookup(src_frame_number); /* Create the new frame structure, with the same duration. * If we want to make an instance of the source frame, the drawing index gets copied from the * source frame. Otherwise, we set the drawing index to the size of the drawings array, since we * are going to add a new drawing copied from the source drawing. */ const int duration = src_frame.is_implicit_hold() ? 0 : layer.get_frame_duration_at(src_frame_number); const int drawing_index = do_instance ? src_frame.drawing_index : int(this->drawings().size()); GreasePencilFrame *dst_frame = layer.add_frame(dst_frame_number, drawing_index, duration); if (dst_frame == nullptr) { return false; } dst_frame->type = src_frame.type; const GreasePencilDrawingBase *src_drawing_base = this->drawing(src_frame.drawing_index); ``` add_frame() might move the frames(), leaving src_frame pointing to invalid address. Blender will crash at the last line when that happens. <video src="/attachments/bee970d3-58a9-42b8-8e8f-f87bf8141a61" controls></video> **Exact steps for others to reproduce the error** `GP_TOOL_FLAG_RETAIN_LAST` should be on in order to reproduce. I don't know how to change this settings so I changed this programmatically (purely for invoking this function). Then keep inserting frames. When the keyframe number exceeds 8, this will happen. Please see the videos as demonstration.

blender_3saB9tKcEy.mp4

3.1 MiB

Chao Li added the

labels 2023-11-01 04:55:31 +01:00

Chao Li referenced this issue

2023-11-01 05:01:22 +01:00

GPv3: Copy/Paste Keyframes #113110

Germano Cavalcante commented

2023-11-01 20:35:28 +01:00

Thanks for the report, @Chao-Li

If I understand correctly, you are saying that there is a bug in the insert_duplicate_frame() function in Blender version 4.1.0 alpha.
The issue occurs when the add_frame() function is called, which can potentially move the frames and leave src_frame pointing to an invalid memory address. And this results in a crash when trying to access src_frame later on.

We usually do not accept crash reports on custom builds. For these cases we suggest to use other channels: https://wiki.blender.org/wiki/Communication/Contact

But I'm giving a vote of confidence that this is indeed a bug (and not intended by the code).

Apparently the insert_duplicate_frame() code was implemented in fd75695bed

@amelief, @filedescriptor, mind checking?

Thanks for the report, @Chao-Li If I understand correctly, you are saying that there is a bug in the `insert_duplicate_frame()` function in Blender version 4.1.0 alpha. The issue occurs when the `add_frame()` function is called, which can potentially move the frames and leave `src_frame` pointing to an invalid memory address. And this results in a crash when trying to access `src_frame` later on. We usually do not accept crash reports on custom builds. For these cases we suggest to use other channels: https://wiki.blender.org/wiki/Communication/Contact But I'm giving a vote of confidence that this is indeed a bug (and not intended by the code). Apparently the `insert_duplicate_frame()` code was implemented in fd75695bed @amelief, @filedescriptor, mind checking?

Germano Cavalcante added

Module

Grease Pencil

Status

Needs Info from Developers

and removed

Status

Needs Triage

labels 2023-11-01 20:35:46 +01:00

~~Chao Li referenced this issue 2024-01-21 21:04:20 +01:00~~

GPv3: Copy/Paste Keyframes #117388

Amélie Fondevilla commented

2024-01-26 18:08:38 +01:00

Hi,
I'm sorry but I cannot reproduce the error.

I don't think it is possible for src_frame to be deleted by the add_frame function (only null frames are removed in this function, and they are not selectable, so they cannot be source frames). The position of the source frame in the frames vector might change, but it does not invalidate the data.

Maybe I'm missing or misunderstanding something here, would you mind checking @filedescriptor ?

Note: to set GP_TOOL_FLAG_RETAIN_LAST from the UI, you need to go to drawing mode and check the snowflake icon in the top bar menu.

Hi, I'm sorry but I cannot reproduce the error. I don't think it is possible for `src_frame` to be deleted by the `add_frame` function (only null frames are removed in this function, and they are not selectable, so they cannot be source frames). The position of the source frame in the frames vector might change, but it does not invalidate the data. Maybe I'm missing or misunderstanding something here, would you mind checking @filedescriptor ? Note: to set `GP_TOOL_FLAG_RETAIN_LAST` from the UI, you need to go to drawing mode and check the snowflake icon in the top bar menu.

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

GPv3: src_frame might point to invalid memory in insert_duplicate_frame() #114352