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

ARM9MemoryBus::UpdateMemoryMap tries to access invalid memory #22

Closed
ghost opened this issue May 5, 2021 · 2 comments
Closed

ARM9MemoryBus::UpdateMemoryMap tries to access invalid memory #22

ghost opened this issue May 5, 2021 · 2 comments

Comments

@ghost
Copy link

ghost commented May 5, 2021

The sequence of events is:

  1. ARM9MemoryBus constructor
  2. UpdateMemoryMap(0, 0x100000000ULL);
  3. case 0x06: table[index] = VisitVRAMByAddress<GetUnsafePointerFunctor>(address);
  4. GetUnsafePointerFunctor
  5. GetUnsafePointer

At the first attempt to read an address above the 0x068A3FFF mark (VRAM 656KB limit) everything blows up.
Could fix it checking before calling VisitVRAMByAddress:

case 0x06: {
if ((address & 0xFFFFF) < 0xa4000) //Check
table[index] = VisitVRAMByAddress<GetUnsafePointerFunctor<u8>>(address);
break;
}

But for current lack of whole design complete understanding and fleroviux intentions, I'm just documenting here and not opening a pull request.

Tested on:

Platform: Win10
Compiler: Clang 11 under MSVCRT (MSVC can't handle the metaprogramming and won't build, at least in the current stable 14.29)
(logically should be an issue in other platforms, logically)

@fleroviux
Copy link
Owner

Can you confirm that this is fixed as of the latest commit 094361c?

The problem was in the vram.hpp file, it defined LCDC VRAM as a region with 41 pages but mask is 63.
This correctly models the mirroring behavior of LCDC VRAM but the Region class template didn't account for the mask >= number of pages case. To avoid a branch I just made the LCDC region 64 pages large, which is still correct, just slightly more wasteful.

@ghost
Copy link
Author

ghost commented May 6, 2021

I knew you would come with a better solution ヾ(≧▽≦*)o
Merged upstream and now works without a flaw! Now it can compile and run without problems under clang+Visual Studio on Windows.

@ghost ghost closed this as completed May 6, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant