thpatch / thcrap

Touhou Community Reliant Automatic Patcher
https://www.thpatch.net
The Unlicense
558 stars 41 forks source link

[IN] Game crashes when completing. #148

Closed Oreeeee closed 2 years ago

Oreeeee commented 3 years ago

So I did an 1cc, game crashed on ending so it hasn't save anything and Extra is still locked.

Here is list of patches that I used: -English Patch -SC-88 Pro soundtrack

I'm going to attach logs later.

Sorry for my bad English.

Oreeeee commented 3 years ago

Ok, here are logs: thcrap_log.3.txt thcrap_log.4.txt thcrap_log.5.txt thcrap_log.txt thcrap_log.1.txt thcrap_log.2.txt

brliron commented 3 years ago

Notes for devs: crash logs in thcrap_log.3.txt, crash in vcruntime140.dll and no stacktrace, using standalone on last version + vpatch.

b65535 commented 2 years ago

Touhou 8 - Imperishable Night crashed for me, too, after completing it on Easy (2CC, Illusionary Barrier Team, Final A, last spell failed, when the window becomes white before displaying the ending and the credits).

I'm using PlayOnLinux 4.3.4 with Wine 6.17.

Log file: thcrap_log.txt

At one occasion of the crash, a window was displayed showing more details about the crash. Unfortunately, I didn't save the content of the window but only wrote down some parts of the error log:

Unhandled exception: page fault on read access to 0xfffffffd in 32-bit code (0x70ba711f)

_snwsconf_s_l + 0x14f() in ucrtbase

th08 ole32 d3d8

Backtrace: _snwsconf_s_l in ucrtbase 0x0021ff88 I10_OUTPUT in ucrtbase BP_ascii_params in thcrap_tsa

The crash on completion of the game doesn't occur when using the original Japanese version, i.e. th08.exe without thcrap.

brliron commented 2 years ago

The crash happens inside the call to memcpy() at https://github.com/thpatch/thcrap/blob/a295b8808218755bac48b8c8be15d97425d63c44/thcrap_tsa/src/th06_msg.cpp#L777

brliron commented 2 years ago

@32th-System Are you sure the memchr call on the previous line will find a \n?

According to the man, memchr returns NULL if the byte isn't found, and the crash happens when reading 0xFFFFFFFD. If we try to read orig_file from the end with a size of 0-orig_file-4, the address matches. And the wine implementation of memcpy is a memmove (https://github.com/wine-mirror/wine/blob/2660cd9fcbb23897b041c43bcac1dc2a7aa3c01f/dlls/msvcrt/string.c#L2853), so it probably starts copying from the end.

32th-System commented 2 years ago

If the file doesn't end in \n, no, because the last line wouldn't end in \n. In that case, the fix would be to set orig_advanced to orig_file_copy + size_in if orig_advanced ends up being NULL as a result of the memchr, in theory

b65535 commented 2 years ago

In the same function is another call of memchr: https://github.com/thpatch/thcrap/blob/a295b8808218755bac48b8c8be15d97425d63c44/thcrap_tsa/src/th06_msg.cpp#L760

Is it guaranteed that a '@' exists? If not, this would cause invalid memory accesses, too.

b65535 commented 2 years ago

@32th-System, I think, being able to deal with a (theoretically) missing '@' would require several changes in the code of the function patch_end_th06 because the whole logic of the function relies on the existence of the '@' characters. Or do you see a simple solution to make the code more robust?

@Oreeeee, until this issue is fixed, you can do the following to workaround the crash:

brliron commented 2 years ago

Instead of using an outdated version of thcrap, you can tell him to ignore the ending patch files. https://www.thpatch.net/wiki/Touhou_Patch_Center:Download#ignore The pattern to ignore should be "*.end.jdiff"

b65535 commented 2 years ago

Instead of using an outdated version of thcrap, you can tell him to ignore the ending patch files. https://www.thpatch.net/wiki/Touhou_Patch_Center:Download#ignore The pattern to ignore should be "*.end.jdiff"

This would be a more elegant workaround. Unfortunately, it didn't work with thcrap 2021-08-30. I changed config/en.js:

{ "archive": "C:/Program Files/Touhou 8 - Imperishable Night/thcrap/repos/thpatch/lang_en/", "ignore": ["*.end.jdiff"] } "th08/*.end.jdiff", "end*" and "th08\\end*" didn't work, either. The endings were still translated which probably would cause crashes on certain endings.

brliron commented 2 years ago

I fixed the crash in 1e5f2186713f4c164328596a5e84e74ac23cb056, I will make a new release with this fix soon. I also fixed the "ignore" config not working on jdiff files. And about b65535's comment on '@' characters, I don't feel like fixing it properly everywhere right now, I guess I will wait and see if it does cause a crash for some people.

brliron commented 2 years ago

The fix is included in the new release

b65535 commented 2 years ago

@brliron, thank you. With release 2021-10-24 TH 8 doesn't crash at the ending anymore (Easy, 2CC, Illusionary Barrier Team, Final A, last spell failed).