search

mupen64plus / mupen64plus-core

Core module of the Mupen64Plus project
1.29k stars 257 forks source link

Crash (Segfault) on 32bit ARM & DynaRec enabled #1026

Closed cmitu closed 1 year ago

cmitu commented 1 year ago

Hi,

I'm using a Mupen64plus build for armhf, on latest RaspiOS running on a Pi4 compiled with GLES/NEON support. Running any ROM results in a mupen64plus crash with Segmentation fault, which seems to happen inside libmupen64plus.

gdb shows in the backtrace the crash location like so:

...
#0  cvt_w_s (fcr31=0x1, source=0xb676ed60 <g_dev+51387744>, dest=0xb676ed40 <g_dev+51387712>) at ../../src/device/r4300/fpu.h:438
#1  0xb3bab70c in g_dev () from /opt/retropie/emulators/mupen64plus/lib/libmupen64plus.so.2

This seems to happen after #1001 was merged, using a 22 May commit doesn't trigger a crash. A summary bisect points to a9df0c988, but I think it's inaccurate since I had to skip a commit during bisection.

mupen64plus-core was compiled with

DEBUG=1 USE_GLES=1 NEON=1 OPTFLAGS= -mcpu=cortex-a72  -mfpu=neon-fp-armv8 -O2
zdateh commented 1 year ago

Does it work with ACCURATE_FPU=0

This option disables the accurate FPU code that was part of #1001 - perhaps necessary on pi anyway. That said, it probably shouldn't crash!

cmitu commented 1 year ago

@zdateh I haven't tried with ACCURATE_FPU=0, but from what I've seen in the makefile, isn't the FPU code enabled only when building with ACCURATE_FPU=1 ?

zdateh commented 1 year ago

sorry, yes i think you're right! it looks to be disabled by default.

m4xw commented 1 year ago

Known issue, heres @Rosalie241 's workaround https://cdn.discordapp.com/attachments/748233471381471272/1128367011790065674/dynarec_fpu_fix.patch Talked to @Gillou68310 before about handling it better but we never finalised it

Rosalie241 commented 1 year ago

The patch is also available here: https://github.com/Rosalie241/mupen64plus-core/tree/fix-dynarec

If desired, I can open a PR so we can temporarely fix it upstream until Gillou has taken a look, but it does fix it on arm64 at least, I don't have an arm32 cpu to test that though, but that decision is also upto @richard42

EDIT: seems like the patch that I sent m4xw didn't have arm32 support included, so I've fixed that in the above branch, so please that test instead.

cmitu commented 1 year ago

@Rosalie241 thank you for the patch - it does solve the issue on arm32 also. There are no more crashes when using your fix-dynarec branch for mupen64plus-core.

I'll close this issue, is there any plan to merge the fixes here, in this repo ?

Rosalie241 commented 1 year ago

That's upto Richard, if he'd like to, I'll open a PR until Gillou comes up with a cleaner/better solution in the near of far future.