MantisBT - Zandronum
View Issue Details
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.
Steps To Reproduce
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
TagsNo tags attached.
Relationships
Attached Fileslog zandronum-crash-05_03_2017-17_14_16.2506.log (12,452) 2017-05-03 15:53
/tracker/file_download.php?file_id=2098&type=bug
log zandronum-crash-05_03_2017-16_54_43.22792.log (12,500) 2017-05-03 15:53
/tracker/file_download.php?file_id=2099&type=bug
7z core.2506.7z (3,066,578) 2017-05-03 15:53
/tracker/file_download.php?file_id=2100&type=bug
? ServerTrailDivByZero.pk3 (347) 2017-05-07 14:37
/tracker/file_download.php?file_id=2105&type=bug
diff crashy.diff (310) 2017-05-28 19:59
/tracker/file_download.php?file_id=2113&type=bug
Issue History
Date ModifiedUsernameFieldChange
2017-05-03 15:20KorshunNew Issue
2017-05-03 15:23KorshunNote Added: 0017540
2017-05-03 15:33KorshunNote Edited: 0017540bug_revision_view_page.php?bugnote_id=17540#r10534
2017-05-03 15:53KorshunFile Added: zandronum-crash-05_03_2017-17_14_16.2506.log
2017-05-03 15:53KorshunFile Added: zandronum-crash-05_03_2017-16_54_43.22792.log
2017-05-03 15:53KorshunFile Added: core.2506.7z
2017-05-04 08:17Edward-sanNote Added: 0017545
2017-05-04 08:17Edward-sanStatusnew => feedback
2017-05-07 09:49Torr SamahoNote Added: 0017561
2017-05-07 13:08KorshunNote Added: 0017577
2017-05-07 13:08KorshunStatusfeedback => new
2017-05-07 14:37KorshunFile Added: ServerTrailDivByZero.pk3
2017-05-07 14:38KorshunNote Added: 0017583
2017-05-09 06:14Torr SamahoNote Added: 0017615
2017-05-10 06:04Torr SamahoStatusnew => feedback
2017-05-13 11:11Edward-sanNote Added: 0017636
2017-05-13 11:56KorshunNote Added: 0017642
2017-05-13 11:56KorshunStatusfeedback => new
2017-05-13 11:59KorshunNote Edited: 0017642bug_revision_view_page.php?bugnote_id=17642#r10590
2017-05-13 12:53Torr SamahoNote Added: 0017646
2017-05-13 12:55Torr SamahoNote Edited: 0017646bug_revision_view_page.php?bugnote_id=17646#r10594
2017-05-13 13:04Edward-sanNote Added: 0017649
2017-05-13 14:46Torr SamahoStatusnew => feedback
2017-05-13 17:39KorshunNote Added: 0017655
2017-05-13 17:39KorshunStatusfeedback => new
2017-05-13 17:41KorshunNote Edited: 0017655bug_revision_view_page.php?bugnote_id=17655#r10596
2017-05-13 17:44KorshunNote Edited: 0017655bug_revision_view_page.php?bugnote_id=17655#r10597
2017-05-13 17:44KorshunNote Edited: 0017655bug_revision_view_page.php?bugnote_id=17655#r10598
2017-05-13 17:45KorshunNote Edited: 0017655bug_revision_view_page.php?bugnote_id=17655#r10599
2017-05-14 08:13Torr SamahoNote Added: 0017659
2017-05-14 11:30KorshunNote Added: 0017662
2017-05-14 11:31KorshunNote Edited: 0017662bug_revision_view_page.php?bugnote_id=17662#r10601
2017-05-14 11:32KorshunNote Edited: 0017662bug_revision_view_page.php?bugnote_id=17662#r10602
2017-05-14 12:12Torr SamahoNote Added: 0017664
2017-05-14 12:13Torr SamahoAssigned To => Torr Samaho
2017-05-14 12:13Torr SamahoStatusnew => feedback
2017-05-14 13:44KorshunNote Added: 0017667
2017-05-14 13:44KorshunStatusfeedback => assigned
2017-05-14 13:46Edward-sanNote Added: 0017668
2017-05-14 13:46Edward-sanStatusassigned => feedback
2017-05-14 13:48KorshunNote Edited: 0017667bug_revision_view_page.php?bugnote_id=17667#r10608
2017-05-14 14:37Torr SamahoNote Added: 0017670
2017-05-14 14:40Torr SamahoNote Edited: 0017670
2017-05-14 14:41Torr SamahoNote Edited: 0017670bug_revision_view_page.php?bugnote_id=17670#r10611
2017-05-14 14:41Torr SamahoNote Revision Dropped: 17670: 0010610
2017-05-14 14:52KorshunNote Added: 0017672
2017-05-14 14:52KorshunStatusfeedback => assigned
2017-05-14 15:09Torr SamahoNote Added: 0017673
2017-05-14 15:59Edward-sanNote Added: 0017678
2017-05-14 16:17KorshunNote Added: 0017680
2017-05-20 12:06Torr SamahoNote Added: 0017721
2017-05-20 12:06Torr SamahoStatusassigned => feedback
2017-05-28 19:59DuskNote Added: 0017784
2017-05-28 19:59DuskNote Edited: 0017784bug_revision_view_page.php?bugnote_id=17784#r10666
2017-05-28 19:59DuskFile Added: crashy.diff
2017-05-28 22:37Edward-sanNote Added: 0017786
2017-05-29 06:31Torr SamahoNote Added: 0017788
2017-05-29 06:32Torr SamahoNote Edited: 0017788bug_revision_view_page.php?bugnote_id=17788#r10668
2017-06-18 19:21DuskNote Added: 0017839
2017-06-18 19:22DuskNote Edited: 0017839bug_revision_view_page.php?bugnote_id=17839#r10689
2017-06-18 21:02WaTaKiDNote Added: 0017840
2017-06-18 21:02WaTaKiDStatusfeedback => needs testing
2017-06-18 21:02WaTaKiDTarget Version => 3.0
2017-06-20 15:09KorshunNote Added: 0017855
2017-06-21 00:27Ru5tK1ngNote Added: 0017857
2017-06-21 00:27Ru5tK1ngStatusneeds testing => resolved
2017-06-21 00:27Ru5tK1ngResolutionopen => fixed
2017-06-21 00:27Ru5tK1ngFixed in Version => 3.0
2018-09-30 21:43Blzut3Statusresolved => closed

Notes
(0017540)
Korshun   
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.
(0017545)
Edward-san   
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?
(0017561)
Torr Samaho   
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?
(0017577)
Korshun   
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?
(0017583)
Korshun   
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.
(0017615)
Torr Samaho   
2017-05-09 06:14   
So ServerTrailDivByZero.pk3 alone should be sufficient to reproduce the problem under Linux? Edward-san, can you check this?
(0017636)
Edward-san   
2017-05-13 11:11   
I still cannot reproduce it. Korshun, which map did you use for the turbosphere?
(0017642)
Korshun   
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.
(0017646)
Torr Samaho   
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?
(0017649)
Edward-san   
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?
(0017655)
Korshun   
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?
(0017659)
Torr Samaho   
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.
(0017662)
Korshun   
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?
(0017664)
Torr Samaho   
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.
(0017667)
Korshun   
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.
(0017668)
Edward-san   
2017-05-14 13:46   
Quote
Sure, do I need to compile it myself?


That's correct.
(0017670)
Torr Samaho   
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.
(0017672)
Korshun   
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.
(0017673)
Torr Samaho   
2017-05-14 15:09   
AFAIK the debugger output of release mode binaries is not fully reliable, so you may get contradicting values.
(0017678)
Edward-san   
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'.
(0017680)
Korshun   
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.
(0017721)
Torr Samaho   
2017-05-20 12:06   
Did you have a chance to test this with a debug build yet?
(0017784)
Dusk   
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)
(0017786)
Edward-san   
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'...
(0017788)
Torr Samaho   
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?
(0017839)
Dusk   
2017-06-18 19:21   
(edited on: 2017-06-18 19:22)
The above code change fixes the crashing when ScaleY is treated likewise.
(0017840)
WaTaKiD   
2017-06-18 21:02   
fixed with: 'https://bitbucket.org/Torr_Samaho/zandronum/commits/e51d0173dda12e8d033f5e156f73d6a8b9bcf314 [^]'
(0017855)
Korshun   
2017-06-20 15:09   
Our 32bit Linux server doesn't crash anymore due to trails.
(0017857)
Ru5tK1ng   
2017-06-21 00:27   
That's good to hear. Reopen if necessary.