-
Notifications
You must be signed in to change notification settings - Fork 106
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
added Apple Silicon support #103
Conversation
Hello @alpencolt, Riding on your last comment on #102. I modified the CMake code to detect the platform as Unix ARM64 when it is on an M1 processor, and the See build log here >
I decided to test it with Vimspector on Neovim (I had earlier used this setup with the X86_64 architecture), but I keep hitting the same roadblock I had when using the Here is my Vimspector log from the run
Are there more changes I need to make in the code before I build? |
After some troubleshooting, it works but still not attaching breakpoints.
|
Looks strange, I see
In case of proper executuion, you should see something like:
make sure you see |
Hi @viewizard, I just tried again, and this time the symbol was loaded for my app, but it still wrote a couple of symbol not loaded for dependencies.
but it still does not hit any of the breakpoints I have in the code. |
Full log added
|
As I see in log, you have:
This mean, debugger found this point and successfully setup breakpoint, only 2 options here:
|
Thank you, @viewizard, Here is what I noticed by stepping through the logs
While I also understand that this M1 is not supported, I will like to work on making it possible. Thank you. |
Debugger part could be checked here: You could enable logging https://github.com/Samsung/netcoredbg#running-netcoredbg (note, this work with Debug build only!)
https://github.com/Samsung/netcoredbg/blob/master/src/debugger/managedcallback.cpp#L487 |
Thank you @viewizard. |
I've built and tested this on my M1 Macbook Pro and have debugging working successfully. I've not tested extensively but I can do all the following:
That's enough for my needs and probably 90% of cases so I'd say this is ready to merge. One caveat though. The part of the runtime used for building in the FWIW I think I found the issue and I'm just waiting for a response: |
Thank you @matthewblott for your detailed test and feedback. |
HI @matthewblott, Can you send me a link to the Discord channel where you had this discussion?
|
Hi @matthewblott, I am having difficulty testing this on my end. I have this branch I am using for testing, but it never stops on a breakpoint https://github.com/CodEaisy/TinySaas/pull/new/netcoredbg-test Here is my setup details:
Vimspector 5c328b5 Do you mind we have a short call at your leisure to resolve this? |
@codeprefect do you want to do a Zoom call? |
@codeprefect I did a quick tutorial on my blog of the steps needed if it helps: |
Note that .net core 3.1 has end of life at december 2022, MS would probably not take any patches to 3.1 branch. You can use .net 6.0 with debugger, just pass extra option to cmake. Clone runtime manually: git clone https://github.com/dotnet/runtime --branch release/6.0 <runtime_path> And then build netcoredbg with I think this will fix the problem you've mentioned in #103 (comment). |
@codeprefect For the Discord logs look for the coreclr section under DotNetEvolution (see screenshot below): |
@gbalykov completely agree, patching 3.1 isn't worth it. |
@codeprefect @matthewblott @gbalykov Great work - I was able to compile with .net 6.0 and get this working on my Mac Studio M1 Max. |
Hey @matthewblott, I have already read your blog—an excellent and concise article, by the way. |
Thanks, @gbalykov and @james-world. I have confirmed we no longer need to depend on 3.1. @matthewblott, I think updating your blog post to reflect recent learnings will be a good idea. Thank you all. |
I made some improvement to the build pipeline,
The build output is placed in the bin directory. |
@codeprefect TLDR: Yes it worked for me using neovim v0.8.0. Methodology:
Then took contents of bin folder and with them replace content of With this setup, was able to set and hit breakpoints and debug a .NET Core 7.0 application. I have the following arm64 .net core SDKs installed:
|
Thank you @james-world , Can you share your |
Sure, for a project created using {
"configurations": {
"debug": {
"adapter": "netcoredbg",
"configuration": {
"request": "launch",
"program": "${workspaceRoot}/bin/Debug/net7.0/HelloWorld.dll",
"args": [],
"stopAtEntry": true
}
}
}
} |
Tested successfully with a console app. Thanks, @james-world and everyone. |
I think I was wrong in thinking that DbgShim is the only native project that nees to be built. The very Maybe the only thing remaining is to include https://github.com/Samsung/netcoredbg/pull/103/files#diff-8d21a04f41ca438004f1b09c7b912e5813ab6f7ea79a86638dd8674298096d41
Not sure if it is needed though. I will test without it and the patch and report my findings. |
Looks like smth is wrong with your setup, I've just checked build with downloaded runtime and build works fine (no runtime is built, no dbgshim is built): only native part of netcoredbg is built and managed part of netcoredbg. Please clean the build dir and try again. Also note that you can pass |
FYI code on my PR for nix works without problems, it may be used as a reference which versions are used etc. I don't think familiarity with nix is required to understand what is used there. |
No, everything is fine. I was mistaken about DbgShim. Please disregard the whole DbgShim thing. I tried the patch but without the modifications to detectplatform.cmake it won't build. After applying it things build as expected. diff --git a/detectplatform.cmake b/detectplatform.cmake
index 7b93bbf..6fa6e9e 100644
--- a/detectplatform.cmake
+++ b/detectplatform.cmake
@@ -56,7 +56,11 @@ endif(CMAKE_SYSTEM_NAME STREQUAL Linux)
if(CMAKE_SYSTEM_NAME STREQUAL Darwin)
set(CLR_CMAKE_PLATFORM_UNIX 1)
- set(CLR_CMAKE_PLATFORM_UNIX_AMD64 1)
+ if(CMAKE_SYSTEM_PROCESSOR STREQUAL arm64)
+ set(CLR_CMAKE_PLATFORM_UNIX_ARM64 1)
+ else()
+ set(CLR_CMAKE_PLATFORM_UNIX_AMD64 1)
+ endif()
set(CLR_CMAKE_PLATFORM_DARWIN 1)
if(CMAKE_VERSION VERSION_LESS "3.4.0")
set(CMAKE_ASM_COMPILE_OBJECT "${CMAKE_C_COMPILER} <FLAGS> <DEFINES> -o <OBJECT> -c <SOURCE>") |
Hello @gbalykov |
@akorchev, I also encountered the failing tests with .NET 7, so I believe the error might be related to the removed code. |
After applying @gbalykov's patch from last week, it now builds with .NET 7, there are no new tests failing from my end.
|
I confirm that this PR builds just fine. Should be ready to merge :) Great job @codeprefect ! |
@@ -2,7 +2,7 @@ | |||
|
|||
<PropertyGroup> | |||
<OutputType>Library</OutputType> | |||
<TargetFramework>netcoreapp3.1</TargetFramework> |
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.
You need to revert all such changes, it depends on runtime that is going to be used to run tests and we use different runtimes. To handle this we simply use sed -i "s/netcoreapp${hardcoded_test_clr_version}/netcoreapp${test_clr_version}/g"
before launch of tests.
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.
Oh, I never saw this, let me do it straightaway.
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.
Done @gbalykov
Note that on .net 6 on x64 mac all tests pass. There are issues with .net 7 related to runtime, see my comment about this #103 (comment). |
Thank you @gbalykov, I believe they are closely related. |
What problems do you face? Build commands seem correct. |
Here are the errors from the build
You can find the complete log here |
@codeprefect you need to build with clang |
Thank you @gbalykov for the update. Linux x64 failing tests
All tests are also passing on Windows x64 |
- added arm64 discovery for M1 via CMAKE_SYSTEM_PROCESSOR
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.
We need to wait for sync with our internal repo, then you'll need to rebase. Also, as I see in your comment in #103 (comment) you have few test fails on .net 6 arm64 mac, but as I mentioned in #103 (comment) all tests pass on .net 6 x64 mac on our side, so these need to be investigated. Regarding .net 7, currently fails for it are expected and happening because of dotnet/runtime#82422 even on linux x64.
@@ -7,17 +7,21 @@ if (CLR_CMAKE_PLATFORM_ARCH_AMD64) | |||
add_definitions(-DAMD64) | |||
add_definitions(-DBIT64=1) # CoreClr <= 3.x | |||
add_definitions(-DHOST_64BIT=1) # CoreClr > 3.x | |||
add_definitions(-DHOST_AMD64) |
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.
This needs to be removed when we sync with our internal repo
I used this PR to build for my M1 Pro w/ 16 GB of ram and it has been working very well. |
I've merged this commit in our internal repo. Now master branch is synced and new release is available, so closing this PR. Note that MacOS arm64 architecture is community supported, because we are not able to test it. Thank you for contribution! |
Description
This change adds support for Apple Silicon processors by switching to the release/6.0 channel and updating some compilation flags.
Fixes #102, #105
Type of change
Please delete options that are not relevant.
[x] New feature (non-breaking change which adds functionality)
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.
Environment