Possible Z_Malloc error on playing demo under x64 build about chocolate-doom HOT 19 CLOSED

A crash occurs on

diff --git a/src/i_system.c b/src/i_system.c
index dae2107c..b32639e8 100644
--- a/src/i_system.c
+++ b/src/i_system.c
@@ -146,7 +146,14 @@ byte *I_ZoneBase (int *size)
     }
     else
     {
-        default_ram = DEFAULT_RAM;
+        if (sizeof(void *) == 8)
+        {
+            default_ram = DEFAULT_RAM * 2;
+        }
+        else
+        {
+            default_ram = DEFAULT_RAM;
+        }
         min_ram = MIN_RAM;
     }
 

from chocolate-doom.

We would have to exactly replicate Vanilla's zone memory fragmentation at any given point. I don't think that's possible at all.

I think we may be very close, the code in z_zone.c is the same. Several things are rewritten using zone memory, notably the WAD system. I tried running a 32-bit Chocolate with the -mb 8 key. The demo crashed, but in a different place and with a different message than the vanilla DOS version.

w_wad.c(418): Z_Malloc: failed on allocation of 733764 bytes

Warning: recursive call to I_Error detected.

Anyway, I think it's worth fixing the 64-bit build for consistency with the 32-bit version.

from chocolate-doom.

Can you try with --disable-zpool? Also use Valgrind or ASAN if you can.

from chocolate-doom.

Can you try with --disable-zpool?

But where exactly this parameter should be added? Is it compile-time, or...?

Never tried Valgrind unfortunately, but still have ASAN in MINGW's Clang.
Build type is "Debug", x64, results are:

lldb -- ./chocolate-doom.exe -file AV.WAD -timedemo avm62402.lmp -nodraw

# lldb -- ./chocolate-doom.exe -file AV.WAD -timedemo avm62402.lmp -nodraw
(lldb) target create "./chocolate-doom.exe"
(rrent executable set to 'R:\MSYS\home\julia\chocolate-doom\debug_clang\src\chocolate-doom.exe' (x86_64).
(ldb) settings set -- target.run-args  "-file" "AV.WAD" "-timedemo" "avm62402.lmp" "-nodraw"
(lldb) r
(lldb) Process 1652 launched: 'R:\MSYS\home\julia\chocolate-doom\debug_clang\src\chocolate-doom.exe' (x86_64)
Process 1652 exited with status = -1 (0xffffffff)
(lldb) bt
error: Command requires a process which is currently stopped.

Next I've tried w/o -nodraw parameter and crash happens almost imideatelly:

# lldb -- ./chocolate-doom.exe -file AV.WAD -timedemo avm62402.lmp
(lldb) target create "./chocolate-doom.exe"
(rrent executable set to 'R:\MSYS\home\julia\chocolate-doom\debug_clang\src\chocolate-doom.exe' (x86_64).
(lldb) settings set -- target.run-args  "-file" "AV.WAD" "-timedemo" "avm62402.lmp"
(lldb) r
(lldb) Process 7464 launched: 'R:\MSYS\home\julia\chocolate-doom\debug_clang\src\chocolate-doom.exe' (x86_64)
Process 7464 stopped
* thread #1, stop reason = Exception 0x80000003 encountered at address 0x7ffb94045871
    frame #0: 0x00007ffb94045872 libclang_rt.asan_dynamic-x86_64.dll`__sanitizer::internal__exit(int) + 18
libclang_rt.asan_dynamic-x86_64.dll`__sanitizer::internal__exit:
->  0x7ffb94045872 <+18>: callq  *0x59a28(%rip)            ; __imp_GetCurrentProcess
    0x7ffb94045878 <+24>: movq   %rax, %rcx
    0x7ffb9404587b <+27>: movl   %esi, %edx
    0x7ffb9404587d <+29>: callq  *0x59b45(%rip)            ; __imp_TerminateProcess
(lldb) bt
* thread #1, stop reason = Exception 0x80000003 encountered at address 0x7ffb94045871
  * frame #0: 0x00007ffb94045872 libclang_rt.asan_dynamic-x86_64.dll`__sanitizer::internal__exit(int) + 18
    frame #1: 0x00007ffb94046b37 libclang_rt.asan_dynamic-x86_64.dll`__sanitizer::Die() + 103
    frame #2: 0x00007ffb9407850d libclang_rt.asan_dynamic-x86_64.dll`__asan::ScopedInErrorReport::~ScopedInErrorReport() + 1133
    frame #3: 0x00007ffb9407b434 libclang_rt.asan_dynamic-x86_64.dll`__asan::ReportGenericError(unsigned long long, unsigned long long, unsigned long long, unsigned long long, bool, unsigned long long, unsigned int, bool) + 1652
    frame #4: 0x00007ffb9407c149 libclang_rt.asan_dynamic-x86_64.dll`__asan_report_load4 + 57
    frame #5: 0x00007ff700aa419e chocolate-doom.exe`R_RenderSegLoop at r_segs.c:257:41
    frame #6: 0x00007ff700aa9c03 chocolate-doom.exe`R_StoreWallRange(start=298, stop=298) at r_segs.c:711:5
    frame #7: 0x00007ff700a95735 chocolate-doom.exe`R_ClipPassWallSegment(first=298, last=298) at r_bsp.c:213:6
    frame #8: 0x00007ff700a96aa0 chocolate-doom.exe`R_AddLine(line=0x0000017682ce1330) at r_bsp.c:350:5
    frame #9: 0x00007ff700a9835a chocolate-doom.exe`R_Subsector(num=186) at r_bsp.c:538:2
    frame #10: 0x00007ff700a983ff chocolate-doom.exe`R_RenderBSPNode(bspnum=32954) at r_bsp.c:566:6
    frame #11: 0x00007ff700a9856a chocolate-doom.exe`R_RenderBSPNode(bspnum=185) at r_bsp.c:576:5
    frame #12: 0x00007ff700a985f9 chocolate-doom.exe`R_RenderBSPNode(bspnum=189) at r_bsp.c:580:2
    frame #13: 0x00007ff700a9856a chocolate-doom.exe`R_RenderBSPNode(bspnum=193) at r_bsp.c:576:5
    frame #14: 0x00007ff700a9856a chocolate-doom.exe`R_RenderBSPNode(bspnum=194) at r_bsp.c:576:5
    frame #15: 0x00007ff700a9856a chocolate-doom.exe`R_RenderBSPNode(bspnum=212) at r_bsp.c:576:5
    frame #16: 0x00007ff700a9856a chocolate-doom.exe`R_RenderBSPNode(bspnum=218) at r_bsp.c:576:5
    frame #17: 0x00007ff700a9856a chocolate-doom.exe`R_RenderBSPNode(bspnum=236) at r_bsp.c:576:5
    frame #18: 0x00007ff700a9856a chocolate-doom.exe`R_RenderBSPNode(bspnum=261) at r_bsp.c:576:5
    frame #19: 0x00007ff700a9856a chocolate-doom.exe`R_RenderBSPNode(bspnum=340) at r_bsp.c:576:5
    frame #20: 0x00007ff700a9856a chocolate-doom.exe`R_RenderBSPNode(bspnum=487) at r_bsp.c:576:5
    frame #21: 0x00007ff700a9856a chocolate-doom.exe`R_RenderBSPNode(bspnum=616) at r_bsp.c:576:5
    frame #22: 0x00007ff700a9856a chocolate-doom.exe`R_RenderBSPNode(bspnum=630) at r_bsp.c:576:5
    frame #23: 0x00007ff700a9856a chocolate-doom.exe`R_RenderBSPNode(bspnum=631) at r_bsp.c:576:5
    frame #24: 0x00007ff700a9856a chocolate-doom.exe`R_RenderBSPNode(bspnum=743) at r_bsp.c:576:5
    frame #25: 0x00007ff700a22994 chocolate-doom.exe`R_RenderPlayerView(player=0x00007ff700c9ba80) at r_main.c:876:5
    frame #26: 0x00007ff7009e2896 chocolate-doom.exe`D_Display at d_main.c:233:2
    frame #27: 0x00007ff7009e39f3 chocolate-doom.exe`D_RunFrame at d_main.c:443:21
    frame #28: 0x00007ff7009e3d3e chocolate-doom.exe`D_DoomLoop at d_main.c:496:9
    frame #29: 0x00007ff7009e7a5d chocolate-doom.exe`D_DoomMain at d_main.c:1985:2
    frame #30: 0x00007ff7009415da chocolate-doom.exe`SDL_main(argc=5, argv=0x00000176fda691a0) at i_main.c:77:5
    frame #31: 0x00007ff700a6b9f1 chocolate-doom.exe`main_getcmdline + 385
    frame #32: 0x00007ff700a02e78 chocolate-doom.exe`main(argc=<unavailable>, argv=<unavailable>, envp=<unavailable>) at crtexewin.c:67:10
    frame #33: 0x00007ff700941315 chocolate-doom.exe`__tmainCRTStartup at crtexe.c:267:15
    frame #34: 0x00007ff700941156 chocolate-doom.exe`WinMainCRTStartup at crtexe.c:157:9
    frame #35: 0x00007ffbf39e257d kernel32.dll`BaseThreadInitThunk + 29
    frame #36: 0x00007ffbf4daaa58 ntdll.dll`RtlUserThreadStart + 40
(lldb)

Most likely, I'm doing something very wrong. DLL libraries are from MSYS/clang64/bin/ and ASan is enabled as compile-time option in debug_clang/CMakeCache.txt using this approach.

from chocolate-doom.

But where exactly this parameter should be added? Is it compile-time, or...?

Yes, it's a configure option.

Never tried Valgrind unfortunately, but still have ASAN in MINGW's Clang.

Valgrind requires Linux. Asan maybe works on windows but never tried it.

If that error report is correct then it looks like angle is way out of bounds for finetangent.

from chocolate-doom.

I think the original av.zip contained a separate variant of map20 to fix vanilla compatibility in some way. Maybe this is related.

from chocolate-doom.

Looks likes it is, I wasn't curious enough. From AV.TXT file:

[1.8 Avmovfix.wad]

Unfortunately we discovered a limmitation regarding map20 and demorecording.
If you attempt a full game run with doom2.exe it will bomb out in map20 
with an error message if the -maxdemo parameter is set high enough.
To avoid this we included an altered version of map20 as avmovfix.wad,
its basically map20 w/o the hughe mountain area. Such a high -maxdemo
valu is only of interest for a possible maxkill movie run, even episode
recordings work fine with no fix added.

Note that his one is only of interest for demorecorders in compet-N context,
and should be ignored if using a port such as zdoom.

Unfortunately, avm62402.lmp desyncs on MAP20 with using Avmovfix.wad because of different map nodes / structure, this is correct behavior, but at least no Z_Malloc crash is happening upon entering Misri Halek. Avm62402 was recorded with DSDA-Doom 0.21.3, which, obliviously, doesn't have limitations for demo recordings, as well as it is completely limit-removing by itself.

Still have a suspicion that something went really wrong with ASan. Really no idea what exactly, such error shouldn't happen at all, it's rendering-related.

@turol, could you please advise farther, how, i.e. when exactly --disable-zpool should be placed/provided? Just to clarify, I'm using Cmake for building, i.e.:
cmake -G "Ninja" -D CMAKE_BUILD_TYPE="Debug" -S . -B debug_clang

from chocolate-doom.

It's only supported in configure at this time. You can manually disable DISABLE_ZPOOL macro or better yet, add support for it to CMakeLists.

from chocolate-doom.

Figured out, thanks! Just added option(DISABLE_ZPOOL "Enable ZPool" OFF), then made cmake --build buildX --target clean on respective builds and recompiled everything. Looks likes no effect, it's still:

• Warning: recursive call to I_Error detected. on GCCs x64 build.
• Normal "timed ... gametics" on GCCs x86 build.
• Same behavior on Clang x64 builds with and w/o ASan.

from chocolate-doom.

Figured out, thanks! Just added option(DISABLE_ZPOOL "Enable ZPool" OFF), then made cmake --build buildX --target clean on respective builds and recompiled everything.

It's not that simple.

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 217765b3..cdb6e12c 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -82,6 +82,8 @@ string(TOLOWER "${PACKAGE_SHORTNAME}" PROGRAM_SPREFIX)
 # With a hyphen, used almost everywhere else.
 set(PROGRAM_PREFIX "${PROGRAM_SPREFIX}-")
 
+option(DISABLE_ZPOOL "Disable ZPool" OFF)
+
 configure_file(cmake/config.h.cin config.h)
 
 configure_file(src/doom-res.rc.in src/doom-res.rc)
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 8b02fe66..9158ad82 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -101,8 +101,13 @@ set(GAME_SOURCE_FILES
     w_file_stdc.c
     w_file_posix.c
     w_file_win32.c
-    w_merge.c           w_merge.h
-    z_zone.c            z_zone.h)
+    w_merge.c           w_merge.h)
+
+if(DISABLE_ZPOOL)
+    list(APPEND GAME_SOURCE_FILES z_native.c z_zone.h)
+else()
+    list(APPEND GAME_SOURCE_FILES z_zone.c z_zone.h)
+endif()
 
 set(GAME_INCLUDE_DIRS "${CMAKE_CURRENT_BINARY_DIR}/../")
 

Then configure: cmake -B build_z -DDISABLE_ZPOOL=ON

from chocolate-doom.

Done! And it's worked, now both GCC's x86 and x64, as well as Clang's x64 executables are able to play the demo from start till end.

from chocolate-doom.

So, it seems to be an allocation issue. The DOS error message reports that 896120 bytes cannot get allocated in the zone memory, which is quite a lot, given that the zone is typically only 4 MB [*] altogether. If the map is loaded with -warp 20 there hasn't been much allocation before, so the zone isn't as fragmented. But if the map is loaded in succession with other maps, the zone will be fragmented and it won't be possible to find a continuous (!) memory region of that size. Nativ allocators won't face this problem, of course, as long as there is a system memory region of that size available.

The difference between x86 and x64 builds may be due to the different zone memory header sizes (because of different sizeof(void*)), which result in different degrees of fragmentation at the point when the map is loaded.

The "Warning: recursive call to I_Error detected." message isn't really helpful, but I guess that the engine was going to report its insufficient memory, but failed to do so because of, well, insufficient memory. I just cannot say for sure...

[*] Edit: That's MIN_RAM, DEFAULT_RAM is actually 16 MB.

from chocolate-doom.

Pragmatic approach, I like it. And since DOOM.exe was never available on 64-bit architectures anyway, I guess we are a bit more free to apply specific fixes like this one.

from chocolate-doom.

I think making this not crash can cause us to record demos which can't play in vanilla or 32-bit chocolate. Do we really want to do that?

Can vanilla play that demo if you specify a larger initial memory size?

from chocolate-doom.

I think making this not crash can cause us to record demos which can't play in vanilla or 32-bit chocolate. Do we really want to do that?

Vanilla DOS exe allocate 8 MiB, 32-bit Chocolate Doom - 16 MiB. Demo avm62402.lmp

I suggest to make the 64-bit Chocolate Doom work too, since the 32-bit one works anyway.

Can vanilla play that demo if you specify a larger initial memory size?

Is this possible with vanilla DOS exe?

from chocolate-doom.

We would have to exactly replicate Vanilla's zone memory fragmentation at any given point. I don't think that's possible at all.

from chocolate-doom.

Vanilla Doom doesn't have -mb parameter for sure, but maybe it is possible to hex-edit executable in same manner as in Doom-Plus? I.e. change heap size from 0x800000 to 0x1000000. But will it work at all, and where's these values in executable - that's the question of the day.

from chocolate-doom.

So it sounds like it's already possible to create demos which are not playable in vanilla. In that case this doesn't matter and changing the initial heap size is okay. I doubt anyone wants to un-refactor enough to make this crash-compatible.

from chocolate-doom.

Whoever makes the PR add a comment about why.

from chocolate-doom.