Blender 2.8: Crash with background nodes #52385

New Issue

Ulysse Martin · 2017-08-13T17:53:07+02:00

Ulysse Martin commented

2017-08-13 17:53:07 +02:00

System Information
Windows 10 64 GTX 970

Blender Version
Broken: First bad commit: https://developer.blender.org/rB2eef097831caf14634cc0fc6749b857096baf3da#2816e222 (git bisect)

Short description of error
Crash when enabling eevee background nodes (windows 32 build)

Exact steps for others to reproduce the error

Build last blender2.8 branch (windows 32)
go in world panel
check use nodes to enable background nodes
crash

**System Information** Windows 10 64 GTX 970 **Blender Version** Broken: First bad commit: https://developer.blender.org/rB2eef097831caf14634cc0fc6749b857096baf3da#2816e222 (git bisect) **Short description of error** Crash when enabling eevee background nodes (windows 32 build) **Exact steps for others to reproduce the error** - Build last blender2.8 branch (windows 32) - go in world panel - check use nodes to enable background nodes - > crash

Ulysse Martin commented

2017-08-13 17:53:07 +02:00

Changed status to: 'Open'

Ulysse Martin commented

2017-08-13 17:53:07 +02:00

Added subscriber: @you.le

Ulysse Martin commented

2017-08-13 17:55:45 +02:00

this patch: http://pasteall.org/509540/diff fixes the issue but I'm not sure this is correct

Clément Foucault commented

2017-08-13 17:57:27 +02:00

Added subscriber: @fclem

Clément Foucault commented

2017-08-13 17:57:27 +02:00

Note that D2788 solve this. But I would prefer this to be investigated / reviewed more to be sure this crash comes effectively from theses strings.

Note that [D2788](https://archive.blender.org/developer/D2788) solve this. But I would prefer this to be investigated / reviewed more to be sure this crash comes effectively from theses strings.

Clément Foucault was assigned by Dalai Felinto

2017-09-04 15:42:13 +02:00

Dalai Felinto commented

2017-09-04 15:50:02 +02:00

Added subscriber: @dfelinto

Dalai Felinto commented

2017-09-04 15:50:02 +02:00

@you.le without a crash report (full backtrace from a debug build) we can't handle this for now.

Ulysse Martin commented

2017-09-04 16:09:56 +02:00

backtrace: http://pasteall.org/pic/show.php?id=118468
blender.exe -d: http://pasteall.org/545272
file I used to produce the crash: http://pasteall.org/blend/index.php?id=47708 (just click on use nodes in world panel)

The bug occurs on my computer on windows 10 64 bits only with windows 32 builds. I'll update if I have more infos. in upbge I fixed it like in the patch except that I changed again this:

GPU_DATATYPE_STR[18] -> GPU_DATATYPE_STR[17]

and

for (i = 1; i <= 17; i++) {
if (GPU_DATATYPE_STR- [x] && gpu_str_prefix(code, GPU_DATATYPE_STR[i])) {
type = i;
break;
}
}

(17->16)

But I didn't investigated much on this. I noone can reproduce the bug, I'll try to investigate more

backtrace: http://pasteall.org/pic/show.php?id=118468 blender.exe -d: http://pasteall.org/545272 file I used to produce the crash: http://pasteall.org/blend/index.php?id=47708 (just click on use nodes in world panel) The bug occurs on my computer on windows 10 64 bits only with windows 32 builds. I'll update if I have more infos. in upbge I fixed it like in the patch except that I changed again this: GPU_DATATYPE_STR[18] -> GPU_DATATYPE_STR[17] and for (i = 1; i <= 17; i++) { if (GPU_DATATYPE_STR- [x] && gpu_str_prefix(code, GPU_DATATYPE_STR[i])) { type = i; break; } } (17->16) But I didn't investigated much on this. I noone can reproduce the bug, I'll try to investigate more

Clément Foucault was unassigned by Ulysse Martin

2017-09-04 17:05:54 +02:00

Ulysse Martin self-assigned this 2017-09-04 17:05:54 +02:00

Ulysse Martin commented

2017-09-04 17:05:54 +02:00

It happens only when I build myself (both Visual studio 2013/2015). I'll redownload libraries to see. Sorry about the time you spent on this

Ray molenkamp commented

2017-09-04 17:51:37 +02:00

Added subscriber: @LazyDodo

Ray molenkamp commented

2017-09-04 17:51:37 +02:00

It's not just you, i have no issues reproducing a crash with

file I used to produce the crash: http://pasteall.org/blend/index.php?id=47708 (just click on use nodes in world panel)

It's not just you, i have no issues reproducing a crash with ``` file I used to produce the crash: http://pasteall.org/blend/index.php?id=47708 (just click on use nodes in world panel) ```

Ulysse Martin commented

2017-09-04 18:52:33 +02:00

@LazyDodo: Thanks very much for the test. I'm currently reinstalling all my dev environment. But if I'm not alone to have this bug...
@dfelinto && @fclem: If we don't find the cause of the bug, is there any inconvenience to put Closure at 5 instead of 17? (I saw that in GPU_material.h there is a comment which says that normally the number must correspond to the data components number: vec4 -> 4... but for Closure the number is not static?).

@LazyDodo: Thanks very much for the test. I'm currently reinstalling all my dev environment. But if I'm not alone to have this bug... @dfelinto && @fclem: If we don't find the cause of the bug, is there any inconvenience to put Closure at 5 instead of 17? (I saw that in GPU_material.h there is a comment which says that normally the number must correspond to the data components number: vec4 -> 4... but for Closure the number is not static?).

Ray molenkamp commented

2017-09-04 21:56:09 +02:00

Moving closure is probably just going to mask the problem. I can't put my finger on it, but it does seem like a codegen bugs in the 32bit compiler

 source/blender/gpu/intern/gpu_codegen.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/source/blender/gpu/intern/gpu_codegen.c b/source/blender/gpu/intern/gpu_codegen.c
index 9fa78bf..452c0a6 100644
--- a/source/blender/gpu/intern/gpu_codegen.c
+++ b/source/blender/gpu/intern/gpu_codegen.c
@@ -88,7 +88,7 @@ typedef struct GPUFunction {
 } GPUFunction;
 
 /* Indices match the GPUType enum */
-static const char *GPU_DATATYPE_STR[18] = {
+static const char *GPU_DATATYPE_STR[] = {
 	"", "float", "vec2", "vec3", "vec4",
 	NULL, NULL, NULL, NULL, "mat3", NULL, NULL, NULL, NULL, NULL, NULL, "mat4",
 	"Closure"
@@ -175,7 +175,7 @@ static void gpu_parse_functions_string(GHash *hash, char *code)
 
 			/* test for type */
 			type = GPU_NONE;
-			for (i = 1; i <= 17; i++) {
+			for (i = 1; i < ARRAY_SIZE(GPU_DATATYPE_STR) ; i++) {
 				if (GPU_DATATYPE_STR[i] && gpu_str_prefix(code, GPU_DATATYPE_STR[i])) {
 					type = i;
 					break;

This works just as well, and gets rid of 'magic' constants in the code at the same time..

Moving closure is probably just going to mask the problem. I can't put my finger on it, but it does seem like a codegen bugs in the 32bit compiler ``` source/blender/gpu/intern/gpu_codegen.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/blender/gpu/intern/gpu_codegen.c b/source/blender/gpu/intern/gpu_codegen.c index 9fa78bf..452c0a6 100644 --- a/source/blender/gpu/intern/gpu_codegen.c +++ b/source/blender/gpu/intern/gpu_codegen.c @@ -88,7 +88,7 @@ typedef struct GPUFunction { } GPUFunction; /* Indices match the GPUType enum */ -static const char *GPU_DATATYPE_STR[18] = { +static const char *GPU_DATATYPE_STR[] = { "", "float", "vec2", "vec3", "vec4", NULL, NULL, NULL, NULL, "mat3", NULL, NULL, NULL, NULL, NULL, NULL, "mat4", "Closure" @@ -175,7 +175,7 @@ static void gpu_parse_functions_string(GHash *hash, char *code) /* test for type */ type = GPU_NONE; - for (i = 1; i <= 17; i++) { + for (i = 1; i < ARRAY_SIZE(GPU_DATATYPE_STR) ; i++) { if (GPU_DATATYPE_STR[i] && gpu_str_prefix(code, GPU_DATATYPE_STR[i])) { type = i; break; ``` This works just as well, and gets rid of 'magic' constants in the code at the same time..

Ulysse Martin commented

2017-09-05 03:57:22 +02:00

I reinstalled all my environment ("Repair"ed MSVC 2013, reinstalled git, tortoise git, tortoise svn, cmake, redownloaded libs, blender sources, cleared the cache in AppData/Roaming).

@LazyDodo: I copied the 2 lines of your patch but it doesn't work for me (tested @7dfcbe)(last sources)

This: http://pasteall.org/545633/diff prevents the crash for me but I have no idea if this is good or not.

The other bug I opened at the same time: https://developer.blender.org/T52392 is still there for me when I apply my patch. So maybe this is my patch which cause this bug.

I noticed 2 differences since the last time I tested blender2.8 branch:

Now my "fix" prevents 2 crashes: 1 when we check use nodes for background (in the test file I shared, 1 when we check use nodes for materials
Last time I tested, iirc my "fix" fixed realtime update of BACKGROUND (not materials) using nodes. Now, realtime update of background using nodes doesn't work on my computer.

In the version from builbot (same hash):

When I go in world tab and check use nodes, it doesn't crash (whereas it crashes with my own build). When I use the file I shared and when I click on use nodes, this crashes.
When I click on use nodes for materials, it crashes

I don't know if the current bug and https://developer.blender.org/T52392 are related. I have no idea on how to fix it.

About crash for background:

blender.crash.txt: http://pasteall.org/545706
build output: http://pasteall.org/545707
warnings/errors: http://pasteall.org/545708

I'll try to study that later.

I'm very often connected to irc and if a dev wants to help on this bug, I can give distant access/control over my computer via team viewer.

notes: It's weird that I have not exactly the same behaviour between my computer and build from buildbot (but the version from buildbot has similar issues too). I don't know if we can access the buildbot specs. Mine are Windows 10 64 bits, Visual Studio 2013 update 5, GTX 970, intel i7 2600k.

Feel free to ask for more infos.

I reinstalled all my environment ("Repair"ed MSVC 2013, reinstalled git, tortoise git, tortoise svn, cmake, redownloaded libs, blender sources, cleared the cache in AppData/Roaming). @LazyDodo: I copied the 2 lines of your patch but it doesn't work for me (tested @7dfcbe)(last sources) This: http://pasteall.org/545633/diff prevents the crash for me but I have no idea if this is good or not. The other bug I opened at the same time: https://developer.blender.org/T52392 is still there for me when I apply my patch. So maybe this is my patch which cause this bug. I noticed 2 differences since the last time I tested blender2.8 branch: - Now my "fix" prevents 2 crashes: 1 when we check use nodes for background (in the test file I shared, 1 when we check use nodes for materials - Last time I tested, iirc my "fix" fixed realtime update of BACKGROUND (not materials) using nodes. Now, realtime update of background using nodes doesn't work on my computer. In the version from builbot (same hash): - When I go in world tab and check use nodes, it doesn't crash (whereas it crashes with my own build). When I use the file I shared and when I click on use nodes, this crashes. - When I click on use nodes for materials, it crashes I don't know if the current bug and https://developer.blender.org/T52392 are related. I have no idea on how to fix it. About crash for background: - blender.crash.txt: http://pasteall.org/545706 - build output: http://pasteall.org/545707 - warnings/errors: http://pasteall.org/545708 I'll try to study that later. I'm very often connected to irc and if a dev wants to help on this bug, I can give distant access/control over my computer via team viewer. notes: It's weird that I have not exactly the same behaviour between my computer and build from buildbot (but the version from buildbot has similar issues too). I don't know if we can access the buildbot specs. Mine are Windows 10 64 bits, Visual Studio 2013 update 5, GTX 970, intel i7 2600k. Feel free to ask for more infos.

Ray molenkamp commented

2017-09-05 05:58:25 +02:00

Nah i'm still getting bad vibes from this, i'd rather get to the bottom on it than move closure to a different spot 'cause that doesn't crash' , pretty sure all it would do is mask the actual problem.

also please attach any crashlog, regular log or .blend file directly to the ticket rather than hosting them off site, (you can just drag files onto the text box and it'll upload automagically)

Anyhow there's one crash with worldNodesCrash.blend , how do i repro the second one ?

Nah i'm still getting bad vibes from this, i'd rather get to the bottom on it than move closure to a different spot 'cause that doesn't crash' , pretty sure all it would do is mask the actual problem. also *please* attach any crashlog, regular log or .blend file directly to the ticket rather than hosting them off site, (you can just drag files onto the text box and it'll upload automagically) Anyhow there's one crash with worldNodesCrash.blend , how do i repro the second one ?

Ulysse Martin commented

2017-09-05 12:09:00 +02:00

@LazyDodo: in this file: http://pasteall.org/blend/index.php?id=47719 (I didn't change anything, I saved it with last blender2.8 sources with my windows 32 debug build), just click on use nodes in the material panel. For me it crashes.

oops sorry, drag and drop:

materialNodesCrash.blend

@LazyDodo: in this file: http://pasteall.org/blend/index.php?id=47719 (I didn't change anything, I saved it with last blender2.8 sources with my windows 32 debug build), just click on use nodes in the material panel. For me it crashes. oops sorry, drag and drop: [materialNodesCrash.blend](https://archive.blender.org/developer/F774593/materialNodesCrash.blend)

Ulysse Martin was unassigned by Ray molenkamp

2017-09-05 15:31:35 +02:00

Clément Foucault was assigned by Ray molenkamp

2017-09-05 15:31:35 +02:00

Ray molenkamp commented

2017-09-05 15:31:35 +02:00

Allright, found it, and can also explain why moving closure to 5 'fixes' it

in gpu_codegen.c in gpu_node_input_link we have the following catch all

	else {
		/* uniform vector */
		input->type = type;
		input->source = GPU_SOURCE_VEC_UNIFORM;

		memcpy(input->vec, link->ptr1, type * sizeof(float));
		if (link->dynamic) {
			input->dynamicvec = link->ptr1;
			input->dynamictype = link->dynamictype;
			input->dynamicdata = link->ptr2;
		}
		MEM_freeN(link);
	}

and in the typedf for GPUInput we have this for vec

	float vec[16];               /* vector data */
	GPUNodeLink *link;

Given the type for closure is 17, this will copy 17 floats from link->ptr1 neatly overwriting input->link with garbage.

This gets later dereferenced and boom we go

@fclem / @dfelinto , I'll leave the fixing up to you guys.

Allright, found it, and can also explain why moving closure to 5 'fixes' it in gpu_codegen.c in gpu_node_input_link we have the following catch all ``` else { /* uniform vector */ input->type = type; input->source = GPU_SOURCE_VEC_UNIFORM; memcpy(input->vec, link->ptr1, type * sizeof(float)); if (link->dynamic) { input->dynamicvec = link->ptr1; input->dynamictype = link->dynamictype; input->dynamicdata = link->ptr2; } MEM_freeN(link); } ``` and in the typedf for GPUInput we have this for vec ``` float vec[16]; /* vector data */ GPUNodeLink *link; ``` Given the type for closure is 17, this will copy 17 floats from link->ptr1 neatly overwriting input->link with garbage. This gets later dereferenced and *boom* we go @fclem / @dfelinto , I'll leave the fixing up to you guys.

Clément Foucault commented

2017-09-05 17:31:46 +02:00

Goddmanit ! Thank you so much @LazyDodo this is a major time savoir.