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

Crash with strong CRC #2839

Closed
Rosalie241 opened this issue May 13, 2024 · 7 comments
Closed

Crash with strong CRC #2839

Rosalie241 opened this issue May 13, 2024 · 7 comments

Comments

@Rosalie241
Copy link
Contributor

Rosalie241 commented May 13, 2024

An RMG user reported a crash when using a Paper Mario texture pack, after investigating, it crashes in GLideN64.

It happens because buf.size() is smaller than bytesPerLine in TxUtil::StrongCRC32:

Thread 25 "Thread::Emulati" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffac6006c0 (LWP 24518)]
0x00007ffff5f7c518 in __memmove_avx_unaligned_erms () from /lib64/libc.so.6
(gdb) bt full
#0  0x00007ffff5f7c518 in __memmove_avx_unaligned_erms () at /lib64/libc.so.6
#1  0x00007fffb8cb3c15 in TxUtil::StrongCRC32
    (src=0x7fff740c16a0 "\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377$\030\022\374\377\3773\377\002", width=-1008, height=16, size=0, rowStride=8)
    at /home/rosalie/dev/RMG/Source/3rdParty/mupen64plus-video-GLideN64/src/GLideNHQ/TxUtil.cpp:536
        y = 0
        bytesPerLine = 4294966792
        crc = 18446744073709551615
        buf = std::vector of length 4294959232, capacity 4294959232 = {238 '\356', 255 '\377', 238 '\356', 239 '\357', 254 '\376', 254 '\376', 255 '\377', 255 '\377', 238 '\356', 255 '\377', 238 '\356', 239 '\357', 254 '\376', 254 '\376', 255 '\377', 255 '\377', 238 '\356', 255 '\377', 238 '\356', 239 '\357', 254 '\376', 254 '\376', 255 '\377', 255 '\377', 238 '\356', 255 '\377', 238 '\356', 239 '\357', 254 '\376', 254 '\376', 255 '\377', 255 '\377', 238 '\356', 255 '\377', 238 '\356', 239 '\357', 254 '\376', 254 '\376', 255 '\377', 255 '\377', 238 '\356', 255 '\377', 238 '\356', 239 '\357', 254 '\376', 254 '\376', 255 '\377', 255 '\377', 238 '\356', 255 '\377', 238 '\356', 239 '\357', 254 '\376', 254 '\376', 255 '\377', 255 '\377', 238 '\356', 255 '\377', 238 '\356', 239 '\357', 254 '\376', 254 '\376', 255 '\377', 255 '\377', 238 '\356', 255 '\377', 238 '\356', 239 '\357', 254 '\376', 254 '\376', 255 '\377', 255 '\377', 238 '\356', 255 '\377', 238 '\356', 239 '\357', 254 '\376', 254 '\376', 255 '\377', 255 '\377', 238 '\356', 255 '\377', 238 '\356', 239 '\357', 254 '\376', 254 '\376', 255 '\377', 255 '\377', 238 '\356', 255 '\377', 238 '\356', 239 '\357', 254 '\376', 254 '\376', 255 '\377', 255 '\377', 238 '\356', 255 '\377', 238 '\356', 239 '\357', 254 '\376', 254 '\376', 255 '\377', 255 '\377', 238 '\356', 255 '\377', 238 '\356', 239 '\357', 254 '\376', 254 '\376', 255 '\377', 255 '\377', 238 '\356', 255 '\377', 238 '\356', 239 '\357', 254 '\376', 254 '\376', 255 '\377', 255 '\377', 238 '\356', 255 '\377', 238 '\356', 239 '\357', 254 '\376', 254 '\376', 255 '\377', 255 '\377', 36 '$', 24 '\030', 18 '\022', 252 '\374', 255 '\377', 255 '\377', 51 '3', 255 '\377', 2 '\002', 0 '\000', 0 '\000', 215 '\327', 255 '\377', 255 '\377', 255 '\377', 255 '\377', 0 '\000', 0 '\000', 112 'p', 253 '\375', 160 '\240', 22 '\026', 43 '+', 128 '\200', 0 '\000', 0 '\000', 112 'p', 245 '\365', 64 '@', 2 '\002', 9 '\t', 7 '\a', 0 '\000', 0 '\000', 0 '\000', 230 '\346', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 243 '\363', 0 '\000', 248 '\370', 3 '\003', 7 '\a', 0 '\000', 0 '\000', 0 '\000', 231 '\347', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 2 '\002', 96 '`', 245 '\365', 64 '@', 2 '\002', 9 '\t', 0 '\000', 0 '\000', 0 '\000', 0 '\000', 242 '\362', 60 '<', 192 '\300', 3 '\003', 0 '\000'...}
        pData = 0x7ffd9c000010 "\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377$\030\022\374\377\3773\377\002"
#2  0x00007fffb8cb3749 in TxUtil::checksum64strong
    (src=0x7fff740c16a0 "\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377$\030\022\374\377\3773\377\002", width=-1008, height=16, size=0, rowStride=8, palette=0x0)
    at /home/rosalie/dev/RMG/Source/3rdParty/mupen64plus-video-GLideN64/src/GLideNHQ/TxUtil.cpp:149
        crc64Ret = 0
#3  0x00007fffb8ca2416 in TxFilter::checksum64strong
    (this=0x7fff48f0b8d0, src=0x7fff740c16a0 "\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377$\030\022\374\377\3773\377\002", width=-1008, height=16, size=0, rowStride=8, palette=0x0)
    at /home/rosalie/dev/RMG/Source/3rdParty/mupen64plus-video-GLideN64/src/GLideNHQ/TxFilter.cpp:587
#4  0x00007fffb8ca0757 in txfilter_checksum_strong
    (src=0x7fff740c16a0 "\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377\356\377\356\357\376\376\377\377$\030\022\374\377\3773\377\002", width=-1008, height=16, size=0, rowStride=8, palette=0x0)
    at /home/rosalie/dev/RMG/Source/3rdParty/mupen64plus-video-GLideN64/src/GLideNHQ/TxFilterExport.cpp:91

surprisingly, I've also discovered that it happens without crashing too:

buf.size(): 32
bytesPerLine: 32
buf.size(): 32
bytesPerLine: 32

Texture pack: https://drive.google.com/file/d/1qi2tvLso7cSnFXrnilN-qoc1fCBW1B9j/view
Mupen64plus save state: state.zip
ROM MD5: A722F8161FF489943191330BF8416496

I've opened #2838 as a workaround for now, which does fix the crash when the strong CRC option is disabled but isn't a proper fix, I do feel it makes sense to not calculate the strong CRC if the option hasn't been enabled but your opinion might differ.

gonetz added a commit that referenced this issue May 25, 2024
@gonetz
Copy link
Owner

gonetz commented May 25, 2024

@Rosalie241 I checked the game from the provided save state.
At some moment the game calls gDPSetTileSize command, which set tile uls greater than lrs, so texture width becomes negative.
It causes crash either in texture checksum calculation or in OpenGL call.
The situation is weird. Probably it is a core issue. I put sanity checks to texture load functions to avoid the crashes, see commit 85bdd45. Please check if the problem is gone.

BTW, great texture pack!

gonetz added a commit that referenced this issue Jun 1, 2024
@Rosalie241
Copy link
Contributor Author

I can confirm your commit fixes the issue, it'd be odd if it's a core issue but I don't know.

Thank you for checking and fixing it ❤️

@Rosalie241
Copy link
Contributor Author

An RMG user reported an issue with Donkey Kong 64 where the water turns invisible sometimes after I updated GLideN64,
I've decided to try it with Project64 and it happens there aswell, commenting out the code added in the commit fixes it again.

ROM MD5: 295D995852DB7BC9E8F29085E924C835

Mupen64plus and Project64 savestates: wherethewater.zip

so it seems that might not be a core issue, tested it on the latest stable Project64.

@gonetz
Copy link
Owner

gonetz commented Jun 30, 2024

so it seems that might not be a core issue, tested it on the latest stable Project64.

Yes, it's my mistake.
I reverted the wrong fix and added a proper (hopefully) one: f119c32

@GhostlyDark
Copy link
Contributor

Confirmed that Paper Mario doesn't crash anymore and the DK64 water is working. The N64 logo in the Zelda games also looked wrong, but anymore.

I've opened #2838 as a workaround for now, which does fix the crash when the strong CRC option is disabled but isn't a proper fix, I do feel it makes sense to not calculate the strong CRC if the option hasn't been enabled but your opinion might differ.

No, the option shouldn't be needed to load in strong CRC dumped textures. The idea is to enable the option whenever a texture artist needs it, dump any problematic textures and deactive it again to maintain compatibility with previous hashes. Strong CRC hashes should work regardless of the option being enabled or not, it should be solely toggled for dumping.

@gonetz
Copy link
Owner

gonetz commented Jun 30, 2024

No, the option shouldn't be needed to load in strong CRC dumped textures. The idea is to enable the option whenever a texture artist needs it, dump any problematic textures and deactive it again to maintain compatibility with previous hashes. Strong CRC hashes should work regardless of the option being enabled or not, it should be solely toggled for dumping.

Yes, exactly. If the crash is fixed without regressions in other games, #2838 should be closed.

@Rosalie241
Copy link
Contributor Author

I can confirm the issues have been fixed, I've also closed the PR I made.

Thank you for looking into it ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants