Anonymous | Login | Signup for a new account | 2024-04-19 09:20 UTC |
My View | View Issues | Change Log | Roadmap | Zandronum Issue Support Ranking | Rules | My Account |
View Issue Details [ Jump to Notes ] | [ Issue History ] [ Print ] | ||||||||
ID | Project | Category | View Status | Date Submitted | Last Update | ||||
0003124 | Zandronum | [All Projects] Bug | public | 2017-05-03 15:20 | 2018-09-30 21:43 | ||||
Reporter | Korshun | ||||||||
Assigned To | Torr Samaho | ||||||||
Priority | high | Severity | crash | Reproducibility | always | ||||
Status | closed | Resolution | fixed | ||||||
Platform | Linux | OS | Ubuntu | OS Version | |||||
Product Version | 3.0-beta | ||||||||
Target Version | 3.0 | Fixed in Version | 3.0 | ||||||
Summary | 0003124: Fix of trail scale bug causes divide by zero on servers | ||||||||
Description | The 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 Files | zandronum-crash-05_03_2017-17_14_16.2506.log [^] (12,452 bytes) 2017-05-03 15:53 zandronum-crash-05_03_2017-16_54_43.22792.log [^] (12,500 bytes) 2017-05-03 15:53 core.2506.7z [^] (3,066,578 bytes) 2017-05-03 15:53 ServerTrailDivByZero.pk3 [^] (347 bytes) 2017-05-07 14:37 crashy.diff [^] (310 bytes) 2017-05-28 19:59 [Show Content] | ||||||||
Notes | |
(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. |
(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? |
(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? |
(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? |
(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. |
(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? |
(0017636) Edward-san (developer) 2017-05-13 11:11 |
I still cannot reproduce it. Korshun, which map did you use for the turbosphere? |
(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. |
(0017646) Torr Samaho (administrator) 2017-05-13 12:53 edited on: 2017-05-13 12:55 |
Quote from Korshun 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 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 (developer) 2017-05-13 13:04 |
Quote I'm not. I use 64 bit. Could it be that the problem? |
(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? |
(0017659) Torr Samaho (administrator) 2017-05-14 08:13 |
Quote from Korshun 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 KorshunZDoom 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 (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? |
(0017664) Torr Samaho (administrator) 2017-05-14 12:12 |
Quote from Korshun 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 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 (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. |
(0017668) Edward-san (developer) 2017-05-14 13:46 |
Quote That's correct. |
(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 That part sounds correct. 65535 is equal to the internal value FRACUNIT, which essentially means that the scale is 1. |
(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. |
(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. |
(0017678) Edward-san (developer) 2017-05-14 15:59 |
Quote Try accessing it with 'Owner.p->anything'. |
(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. |
(0017721) Torr Samaho (administrator) 2017-05-20 12:06 |
Did you have a chance to test this with a debug build yet? |
(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):
|
(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:
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:
written like this? Scale(a,b,a) is technically the same as 'b'... |
(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
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 (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. |
(0017840) WaTaKiD (updater) 2017-06-18 21:02 |
fixed with: 'https://bitbucket.org/Torr_Samaho/zandronum/commits/e51d0173dda12e8d033f5e156f73d6a8b9bcf314 [^]' |
(0017855) Korshun (reporter) 2017-06-20 15:09 |
Our 32bit Linux server doesn't crash anymore due to trails. |
(0017857) Ru5tK1ng (updater) 2017-06-21 00:27 |
That's good to hear. Reopen if necessary. |
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 |
Copyright © 2000 - 2024 MantisBT Team |