-
Notifications
You must be signed in to change notification settings - Fork 513
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
Add Apple silicon slice to Mono runtime built for macOS #1309
Conversation
Signed-off-by: Tautvydas Žilys <[email protected]>
Signed-off-by: Tautvydas Žilys <[email protected]>
Signed-off-by: Tautvydas Žilys <[email protected]>
…undefined on darwin but was added recently which leads to unexpected behavior Signed-off-by: Tautvydas Žilys <[email protected]>
6661111
to
3ddb80f
Compare
@@ -57,7 +57,7 @@ static MonoCodeManagerCallbacks code_manager_callbacks; | |||
#define MAX_WASTAGE 32 | |||
#define MIN_BSIZE 32 | |||
|
|||
#ifdef __x86_64__ | |||
#if defined(__x86_64__) && !defined(__APPLE__) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW we fixed this a bit differently upstream: dotnet/runtime@cc501c2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Looks like our fork code is quite a bit different. We'll get rid of this change when we pull upstream.
Signed-off-by: Tautvydas Žilys <[email protected]>
Signed-off-by: Tautvydas Žilys <[email protected]>
3ddb80f
to
bd8d105
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
@@ -2804,6 +2806,7 @@ mono_jit_runtime_invoke (MonoMethod *method, void *obj, void **params, MonoObjec | |||
if (!is_ok (error)) | |||
return NULL; | |||
} else { | |||
MONO_SCOPE_ENABLE_JIT_EXEC(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems odd this is needed in mono_jit_runtime_invoke
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without this, JITted code won't be marked executable and you'll get EXC_BAD_ACCESS when trying to jump to it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't we mark it as executable after JIT'ing instead of before executing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Executable mode is thread local state. That means changing the mode affects it for all code that runs on current thread. If we were to switch to executable mode right after JIT, we might mess up the state of something else running on the same thread (for instance, what if somebody embeds CEF?). Therefore we scope it: just before needing certain mode, we switch to it. And then after, we switch back to whatever it was.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Main concern is the presence of WRITE/EXEC macros outside of arch specific code and in places where writing seems to not occur
I don't really know how to abstract those macros out of generic code. Basically we need to change to "writable" mode just before writing to executable memory, and "execute mode" before jumping to it. I will go through all of the places to make sure they're necessary, though. |
d101dd1
to
e72e1a8
Compare
e72e1a8
to
db492f6
Compare
The Mono runtime changes were implemented & tested by Apple on Apple silicon hardware. I implemented the changes in our build scripts.
The biggest change in here is how JIT operates. On Apple silicon Macs, you're not allowed to both execute and write to write/execute pages at the same time. The way I understand it is that each thread has a state whether it can write or execute instructions in these pages. It is controlled using
pthread_jit_write_protect_np
function. Before the JIT can write to those pages, it needs to change the thread state to "non-protected". Before executing the JITed code, the thread state must be changed to "write-protected". This is handled using the new macros inmono/mini.h
. They were implemented inline for performance reasons.Other changes include adding ARM64 target to configure script and fixing build on the new SDK.
Please note that CI might fail - up until today I couldn't push the changes and could only run stuff locally (where it worked). I'm working on ironing out all the issues as we speak.
EDIT: CI is now green and we're producing all macOS binaries properly: