Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Map crashes #34

Closed
4 tasks done
viti95 opened this issue Jun 6, 2021 · 20 comments
Closed
4 tasks done

Map crashes #34

viti95 opened this issue Jun 6, 2021 · 20 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@viti95
Copy link
Owner

viti95 commented Jun 6, 2021

@viti95 viti95 added bug Something isn't working help wanted Extra attention is needed labels Jun 6, 2021
@viti95 viti95 changed the title Freedoom 2 crashes randomly on MAP15 Map crashes Aug 24, 2022
@viti95
Copy link
Owner Author

viti95 commented Aug 24, 2022

Merged all map crashes issues onto this one

@RamonUnch
Copy link
Contributor

I also get a crash on TNT map02 when going turning right at the end of the stairs that lead outside.
Using DOSBox0.74-3. and latest Fast-doom build or even 0.7, 0.666, 0.5, 0.4, 0.3, 0.2 and 0.1
In the DOSBOX console I get: Illegal read from 6f9d3214, CS:IP 838: 152464 and similar (hard to capture) because DOSBOX fully crashes.

@viti95
Copy link
Owner Author

viti95 commented Oct 2, 2022

So it even crashes on build 0.1? Wow that's a very very early release. Can you try with DosBox-X to see if it catches better the exception?

@RamonUnch
Copy link
Contributor

cd47382

This commit seems to be the culprit
If i revert it then it seems fine even with the latest build.

@RamonUnch
Copy link
Contributor

I was trying to play back TNTmax.LMP movie by Never_Again with this revert, it seems to play back much further. I still got a crash in a late map but not map02

@viti95
Copy link
Owner Author

viti95 commented Oct 2, 2022

Uhhh very interesting, wasn't expecting a bug due to a Lee Killough optimization. I'm going to try also if it affects any of the other map crashes reports. Differences tested:

  • Plutonia MAP12 (demo3) now doesn't crash, but gets desynced anyway.
  • Ultimate Doom E4M9 nor E4M3 doesn't crash after more than 1 minute.

So I guess is safe to revert cd47382, maybe memmove is buggy on OpenWatcom but not on DJGPP.

viti95 added a commit that referenced this issue Oct 2, 2022
This early commit causes crashes on maps E4M3, E4M9 and Plutonia MAP12 (#34)

I'm pretty sure this issue due to a buggy memmove function on OpenWatcom, but will look onto it.
@RamonUnch
Copy link
Contributor

Well I see those implementations in the open watcom source tree:

https://github.com/open-watcom/open-watcom-v2/blob/047386398d3b538c8ba9792714e6143b1f44f0ca/bld/clib/memory/c/memmove.c
https://github.com/open-watcom/open-watcom-v2/blob/047386398d3b538c8ba9792714e6143b1f44f0ca/bld/clib/memory/c/rmemmove.c

not sure which one gets used in the end.
This is the only time memmove was used in the fastdoom source.

@RamonUnch
Copy link
Contributor

RamonUnch commented Oct 2, 2022

Actually I still get a crash on pluutonia map 12 and this even happens with the original PCDOOM-v2 so it is not your fault.
I get a crash with both dosboxX and Dosbox.
I think this is related to this: nukeykt/PCDoom-v2#23

and thus maybe do look at what chocolate-doom did:
chocolate-doom/chocolate-doom#48

EDIT: No it is not visplane overflow ignore this plutonia behaves fine under vanilla anyway...

@RamonUnch
Copy link
Contributor

Sorry for noizy information, I made a mistake and actually I get no crash with PCDoom-v2-master branch.

@RamonUnch
Copy link
Contributor

RamonUnch commented Oct 2, 2022

Plutonia Map12 insta-crash for me started with 7f22fcb

To fix it I just replaced:

in p_mobj.c at line618
-    if (mthing->type == 11 || mthing->type == 2 || mthing->type == 3 || mthing->type == 4)
+    if (!mthing || mthing->type == 11 || mthing->type == 2 || mthing->type == 3 || mthing->type == 4 || mthing->type < 1)

Now 30pl2930.lmp plays back fully.

@RamonUnch
Copy link
Contributor

RamonUnch commented Oct 2, 2022

Actually the patch I did was:
my mistak, because in some maps it can happen that mthing is NULL (I guess?),

in p_mobj.c at line618
-    if (mthing->type == 11 || mthing->type == 2 || mthing->type == 3 || mthing->type == 4)
+    if (!mthing || mthing->type == 11 || mthing->type == 2 || mthing->type == 3 || mthing->type == 4 || mthing->type < 1)

I guess that this is treating a symptom and the root cause must be something else.

@viti95
Copy link
Owner Author

viti95 commented Oct 3, 2022

The patch checking for NULL mthing actually works, I was able to test MAP12 without crashes, but yeah the issue must be coming from somewhere else. Anyway I think is good to have a check for NULL objects to avoid problems.

@RamonUnch
Copy link
Contributor

Well even with all those I get a crash in TNT MAP20 in some cases. I also get desync... I will investigate further

@RamonUnch
Copy link
Contributor

MAP20 crashes since 34a2bed
If I revert then it no longer crashes in map 20
I guess the optimization must have some edge cases. to get a reproducible crash go around coordinates: x=8609, y=-3158, A=176.
This is approximate, as I do not see coords in fastdoom. but this is the area t crashes.
There is also a desync in map 20 that occurred between 0.7 and 0.8.16 but this is not directly related.

@viti95
Copy link
Owner Author

viti95 commented Oct 3, 2022

Looks like a precision loss error, maybe reverting it also fixes #52

@RamonUnch
Copy link
Contributor

The desync was my fault, I did not apply all fdoom patches, so now I can play back TNT map20 without problems, I can also play back the whole tntmax.lmp 4 hour movie...

@RamonUnch
Copy link
Contributor

#52 is indeed fixed for me.
Also when playing back MAP20 with large screen the problem did not occur, I am often playing back demo in low details and small screen.

@viti95
Copy link
Owner Author

viti95 commented Oct 3, 2022

Ok I revert 34a2bed until we find the edge cases that makes this commit crash in some maps/screen sizes.

viti95 added a commit that referenced this issue Oct 3, 2022
@viti95
Copy link
Owner Author

viti95 commented Oct 3, 2022

I've been testing also FreeDoom2 MAP15 and it works without issues.

@viti95
Copy link
Owner Author

viti95 commented Oct 17, 2022

I'm also closing this as it seems all map crashes have been fixed. If we find a new crash I'll reopen it.

@viti95 viti95 closed this as completed Oct 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants