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

Unstable E7 mapper debuging #1017

Open
Saycrain opened this issue Feb 20, 2024 · 16 comments
Open

Unstable E7 mapper debuging #1017

Saycrain opened this issue Feb 20, 2024 · 16 comments
Assignees
Labels

Comments

@Saycrain
Copy link

Saycrain commented Feb 20, 2024

When debugging an E7 mapper game, there's a high chance that the emulator will crash. it happens on boot, randomly when entering the debugger after boot, when banks change before entering the debugger and other cases aswell.

there's a demo program that can be tested with from Atariage, for both ntsc and pal.

the demo will run fine on the emulator as long you don't try and touch the debugger at all.

Running the emulator on Linux Mint 21.3 Virginia, Kernel: 5.15.0-94-generic x86_64 bits

@thrust26 thrust26 self-assigned this Feb 20, 2024
@thrust26
Copy link
Member

Yes, the debugger has problems with E7, but at least for me it never crashes.

Which Stella version are you using?

@Saycrain
Copy link
Author

Saycrain commented Feb 20, 2024

I'm using 6.7.1 now after I saw the problem in an earlier version. but it is doing the same still.

and when I said "crashes on boot" I mend when the debugger is set to start up when the emulator loads up the rom.

@thrust26
Copy link
Member

@sa666666 Can you reproduce the problem under Linux?

@sa666666
Copy link
Member

I don't get crashes in Linux. But I notice in 6.7.1 that once the display gets near the bottom and starts drawing the red control area by tracing scanlines, the disassembly doesn't show up anymore. This doesn't happen in the most recent code; it shows the disassembly for the entire frame.

So something has changed in the codebase since 6.7.1, and it may fix this issue. I guess we won't know until we release, or get the reporter to test it against 7.0_pre.

@sa666666
Copy link
Member

Starting in debug mode also shows weirdness in the disassembly, which is fine in the latest code. So there was definitely some new code that affected this issue.

@Saycrain
Copy link
Author

well. I'll gladly look into it more if the next release make it more stable where I can test things out. for at the moment it's unusable on my end with that mapper for development.

@Saycrain
Copy link
Author

I went ahead and took a look at what's going on when it crashes through the output to the terminal, with this being what it printed out from launch of the emulator to the crash when I enter the debugger.

image

@DirtyHairy
Copy link
Member

Opening the debugger (current git) on the elite demo triggers the UBSAN trace below, although do not get any crash. Thiis seems consistent with the screenshot above:

/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/array:219:16: runtime error: index 48 out of bounds for type 'bool[32]'
    #0 0x100b7a710 in std::__1::array<bool, 32ul>::operator[][abi:v160006](unsigned long) array:219
    #1 0x100c6fcd4 in DiStella::DiStella(CartDebug const&, std::__1::vector<CartDebug::DisassemblyTag, std::__1::allocator<CartDebug::DisassemblyTag>>&, CartDebug::BankInfo&, DiStella::Settings const&, std::__1::array<unsigned short, 4096ul>&, std::__1::array<unsigned short, 4096ul>&, CartDebug::ReservedEquates&) DiStella.cxx:92
    #2 0x100c83340 in DiStella::DiStella(CartDebug const&, std::__1::vector<CartDebug::DisassemblyTag, std::__1::allocator<CartDebug::DisassemblyTag>>&, CartDebug::BankInfo&, DiStella::Settings const&, std::__1::array<unsigned short, 4096ul>&, std::__1::array<unsigned short, 4096ul>&, CartDebug::ReservedEquates&) DiStella.cxx:36
    #3 0x100b85788 in CartDebug::disassemble(int, unsigned short, CartDebug::Disassembly&, std::__1::map<unsigned short, int, std::__1::less<unsigned short>, std::__1::allocator<std::__1::pair<unsigned short const, int>>>&, bool) CartDebug.cxx:355
    #4 0x100b820bc in CartDebug::disassembleAddr(unsigned short, bool) CartDebug.cxx:269
    #5 0x100b886ac in CartDebug::disassemblePC(bool) CartDebug.cxx:296
    #6 0x100888904 in TabWidget::updateActiveTab() TabWidget.cxx:140
    #7 0x100210460 in Dialog::open() Dialog.cxx:126
    #8 0x1001e8cb0 in DialogContainer::reStack() DialogContainer.cxx:200
    #9 0x10241c2f8 in EventHandler::changeStateByEvent(Event::Type) EventHandler.cxx:1929
    #10 0x101ea862c in PhysicalKeyboardHandler::handleEvent(StellaKey, StellaMod, bool, bool) PKeyboardHandler.cxx:579
    #11 0x1023e88dc in EventHandler::poll(unsigned long long) EventHandler.cxx:269
    #12 0x102787618 in OSystem::mainLoop() OSystem.cxx:968
    #13 0x101dbf584 in main main.cxx:344

@DirtyHairy
Copy link
Member

DirtyHairy commented Mar 3, 2024

92a7137 fixes this. I am totally unfamiliar with the disassembler code though, so @sa666666 or @thrust26 please take a look, there may be more broken here.

@thrust26
Copy link
Member

thrust26 commented Mar 3, 2024

All the problems described in the issue are the result of using code which was meant for 4K ROMs only (DiStella) and trying to base Stella's disassembler on it. Also Stella's disassembler was originally aimed at simple 4K banking. E7 is most different here. The whole disassembler code base is quite messed up and needs a major rewrite.

The fix doesn't hurt, but it only cures another symptom.

thrust26 added a commit that referenced this issue Mar 3, 2024
@thrust26
Copy link
Member

thrust26 commented Mar 3, 2024

@DirtyHairy I have reverted your changes and tried another fix, which is closer to the original problem.

Can you check this one, please?

BTW: This describes all mirrors. As you can see, the RIOT mirrors are "rather complex".

@DirtyHairy
Copy link
Member

Yeah, that one works, too. I think the correct approach for the "mirrors" is not enumerating them in a switch, but to go through the address decoding sequence.

@DirtyHairy
Copy link
Member

@thrust26
Copy link
Member

thrust26 commented Mar 3, 2024

Like I said above, this is up for a major rewrite. Volunteers welcome! 😉

@sa666666
Copy link
Member

sa666666 commented Mar 4, 2024

While it's true that the disassembler is basically DiStella which was designed for 4K ROMs, it should still work. After all, the 2600 only sees 4K of address space at a time, so there's no reason why this wouldn't work. I don't think a major rewrite is in order.

@thrust26
Copy link
Member

thrust26 commented Mar 4, 2024

Yes and no. DiStella is not able to refer to addresses outside its current address range, because it only sees the currently disassembled bank (which can be of any size up to 4K). Therefore this information has to be provided by our own disassembler class. And there it gets complicated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants