libretro / gpsp

gpSP for libretro.
GNU General Public License v2.0
51 stars 51 forks source link

64-bit integer division causes games to crash #225

Closed exelotl closed 6 months ago

exelotl commented 9 months ago

Whenever a 64-bit integer division occurs (signed or unsigned), my game hangs.

I guess the underlying cause isn't specific to division, since division is done in software on the GBA. Maybe there's some instruction within the 64-bit division routine in libgcc that's not emulated properly?

Here's an example:

// Based on obj_demo.c by cearn

#include <string.h>
#include <tonc.h>
#include "metr.h"

OBJ_ATTR obj_buffer[128];

void obj_test() {

  uint x = 96, y = 32;
  u32 tid = 0, pb = 0;      // tile id, pal-bank

  OBJ_ATTR *metr = &obj_buffer[0];
  obj_set_attr(metr, ATTR0_SQUARE, ATTR1_SIZE_64, ATTR2_PALBANK(pb) | tid);

  while (1) {
    vid_vsync();
    key_poll();

    // You can move with the arrow keys:
    x += 2 * key_tri_horz();
    y += 2 * key_tri_vert();

    if (key_hit(KEY_START)) {

      // Pressing START should *scale* the X coordinate by a small amount.
      // But on gpSP it locks up the game instead.

      x = (x * ((u64) 61000)) / ((u64) 59737);
    }

    metr->attr2 = ATTR2_BUILD(tid, pb, 0);
    obj_set_pos(metr, x, y);

    oam_copy(oam_mem, obj_buffer, 1);  // Copy into real OAM.
  }
}

int main() {
  memcpy(&tile_mem[4][0], metrTiles, metrTilesLen);
  memcpy(pal_obj_mem, metrPal, metrPalLen);

  oam_init(obj_buffer, 128);
  REG_DISPCNT = DCNT_OBJ | DCNT_OBJ_1D;

  obj_test();

  return 0;
}

Here's the project (build files included, can be recompiled with devkitARM): gpsp_bug.zip

davidgfnet commented 9 months ago

Hey there, just adding this to not forget about what the issue is. The issue is with the STM instructions. In particular, the divmod code (likely handwritten asm) for the armv4 uses a "push {sp, lr}" instruction, which in gpsp is incorrectly emulated. The new SP value is written to memory, as opposed to the old value. According to GBATEK and other emus, there's a few corner cases for this instruction which we do not handle. Thanks for the report. This is a high quality report and helps a lot :)

davidgfnet commented 7 months ago

Allright! I fixed this issue on the interpreter (in my fork) and seems to properly run the attached demo. I will rewrite some of the dynarec code to bring the fix to all platforms. Thanks again for the report! It seems many homebrew run into this due to some library code that devkitpro's toolchain ships.

davidgfnet commented 7 months ago

Added a few fixes. Doesnt fix the issue a 100% yet (only ARM mode for now), and the dynarec doesn't correctly emulate the CPU behavriou, but it behaves like mGBA does, so it should work. Lemme know if it still fails in some other funny way :) I will keep this open until I fix the other instructions and some other funny corner cases. Thanks!

davidgfnet commented 6 months ago

Not all LDM/STM instructions have been fixed, however I wrote a few tests to track this (at https://github.com/davidgfnet/test-rom-suite/). I'm gonna close this since the reported issue is indeed fixed, and perhaps file an issue to ensure we fix other corner cases. Thanks for the report! Fixed a few other homebrews