BUG: Libco defect in many libretro-cores causes crashes in Windows x64

[snaphat]

Many of the libretro cores contain a defect in libco that causes floating point register corruption under x86_64 Windows due to incorrect handwritten 'machine code' found here for example.

Specifically, the issue is that the restore code uses the wrong opcode for MOVAPS when attempting to restore callee-saved registers. The bug appears to originate in the earliest incarnations of libco. It seems to have been fixed some years ago in some of the libretro repositories when an update was pulled from the mainline libco repository (e.g. here). Many of the repositories hold a broken copy though, and it does result in real-world crashes and/or broken behavior.

More specifically, lines 62 through 71 use opcode 0x29 instead of 0x28. Semantically, the difference is as follows:

• 0x28 : (reg <-- [mem])
• 0x29 : ([mem] <-- reg)

The former is for storing a memory value into a register, and the latter is for storing register contents into a memory location.

The earliest incarnations of the code were uncommented and written in an era when 64-bit processors were rare, so it isn't surprising it wasn't fixed in mainline for almost a decade. Interestingly, eventually the incorrect code was commented with the actual assembly it represents and even it shows the operations are incorrect (in the libretro copies). The original author of those comments probably didn't realize the implications at the time.

This issue was found while investigating a crash bug in scummvm.

I've determined that the following scummvm games are affected by this issue and either crash or have severe display issues as result of the fault:

• Beneathe a Steel Sky
• The Bizarre Adventures of Woodruff and the Schnibble
• Bud Tucker in Double Trouble
• Gobliins 2
• Goblins 3 / Goblins Quest 3
• Legend of Kyrandia
• Legend of Kyrandia 3
• Might and Magic IV: Clouds of Xeen
• Might and Magic V: Darkside of Xeen
• Might and Magic IV & V: World of Xeen
• Might and Magic: Swords of Xeen
• Toonstruck

The following scummvm games are most likely affected by this issue but I am unable to confirm:

• Drascula - The Vampire Strikes Back
• Leisure Suit Larry 7 - Love for Sail
• The Neverhood Chronicles
• Phantasmagoria 2
• Starship Titanic
• Touche
• Tony Tough and the Night of the Roasted Moths

Here is a list of related open issue reports that have a high probability of having been caused by this bug:

• https://github.com/libretro/scummvm/issues/82
• https://github.com/libretro/scummvm/issues/95
• https://github.com/libretro/scummvm/issues/96
• https://github.com/libretro/scummvm/issues/152
• https://github.com/libretro/scummvm/issues/158
• https://github.com/libretro/scummvm/issues/168
• RetroArch causes malfunction in some games running ScummVM core RetroArch#11763
• https://forums.libretro.com/t/scummvm-problem/22828
• https://forums.libretro.com/t/scummvm-core-crashes-with-phantasmagoria-2/27107

The following is a list of ALL cores that currently contain the faulty code in the main branch. I am unable to determine if this causes fault behavior (in practice) on any of the cores outside of scummvm:
- 3dengine

• bnes
  - boom3
  - bsnes2014
  - bsnes-cplusplus98
  - bsnes-mercury
  - desmume
  - desmume2015
  - dosbox
  - fbalpha
  - flycast
• fsuae
  - mame2000
  - mame2016
• mpv
  - beetle-bsnes
  - melonds
  - parallel-n64
• parallext
  - reminiscence
  - scummvm

The following is a list of additional subrepositories within the libretro organization that currently contain the faulty code in the main branch:

• ChaiLove's subrepos:
     - opentyrian test
     - sdl-tetris test

The following is a list of 3rd party libretro repos that currently contain the faulty code in the main branch:

• scummvm-backend

[hizzlekizzle]

well damn, thanks for the excellent bug report!

[snaphat]

Couple of things I neglected to mention are as follows:

1. Fault behavior (in practice) will be dependent on the exact compiler used, version, compilation options, and core code itself. So some games may just work in scummvm on one version of retroarch or core and then break in another, etc.
2. -O2 compilation does not result in fault behavior because the xmm registers are not used to store pointers. This is why the debug build of retroarch does not exhibit the bug (as some of the reports indicate).
3. Declaring local variables as volatile works around the issue by ensuring that they are not saved into xmm registers.
4. Binaries compiled with -03 and -g exhibit the issue. Noting this due to a comment I saw in one of the linked threads regarding debugging this issue on release builds of Retroarch. Issues like these that only show up in 'release builds' can be debugged by removing the stripping, adding -g, and providing an executable to users.

[i30817]

Does this only affect windows and not linux?

[snaphat]

Does this only affect windows and not linux?

Linux uses the System V ABI which doesn't require callee-saving the XMM6-XMM15 registers so it shouldn't be affected by this issue specifically.

I reviewed the libco call-site code for both Windows 64-bit and System V 64-bit and didn't see any issues outside of this particular XMM issue. All other callee-save registers appear to be properly saved and restored in both the inline assembly and inline machine code for both ABIs.

For reference to the calling conventions, see Table 4 here.