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

Check for valid instruction info before attempting to read from it. #997

Closed
wants to merge 1 commit into from

Conversation

skidau
Copy link
Contributor

@skidau skidau commented Sep 6, 2014

Check for valid instruction info before attempting to read from it. Fixes the crash that occurred on boot.

@degasus
Copy link
Member

degasus commented Sep 6, 2014

What's about line 39?

@skidau
Copy link
Contributor Author

skidau commented Sep 6, 2014

It is valid for info to be nullptr on line 39.

@delroth
Copy link
Member

delroth commented Sep 6, 2014

What do you mean by "valid"? It will segfault Dolphin.

@BhaaLseN
Copy link
Member

BhaaLseN commented Sep 6, 2014

If the tables are incomplete, yes. The change is that the subop switch will change the info-reference to something else, which might in turn be null then.

@delroth
Copy link
Member

delroth commented Sep 6, 2014

But why do we have null in the subops tables instead of OPTYPE_INVALID entries?

@BhaaLseN
Copy link
Member

BhaaLseN commented Sep 6, 2014

Dunno, maybe because its far too many to type? That table has been around for like, forever.
Short-term it fixes pretty much every game crashing, as far as I got that from the forums; so I'd possibly go with that (master shouldn't be broken for too long, if we can avoid it). Unless skid wants to do the grinding work of adding all the entries.

@comex
Copy link
Contributor

comex commented Sep 6, 2014

The change should just be reverted. GetOpInfo is expected to return null for invalid ops in multiple places, including IsValidInstruction; indeed, this suggests that the preexisting assert message is broken. Any fix for the segfaults needs to take this into account.

edit: Are there even any OPTYPE_INVALID instructions?

@shuffle2 shuffle2 closed this Sep 6, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
6 participants