Crash related to custom data draw (Blender with ASAN) #49297

New Issue

Alexander Romanov · 2016-09-08T15:38:59+02:00

Alexander Romanov commented

2016-09-08 15:38:59 +02:00

Exact steps for others to reproduce the error

Open main.blend in Blender compiled with ASAN and see crash

**Exact steps for others to reproduce the error** Open [main.blend](https://archive.blender.org/developer/F356124/main.blend) in Blender compiled with ASAN and see crash

Alexander Romanov commented

2016-09-08 15:38:59 +02:00

Changed status to: 'Open'

Alexander Romanov commented

2016-09-08 15:38:59 +02:00

Added subscriber: @AlexanderRomanov

Alexander Romanov self-assigned this 2016-09-09 09:13:09 +02:00

Sergey Sharybin commented

2016-09-09 10:52:19 +02:00

Added subscriber: @Sergey

Sergey Sharybin commented

2016-09-09 10:52:19 +02:00

The root of the issue is that active render index for UVs is wrong. which should not be happening. So question is: how did you run into such situation? This is to be fixed..

But since such situation was possible, should we apply something like this?

P393: Fix/workaround for #49297

diff --git a/source/blender/blenkernel/intern/DerivedMesh.c b/source/blender/blenkernel/intern/DerivedMesh.c
index 5f759c6..8168817 100644
--- a/source/blender/blenkernel/intern/DerivedMesh.c
+++ b/source/blender/blenkernel/intern/DerivedMesh.c
@@ -3474,13 +3474,17 @@ void DM_calc_loop_tangents(
 
 		/* Update active layer index */
 		uv_index = CustomData_get_layer_index_n(&dm->loopData, CD_MLOOPUV, act_uv_n);
-		tan_index = CustomData_get_named_layer_index(&dm->loopData, CD_TANGENT, dm->loopData.layers[uv_index].name);
-		CustomData_set_layer_active_index(&dm->loopData, CD_TANGENT, tan_index);
+		if (uv_index != -1) {
+			tan_index = CustomData_get_named_layer_index(&dm->loopData, CD_TANGENT, dm->loopData.layers[uv_index].name);
+			CustomData_set_layer_active_index(&dm->loopData, CD_TANGENT, tan_index);
+		}
 
 		/* Update render layer index */
 		uv_index = CustomData_get_layer_index_n(&dm->loopData, CD_MLOOPUV, ren_uv_n);
-		tan_index = CustomData_get_named_layer_index(&dm->loopData, CD_TANGENT, dm->loopData.layers[uv_index].name);
-		CustomData_set_layer_render_index(&dm->loopData, CD_TANGENT, tan_index);
+		if (uv_index != -1) {
+			tan_index = CustomData_get_named_layer_index(&dm->loopData, CD_TANGENT, dm->loopData.layers[uv_index].name);
+			CustomData_set_layer_render_index(&dm->loopData, CD_TANGENT, tan_index);
+		}
 	}
 }

The root of the issue is that active render index for UVs is wrong. which should not be happening. So question is: how did you run into such situation? This is to be fixed.. But since such situation was possible, should we apply something like this? [P393: Fix/workaround for #49297](https://archive.blender.org/developer/P393.txt) ``` diff --git a/source/blender/blenkernel/intern/DerivedMesh.c b/source/blender/blenkernel/intern/DerivedMesh.c index 5f759c6..8168817 100644 --- a/source/blender/blenkernel/intern/DerivedMesh.c +++ b/source/blender/blenkernel/intern/DerivedMesh.c @@ -3474,13 +3474,17 @@ void DM_calc_loop_tangents( /* Update active layer index */ uv_index = CustomData_get_layer_index_n(&dm->loopData, CD_MLOOPUV, act_uv_n); - tan_index = CustomData_get_named_layer_index(&dm->loopData, CD_TANGENT, dm->loopData.layers[uv_index].name); - CustomData_set_layer_active_index(&dm->loopData, CD_TANGENT, tan_index); + if (uv_index != -1) { + tan_index = CustomData_get_named_layer_index(&dm->loopData, CD_TANGENT, dm->loopData.layers[uv_index].name); + CustomData_set_layer_active_index(&dm->loopData, CD_TANGENT, tan_index); + } /* Update render layer index */ uv_index = CustomData_get_layer_index_n(&dm->loopData, CD_MLOOPUV, ren_uv_n); - tan_index = CustomData_get_named_layer_index(&dm->loopData, CD_TANGENT, dm->loopData.layers[uv_index].name); - CustomData_set_layer_render_index(&dm->loopData, CD_TANGENT, tan_index); + if (uv_index != -1) { + tan_index = CustomData_get_named_layer_index(&dm->loopData, CD_TANGENT, dm->loopData.layers[uv_index].name); + CustomData_set_layer_render_index(&dm->loopData, CD_TANGENT, tan_index); + } } } ```

Alexander Romanov commented

2016-09-09 11:08:57 +02:00

Don't know how to reproduce this from scratch. I'm testing all files from Blend4Web SDK and the original file with this bug is from the end of 2014.
I've just started to investigate this bug.

But the good news is that this is the last crash that I met)

Don't know how to reproduce this from scratch. I'm testing all files from Blend4Web SDK and the original file with this bug is from the end of 2014. I've just started to investigate this bug. But the good news is that this is the last crash that I met)

Alexander Romanov commented

2016-09-09 13:36:30 +02:00

I think we should check the type of layer where we expect UV. Crash occurs because of broken logic there.
P394: possible fix for #49297

diff --git a/source/blender/blenkernel/intern/DerivedMesh.c b/source/blender/blenkernel/intern/DerivedMesh.c
index 5f759c6..ceb3ff0 100644
--- a/source/blender/blenkernel/intern/DerivedMesh.c
+++ b/source/blender/blenkernel/intern/DerivedMesh.c
@@ -3306,14 +3306,25 @@ void DM_calc_loop_tangents_step_0(
 	*ract_uv_n = CustomData_get_active_layer(loopData, CD_MLOOPUV);
 	ract_uv_name- [x] = 0;
 	if (*ract_uv_n != -1) {
-		strcpy(ract_uv_name, loopData->layers[*ract_uv_n + layer_index].name);
+		int index = *ract_uv_n + layer_index;
+		if (index < loopData->totlayer && loopData->layers[index].type == CD_MLOOPUV)
+			strcpy(ract_uv_name, loopData->layers[index].name);
+		else
+			*ract_uv_n = -1;
 	}
 
 	/* Active tangent in render */
 	*rren_uv_n = CustomData_get_render_layer(loopData, CD_MLOOPUV);
 	rren_uv_name- [x] = 0;
 	if (*rren_uv_n != -1) {
-		strcpy(rren_uv_name, loopData->layers[*rren_uv_n + layer_index].name);
+		int index = *rren_uv_n + layer_index;
+		/* It is not known why, but layer.active_rnd can have wrong value,
+		 * therefore we should check the layer type
+		 * The same is done above for layer.active see #49297 */
+		if (index < loopData->totlayer && loopData->layers[index].type == CD_MLOOPUV)
+			strcpy(rren_uv_name, loopData->layers[index].name);
+		else
+			*rren_uv_n = -1;
 	}
 
 	/* If active tangent not in tangent_names we take it into account */
@@ -3473,14 +3484,18 @@ void DM_calc_loop_tangents(
 		int uv_index, tan_index;
 
 		/* Update active layer index */
-		uv_index = CustomData_get_layer_index_n(&dm->loopData, CD_MLOOPUV, act_uv_n);
-		tan_index = CustomData_get_named_layer_index(&dm->loopData, CD_TANGENT, dm->loopData.layers[uv_index].name);
-		CustomData_set_layer_active_index(&dm->loopData, CD_TANGENT, tan_index);
+		if (act_uv_n != -1) {
+			uv_index = CustomData_get_layer_index_n(&dm->loopData, CD_MLOOPUV, act_uv_n);
+			tan_index = CustomData_get_named_layer_index(&dm->loopData, CD_TANGENT, dm->loopData.layers[uv_index].name);
+			CustomData_set_layer_active_index(&dm->loopData, CD_TANGENT, tan_index);
+		}
 
 		/* Update render layer index */
-		uv_index = CustomData_get_layer_index_n(&dm->loopData, CD_MLOOPUV, ren_uv_n);
-		tan_index = CustomData_get_named_layer_index(&dm->loopData, CD_TANGENT, dm->loopData.layers[uv_index].name);
-		CustomData_set_layer_render_index(&dm->loopData, CD_TANGENT, tan_index);
+		if (ren_uv_n != -1) {
+			uv_index = CustomData_get_layer_index_n(&dm->loopData, CD_MLOOPUV, ren_uv_n);
+			tan_index = CustomData_get_named_layer_index(&dm->loopData, CD_TANGENT, dm->loopData.layers[uv_index].name);
+			CustomData_set_layer_render_index(&dm->loopData, CD_TANGENT, tan_index);
+		}
 	}
 }

I think we should check the type of layer where we expect UV. Crash occurs because of broken logic there. [P394: possible fix for #49297](https://archive.blender.org/developer/P394.txt) ``` diff --git a/source/blender/blenkernel/intern/DerivedMesh.c b/source/blender/blenkernel/intern/DerivedMesh.c index 5f759c6..ceb3ff0 100644 --- a/source/blender/blenkernel/intern/DerivedMesh.c +++ b/source/blender/blenkernel/intern/DerivedMesh.c @@ -3306,14 +3306,25 @@ void DM_calc_loop_tangents_step_0( *ract_uv_n = CustomData_get_active_layer(loopData, CD_MLOOPUV); ract_uv_name- [x] = 0; if (*ract_uv_n != -1) { - strcpy(ract_uv_name, loopData->layers[*ract_uv_n + layer_index].name); + int index = *ract_uv_n + layer_index; + if (index < loopData->totlayer && loopData->layers[index].type == CD_MLOOPUV) + strcpy(ract_uv_name, loopData->layers[index].name); + else + *ract_uv_n = -1; } /* Active tangent in render */ *rren_uv_n = CustomData_get_render_layer(loopData, CD_MLOOPUV); rren_uv_name- [x] = 0; if (*rren_uv_n != -1) { - strcpy(rren_uv_name, loopData->layers[*rren_uv_n + layer_index].name); + int index = *rren_uv_n + layer_index; + /* It is not known why, but layer.active_rnd can have wrong value, + * therefore we should check the layer type + * The same is done above for layer.active see #49297 */ + if (index < loopData->totlayer && loopData->layers[index].type == CD_MLOOPUV) + strcpy(rren_uv_name, loopData->layers[index].name); + else + *rren_uv_n = -1; } /* If active tangent not in tangent_names we take it into account */ @@ -3473,14 +3484,18 @@ void DM_calc_loop_tangents( int uv_index, tan_index; /* Update active layer index */ - uv_index = CustomData_get_layer_index_n(&dm->loopData, CD_MLOOPUV, act_uv_n); - tan_index = CustomData_get_named_layer_index(&dm->loopData, CD_TANGENT, dm->loopData.layers[uv_index].name); - CustomData_set_layer_active_index(&dm->loopData, CD_TANGENT, tan_index); + if (act_uv_n != -1) { + uv_index = CustomData_get_layer_index_n(&dm->loopData, CD_MLOOPUV, act_uv_n); + tan_index = CustomData_get_named_layer_index(&dm->loopData, CD_TANGENT, dm->loopData.layers[uv_index].name); + CustomData_set_layer_active_index(&dm->loopData, CD_TANGENT, tan_index); + } /* Update render layer index */ - uv_index = CustomData_get_layer_index_n(&dm->loopData, CD_MLOOPUV, ren_uv_n); - tan_index = CustomData_get_named_layer_index(&dm->loopData, CD_TANGENT, dm->loopData.layers[uv_index].name); - CustomData_set_layer_render_index(&dm->loopData, CD_TANGENT, tan_index); + if (ren_uv_n != -1) { + uv_index = CustomData_get_layer_index_n(&dm->loopData, CD_MLOOPUV, ren_uv_n); + tan_index = CustomData_get_named_layer_index(&dm->loopData, CD_TANGENT, dm->loopData.layers[uv_index].name); + CustomData_set_layer_render_index(&dm->loopData, CD_TANGENT, tan_index); + } } } ```

Sergey Sharybin commented

2016-09-11 13:45:42 +02:00

Checking type earlier is same as checking whether CustomData_get_layer_index_n() gave proper index for a given layer type or do i miss something?

Checking type earlier is same as checking whether `CustomData_get_layer_index_n()` gave proper index for a given layer type or do i miss something?

Alexander Romanov commented

2016-09-12 13:50:26 +02:00

In #49297#391167, @Sergey wrote:
Checking type earlier is same as checking whether CustomData_get_layer_index_n() gave proper index for a given layer type or do i miss something?

Another attempt. Return 0 index because DM_calc_loop_tangents_step_0 will not be invoked without CD_MLOOPUV
P396: possible fix for #49297 v2

diff --git a/source/blender/blenkernel/intern/DerivedMesh.c b/source/blender/blenkernel/intern/DerivedMesh.c
index 5f759c6..8c40d27 100644
--- a/source/blender/blenkernel/intern/DerivedMesh.c
+++ b/source/blender/blenkernel/intern/DerivedMesh.c
@@ -3306,14 +3306,20 @@ void DM_calc_loop_tangents_step_0(
 	*ract_uv_n = CustomData_get_active_layer(loopData, CD_MLOOPUV);
 	ract_uv_name- [x] = 0;
 	if (*ract_uv_n != -1) {
-		strcpy(ract_uv_name, loopData->layers[*ract_uv_n + layer_index].name);
+		if (CustomData_get_layer_index_n(loopData, CD_MLOOPUV, *ract_uv_n) >= 0)
+			strcpy(ract_uv_name, loopData->layers[*ract_uv_n + layer_index].name);
+		else
+			*ract_uv_n = 0;
 	}
 
 	/* Active tangent in render */
 	*rren_uv_n = CustomData_get_render_layer(loopData, CD_MLOOPUV);
 	rren_uv_name- [x] = 0;
 	if (*rren_uv_n != -1) {
-		strcpy(rren_uv_name, loopData->layers[*rren_uv_n + layer_index].name);
+		if (CustomData_get_layer_index_n(loopData, CD_MLOOPUV, *rren_uv_n) >= 0)
+			strcpy(rren_uv_name, loopData->layers[*rren_uv_n + layer_index].name);
+		else
+			*rren_uv_n = 0;
 	}
 
 	/* If active tangent not in tangent_names we take it into account */

> In #49297#391167, @Sergey wrote: > Checking type earlier is same as checking whether `CustomData_get_layer_index_n()` gave proper index for a given layer type or do i miss something? Another attempt. Return 0 index because DM_calc_loop_tangents_step_0 will not be invoked without CD_MLOOPUV [P396: possible fix for #49297 v2](https://archive.blender.org/developer/P396.txt) ``` diff --git a/source/blender/blenkernel/intern/DerivedMesh.c b/source/blender/blenkernel/intern/DerivedMesh.c index 5f759c6..8c40d27 100644 --- a/source/blender/blenkernel/intern/DerivedMesh.c +++ b/source/blender/blenkernel/intern/DerivedMesh.c @@ -3306,14 +3306,20 @@ void DM_calc_loop_tangents_step_0( *ract_uv_n = CustomData_get_active_layer(loopData, CD_MLOOPUV); ract_uv_name- [x] = 0; if (*ract_uv_n != -1) { - strcpy(ract_uv_name, loopData->layers[*ract_uv_n + layer_index].name); + if (CustomData_get_layer_index_n(loopData, CD_MLOOPUV, *ract_uv_n) >= 0) + strcpy(ract_uv_name, loopData->layers[*ract_uv_n + layer_index].name); + else + *ract_uv_n = 0; } /* Active tangent in render */ *rren_uv_n = CustomData_get_render_layer(loopData, CD_MLOOPUV); rren_uv_name- [x] = 0; if (*rren_uv_n != -1) { - strcpy(rren_uv_name, loopData->layers[*rren_uv_n + layer_index].name); + if (CustomData_get_layer_index_n(loopData, CD_MLOOPUV, *rren_uv_n) >= 0) + strcpy(rren_uv_name, loopData->layers[*rren_uv_n + layer_index].name); + else + *rren_uv_n = 0; } /* If active tangent not in tangent_names we take it into account */ ```

Alexander Romanov was unassigned by Dalai Felinto

2019-12-23 16:37:00 +01:00

Aaron Carlisle commented

2020-01-22 18:24:37 +01:00

Changed status from 'Confirmed' to: 'Resolved'

Aaron Carlisle closed this issue

2020-01-22 18:24:37 +01:00

Aaron Carlisle self-assigned this 2020-01-22 18:24:37 +01:00

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

Crash related to custom data draw (Blender with ASAN) #49297