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

[Gensplus-gx] stack pointer overflow causes Bizhawk to crash #1760

Closed
ReaperMan02 opened this issue Dec 15, 2019 · 14 comments
Closed

[Gensplus-gx] stack pointer overflow causes Bizhawk to crash #1760

ReaperMan02 opened this issue Dec 15, 2019 · 14 comments
Labels
Core: Genplus-gx Sega Genesis / Mega Drive core Homebrew Affects homebrew/romhack/bootleg Working as intended (wontfix) Intended behaviour perceived as a bug, or an unreasonable feature request

Comments

@ReaperMan02
Copy link

So, when researching on how the Genesis handles various exceptions, I managed to overflow the stack pointer from FFFF FFFC to what I would guess would be 0000 0002, which causes Bizhawk to terminate without any error messages

Bug is confirmed as of Commit #b6fa9c009

Easiest way to do this is as follows:

  1. Open up the Genesis ROM Battletech (or any other rom with a similar initial stack pointer)
  2. edit the first instruction (in the case of Battletech, located at $200, like most games) to RTE (#$4E73)
  3. Open it in Bizhawk
  4. Goodbye Bizhawk

That's about it, really. Just hits an overflow and poof, gone. Hopefully it'll be a trivial fix. Adjusting the stack pointer to a lower value was the workaround that I used, so it had nothing to do with executing an RTE while not in an exception.

@YoshiRulz YoshiRulz added App: EmuHawk Relating to EmuHawk frontend Core: Genplus-gx Sega Genesis / Mega Drive core Homebrew Affects homebrew/romhack/bootleg labels Dec 15, 2019
@vadosnaprimer
Copy link
Contributor

It's unrelated to these?
96ac5d1
f883065
2978775
c96ff28

@ReaperMan02
Copy link
Author

I don't believe so. This deals specifically with register A7, which is a 32 bit unsigned integer overflow. Oddly, adding to the register directly doesn't trigger it, so it only seems to happen with an RTE with a stack pointer set to around FFFF FFFC, so it might also just be related to the RTE instruction.

@nattthebear
Copy link
Contributor

Can someone provide a legal to distribute rom which exhibits this?

@nattthebear nattthebear removed the App: EmuHawk Relating to EmuHawk frontend label May 30, 2020
@nattthebear
Copy link
Contributor

The m68k core makes an assumption that the stack is in writable address space, and bombs out when it isn't. From previous experiments, we know that adding any sort of check here is quite expensive. In GPGX upstream, this trick will actually overwrite the "ROM" data in memory. Either way, the situation isn't recoverable and it's not worth a speed loss on all roms to crash differently.

@nattthebear
Copy link
Contributor

For future reference, the crash is in INLINE void m68ki_push_32(uint value) when the old SP is 2, so the new SP is 0xFFFFFFFE

@nattthebear nattthebear added the Working as intended (wontfix) Intended behaviour perceived as a bug, or an unreasonable feature request label May 31, 2020
@ReaperMan02
Copy link
Author

While the speed concerns are warranted, wouldn't it make sense to at the very least catch the exception and recover so that it doesn't shit itself and take bizhawk down with it? There shouldn't be much of a speed penalty if done that way, unless I'm mistaken.

@zeromus
Copy link
Contributor

zeromus commented May 31, 2020

other things could have been stomped to destroy the process before one of the random things that we could theoretically catch an exception on actually get hit. practically nobody uses "memory stomped on" exceptions, because they're practically useless for that reason. we're not going to begin warranting that unmanaged cores are resilient against your corrupting.

@nattthebear
Copy link
Contributor

It is difficult to actually recover from SIGSEGV in any meaningful way, because it means a bunch of custom native code with OS interface to obtain the signal and react appropriately. Plus, there are problems with .NET's stack unwinder interacting with Waterbox that I haven't solved. All of this may happen some day, as JIT cores need "fastmem" type capabilities, but it won't be for the benefit of GPGX.

@ReaperMan02
Copy link
Author

Alright I suppose, fair enough. Just sucks that there's a core in bizhawk that's prone to crashing under certain circumstances.

@zeromus
Copy link
Contributor

zeromus commented May 31, 2020

IMO, we should post an upstream bug. it's probably possible for a game to do this intentionally and then recover as a way of making GPGX malfunction. anything we can write a hypothetical scenario for "emulator detection", they ought to fix. I don't se what's so hard about fixing this one; writes to rom address space need to be blocked on principle. the performance hit shouldnt really enter consideration. games can do this kind of wacky stuff all on their own and it needs to be protected.

@nattthebear
Copy link
Contributor

If anyone wants to post an upstream bug, do test it out against upstream first; it's possible they have a later fix that we do not.

Bizhawk is already deterministically blocking the write, it just can't recover from that blocked write.

@getCursorsExe
Copy link

I replicated the crash and I see a loading icon on my mouse cursor even though nothing is loading.

@zeromus
Copy link
Contributor

zeromus commented Apr 30, 2021

Have you ever used a computer before? Windows does this when a process is hung, moments away from crashing

@getCursorsExe
Copy link

We only get "Access violation" close message in Visual Studio. No deatils, no location, just this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core: Genplus-gx Sega Genesis / Mega Drive core Homebrew Affects homebrew/romhack/bootleg Working as intended (wontfix) Intended behaviour perceived as a bug, or an unreasonable feature request
Projects
None yet
Development

No branches or pull requests

6 participants