[clr-android] Add CoreCLR debugger support for Android#10889
[clr-android] Add CoreCLR debugger support for Android#10889kotlarmilos wants to merge 1 commit intomainfrom
Conversation
- Introduced targets for setting up and running the CoreCLR remote debugger. - Added conditions to enable CoreCLR debugging based on project properties. - Updated environment variables for CoreCLR profiling. - Implemented tests to verify CoreCLR debugger environment variables and behavior in release builds.
jonathanpeppers
left a comment
There was a problem hiding this comment.
I think one thing that is missing is some kind of end-to-end test. Maybe it can't actually debug, but could a test do something like:
- Build run an app with CoreCLR debugger enabled
- Verify it launches
- Use TcpClient or equivalent to just verify the debugger is listening
I think that would catch a lot of potential bugs.
| <Target Name="RunWithCoreClrDebugger" | ||
| DependsOnTargets="Install;_SetupCoreClrDebugger"> |
There was a problem hiding this comment.
This is a new public target, what calls it?
I think I have some general questions on how this is supposed to integrate with IDEs.
| <AndroidEnableCoreClrDebugger Condition=" '$(AndroidEnableCoreClrDebugger)' == '' and '$(_AndroidRuntime)' == 'CoreCLR' and '$(Configuration)' != 'Release' ">true</AndroidEnableCoreClrDebugger> | ||
| <AndroidEnableCoreClrDebugger Condition=" '$(AndroidEnableCoreClrDebugger)' == '' ">false</AndroidEnableCoreClrDebugger> |
There was a problem hiding this comment.
Question about naming: should this just be DebuggerSupport? Same as feature switch? or a new name like EnableDebugger? We'd also unify with the existing Mono settings for the debugger.
Setting a single property would be ideal, and then it would do the same thing no matter what runtime is used.
| <CoreClrDebuggerPort Condition=" '$(CoreClrDebuggerPort)' == '' ">4711</CoreClrDebuggerPort> | ||
| <CoreClrDebuggerIsServer Condition=" '$(CoreClrDebuggerIsServer)' == '' ">1</CoreClrDebuggerIsServer> |
There was a problem hiding this comment.
Again, these it seems like we should choose names that don't have CoreClr in them and try to unify with what Mono has. Picking names like DebuggerPort and DebuggerIsServer -- and compare what Mono has right now.
| public string? AndroidSequencePointsMode { get; set; } | ||
| public bool EnableSGenConcurrent { get; set; } | ||
| public string? CustomBundleConfigFile { get; set; } | ||
| public bool EnableCoreClrDebugger { get; set; } |
There was a problem hiding this comment.
Seems like this could check:
- Is it CoreCLR
- Is the general purpose debugger MSBuild property set
| <AndroidAdb | ||
| ToolExe="$(AdbToolExe)" | ||
| ToolPath="$(AdbToolPath)" | ||
| AdbTarget="$(AdbTarget)" | ||
| Command="shell" | ||
| Arguments="setprop debug.coreclr.isserver $(CoreClrDebuggerIsServer)" | ||
| /> | ||
| <AndroidAdb | ||
| ToolExe="$(AdbToolExe)" | ||
| ToolPath="$(AdbToolPath)" | ||
| AdbTarget="$(AdbTarget)" | ||
| Command="shell" | ||
| Arguments="setprop debug.coreclr.ip 127.0.0.1" | ||
| /> | ||
| <AndroidAdb | ||
| ToolExe="$(AdbToolExe)" | ||
| ToolPath="$(AdbToolPath)" | ||
| AdbTarget="$(AdbTarget)" | ||
| Command="shell" | ||
| Arguments="setprop debug.coreclr.port $(CoreClrDebuggerPort)" | ||
| /> | ||
|
|
There was a problem hiding this comment.
These are a lot of adb calls, do they all need to be system properties? We also have the ability to bake environment variables into apps, or use dotnet run -e DOTNET_CORE_CLR_IP=127.0.0.1.
Description
When building a CoreCLR Android app in Debug mode, the build now configures the gRPC-based remote debugger. It sets the profiler environment variables
CORECLR_ENABLE_PROFILING,CORECLR_PROFILER,CORECLR_PROFILER_PATHso CoreCLR loads libremotemscordbitarget.so as a CoreCLR profiler at startup. It excludes the profiler library from JNI preload because it is loaded by the profiler. New MSBuild targets handle adb port forwarding and app launch for debug sessions.Tested end-to-end on a Pixel 7a arm64 - profiler loads, gRPC connection establishes over adb, initialize return S_OK, and the host receives the CreateProcess callback.
Note: This depends on diagnostics Android changes which are in PR.