Zandronum Chat on our Discord Server Get the latest version: 3.1
Source Code

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0003124Zandronum[All Projects] Bugpublic2017-05-03 15:202018-09-30 21:43
ReporterKorshun 
Assigned ToTorr Samaho 
PriorityhighSeveritycrashReproducibilityalways
StatusclosedResolutionfixed 
PlatformLinuxOSUbuntuOS Version
Product Version3.0-beta 
Target Version3.0Fixed in Version3.0 
Summary0003124: Fix of trail scale bug causes divide by zero on servers
DescriptionThe bugfix for'https://zandronum.com/tracker/view.php?id=3030 [^]' apparenly causes server crashes on our sectorcraft server.
Additional Information(gdb) backtrace
#0 0xb7708c31 in __kernel_vsyscall ()
#1 0xb764df7f in waitpid () at ../sysdeps/unix/syscall-template.S:84
0000002 0x0818454c in crash_catcher (siginfo=<optimized out>, signum=<optimized out>, context=<optimized out>)
    at /home/blzut3/Code/Public/Zandronum/src/sdl/crashcatcher.c:271
0000003 crash_catcher (signum=8, siginfo=0x87f40cc <altstack+7276>, context=0x87f414c <altstack+7404>) at /home/blzut3/Code/Public/Zandronum/src/sdl/crashcatcher.c:212
0000004 <signal handler called>
0000005 0x0845df2e in Scale (c=0, b=65536, a=0) at /home/blzut3/Code/Public/Zandronum/src/./gccinlines.h:45
0000006 APowerSpeed::DoEffect (this=0xb6f4c88) at /home/blzut3/Code/Public/Zandronum/src/g_shared/a_artifacts.cpp:1321
0000007 APowerSpeed::DoEffect (this=0xb6f4c88) at /home/blzut3/Code/Public/Zandronum/src/g_shared/a_artifacts.cpp:1262
0000008 0x08313533 in AActor::Tick (this=0xb483850) at /home/blzut3/Code/Public/Zandronum/src/p_mobj.cpp:4040
0000009 0x083da2e6 in ClientMoveCommand::process (this=0xb190468, ulClient=1) at /home/blzut3/Code/Public/Zandronum/src/sv_main.cpp:5294
0000010 0x083e3d8d in SERVER_Tick () at /home/blzut3/Code/Public/Zandronum/src/sv_main.cpp:603
0000011 0x081f2da5 in D_DoomLoop () at /home/blzut3/Code/Public/Zandronum/src/d_main.cpp:1334
0000012 0x081f5617 in D_DoomMain () at /home/blzut3/Code/Public/Zandronum/src/d_main.cpp:3265
0000013 0x081602dd in main (argc=11, argv=0xbfed3394) at /home/blzut3/Code/Public/Zandronum/src/sdl/i_main.cpp:380
Attached Fileslog file icon zandronum-crash-05_03_2017-17_14_16.2506.log [^] (12,452 bytes) 2017-05-03 15:53
log file icon zandronum-crash-05_03_2017-16_54_43.22792.log [^] (12,500 bytes) 2017-05-03 15:53
7z file icon core.2506.7z [^] (3,066,578 bytes) 2017-05-03 15:53
? file icon ServerTrailDivByZero.pk3 [^] (347 bytes) 2017-05-07 14:37
diff file icon crashy.diff [^] (310 bytes) 2017-05-28 19:59 [Show Content]

- Relationships

-  Notes
User avatar (0017540)
Korshun (reporter)
2017-05-03 15:23
edited on: 2017-05-03 15:33

Cannot reproduce this on Windows. This crash happens when a player picks up a turbosphere.

User avatar (0017545)
Edward-san (developer)
2017-05-04 08:17

I cannot reproduce this on my local server. Does it happen with just summoning the turbosphere and make the player pick it up?
User avatar (0017561)
Torr Samaho (administrator)
2017-05-07 09:49

So the crash doesn't happen under Windows? Does it also happen under Linux without skulltag_actors_3-0_170205.pk3 skulltag_data_126.pk3 sectorcraft-0.8.3fix.pk3 or does the crash only happen with these wads loaded?
User avatar (0017577)
Korshun (reporter)
2017-05-07 13:08

We found out that the issue is not reproducible without sectorcraft. We tried several simple example wads. I can try to remove parts out of sectorcraft until the cause of the issue is found.

By the way, why is this code running serverside at all?
User avatar (0017583)
Korshun (reporter)
2017-05-07 14:38

Removing parts of Sectorcraft until only the bug is left yielded the minimal example wad. To reproduce:

1. Run on a Linux server.
2. Choose Doomer class, as Sectorcrafter does not crash the server.
3. Pick up a turbosphere.
4. Run fast enough to make trails appear.
User avatar (0017615)
Torr Samaho (administrator)
2017-05-09 06:14

So ServerTrailDivByZero.pk3 alone should be sufficient to reproduce the problem under Linux? Edward-san, can you check this?
User avatar (0017636)
Edward-san (developer)
2017-05-13 11:11

I still cannot reproduce it. Korshun, which map did you use for the turbosphere?
User avatar (0017642)
Korshun (reporter)
2017-05-13 11:56
edited on: 2017-05-13 11:59

Any map works. Tested on Doom 2 MAP01. I suspect it's maybe something system-specific. Are you using the 32-bit version of zandronum? Or maybe you need to use the exact skulltag data wads, and not skulltag_content? I admit we haven't tested skulltag_content.

Still, why does the server run skin scale calculations at all? It isn't supposed to. Also, both classes in the wad are absolutely identical, yet only the second one crashes.

User avatar (0017646)
Torr Samaho (administrator)
2017-05-13 12:53
edited on: 2017-05-13 12:55

Quote from Korshun

Still, why does the server run skin scale calculations at all? It isn't supposed to.

Why do you think so? Because your player is using the base skin? In that case, it shouldn't do these skin calculations. What do you get as skin, if you call "playerinfo NUM", where NUM is the number of the player in question, on the server?

EDIT:
Quote from Korshun

Or maybe you need to use the exact skulltag data wads, and not skulltag_content? I admit we haven't tested skulltag_content.

Why are you talking about the Skulltag wads? Isn't ServerTrailDivByZero.pk3 be supposed to be sufficient to reproduce the problem alone without any other additional wads loaded?

User avatar (0017649)
Edward-san (developer)
2017-05-13 13:04

Quote
Are you using the 32-bit version of zandronum?


I'm not. I use 64 bit. Could it be that the problem?
User avatar (0017655)
Korshun (reporter)
2017-05-13 17:39
edited on: 2017-05-13 17:45

I have confirmed that the bug only happens with 32bit linux server.

I am talking about Skulltag wads because Turbosphere is part of Skulltag actors. Picking it up with the attached minimal example wad reproduces this bug. I have confirmed that the version of skulltag content doesn't matter.

playerinfo says "Base (0)" for Sectorcrafter (which doesn't crash) and "Base (1)" for Doomer (which does crash).

Why does the server try to calculate the trails' scale, which is purely a graphics effect and doesn't affect game simulation?

User avatar (0017659)
Torr Samaho (administrator)
2017-05-14 08:13

Quote from Korshun
I am talking about Skulltag wads because Turbosphere is part of Skulltag actors. Picking it up with the attached minimal example wad reproduces this bug. I have confirmed that the version of skulltag content doesn't matter.

Except for the sprites, the Turboshere is integrated in zandronum.pk3. So you don't need any Skulltag wads to give yourself the Turboshere. Just to double check, does the crash also happen if you just load ServerTrailDivByZero.pk3 and give yourself a turbosphere with "give turbosphere"?

Quote from Korshun
Why does the server try to calculate the trails' scale, which is purely a graphics effect and doesn't affect game simulation?
ZDoom makes no separation between purely visual effects and things that affect the game state. Thus, the server has to keep track of all actor properties. The only thing it does not do, is the actual rendering.
User avatar (0017662)
Korshun (reporter)
2017-05-14 11:30
edited on: 2017-05-14 11:32

Yes, it happens without skulltag wads.

So it's because some mod might do GetActorProperty(trail, APROP_ScaleX) and use it for gameplay calculations. But how can a mod access player trails without ZScript? Do mods that rely on this exist? Isn't this a Skulltag-specific feature that doesn't exist in ZDoom and can be safely clientsided without changing ZDoom behavior?

Or is it because a mod can replace player trail actor and then access its Scale properties?

User avatar (0017664)
Torr Samaho (administrator)
2017-05-14 12:12

Quote from Korshun

So it's because some mod might do GetActorProperty(trail, APROP_ScaleX) and use it for gameplay calculations.

No. As I said above, it's because ZDoom makes no separation between purely visual effects and things that affect the game state. Thus, there is no feasible way to exclude the handling of visual only properties on the server. Of course, I could manually skip this particular occasion on the server, but that wouldn't solve the underlying problem that is causing the crash. The scale code should work on the server. We need to find out why it's crashing. Can you test it with a server binary compiled in debug mode on your system?

Quote from Korshun

Isn't this a Skulltag-specific feature that doesn't exist in ZDoom and can be safely clientsided without changing ZDoom behavior?

No, ZDoom's skin code also supports the scale property and ZDoom also uses PlayerSpeedTrail. As far as I can tell, the fact that GZDoom 1.8.6 doesn't apply the scale of the skin to the trail is a bug.
User avatar (0017667)
Korshun (reporter)
2017-05-14 13:44
edited on: 2017-05-14 13:48

Sure, do I need to compile it myself?

I tried debugging with the official build and its debug symbols. It shows the values of all variables, except it can't access Owner->anything because "Could not find operator->."

What's strange is that speedMo->scaleX and speedMo->scaleY are 65535, not 0. But these values were copied from Owner->scaleX and Owner->scaleY. And then Scale is called with the values from Owner, but suddenly Owner->scaleX is 0 here.

User avatar (0017668)
Edward-san (developer)
2017-05-14 13:46

Quote
Sure, do I need to compile it myself?


That's correct.
User avatar (0017670)
Torr Samaho (administrator)
2017-05-14 14:37
edited on: 2017-05-14 14:41

Here is some on how to compile in debug mode under Linux and how to run the server in the debugger.

EDIT:
Quote from Korshun

What's strange is that speedMo->scaleX and speedMo->scaleY are 65535, not 0

That part sounds correct. 65535 is equal to the internal value FRACUNIT, which essentially means that the scale is 1.

User avatar (0017672)
Korshun (reporter)
2017-05-14 14:52

Sure, that's not strange at all. I know about fixed-point numbers. What is strange is that Owner->scaleX mysteriously becomes 0 and causes the crash, despite being 65535 earlier, as can be seen from the copy of the value that is assigned to speedMo->scaleX.

Thanks for the instructions.
User avatar (0017673)
Torr Samaho (administrator)
2017-05-14 15:09

AFAIK the debugger output of release mode binaries is not fully reliable, so you may get contradicting values.
User avatar (0017678)
Edward-san (developer)
2017-05-14 15:59

Quote

 It shows the values of all variables, except it can't access Owner->anything because "Could not find operator->."


Try accessing it with 'Owner.p->anything'.
User avatar (0017680)
Korshun (reporter)
2017-05-14 16:17

It works and says Owner.p->scaleX and scaleY are 65535. But a and c in Scale are 0. Should build a debug build to get rid of inconsistencies.
User avatar (0017721)
Torr Samaho (administrator)
2017-05-20 12:06

Did you have a chance to test this with a debug build yet?
User avatar (0017784)
Dusk (developer)
2017-05-28 19:59
edited on: 2017-05-28 19:59

VM testing finds the following:
- the crash does not happen in a debug build
- the crash seems to only happen with GCC (version 5.4.0 tested) and does not happen with Clang.

It is sufficient to simply add the following to D_DoomMain to trigger the crash at startup (full diff attached):

 void D_DoomMain (void)
 {
+ volatile const int x = Scale(65536, 65536, 65536);
+ printf("%d\n", x)


User avatar (0017786)
Edward-san (developer)
2017-05-28 22:37

Well, it doesn't happen with Clang because it doesn't use gccinlines.h, as it is written in src/m_fixed.h:

#if defined(__GNUC__) && defined(__i386__) && !defined(__clang__)
#include "gccinlines.h"


does it still crash if you replace that "gccinlines.h" with "basicinlines.h" and recompile with GCC? If it works, maybe we have a solution to this problem.

Another thing, Torr: why is this code you added:


speedMo->scaleX = Scale(Owner->scaleX, skins[skinidx].ScaleX, Owner->scaleX);


written like this? Scale(a,b,a) is technically the same as 'b'...
User avatar (0017788)
Torr Samaho (administrator)
2017-05-29 06:31
edited on: 2017-05-29 06:32

The code is supposed to scale speedMo->scaleX with "skins[skinidx].ScaleX / Owner->scaleX". Since the code earlier used "speedMo->scaleX = Owner->scaleX;" the code you quoted should do the same as

speedMo->scaleX = Scale(speedMo->scaleX, skins[skinidx].ScaleX, Owner->scaleX);

You are right though that in the end we end up with speedMo->scaleX = skins[skinidx].ScaleX, which may be incorrect in case the player body itself has a scale different from 1. Does anybody have an example with a skin with "scale != 1" for a player class with "scale != 1" at hand?

User avatar (0017839)
Dusk (developer)
2017-06-18 19:21
edited on: 2017-06-18 19:22

The above code change fixes the crashing when ScaleY is treated likewise.

User avatar (0017840)
WaTaKiD (updater)
2017-06-18 21:02

fixed with: 'https://bitbucket.org/Torr_Samaho/zandronum/commits/e51d0173dda12e8d033f5e156f73d6a8b9bcf314 [^]'
User avatar (0017855)
Korshun (reporter)
2017-06-20 15:09

Our 32bit Linux server doesn't crash anymore due to trails.
User avatar (0017857)
Ru5tK1ng (updater)
2017-06-21 00:27

That's good to hear. Reopen if necessary.

Issue Community Support
This issue is already marked as resolved.
If you feel that is not the case, please reopen it and explain why.
Supporters: Korshun
Opponents: No one explicitly opposes this issue yet.

- Issue History
Date Modified Username Field Change
2017-05-03 15:20 Korshun New Issue
2017-05-03 15:23 Korshun Note Added: 0017540
2017-05-03 15:33 Korshun Note Edited: 0017540 View Revisions
2017-05-03 15:53 Korshun File Added: zandronum-crash-05_03_2017-17_14_16.2506.log
2017-05-03 15:53 Korshun File Added: zandronum-crash-05_03_2017-16_54_43.22792.log
2017-05-03 15:53 Korshun File Added: core.2506.7z
2017-05-04 08:17 Edward-san Note Added: 0017545
2017-05-04 08:17 Edward-san Status new => feedback
2017-05-07 09:49 Torr Samaho Note Added: 0017561
2017-05-07 13:08 Korshun Note Added: 0017577
2017-05-07 13:08 Korshun Status feedback => new
2017-05-07 14:37 Korshun File Added: ServerTrailDivByZero.pk3
2017-05-07 14:38 Korshun Note Added: 0017583
2017-05-09 06:14 Torr Samaho Note Added: 0017615
2017-05-10 06:04 Torr Samaho Status new => feedback
2017-05-13 11:11 Edward-san Note Added: 0017636
2017-05-13 11:56 Korshun Note Added: 0017642
2017-05-13 11:56 Korshun Status feedback => new
2017-05-13 11:59 Korshun Note Edited: 0017642 View Revisions
2017-05-13 12:53 Torr Samaho Note Added: 0017646
2017-05-13 12:55 Torr Samaho Note Edited: 0017646 View Revisions
2017-05-13 13:04 Edward-san Note Added: 0017649
2017-05-13 14:46 Torr Samaho Status new => feedback
2017-05-13 17:39 Korshun Note Added: 0017655
2017-05-13 17:39 Korshun Status feedback => new
2017-05-13 17:41 Korshun Note Edited: 0017655 View Revisions
2017-05-13 17:44 Korshun Note Edited: 0017655 View Revisions
2017-05-13 17:44 Korshun Note Edited: 0017655 View Revisions
2017-05-13 17:45 Korshun Note Edited: 0017655 View Revisions
2017-05-14 08:13 Torr Samaho Note Added: 0017659
2017-05-14 11:30 Korshun Note Added: 0017662
2017-05-14 11:31 Korshun Note Edited: 0017662 View Revisions
2017-05-14 11:32 Korshun Note Edited: 0017662 View Revisions
2017-05-14 12:12 Torr Samaho Note Added: 0017664
2017-05-14 12:13 Torr Samaho Assigned To => Torr Samaho
2017-05-14 12:13 Torr Samaho Status new => feedback
2017-05-14 13:44 Korshun Note Added: 0017667
2017-05-14 13:44 Korshun Status feedback => assigned
2017-05-14 13:46 Edward-san Note Added: 0017668
2017-05-14 13:46 Edward-san Status assigned => feedback
2017-05-14 13:48 Korshun Note Edited: 0017667 View Revisions
2017-05-14 14:37 Torr Samaho Note Added: 0017670
2017-05-14 14:40 Torr Samaho Note Edited: 0017670
2017-05-14 14:41 Torr Samaho Note Edited: 0017670 View Revisions
2017-05-14 14:41 Torr Samaho Note Revision Dropped: 17670: 0010610
2017-05-14 14:52 Korshun Note Added: 0017672
2017-05-14 14:52 Korshun Status feedback => assigned
2017-05-14 15:09 Torr Samaho Note Added: 0017673
2017-05-14 15:59 Edward-san Note Added: 0017678
2017-05-14 16:17 Korshun Note Added: 0017680
2017-05-20 12:06 Torr Samaho Note Added: 0017721
2017-05-20 12:06 Torr Samaho Status assigned => feedback
2017-05-28 19:59 Dusk Note Added: 0017784
2017-05-28 19:59 Dusk Note Edited: 0017784 View Revisions
2017-05-28 19:59 Dusk File Added: crashy.diff
2017-05-28 22:37 Edward-san Note Added: 0017786
2017-05-29 06:31 Torr Samaho Note Added: 0017788
2017-05-29 06:32 Torr Samaho Note Edited: 0017788 View Revisions
2017-06-18 19:21 Dusk Note Added: 0017839
2017-06-18 19:22 Dusk Note Edited: 0017839 View Revisions
2017-06-18 21:02 WaTaKiD Note Added: 0017840
2017-06-18 21:02 WaTaKiD Status feedback => needs testing
2017-06-18 21:02 WaTaKiD Target Version => 3.0
2017-06-20 15:09 Korshun Note Added: 0017855
2017-06-21 00:27 Ru5tK1ng Note Added: 0017857
2017-06-21 00:27 Ru5tK1ng Status needs testing => resolved
2017-06-21 00:27 Ru5tK1ng Resolution open => fixed
2017-06-21 00:27 Ru5tK1ng Fixed in Version => 3.0
2018-09-30 21:43 Blzut3 Status resolved => closed






Questions or other issues? Contact Us.

Links


Copyright © 2000 - 2024 MantisBT Team
Powered by Mantis Bugtracker