search

libretro / dosbox-svn

GNU General Public License v2.0
6 stars 17 forks source link

Core status on various platforms #35

Open andres-asm opened 4 years ago

andres-asm commented 4 years ago

Due to the recent libco update from upstream some builds are failing.

Build status:

Testing status:

andres-asm commented 4 years ago

Switch

The libco implementation we had was by @webgeek1234, this one is new and seems it relies on mman. @m4xw, do you think it can be fixed or should we go back to the old implementation for aarch64?

andres-asm commented 4 years ago

Android

Had it crash a couple times

11-24 16:45:56.633  3963  3997 F libc    : Fatal signal 11 (SIGSEGV), code 1, fault addr 0x8 in tid 3997 (Thread-2)
11-24 16:45:56.633   235   235 W         : debuggerd: handling request: pid=3963 uid=10083 gid=10083 tid=3997
11-24 16:45:56.671   318   454 W nvaudio_hw: UNDERRUN DETECTED for music-playback ret -1 avail 0
11-24 16:45:56.635  4004  4004 W debuggerd: type=1400 audit(0.0:1055): avc: denied { search } for name="com.retroarch.ra32" dev="mmcblk0p29" ino=631879 scontext=u:r:debuggerd:s0 tcontext=u:object_r:app_data_file:s0:c512,c768 tclass=dir permissive=0
11-24 16:45:56.697  4004  4004 F DEBUG   : *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
11-24 16:45:56.698  4004  4004 F DEBUG   : LineageOS Version: '14.1-20180328-NIGHTLY-foster'
11-24 16:45:56.698  4004  4004 F DEBUG   : Build fingerprint: 'NVIDIA/loki_e_wifi/foster:7.0/NRD90M/2427173_1038.2788:user/release-keys'
11-24 16:45:56.698  4004  4004 F DEBUG   : Revision: '0'
11-24 16:45:56.698  4004  4004 F DEBUG   : ABI: 'arm'
11-24 16:45:56.698  4004  4004 F DEBUG   : pid: 3963, tid: 3997, name: Thread-2  >>> com.retroarch.ra32 <<<
11-24 16:45:56.698  4004  4004 F DEBUG   : signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x8
11-24 16:45:56.698  4004  4004 F DEBUG   :     r0 00000000  r1 6cf9afc0  r2 6cf9afe8  r3 6c6bee3c
11-24 16:45:56.698  4004  4004 F DEBUG   :     r4 6e7ff000  r5 6e7ed000  r6 6e7ff36c  r7 6c96b788
11-24 16:45:56.698  4004  4004 F DEBUG   :     r8 57dfd3b8  r9 6e7ff168  sl 50bc9490  fp 57dfcc60
11-24 16:45:56.698  4004  4004 F DEBUG   :     ip 6cf32ffc  sp 57dfc510  lr 6c88c4dc  pc 6c88e59c  cpsr 600d0010
11-24 16:45:56.700  4004  4004 F DEBUG   :
11-24 16:45:56.700  4004  4004 F DEBUG   : backtrace:
11-24 16:45:56.700  4004  4004 F DEBUG   :     #00 pc 0038259c  /data/app/com.retroarch.ra32-1/lib/arm/libretroarch-activity.so (menu_widgets_frame+8668)
11-24 16:45:56.700  4004  4004 F DEBUG   :     #01 pc 001a557c  /data/app/com.retroarch.ra32-1/lib/arm/libretroarch-activity.so
11-24 16:45:56.700  4004  4004 F DEBUG   :     #02 pc 00460948  /data/app/com.retroarch.ra32-1/lib/arm/libretroarch-activity.so
11-24 16:45:56.701  4004  4004 F DEBUG   :     #03 pc 00324150  /data/app/com.retroarch.ra32-1/lib/arm/libretroarch-activity.so
11-24 16:45:56.701  4004  4004 F DEBUG   :     #04 pc 00047bf3  /system/lib/libc.so (_ZL15__pthread_startPv+22)
11-24 16:45:56.701  4004  4004 F DEBUG   :     #05 pc 00019f1d  /system/lib/libc.so (__start_thread+6)```

When it doesn't crash it's jerky and sound is messed up with external. The crash seems to be entirely in retroarch side though. Also happened pre-libco update so it's probably irrelevant.

This issue only happens with external too (notification gets screwed up and sound gets locked): https://photos.app.goo.gl/viqZLYyqnXueQ6t58

Changing from external locks up RA, changing between the two internal methods is fine.

m4xw commented 4 years ago

@fr500 Dont define LIBCO_MPROTECT if u have and wrap the sys/mman.h include in LIBCO_MPROTECT, the author seems to forgot that.

realnc commented 4 years ago

Updated the bundled libco to not include mman.h when not using mprotect: 6c4086498851ebfbdd43a0f1d18d1ffd3a3065f1

If this builds now on Switch, I'll submit the change to upstream as well.

m4xw commented 4 years ago

@realnc @fr500 Job succeeded, however can't test if it actually works rn. https://git.m4xw.net/libretro/cores/dosbox-svn/-/jobs/6147

m4xw commented 4 years ago

@realnc apparently it crashes

realnc commented 4 years ago

It was using armeabi.c before, right? Should we keep using that for ARM in general?

m4xw commented 4 years ago

No it uses aarch64 (switch)

m4xw commented 4 years ago

Also i cant tell u rn where it crashes to begin with, am at work myself. Could be somewhere other than libco

realnc commented 4 years ago

Do you know whether or not __arm__ is defined as well as __aarch64__?

m4xw commented 4 years ago

@realnc __aarch64__ is defined as well as __SWITCH__, don't think __arm__ is defined, if so then the aarch64 needs to take precedence.

realnc commented 4 years ago

OK, I just pushed a throw-away commit to test if arm.c is being picked. It will #error if that's the case.

m4xw commented 4 years ago

@realnc seems to build https://git.m4xw.net/libretro/cores/dosbox-svn/-/jobs/6152 try adding __attribute__((used)) here https://github.com/libretro/dosbox-svn/blob/libretro/libretro/deps/libco/aarch64.c#L24 you also might still want to pagealign it to 0x1000

realnc commented 4 years ago

OK. did both. e9dd94e56fd815396c9e1a4b25d2d390ae80878f

m4xw commented 4 years ago

Still crash, will need to ask a tester for the log and check the debug elf

m4xw commented 4 years ago

LR is https://github.com/libretro/dosbox-svn/blob/libretro/libretro/libretro.cpp#L1441 so yea, seems its libcro, error is Instruction Abort, its basically executing a NO_X segment. Seems it creates a 2nd .text section according to IDA, lmao image

m4xw commented 4 years ago

@realnc that should be correct __attribute__((section(".text"))); This should be changed for everything that uses gcc

realnc commented 4 years ago

@m4xw section is a macro from settings.h. However, it appends a #?

m4xw commented 4 years ago 
#elif defined(__linux__) || defined(__HAIKU__) || \
      defined(__FreeBSD__) || defined(__DragonFly__) || \
      defined(HAVE_LIBNX)
    __attribute__((section(".text")));
#elif defined(__MACH__)
    __attribute__((section("__TEXT,.text")));
#else

Taken from flycast and known working, for MACH it doesnt add the "." too, the section name must match, otherwise its a new section and you would have to explicitly tell it to be executable. # becomes _ in practice here so its never added in .text So no idea why that even works for some platforms

realnc commented 4 years ago

On Linux, without the #:

Warning: ignoring changed section attributes for .text

No such warning with #.

m4xw commented 4 years ago

yea because it creates a new section, otherwise that will also flag the .text as data (so read write execute), some AV's might trigger on that. Theres no other way, or u have to use __attribute__(section(".text#", "rx")) Also at that point, call it .libco or smth

m4xw commented 4 years ago

But i guess on the platforms where it works, it creates already a RWX section. I mean linux should never even use that codepath tho, because it has mman

realnc commented 4 years ago

Aha. The core didn't actually define LIBCO_MPROTECT. It was always using the .text codepath.

I changed that so that macOS and Linux use mprotect, and removed the # from the section tag. Rolled back the previous commit. As well.

72458747b09a2df386d4b12fd46563dbdb3a90ef e4512a2fd4ddcd5e872c34d5885645cabd477f27

m4xw commented 4 years ago

@fr500 @realnc confirmed working for switch.

realnc commented 4 years ago

Thank you very much for the help, @m4xw!