Skip to content

Commit 54ce2c4

Browse files
authored
Fix EventPipe on Android CoreClr. (#112270)
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.
1 parent 12fa864 commit 54ce2c4

File tree

5 files changed

+55
-43
lines changed

5 files changed

+55
-43
lines changed

src/mono/mono/component/CMakeLists.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ set(MONO_DIAGNOSTICS_TRACING_COMPONENT_NAME "diagnostics_tracing")
99
set(MONO_DEBUGGER_COMPONENT_NAME "debugger")
1010
set(MONO_MARSHAL_ILGEN_COMPONENT_NAME "marshal-ilgen")
1111

12+
set(EP_GENERATED_HEADER_PATH "${CMAKE_CURRENT_BINARY_DIR}")
13+
include (${SHARED_EVENTPIPE_SOURCE_PATH}configure.cmake)
14+
include_directories(${SHARED_EVENTPIPE_CONFIG_HEADER_PATH})
15+
1216
add_subdirectory(${SHARED_CONTAINERS_SOURCE_PATH} containers)
1317
add_subdirectory(${SHARED_EVENTPIPE_SOURCE_PATH} eventpipe)
1418

src/native/eventpipe/CMakeLists.txt

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,13 @@
1-
include(CheckSymbolExists)
2-
include(CheckIncludeFile)
3-
4-
check_include_file(
5-
sys/socket.h
6-
HAVE_SYS_SOCKET_H
7-
)
8-
9-
check_symbol_exists(
10-
accept4
11-
sys/socket.h
12-
HAVE_ACCEPT4)
13-
14-
# Use TCP for EventPipe on mobile platforms
15-
if (CLR_CMAKE_HOST_IOS OR CLR_CMAKE_HOST_TVOS OR CLR_CMAKE_HOST_ANDROID)
16-
set(FEATURE_PERFTRACING_PAL_TCP 1)
17-
set(FEATURE_PERFTRACING_DISABLE_DEFAULT_LISTEN_PORT 1)
18-
endif()
19-
20-
configure_file(${CLR_SRC_NATIVE_DIR}/eventpipe/ep-shared-config.h.in ${CMAKE_CURRENT_BINARY_DIR}/ep-shared-config.h)
21-
221
# Define the DiagnosticsServer and EventPipe as interface libraries.
232
# We must define them as interface libraries as each runtime builds the same set of files slightly differently.
243
# Defining it as an interface library allows us to specify common sources, include directories, dependencies, etc.
254
# in one place, but also allow each runtime to add any settings that are specific to that runtime.
265
# This includes, but is not limited to each runtime's implementation of the ds-rt.h and ep-rt.h contracts.
276

7+
if (NOT DEFINED SHARED_EVENTPIPE_CONFIG_HEADER_PATH)
8+
message(FATAL_ERROR "Required configuration SHARED_EVENTPIPE_CONFIG_HEADER_PATH not set.")
9+
endif (NOT DEFINED SHARED_EVENTPIPE_CONFIG_HEADER_PATH)
10+
2811
add_library(dn-diagnosticserver INTERFACE)
2912

3013
target_sources(dn-diagnosticserver INTERFACE
@@ -38,10 +21,12 @@ target_sources(dn-diagnosticserver INTERFACE
3821
ds-server.c)
3922

4023
target_include_directories(dn-diagnosticserver INTERFACE ${CMAKE_CURRENT_BINARY_DIR})
24+
target_include_directories(dn-diagnosticserver INTERFACE ${SHARED_EVENTPIPE_CONFIG_HEADER_PATH})
4125
target_link_libraries(dn-diagnosticserver INTERFACE dn-containers)
4226

4327
add_library(dn-diagnosticserver-pal INTERFACE)
4428
target_include_directories(dn-diagnosticserver-pal INTERFACE ${CMAKE_CURRENT_BINARY_DIR})
29+
target_include_directories(dn-diagnosticserver-pal INTERFACE ${SHARED_EVENTPIPE_CONFIG_HEADER_PATH})
4530
target_link_libraries(dn-diagnosticserver-pal INTERFACE dn-containers)
4631

4732
if (FEATURE_PERFTRACING_PAL_TCP)
@@ -83,4 +68,5 @@ target_sources(dn-eventpipe INTERFACE
8368
ep-thread.c)
8469

8570
target_include_directories(dn-eventpipe INTERFACE ${CMAKE_CURRENT_BINARY_DIR})
71+
target_include_directories(dn-eventpipe INTERFACE ${SHARED_EVENTPIPE_CONFIG_HEADER_PATH})
8672
target_link_libraries(dn-eventpipe INTERFACE dn-containers)
Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,26 @@
11
include(CheckSymbolExists)
2+
include(CheckIncludeFile)
3+
4+
check_include_file(
5+
sys/socket.h
6+
HAVE_SYS_SOCKET_H
7+
)
28

39
check_symbol_exists(
410
accept4
511
sys/socket.h
612
HAVE_ACCEPT4)
713

14+
# Use TCP for EventPipe on mobile platforms
15+
if (CLR_CMAKE_HOST_IOS OR CLR_CMAKE_HOST_TVOS OR CLR_CMAKE_HOST_ANDROID)
16+
set(FEATURE_PERFTRACING_PAL_TCP 1)
17+
set(FEATURE_PERFTRACING_DISABLE_DEFAULT_LISTEN_PORT 1)
18+
endif()
19+
820
if (NOT DEFINED EP_GENERATED_HEADER_PATH)
921
message(FATAL_ERROR "Required configuration EP_GENERATED_HEADER_PATH not set.")
1022
endif (NOT DEFINED EP_GENERATED_HEADER_PATH)
1123

1224
configure_file(${CLR_SRC_NATIVE_DIR}/eventpipe/ep-shared-config.h.in ${EP_GENERATED_HEADER_PATH}/ep-shared-config.h)
1325

14-
set (SHARED_EVENTPIPE_CONFIG_HEADERS "${EP_GENERATED_HEADER_PATH}/ep-shared-config.h")
26+
set (SHARED_EVENTPIPE_CONFIG_HEADER_PATH "${EP_GENERATED_HEADER_PATH}")

src/tasks/AndroidAppBuilder/ApkBuilder.cs

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,10 @@ public partial class ApkBuilder
4848
public ITaskItem[] ExtraLinkerArguments { get; set; } = Array.Empty<ITaskItem>();
4949
public string[] NativeDependencies { get; set; } = Array.Empty<string>();
5050
public string RuntimeFlavor { get; set; } = nameof(RuntimeFlavorEnum.Mono);
51+
5152
private RuntimeFlavorEnum parsedRuntimeFlavor;
53+
private bool IsMono => parsedRuntimeFlavor == RuntimeFlavorEnum.Mono;
54+
private bool IsCoreCLR => parsedRuntimeFlavor == RuntimeFlavorEnum.CoreCLR;
5255

5356
private TaskLoggingHelper logger;
5457

@@ -62,6 +65,11 @@ public ApkBuilder(TaskLoggingHelper logger)
6265
string mainLibraryFileName,
6366
string runtimeHeaders)
6467
{
68+
if (!Enum.TryParse(RuntimeFlavor, true, out parsedRuntimeFlavor))
69+
{
70+
throw new ArgumentException($"Unknown RuntimeFlavor value: {RuntimeFlavor}. '{nameof(RuntimeFlavor)}' must be one of: {string.Join(",", Enum.GetNames(typeof(RuntimeFlavorEnum)))}");
71+
}
72+
6573
if (string.IsNullOrEmpty(AppDir) || !Directory.Exists(AppDir))
6674
{
6775
throw new ArgumentException($"AppDir='{AppDir}' is empty or doesn't exist");
@@ -106,9 +114,14 @@ public ApkBuilder(TaskLoggingHelper logger)
106114
throw new InvalidOperationException("Interpreter and AOT cannot be enabled at the same time");
107115
}
108116

109-
if (!string.IsNullOrEmpty(DiagnosticPorts) && !Array.Exists(RuntimeComponents, runtimeComponent => string.Equals(runtimeComponent, "diagnostics_tracing", StringComparison.OrdinalIgnoreCase)))
117+
if (IsMono && !string.IsNullOrEmpty(DiagnosticPorts) && !Array.Exists(RuntimeComponents, runtimeComponent => string.Equals(runtimeComponent, "diagnostics_tracing", StringComparison.OrdinalIgnoreCase)))
110118
{
111-
throw new ArgumentException($"Using DiagnosticPorts requires diagnostics_tracing runtime component, which was not included in 'RuntimeComponents' item group. @RuntimeComponents: '{string.Join(", ", RuntimeComponents)}'");
119+
throw new ArgumentException($"Using DiagnosticPorts targeting Mono requires diagnostics_tracing runtime component, which was not included in 'RuntimeComponents' item group. @RuntimeComponents: '{string.Join(", ", RuntimeComponents)}'");
120+
}
121+
122+
if (IsCoreCLR && StaticLinkedRuntime)
123+
{
124+
throw new ArgumentException("Static linking is not supported for CoreCLR runtime");
112125
}
113126

114127
// Try to get the latest build-tools version if not specified
@@ -148,11 +161,6 @@ public ApkBuilder(TaskLoggingHelper logger)
148161
throw new ArgumentException($"{buildToolsFolder} was not found.");
149162
}
150163

151-
if (!Enum.TryParse(RuntimeFlavor, true, out parsedRuntimeFlavor))
152-
{
153-
throw new ArgumentException($"Unknown RuntimeFlavor value: {RuntimeFlavor}. '{nameof(RuntimeFlavor)}' must be one of: {string.Join(",", Enum.GetNames(typeof(RuntimeFlavorEnum)))}");
154-
}
155-
156164
var assemblerFiles = new StringBuilder();
157165
var assemblerFilesToLink = new StringBuilder();
158166
var aotLibraryFiles = new List<string>();
@@ -260,22 +268,17 @@ public ApkBuilder(TaskLoggingHelper logger)
260268
else
261269
{
262270
string runtimeLib = "";
263-
if (parsedRuntimeFlavor == RuntimeFlavorEnum.CoreCLR)
271+
if (StaticLinkedRuntime && IsMono)
264272
{
265-
if (StaticLinkedRuntime)
266-
throw new ArgumentException("Static linking is not supported for CoreCLR runtime");
267-
runtimeLib = Path.Combine(AppDir, "libcoreclr.so");
273+
runtimeLib = Path.Combine(AppDir, "libmonosgen-2.0.a");
268274
}
269-
else
275+
else if (IsMono)
270276
{
271-
if (StaticLinkedRuntime)
272-
{
273-
runtimeLib = Path.Combine(AppDir, "libmonosgen-2.0.a");
274-
}
275-
else
276-
{
277-
runtimeLib = Path.Combine(AppDir, "libmonosgen-2.0.so");
278-
}
277+
runtimeLib = Path.Combine(AppDir, "libmonosgen-2.0.so");
278+
}
279+
else if (IsCoreCLR)
280+
{
281+
runtimeLib = Path.Combine(AppDir, "libcoreclr.so");
279282
}
280283

281284
if (!File.Exists(runtimeLib))
@@ -332,7 +335,7 @@ public ApkBuilder(TaskLoggingHelper logger)
332335
nativeLibraries += assemblerFilesToLink.ToString();
333336

334337
string aotSources = assemblerFiles.ToString();
335-
string monodroidSource = (parsedRuntimeFlavor == RuntimeFlavorEnum.CoreCLR) ?
338+
string monodroidSource = IsCoreCLR ?
336339
"monodroid-coreclr.c" : (IsLibraryMode) ? "monodroid-librarymode.c" : "monodroid.c";
337340

338341
string cmakeLists = Utils.GetEmbeddedResource("CMakeLists-android.txt")

src/tasks/AndroidAppBuilder/Templates/monodroid-coreclr.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,13 @@ mono_droid_runtime_init (const char* executable)
183183
{
184184
LOG_INFO ("mono_droid_runtime_init (CoreCLR) called with executable: %s", executable);
185185

186+
// build using DiagnosticPorts property in AndroidAppBuilder
187+
// or set DOTNET_DiagnosticPorts env via adb, xharness when undefined.
188+
// NOTE, using DOTNET_DiagnosticPorts requires app build using AndroidAppBuilder and RuntimeComponents to include 'diagnostics_tracing' component
189+
#ifdef DIAGNOSTIC_PORTS
190+
setenv ("DOTNET_DiagnosticPorts", DIAGNOSTIC_PORTS, true);
191+
#endif
192+
186193
if (bundle_executable_path(executable, g_bundle_path, &g_executable_path) < 0)
187194
{
188195
LOG_ERROR("Failed to resolve full path for: %s", executable);

0 commit comments

Comments
 (0)