MacOS support

[campital]

These changes build off of my previous PR so it might be better to just accept this one if you like the changes.

The majority of the additions were in the MacDolphinProcess files, where the memory scanning functionality for the Mach kernel was implemented. There were some major performance issues at first, mainly because vm_read on Mac is much slower than the equivalent APIs on Windows and Linux (and because I used a 2013 MacBook Air). 500mb of data were being allocated, deallocated, and read from Dolphin each second for the cache, so I removed the cache and it sped up considerably. Not sure if this is a problem, but it also seems to perform better on Linux. It still eats up a bit of CPU, but profiling shows that no function in particular is to blame.

The README has been updated on how to build and run for MacOS.

[aldelaro5]

First sorry for responding so late, but I have been busy with other projects and only come back to handle some stuff for a week on this one.

I don't know if you still want to work on this (if not, I might have to pull your pr and coauthor), but there are couple of major issues preventing me to merge this. Mainly, I can't have the encoding features AND the mac os support in the same PR, you would need to split this pr to ONLY mac os and if needed, update your encoding pr.

The other issue is the cache because I am p sure the reason I ended up HAVING to do it is due to the viewer since the view should only change what parts of the ram is shown while it's already read. I honestly don't like it, but unless I can be convinced it doesn't cause issues with the viewer I can't exactly make this go forward (there's also the fact you seemed to run rather old hardware at the time of the PR while dolphin already expects you to have p good hardware). So I would need to be more convinced that removing the cache is needed.

I also see other issues like some files don't have a line at the end and for some reasons, you seemed to have changed the csproj more than it should be needed.

Do you still want to work on this? As I said, if it's not possible, I could always pull your branch and coauthor the PR to a mergable state myself, but it's up to you.

[campital]

Thank you for taking a look at my PR.

If I remember correctly, the CPU usage with DME open (and idle) was around 90% on macOS. Most of this time was spent on reading process memory for the cache, so it needed to be removed. The performance hit may be less pronounced on newer systems, but I think the cache probably needs to be made more performant anyways (I removed it completely, thinking that the cache was only an optimization that turned detrimental). I can look into the code again later to see if removing the cache had any unwanted side effects, and if it did, I will fix that.

Also, how do you recommend me to split the PRs? I can fork master again to a separate branch and re-commit the changes to implement macOS support (minus the Unicode support). In that case, I would delete this PR. Does that work?

I will also review the formatting and revert .csproj in both PRs, if that works for you.

[aldelaro5]

If I remember correctly, the CPU usage with DME open (and idle) was around 90% on macOS. Most of this time was spent on reading process memory for the cache, so it needed to be removed. The performance hit may be less pronounced on newer systems, but I think the cache probably needs to be made more performant anyways (I removed it completely, thinking that the cache was only an optimization that turned detrimental). I can look into the code again later to see if removing the cache had any unwanted side effects, and if it did, I will fix that.

I know my memories are very vague, but ik I eventually had to put the cache at least for the viewer to be happy so this is why I am hesitant. If however, there's enough reasons for me to know it could be removed, sure, but I am p sure that the reason I ended up doing it is because it wasn't feasible to read the block of ram needed for the viewer everytime you would scroll. It was WAY more performant to do one big read every once in a while vs many small reads. This is also why I put the timer settings so you can make it update slowly.

So yeah, I am very unsure here.

Also, how do you recommend me to split the PRs? I can fork master again to a separate branch and re-commit the changes to implement macOS support (minus the Unicode support). In that case, I would delete this PR. Does that work?

You can just force push cause you already have a PR opened about string encoding so you could just force push that one with your encoding stuff if needed. For this PR, same thing, you can just force push (and you may as well squash the commits cause it can make the history cleaner).

Point is, you don't NEED to close any PRs for now.

I will also review the formatting and revert .csproj in both PRs, if that works for you.

Yeah, I would have pointed these out in a deeper reivew, but since this had organizational issues, I only glossed the changes and saw these.

Btw I need to ask because I am curious: I tried years ago to make this work on a mac vm and it refused to have the perm to even read a remote process and from what I gathered, you needed to sign the thing, but to do that, the only way I found was to buy a certificate which cost an annual fee. How did you got it to work without doing this?

[campital]

Sorry for the delay!

I have measured the performance of removing the cache on Linux, Windows, and macOS and there seems to be little overall change other than the speedup on macOS (and a slight improvement on Linux). There is a small increase in CPU usage, though, when scrolling in MemViewer (on Windows mostly), but it is not critical.

For the macOS permissions, the executable needs to be signed with a code signing certificate and it needs to be given debugger entitlements (found in Source/entitlements.xml). Currently, I have updated the README with instructions to do this (and to create a free self-signed code certificate). It is also possible to create a code signing certificate for distribution, but those are usually expensive and need to be purchased from a CA.

Also, I have just separated the PRs, so this PR now only contains changes to implement macOS support. I also tried to address the formatting in both branches, along with fixing a few remaining problems in this PR.

How does everything look at the moment? Let me know if there is anything else I should modify.

[Hibyehello]

So I got a build error with this PR, that happened to occur in Common/CommonUtils.h, but I added another #Elif for mac that uses linux's elif template. I had to add #import <libkern/OSByteOrder.h>, under another #elif for apple, and before the return bSwap16, etc I added #define bswap_16(x) _OSSwapInt16(x) using functions from the import. The Error I ended up getting was a segmentation fault on MacOS Catalina. If anyone wants I could add my edits in a txt to here

[campital]

Thank you for your report, @Hibyehello. Are you certain you are compiling the correct branch? I already added byte-swap macros in CommonUtils.h guarded with #elif __APPLE__. I don't see any reason for this to not work. In the end, though, were you able to compile? Where did the segmentation fault occur? Did you sign the executable with entitlements?

If you think you are doing everything correctly (as outlined in the new README) and there are still issues, let me know.

[Hibyehello]

Well from I had gotten there was not any Apple specific code in commonutils.h, but let me look, and I will keep testing

[Hibyehello]

What I had pulled didn't have the code but I see the code you have began adding
edit: I forgot to switch branches

[Hibyehello]

So after building I was unable to hook it to dolphin after doing all the certificate stuff, it said it saw the process, but couldn't see that the game was running.

[Hibyehello]

This is what happens, and I have signed the app. It just won't hook to dolphin

[campital]

Not sure where the issue is occurring, but I suspect the code may not be reliably finding the emulator memory region (MacDolphinProcess.cpp). If you want, try running vmmap Dolphin | grep 32.0M before and after starting emulation and post the output here.

Also, what version of Dolphin are you using? It seems to work for me on 5.0-14095.

[Hibyehello]

Before emulation starts: MALLOC_SMALL 00007f902b800000-00007f902d800000 [ 32.0M 7768K 7768K 0K] rw-/rwx SM=PRV MallocHelperZone_0x10c916000
After emulation starts: shared memory 000000012a959000-000000012c959000 [ 32.0M 32.0M 32.0M 0K] rw-/rw- SM=SHM
shared memory 00000001424e7000-00000001444e7000 [ 32.0M 32.0M 32.0M 0K] rw-/rw- SM=SHM
MALLOC_SMALL 00007f902b800000-00007f902d800000 [ 32.0M 14.7M 14.7M 0K] rw-/rwx SM=PRV MallocHelperZone_0x10c916000

Using Dolphin version 5.0-11653

[campital]

Not sure then. Are you certain the executable has the proper entitlements? Try codesign -d --entitlements :- ./Dolphin-memory-engine and look for <key>com.apple.security.cs.debugger</key> in the output.

[Hibyehello]

I get <key>com.apple.security.cs.debugger</key> as the output

[Hibyehello]

Could this be an issue with my version of mac os, since I am running Mac Os 10.12 (Sierra)

[campital]

I recently got a 2020 MacBook Air on macOS 11.3 and was able to test my changes. Initially, DME was unable to access Dolphin's memory because it seems macOS 11 requires processes to have a certain entitlement (com.apple.security.get-task-allow) to be debugged. However, removing Dolphin's preexisting signature and applying a new self-signed one with the extra entitlement solves this problem. To make this easier, I created the MacSetup.sh script which automatically signs DME's executable and Dolphin's executable, provided a self-signed code certificate.

@Hibyehello, perhaps using the new MacSetup.sh script to sign Dolphin.app will fix your issue. I'm not sure how older versions of macOS handle debugging access.

[Hibyehello]

I'll test this when I get time, and report back.

[Hibyehello]

Sorry I've been busy, but I tried this today and I got

/var/folders/20/rcn061sj6tx02r098cj6b8600000gt/T/tmp.0iVfJViQ: unrecognized blob type (accepting blindly)
/var/folders/20/rcn061sj6tx02r098cj6b8600000gt/T/tmp.0iVfJViQ: invalid length in entitlement blob
Dolphin could not be re-signed!

so DME won't hook

[LapisLazulis]

Seems like it's broken on newer Dolphin versions; at least for me on an M1 MBA on Monterey; this worked before but somehow stopped working.

[campital]

@LapisLazulis, what exactly doesn't work? Everything seems to be fine for me on macOS 12.4, Dolphin 5.0-16380 (most recent beta), and a 2020 M1 MBA.

Also, have you run MacSetup.sh (in Source)? There are instructions in the README for creating a code-signing certificate and running this script. More recent versions of macOS require processes (Dolphin) to be signed with a specific entitlement to be debugged. Dolphin must also be re-signed using this script whenever a new update is installed or the executable is overwritten. Let me know if this doesn't resolve the issue.

[dreamsyntax]

Hi @campital, can you please rebase if this is still something you would like in DME?
I have a Mac and will be vetting this for possible release in the next release.

[dreamsyntax]

After looking over the code, I don't think it will be a big deal to revisit and rebase after the Qt 6 merge.

[dreamsyntax]

This will not be present in the final Qt 5 release.
Additionally, I will not be looking into this unless campital returns and/or there is interest in a mac version.

[dreamsyntax]

@campital please mark as ready for review when you are done

[campital]

I have copied over the build.yml from the other PR, so this should be ready to merge if everything else looks correct.

[dreamsyntax]

I have copied over the build.yml from the other PR, so this should be ready to merge if everything else looks correct.

Ci:

Invalid workflow file: .github/workflows/build.yml#L106
You have an error in your yaml syntax on line 106

Also, the other PR broke windows (probably because removed the bash line for some reason).

[campital]

Whoops. I believe it has been fixed.

[dreamsyntax]

Can you please confirm:

• Following the steps, it works as expected on Intel Mac
• It really needs Qt5 for mac? Or is that a typo and it should be Qt6 for mac?
• If using this on ARM (Silicon) through Rosetta, do you also need to run Dolphin in Rosetta?

[RainbowTabitha]

Can you please confirm:

• Following the steps, it works as expected on Intel Mac

I dont have one to test

• It really needs Qt5 for mac? Or is that a typo and it should be Qt6 for mac?

No thats not true, Qt6 should be fine in theory.

• If using this on ARM (Silicon) through Rosetta, do you also need to run Dolphin in Rosetta?

Thats a really good question

I can answer some of these

[RainbowTabitha]

I would also recomend modifying the CMake file to be similar to the other PR #96, so it bundled an .app bundle instead of just an exectuable

[RainbowTabitha]

@campital do you have Discord, would like to share some info about this PR with you.

[campital]

Everything works on Intel Mac as far as I can tell (tested on MBA 2018). Rosetta does not seem to be a problem (tested on all 4 configurations).

I also added a build system for packaging a standalone application. However, there is currently a bug in the Homebrew package for Qt that prevents the macdeployqt tool from running properly (https://github.com/orgs/Homebrew/discussions/2823), which makes it difficult to package the required libraries in the app bundle. I tried multiple workarounds to no avail. As a temporary solution, I have created a submodule with compiled universal binaries for qtbase. To create an app package with the proper frameworks, you need to first run cmake with -DCMAKE_PREFIX_PATH="/path/to/Externals/Qt-macOS" and then Tools/MacDeploy.sh. The build process with Homebrew Qt still works but does not produce a standalone package as before. This isn't a particularly clean solution, but I'm not aware of any working alternatives.

I can provide README instructions for building the standalone package but not sure if it's appropriate since the previous build system works for most cases.

[dreamsyntax]

Any update here?

I'll be testing on Mac shortly and just merging if things appear to work fine. If more changes need to be done let me know or open a new PR.

[campital]

Should be ready to merge. @EndangeredNayla was experiencing a rare crash bug (https://bugreports.qt.io/browse/QTBUG-119513) but I was unable to reproduce and should be fixed in the current version of Qt.

[dreamsyntax]

In that case, yolo merge time.