From 77b9c7aca792c74dffc44829271fd3d51d7b0f8c Mon Sep 17 00:00:00 2001 From: lateralusX Date: Fri, 7 Feb 2025 12:31:36 +0100 Subject: [PATCH] Fix EventPipe on Android CoreClr. EventPipe CMake was causing issues when building CoreClr due to creating two versions of ep-shared-config.h with different set of variables when targeting iOS/tvOS/Android. Both include paths ended up being added and together with EventPipe's unity build, this caused issues since the wrong ep-shared-config.h was picked up, incorrect enabled default listeners as result of having wrong values for FEATURE_PERFTRACING_DISABLE_DEFAULT_LISTEN_PORT and FEATURE_PERFTRACING_PAL_TCP. Mono doesn't have this issue since it only creates one ep-shared-config.h. Commit makes sure we only setup on ep-shared-config.h and eventpipe libraries adds the include folder to its library targets. Commit also adds support to set DOTNET_DiagnosticPorts as part of building Android sample to simplify enable/disable diagnostics, similar to how Mono Android sample works. Couple of smaller adjustments to ApkBuilder. --- src/mono/mono/component/CMakeLists.txt | 4 ++ src/native/eventpipe/CMakeLists.txt | 28 +++--------- src/native/eventpipe/configure.cmake | 14 +++++- src/tasks/AndroidAppBuilder/ApkBuilder.cs | 45 ++++++++++--------- .../Templates/monodroid-coreclr.c | 7 +++ 5 files changed, 55 insertions(+), 43 deletions(-) diff --git a/src/mono/mono/component/CMakeLists.txt b/src/mono/mono/component/CMakeLists.txt index 0e663e690fc4fd..5b133d79ae91ed 100644 --- a/src/mono/mono/component/CMakeLists.txt +++ b/src/mono/mono/component/CMakeLists.txt @@ -9,6 +9,10 @@ set(MONO_DIAGNOSTICS_TRACING_COMPONENT_NAME "diagnostics_tracing") set(MONO_DEBUGGER_COMPONENT_NAME "debugger") set(MONO_MARSHAL_ILGEN_COMPONENT_NAME "marshal-ilgen") +set(EP_GENERATED_HEADER_PATH "${CMAKE_CURRENT_BINARY_DIR}") +include (${SHARED_EVENTPIPE_SOURCE_PATH}configure.cmake) +include_directories(${SHARED_EVENTPIPE_CONFIG_HEADER_PATH}) + add_subdirectory(${SHARED_CONTAINERS_SOURCE_PATH} containers) add_subdirectory(${SHARED_EVENTPIPE_SOURCE_PATH} eventpipe) diff --git a/src/native/eventpipe/CMakeLists.txt b/src/native/eventpipe/CMakeLists.txt index 311293ca5f45cc..f7301b5933ae04 100644 --- a/src/native/eventpipe/CMakeLists.txt +++ b/src/native/eventpipe/CMakeLists.txt @@ -1,30 +1,13 @@ -include(CheckSymbolExists) -include(CheckIncludeFile) - -check_include_file( - sys/socket.h - HAVE_SYS_SOCKET_H -) - -check_symbol_exists( - accept4 - sys/socket.h - HAVE_ACCEPT4) - -# Use TCP for EventPipe on mobile platforms -if (CLR_CMAKE_HOST_IOS OR CLR_CMAKE_HOST_TVOS OR CLR_CMAKE_HOST_ANDROID) - set(FEATURE_PERFTRACING_PAL_TCP 1) - set(FEATURE_PERFTRACING_DISABLE_DEFAULT_LISTEN_PORT 1) -endif() - -configure_file(${CLR_SRC_NATIVE_DIR}/eventpipe/ep-shared-config.h.in ${CMAKE_CURRENT_BINARY_DIR}/ep-shared-config.h) - # Define the DiagnosticsServer and EventPipe as interface libraries. # We must define them as interface libraries as each runtime builds the same set of files slightly differently. # Defining it as an interface library allows us to specify common sources, include directories, dependencies, etc. # in one place, but also allow each runtime to add any settings that are specific to that runtime. # This includes, but is not limited to each runtime's implementation of the ds-rt.h and ep-rt.h contracts. +if (NOT DEFINED SHARED_EVENTPIPE_CONFIG_HEADER_PATH) + message(FATAL_ERROR "Required configuration SHARED_EVENTPIPE_CONFIG_HEADER_PATH not set.") +endif (NOT DEFINED SHARED_EVENTPIPE_CONFIG_HEADER_PATH) + add_library(dn-diagnosticserver INTERFACE) target_sources(dn-diagnosticserver INTERFACE @@ -38,10 +21,12 @@ target_sources(dn-diagnosticserver INTERFACE ds-server.c) target_include_directories(dn-diagnosticserver INTERFACE ${CMAKE_CURRENT_BINARY_DIR}) +target_include_directories(dn-diagnosticserver INTERFACE ${SHARED_EVENTPIPE_CONFIG_HEADER_PATH}) target_link_libraries(dn-diagnosticserver INTERFACE dn-containers) add_library(dn-diagnosticserver-pal INTERFACE) target_include_directories(dn-diagnosticserver-pal INTERFACE ${CMAKE_CURRENT_BINARY_DIR}) +target_include_directories(dn-diagnosticserver-pal INTERFACE ${SHARED_EVENTPIPE_CONFIG_HEADER_PATH}) target_link_libraries(dn-diagnosticserver-pal INTERFACE dn-containers) if (FEATURE_PERFTRACING_PAL_TCP) @@ -83,4 +68,5 @@ target_sources(dn-eventpipe INTERFACE ep-thread.c) target_include_directories(dn-eventpipe INTERFACE ${CMAKE_CURRENT_BINARY_DIR}) +target_include_directories(dn-eventpipe INTERFACE ${SHARED_EVENTPIPE_CONFIG_HEADER_PATH}) target_link_libraries(dn-eventpipe INTERFACE dn-containers) diff --git a/src/native/eventpipe/configure.cmake b/src/native/eventpipe/configure.cmake index 889894e5f4fdbb..b0af4301d6a914 100644 --- a/src/native/eventpipe/configure.cmake +++ b/src/native/eventpipe/configure.cmake @@ -1,14 +1,26 @@ include(CheckSymbolExists) +include(CheckIncludeFile) + +check_include_file( + sys/socket.h + HAVE_SYS_SOCKET_H +) check_symbol_exists( accept4 sys/socket.h HAVE_ACCEPT4) +# Use TCP for EventPipe on mobile platforms +if (CLR_CMAKE_HOST_IOS OR CLR_CMAKE_HOST_TVOS OR CLR_CMAKE_HOST_ANDROID) + set(FEATURE_PERFTRACING_PAL_TCP 1) + set(FEATURE_PERFTRACING_DISABLE_DEFAULT_LISTEN_PORT 1) +endif() + if (NOT DEFINED EP_GENERATED_HEADER_PATH) message(FATAL_ERROR "Required configuration EP_GENERATED_HEADER_PATH not set.") endif (NOT DEFINED EP_GENERATED_HEADER_PATH) configure_file(${CLR_SRC_NATIVE_DIR}/eventpipe/ep-shared-config.h.in ${EP_GENERATED_HEADER_PATH}/ep-shared-config.h) -set (SHARED_EVENTPIPE_CONFIG_HEADERS "${EP_GENERATED_HEADER_PATH}/ep-shared-config.h") \ No newline at end of file +set (SHARED_EVENTPIPE_CONFIG_HEADER_PATH "${EP_GENERATED_HEADER_PATH}") \ No newline at end of file diff --git a/src/tasks/AndroidAppBuilder/ApkBuilder.cs b/src/tasks/AndroidAppBuilder/ApkBuilder.cs index 7ed5564638ede9..fdeea4e0280516 100644 --- a/src/tasks/AndroidAppBuilder/ApkBuilder.cs +++ b/src/tasks/AndroidAppBuilder/ApkBuilder.cs @@ -48,7 +48,10 @@ public partial class ApkBuilder public ITaskItem[] ExtraLinkerArguments { get; set; } = Array.Empty(); public string[] NativeDependencies { get; set; } = Array.Empty(); public string RuntimeFlavor { get; set; } = nameof(RuntimeFlavorEnum.Mono); + private RuntimeFlavorEnum parsedRuntimeFlavor; + private bool IsMono => parsedRuntimeFlavor == RuntimeFlavorEnum.Mono; + private bool IsCoreCLR => parsedRuntimeFlavor == RuntimeFlavorEnum.CoreCLR; private TaskLoggingHelper logger; @@ -62,6 +65,11 @@ public ApkBuilder(TaskLoggingHelper logger) string mainLibraryFileName, string runtimeHeaders) { + if (!Enum.TryParse(RuntimeFlavor, true, out parsedRuntimeFlavor)) + { + throw new ArgumentException($"Unknown RuntimeFlavor value: {RuntimeFlavor}. '{nameof(RuntimeFlavor)}' must be one of: {string.Join(",", Enum.GetNames(typeof(RuntimeFlavorEnum)))}"); + } + if (string.IsNullOrEmpty(AppDir) || !Directory.Exists(AppDir)) { throw new ArgumentException($"AppDir='{AppDir}' is empty or doesn't exist"); @@ -106,9 +114,14 @@ public ApkBuilder(TaskLoggingHelper logger) throw new InvalidOperationException("Interpreter and AOT cannot be enabled at the same time"); } - if (!string.IsNullOrEmpty(DiagnosticPorts) && !Array.Exists(RuntimeComponents, runtimeComponent => string.Equals(runtimeComponent, "diagnostics_tracing", StringComparison.OrdinalIgnoreCase))) + if (IsMono && !string.IsNullOrEmpty(DiagnosticPorts) && !Array.Exists(RuntimeComponents, runtimeComponent => string.Equals(runtimeComponent, "diagnostics_tracing", StringComparison.OrdinalIgnoreCase))) { - throw new ArgumentException($"Using DiagnosticPorts requires diagnostics_tracing runtime component, which was not included in 'RuntimeComponents' item group. @RuntimeComponents: '{string.Join(", ", RuntimeComponents)}'"); + throw new ArgumentException($"Using DiagnosticPorts targeting Mono requires diagnostics_tracing runtime component, which was not included in 'RuntimeComponents' item group. @RuntimeComponents: '{string.Join(", ", RuntimeComponents)}'"); + } + + if (IsCoreCLR && StaticLinkedRuntime) + { + throw new ArgumentException("Static linking is not supported for CoreCLR runtime"); } // Try to get the latest build-tools version if not specified @@ -148,11 +161,6 @@ public ApkBuilder(TaskLoggingHelper logger) throw new ArgumentException($"{buildToolsFolder} was not found."); } - if (!Enum.TryParse(RuntimeFlavor, true, out parsedRuntimeFlavor)) - { - throw new ArgumentException($"Unknown RuntimeFlavor value: {RuntimeFlavor}. '{nameof(RuntimeFlavor)}' must be one of: {string.Join(",", Enum.GetNames(typeof(RuntimeFlavorEnum)))}"); - } - var assemblerFiles = new StringBuilder(); var assemblerFilesToLink = new StringBuilder(); var aotLibraryFiles = new List(); @@ -260,22 +268,17 @@ public ApkBuilder(TaskLoggingHelper logger) else { string runtimeLib = ""; - if (parsedRuntimeFlavor == RuntimeFlavorEnum.CoreCLR) + if (StaticLinkedRuntime && IsMono) { - if (StaticLinkedRuntime) - throw new ArgumentException("Static linking is not supported for CoreCLR runtime"); - runtimeLib = Path.Combine(AppDir, "libcoreclr.so"); + runtimeLib = Path.Combine(AppDir, "libmonosgen-2.0.a"); } - else + else if (IsMono) { - if (StaticLinkedRuntime) - { - runtimeLib = Path.Combine(AppDir, "libmonosgen-2.0.a"); - } - else - { - runtimeLib = Path.Combine(AppDir, "libmonosgen-2.0.so"); - } + runtimeLib = Path.Combine(AppDir, "libmonosgen-2.0.so"); + } + else if (IsCoreCLR) + { + runtimeLib = Path.Combine(AppDir, "libcoreclr.so"); } if (!File.Exists(runtimeLib)) @@ -332,7 +335,7 @@ public ApkBuilder(TaskLoggingHelper logger) nativeLibraries += assemblerFilesToLink.ToString(); string aotSources = assemblerFiles.ToString(); - string monodroidSource = (parsedRuntimeFlavor == RuntimeFlavorEnum.CoreCLR) ? + string monodroidSource = IsCoreCLR ? "monodroid-coreclr.c" : (IsLibraryMode) ? "monodroid-librarymode.c" : "monodroid.c"; string cmakeLists = Utils.GetEmbeddedResource("CMakeLists-android.txt") diff --git a/src/tasks/AndroidAppBuilder/Templates/monodroid-coreclr.c b/src/tasks/AndroidAppBuilder/Templates/monodroid-coreclr.c index 4489b0f23bfb83..210b8025312ec3 100644 --- a/src/tasks/AndroidAppBuilder/Templates/monodroid-coreclr.c +++ b/src/tasks/AndroidAppBuilder/Templates/monodroid-coreclr.c @@ -183,6 +183,13 @@ mono_droid_runtime_init (const char* executable) { LOG_INFO ("mono_droid_runtime_init (CoreCLR) called with executable: %s", executable); + // build using DiagnosticPorts property in AndroidAppBuilder + // or set DOTNET_DiagnosticPorts env via adb, xharness when undefined. + // NOTE, using DOTNET_DiagnosticPorts requires app build using AndroidAppBuilder and RuntimeComponents to include 'diagnostics_tracing' component +#ifdef DIAGNOSTIC_PORTS + setenv ("DOTNET_DiagnosticPorts", DIAGNOSTIC_PORTS, true); +#endif + if (bundle_executable_path(executable, g_bundle_path, &g_executable_path) < 0) { LOG_ERROR("Failed to resolve full path for: %s", executable);