Blender 2.8: Crash with background nodes
Closed, ResolvedPublic

Description

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

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

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.

Dalai Felinto (dfelinto) triaged this task as Confirmed priority.Sep 4 2017, 3:42 PM
Dalai Felinto (dfelinto) lowered the priority of this task from Confirmed to Incomplete.Sep 4 2017, 3:50 PM

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

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[i] && 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

Ulysse Martin (youle) lowered the priority of this task from Incomplete to Low.

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

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)

@LazyDodo (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...
@Dalai Felinto (dfelinto) && @Clément Foucault (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 (LazyDodo) added a comment.EditedSep 4 2017, 9:56 PM

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..

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 (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:

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.

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 ?

@LazyDodo (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:

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

@Clément Foucault (fclem) / @Dalai Felinto (dfelinto) , I'll leave the fixing up to you guys.

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