Page 1 of 1

[ZScript] GC crash

PostPosted: Mon May 15, 2017 2:59 pm
by RaveYard
Very specific ZScript code that causes crash upon garbage collection.

If struct ChunkSegmentRenderContext (located in "ZScript/DoomCraft/Core/Main.txt") is copy pasted into ZScript.txt, everything works fine.

http://www.mediafire.com/file/eqdbfs6d0 ... _Crash.pk3

Code: Select allExpand view
Code: C0000005 (Access Violation - tried to read address FFFFFFFFFFFFFFFF)


Bad luck... :cry:

Re: [ZScript] GC crash

PostPosted: Fri May 19, 2017 5:08 am
by Graf Zahl
There's a code generation issue with the included struct which breaks the array access in Chunk.Init.

Re: [ZScript] GC crash

PostPosted: Mon May 22, 2017 8:29 am
by RaveYard
Any progress on this?

Another issue like this popped up in my mod and it's now completely unplayable... :|

Re: [ZScript] GC crash

PostPosted: Sat Sep 02, 2017 4:36 am
by Graf Zahl
I just want to note that this is partially fixed, but the generated code still isn't 100% correct - so it looks like there's a second issue at play.

Re: [ZScript] GC crash

PostPosted: Sat Aug 15, 2020 6:35 am
by phantombeta
"Array<struct> [size]" (The internal VM type for structs) is getting the wrong element size for structs. For some reason, the ChunkSegment struct gets an ElementSize of 8, despite taking up 24 bytes.
It appears that the type data at compile time thinks the struct's size is 8, while the final type data doesn't.

Edit: I believe I've found the source of the issue. It's caused by the field compilation function only checking if the type's size is 0 when making sure the type has a known size.

Re: [ZScript] GC crash

PostPosted: Sat Aug 15, 2020 7:43 am
by phantombeta
Sorry for the double post, but IMO it's important enough to warrant a new post:
I've PR'ed a fix - seems to work fine with no issues, but idk, felt safer to PR it.

Re: [ZScript] GC crash

PostPosted: Sat Aug 15, 2020 7:55 am
by Graf Zahl
Have you found out what makes the compiler miscalculate? Adding a patch is fine but this looks a bit like it tries to hide the real bug by ignoring the original value.

Re: [ZScript] GC crash

PostPosted: Sat Aug 15, 2020 8:20 am
by phantombeta
Graf Zahl wrote:Have you found out what makes the compiler miscalculate? Adding a patch is fine but this looks a bit like it tries to hide the real bug by ignoring the original value.

It's not that it was miscalculating, it's because the fields simply hadn't been compiled yet.

So, the compilation of classes' and structs' fields is done in a loop until it has made sure all fields have been compiled. This is for reasons that'll be clear soon.
While compiling "ChunkSegment"'s fields, it'll hit "renderContext". This is of type "ChunkSegmentRenderContext", which is in a include file and hasn't been compiled yet; Since it's not compiled yet, its size is unknown.
This is okay, as it'll stop the struct's compilation for now, and let it try again for the next iteration of the loop, where the field's size should already be known - but note that, by this point, the "Size" member is already non-zero.

Some time after that, it'll reach the "Chunk" class. Note that this is the same iteration as before: that means "ChunkSegment" still hasn't been fully compiled yet, and its actual size is not known. Next, it tries to compile the "segments" array. This would normally fail and defer to the next iteration (like "ChunkSegment" did), but it doesn't as ChunkSegment stopped compiling in the middle of the fields, and because of that, the size is non-zero.
As the only way it checks if a type has a known size is by checking if it's not zero, that means it thinks "ChunkSegment" is done compiling prematurely, and miscompiles "Chunk"'s fields.

There's only two ways to fix this as far as I know: Add a way for it to actually know if the type really has a known size, so it can fail and defer to the next iteration (what I did) or completely rewriting that part of the compiler to work recursively instead of the way it does right now. Both are perfectly valid ways to fix it.
Obviously, the latter is not really feasible, as it would require a fairly massive amount of work.

Re: [ZScript] GC crash

PostPosted: Sat Aug 15, 2020 8:24 am
by Graf Zahl
Thanks for the explanation. Yes, in that case the fix is ok. Rewriting the lookup to be recursive wouldn't really make it any more robust, it'd cause a completely different group of problems, the amount of work required nonwithstanding.