[ZScript] GC crash

Bugs that have been investigated and resolved somehow.

Moderator: GZDoom Developers

Forum rules
Please don't bump threads here if you have a problem - it will often be forgotten about if you do. Instead, make a new thread here.
Post Reply
RaveYard
Posts: 186
Joined: Fri Apr 12, 2013 10:51 am

[ZScript] GC crash

Post 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 all

Code: C0000005 (Access Violation - tried to read address FFFFFFFFFFFFFFFF)
Bad luck... :cry:
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: [ZScript] GC crash

Post by Graf Zahl »

There's a code generation issue with the included struct which breaks the array access in Chunk.Init.
RaveYard
Posts: 186
Joined: Fri Apr 12, 2013 10:51 am

Re: [ZScript] GC crash

Post by RaveYard »

Any progress on this?

Another issue like this popped up in my mod and it's now completely unplayable... :|
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: [ZScript] GC crash

Post 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.
User avatar
phantombeta
Posts: 2084
Joined: Thu May 02, 2013 1:27 am
Operating System Version (Optional): Windows 10
Graphics Processor: nVidia with Vulkan support
Location: Brazil

Re: [ZScript] GC crash

Post 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.
User avatar
phantombeta
Posts: 2084
Joined: Thu May 02, 2013 1:27 am
Operating System Version (Optional): Windows 10
Graphics Processor: nVidia with Vulkan support
Location: Brazil

Re: [ZScript] GC crash

Post 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.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: [ZScript] GC crash

Post 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.
User avatar
phantombeta
Posts: 2084
Joined: Thu May 02, 2013 1:27 am
Operating System Version (Optional): Windows 10
Graphics Processor: nVidia with Vulkan support
Location: Brazil

Re: [ZScript] GC crash

Post 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.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: [ZScript] GC crash

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

Return to “Closed Bugs [GZDoom]”