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

SymbolDB: Multiple symbols detection allowed #4299

Merged
merged 3 commits into from Jan 12, 2017

Conversation

sepalani
Copy link
Contributor

@sepalani sepalani commented Oct 4, 2016

This PR allows multiple functions with the same stripped name to be patched.

NB: This PR doesn't fix the misused array, check this PR instead.


This change is Reviewable

@BhaaLseN
Copy link
Member

BhaaLseN commented Oct 4, 2016

If you change that line anyways, why not stick to this one instead? Or rather, fix it anyways since its bound to cause a conflict either way.
And, this isn't really visible from the diff, but is the singular version still being used somewhere?

@sepalani
Copy link
Contributor Author

sepalani commented Oct 4, 2016

I decided to separate both PR because one of them is unrelated to this change (git is pretty convenient concerning conflict resolution so it's no big deal). Then I know that the other one shouldn't impact anything else.

I found one place where the singular version is used, but I'm not sure that the only one and if it matters. It's in the Debugger_SymbolMap.cpp file in the function AddAutoBreakpoints enclosed by preprocessor directive. The code seems just to automatically add a breakpoint where PPCHalt is located and I've never found this function more than once. Plus, its signature is present in the totaldb.dsy, so unless someone adds it manually to a map file it shouldn't appear more than once.

However, if you want me to get rid of the singular version I can replace it, anyway.

@sepalani
Copy link
Contributor Author

sepalani commented Oct 5, 2016

I figured out that games can have multiple times the same function, so I did the same for the GetSymbolsFromHash method. Should I get rid of the singular version ones?

@sepalani
Copy link
Contributor Author

I used a std::set in order to map a hash to multiple address/symbol. Now, multiple symbols are detected properly, but... As expected, some of them are just false-positive. Furthermore, the checksum system is pretty inefficient right now. Functions with different codes are detected as the same symbol, not really accurate indeed (i.e. lhz r0, 0x5000(r3) equals lhz r0, 0x5004(r3) according to the checksum system).

However, most "system" functions are detected accurately. And by the way, due to these recent changes in the OSReport/Logs system, a lot more debug messages are printed.

In sum, if you don't care about this function detection inaccuracy, this PR is ready to be reviewed & merged. Otherwise I'll close this one and make another WIP PR where the goal would be to rewrite those in order to be the more accurate possible.

@sepalani sepalani changed the title SymbolDB: GetSymbolsFromName added SymbolDB: Multiple symbols detection allowed Nov 3, 2016
@Parlane Parlane merged commit 5790f15 into dolphin-emu:master Jan 12, 2017
@sepalani sepalani deleted the hle_symbols branch January 17, 2017 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants