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

Panic if game does an invalid memory access. #991

Merged
merged 2 commits into from
Oct 21, 2014

Conversation

phire
Copy link
Member

@phire phire commented Sep 6, 2014

This will make it easier to track down bugs. Previously non-mmu games would fall through to the mmu code path which would kindly hide the error.

{
PanicAlertT("Invalid Read at 0x%08x, PC = 0x%08x ", em_address, PC);

}
}

This comment was marked as off-topic.

Previously, if a gamecube game wrote to an EXRAM address, dolphin would
segfault.
Previously it would fall through to the mmu code path, and raise a dsi
exception, which it would never check for, so it would continue
executing code silently.
@phire
Copy link
Member Author

phire commented Sep 6, 2014

Those issues are fixed.

@lioncash
Copy link
Member

lioncash commented Sep 6, 2014

Looks good to me

@@ -135,6 +136,10 @@ inline void ReadFromHardware(T &_var, const u32 em_address, const u32 effective_
_var = bswap((*(const T*)&m_pRAM[tlb_addr & RAM_MASK]));
}
}
else
{
PanicAlertT("Invalid Read at 0x%08x, PC = 0x%08x ", em_address, PC);

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

@ghost
Copy link

ghost commented Sep 13, 2014

Why is the mmu path hiding errors? Sounds like that's what needs to be fixed rather than panicking. Throwing a DSI/ISI might be expected behaviour ala Clone Wars.

@phire
Copy link
Member Author

phire commented Sep 13, 2014

DSI are signaled by setting an exception flag in PPCState.

In order to actually call the exception handler, the jit has to add a cmp/branch after every memory load/store to check if the exception flag has been set, and then generate exception exit code to clean up all the registers and jump to the dispatcher.

This gets very expensive, so it is currently only implemented when you check the enable mmu box. Most games never implement a meaningful DSI handler, so a DSI is game over, stack trace.

@phire
Copy link
Member Author

phire commented Sep 13, 2014

Besides, isn't Clone Wars an MMU game.

comex added a commit that referenced this pull request Oct 21, 2014
Panic if game does an invalid memory access.
@comex comex merged commit 24e72cd into dolphin-emu:master Oct 21, 2014
@Linktothepast
Copy link
Contributor

Now mmu is needed with fzero, plus single core for stability, couldn't get any worse really for that game.

@JMC47
Copy link
Contributor

JMC47 commented Oct 21, 2014

This kind of goes for what @tueidj said, that F-Zero GX should need full MMU.

@Linktothepast
Copy link
Contributor

It still needs single core for stability and a projection hack for the videos... It is messed up in master...

@JMC47
Copy link
Contributor

JMC47 commented Oct 21, 2014

Oh yeah, the Sonic Unleashed Hack needs to be fixed in both backends, and @comex still broke syncGPU. But, we should /try/ to get this working with MMU Speedhack again. If it was possible before, it should be more possible now if what I'm thinking is true.

@ghost
Copy link

ghost commented Oct 21, 2014

Probably want to also recheck https://code.google.com/p/dolphin-emu/issues/detail?id=7298 against this commit.

@phire phire deleted the dsi_should_crash branch July 4, 2015 12:55
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

8 participants