From ddacda022b2d9f0f1d150ec234d5225d97d682b3 Mon Sep 17 00:00:00 2001 From: Justin Anderson Date: Thu, 25 May 2023 10:34:19 -0700 Subject: [PATCH 01/23] ApplyStartupHook diagnostic IPC command --- src/coreclr/dlls/mscoree/exports.cpp | 108 +++++++++++++++++- src/coreclr/inc/hostinformation.h | 1 + .../nativeaot/Runtime/eventpipe/ds-rt-aot.h | 7 ++ .../vm/eventing/eventpipe/ds-rt-coreclr.h | 32 ++++++ src/coreclr/vm/hostinformation.cpp | 8 ++ .../src/System/StartupHookProvider.cs | 21 +++- src/mono/mono/eventpipe/ds-rt-mono.h | 8 ++ src/native/corehost/host_runtime_contract.h | 8 ++ .../hostpolicy/hostpolicy_context.cpp | 27 +++++ src/native/eventpipe/ds-process-protocol.c | 90 +++++++++++++++ src/native/eventpipe/ds-process-protocol.h | 26 +++++ src/native/eventpipe/ds-rt.h | 4 + src/native/eventpipe/ds-types.h | 4 +- 13 files changed, 336 insertions(+), 8 deletions(-) diff --git a/src/coreclr/dlls/mscoree/exports.cpp b/src/coreclr/dlls/mscoree/exports.cpp index d14471f3b6bb8d..53b047223d666f 100644 --- a/src/coreclr/dlls/mscoree/exports.cpp +++ b/src/coreclr/dlls/mscoree/exports.cpp @@ -147,6 +147,8 @@ static void ConvertConfigPropertiesToUnicode( LPCWSTR* propertyValuesW = new (nothrow) LPCWSTR[propertyCount]; ASSERTE_ALL_BUILDS(propertyValuesW != nullptr); + LPCWSTR diagnostic_startup_hooks_value = nullptr; + for (int propertyIndex = 0; propertyIndex < propertyCount; ++propertyIndex) { propertyKeysW[propertyIndex] = StringToUnicode(propertyKeys[propertyIndex]); @@ -189,12 +191,99 @@ static void ConvertConfigPropertiesToUnicode( if (hostContractLocal->pinvoke_override != nullptr) *pinvokeOverride = hostContractLocal->pinvoke_override; } + else if (strcmp(propertyKeys[propertyIndex], HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS) == 0) + { + diagnostic_startup_hooks_value = propertyValuesW[propertyIndex]; + } + } + + if (nullptr != diagnostic_startup_hooks_value) + { + HostInformation::SetProperty(HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS, diagnostic_startup_hooks_value); } *propertyKeysWRef = propertyKeysW; *propertyValuesWRef = propertyValuesW; } +static void CreateAppDomainProperties( + LPCWSTR* configPropertyKeys, + LPCWSTR* configPropertyValues, + int configPropertyCount, + NewArrayHolder& appDomainPropertyKeys, + NewArrayHolder& appDomainPropertyValues, + int& appDomainPropertyCount, + ConstWStringArrayHolder& allocatedStrings) +{ + // Dynamically create list of properties to provide to app domain creation + // Some properties may have been dynamically specified through other means, + // such as the diagnostic IPC channel setting startup hooks. + appDomainPropertyCount = configPropertyCount; + + // Check if the diagnostic startup hook was specified in the host + bool diagnostic_startup_hooks_handled = true; + const WCHAR* diagnostic_startup_hooks_value = nullptr; + SString diagnostic_startup_hooks_value_str; + if (HostInformation::GetProperty(HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS, diagnostic_startup_hooks_value_str)) + { + diagnostic_startup_hooks_handled = false; + diagnostic_startup_hooks_value = diagnostic_startup_hooks_value_str.GetCopyOfUnicodeString(); + appDomainPropertyCount++; + } + + // These holders will not take ownership of the strings but provide a wrapper + // in order to allow using them as a single indexable array. The owners are still + // responsible for deleting the strings. + appDomainPropertyKeys = new (nothrow) LPCWSTR[appDomainPropertyCount]; + ASSERTE_ALL_BUILDS(appDomainPropertyKeys != nullptr); + + appDomainPropertyValues = new (nothrow) LPCWSTR[appDomainPropertyCount]; + ASSERTE_ALL_BUILDS(appDomainPropertyValues != nullptr); + + // Add properties that were explicitly passed for initialization + int propertyIndex = 0; + while (propertyIndex < configPropertyCount) + { + // If one of the properties that was passed for intialization is the diganostic startup + // hook AND the diagnostic startup hook was set in the host information, replace the + // value by prefering the host information value. + if (u16_strcmp(configPropertyKeys[propertyIndex], TEXT(HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS)) == 0 && + nullptr != diagnostic_startup_hooks_value) + { + appDomainPropertyKeys[propertyIndex] = TEXT(HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS); + appDomainPropertyValues[propertyIndex] = diagnostic_startup_hooks_value; + + if (!diagnostic_startup_hooks_handled) + { + // Skip dynamically adding the diagnostic startup hook to the array holder since + // it already exists in the properties passed for initialization. + diagnostic_startup_hooks_handled = true; + appDomainPropertyCount--; + } + } + else + { + appDomainPropertyKeys[propertyIndex] = configPropertyKeys[propertyIndex]; + appDomainPropertyValues[propertyIndex] = configPropertyValues[propertyIndex]; + } + propertyIndex++; + } + + // Append diagnostic startup hook property if it has not been handled yet + if (!diagnostic_startup_hooks_handled) + { + appDomainPropertyKeys[propertyIndex] = TEXT(HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS); + appDomainPropertyValues[propertyIndex] = diagnostic_startup_hooks_value; + propertyIndex++; + } + + allocatedStrings.Set(new (nothrow) LPCWSTR[1], 1); + ASSERTE_ALL_BUILDS(allocatedStrings != nullptr); + allocatedStrings[0] = diagnostic_startup_hooks_value; + + ASSERTE_ALL_BUILDS(propertyIndex == appDomainPropertyCount); +} + coreclr_error_writer_callback_fn g_errorWriter = nullptr; // @@ -320,14 +409,27 @@ int coreclr_initialize( hr = host->Start(); IfFailRet(hr); + NewArrayHolder appDomainPropertyKeys; + NewArrayHolder appDomainPropertyValues; + int appDomainPropertyCount; + ConstWStringArrayHolder allocatedStrings; + CreateAppDomainProperties( + propertyKeysW, + propertyValuesW, + propertyCount, + appDomainPropertyKeys, + appDomainPropertyValues, + appDomainPropertyCount, + allocatedStrings); + hr = host->CreateAppDomainWithManager( appDomainFriendlyNameW, APPDOMAIN_SECURITY_DEFAULT, NULL, // Name of the assembly that contains the AppDomainManager implementation NULL, // The AppDomainManager implementation type name - propertyCount, - propertyKeysW, - propertyValuesW, + appDomainPropertyCount, + appDomainPropertyKeys, + appDomainPropertyValues, (DWORD *)domainId); if (SUCCEEDED(hr)) diff --git a/src/coreclr/inc/hostinformation.h b/src/coreclr/inc/hostinformation.h index d57b4729d30e68..5ff2e352fef680 100644 --- a/src/coreclr/inc/hostinformation.h +++ b/src/coreclr/inc/hostinformation.h @@ -11,6 +11,7 @@ class HostInformation public: static void SetContract(_In_ host_runtime_contract* hostContract); static bool GetProperty(_In_z_ const char* name, SString& value); + static bool SetProperty(_In_z_ const char* name, const SString& value); }; #endif // _HOSTINFORMATION_H_ diff --git a/src/coreclr/nativeaot/Runtime/eventpipe/ds-rt-aot.h b/src/coreclr/nativeaot/Runtime/eventpipe/ds-rt-aot.h index df255f7186e193..ee20ab37e70668 100644 --- a/src/coreclr/nativeaot/Runtime/eventpipe/ds-rt-aot.h +++ b/src/coreclr/nativeaot/Runtime/eventpipe/ds-rt-aot.h @@ -272,6 +272,13 @@ ds_rt_disable_perfmap (void) return DS_IPC_E_NOTSUPPORTED; } +static +uint32_t +ds_rt_apply_startup_hook (const ep_char16_t *startup_hook_path) +{ + return DS_IPC_E_NOTSUPPORTED; +} + /* * DiagnosticServer. */ diff --git a/src/coreclr/vm/eventing/eventpipe/ds-rt-coreclr.h b/src/coreclr/vm/eventing/eventpipe/ds-rt-coreclr.h index 18d2ce62bdfc69..9d7b39e12a39f7 100644 --- a/src/coreclr/vm/eventing/eventpipe/ds-rt-coreclr.h +++ b/src/coreclr/vm/eventing/eventpipe/ds-rt-coreclr.h @@ -329,6 +329,38 @@ ds_rt_disable_perfmap (void) #endif // FEATURE_PERFMAP } +static +uint32_t +ds_rt_apply_startup_hook (const ep_char16_t *startup_hook_path) +{ + HRESULT hr = S_OK; + // This is set to true when the EE has initialized, which occurs after + // the diagnostic suspension point has completed. + if (g_fEEStarted) + { + // TODO: Support loading and executing startup hook after EE has completely initialized. + return DS_IPC_E_INVALIDARG; + } + else + { + // Prepend the new startup hook to the existing list of diagnostic startup hooks + SString new_paths(reinterpret_cast(startup_hook_path)); + + SString old_paths; + if (HostInformation::GetProperty(HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS, old_paths) && + !old_paths.IsEmpty()) + { + new_paths.Append(PATH_SEPARATOR_STR_W); + new_paths.Append(old_paths); + } + + if (!HostInformation::SetProperty(HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS, new_paths)) + return DS_IPC_E_FAIL; + } + + return DS_IPC_S_OK; +} + /* * DiagnosticServer. */ diff --git a/src/coreclr/vm/hostinformation.cpp b/src/coreclr/vm/hostinformation.cpp index b440f6f2168ab7..f5215ca11a382f 100644 --- a/src/coreclr/vm/hostinformation.cpp +++ b/src/coreclr/vm/hostinformation.cpp @@ -40,3 +40,11 @@ bool HostInformation::GetProperty(_In_z_ const char* name, SString& value) return lenActual > 0 && lenActual <= len; } + +bool HostInformation::SetProperty(_In_z_ const char* name, const SString& value) +{ + if (s_hostContract == nullptr || s_hostContract->set_runtime_property == nullptr) + return false; + + return s_hostContract->set_runtime_property(name, value.GetUTF8(), s_hostContract->context); +} diff --git a/src/libraries/System.Private.CoreLib/src/System/StartupHookProvider.cs b/src/libraries/System.Private.CoreLib/src/System/StartupHookProvider.cs index 78b86b6583d9a0..d21640f7b2c402 100644 --- a/src/libraries/System.Private.CoreLib/src/System/StartupHookProvider.cs +++ b/src/libraries/System.Private.CoreLib/src/System/StartupHookProvider.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.Tracing; using System.Diagnostics.CodeAnalysis; @@ -32,8 +33,21 @@ private static void ProcessStartupHooks() if (!IsSupported) return; + List startupHookParts = new(); + + string? diagnosticStartupHooksVariable = AppContext.GetData("DIAGNOSTIC_STARTUP_HOOKS") as string; + if (null != diagnosticStartupHooksVariable) + { + startupHookParts.AddRange(diagnosticStartupHooksVariable.Split(Path.PathSeparator)); + } + string? startupHooksVariable = AppContext.GetData("STARTUP_HOOKS") as string; - if (startupHooksVariable == null) + if (null != startupHooksVariable) + { + startupHookParts.AddRange(startupHooksVariable.Split(Path.PathSeparator)); + } + + if (startupHookParts.Count == 0) { return; } @@ -47,9 +61,8 @@ private static void ProcessStartupHooks() }; // Parse startup hooks variable - string[] startupHookParts = startupHooksVariable.Split(Path.PathSeparator); - StartupHookNameOrPath[] startupHooks = new StartupHookNameOrPath[startupHookParts.Length]; - for (int i = 0; i < startupHookParts.Length; i++) + StartupHookNameOrPath[] startupHooks = new StartupHookNameOrPath[startupHookParts.Count]; + for (int i = 0; i < startupHookParts.Count; i++) { string startupHookPart = startupHookParts[i]; if (string.IsNullOrEmpty(startupHookPart)) diff --git a/src/mono/mono/eventpipe/ds-rt-mono.h b/src/mono/mono/eventpipe/ds-rt-mono.h index 3eea7f1ebe04cd..86898cfccb0dd9 100644 --- a/src/mono/mono/eventpipe/ds-rt-mono.h +++ b/src/mono/mono/eventpipe/ds-rt-mono.h @@ -239,6 +239,14 @@ ds_rt_disable_perfmap (void) return DS_IPC_E_NOTSUPPORTED; } +static +uint32_t +ds_rt_apply_startup_hook (const ep_char16_t *startup_hook_path) +{ + // TODO: Implement. + return DS_IPC_E_NOTSUPPORTED; +} + /* * DiagnosticServer. */ diff --git a/src/native/corehost/host_runtime_contract.h b/src/native/corehost/host_runtime_contract.h index 11d98c3494de47..384820dd1841e4 100644 --- a/src/native/corehost/host_runtime_contract.h +++ b/src/native/corehost/host_runtime_contract.h @@ -23,6 +23,7 @@ #define HOST_PROPERTY_PINVOKE_OVERRIDE "PINVOKE_OVERRIDE" #define HOST_PROPERTY_PLATFORM_RESOURCE_ROOTS "PLATFORM_RESOURCE_ROOTS" #define HOST_PROPERTY_TRUSTED_PLATFORM_ASSEMBLIES "TRUSTED_PLATFORM_ASSEMBLIES" +#define HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS "DIAGNOSTIC_STARTUP_HOOKS" struct host_runtime_contract { @@ -39,6 +40,13 @@ struct host_runtime_contract size_t value_buffer_size, void* contract_context); + // Set the value of a runtime property. + // Returns true if able to set successfully, false otherwise. + bool(HOST_CONTRACT_CALLTYPE* set_runtime_property)( + const char* key, + const char* value, + void* contract_context); + // Probe an app bundle for `path`. Sets its location (`offset`, `size`) in the bundle if found. // Returns true if found, false otherwise. bool(HOST_CONTRACT_CALLTYPE* bundle_probe)( diff --git a/src/native/corehost/hostpolicy/hostpolicy_context.cpp b/src/native/corehost/hostpolicy/hostpolicy_context.cpp index 4c93ea300d7c52..bced955e1c6feb 100644 --- a/src/native/corehost/hostpolicy/hostpolicy_context.cpp +++ b/src/native/corehost/hostpolicy/hostpolicy_context.cpp @@ -133,6 +133,32 @@ namespace return -1; } + + bool HOST_CONTRACT_CALLTYPE set_runtime_property( + const char* key, + const char* value, + void* contract_context) + { + hostpolicy_context_t* context = static_cast(contract_context); + + pal::string_t key_str; + if (!pal::clr_palstring(key, &key_str)) + return false; + + if (nullptr != value) + { + pal::string_t value_str; + if (!pal::clr_palstring(value, &value_str)) + return false; + + context->coreclr_properties.add(key_str.c_str(), value_str.c_str()); + } + else + { + context->coreclr_properties.remove(key_str.c_str()); + } + return true; + } } bool hostpolicy_context_t::should_read_rid_fallback_graph(const hostpolicy_init_t &init) @@ -378,6 +404,7 @@ int hostpolicy_context_t::initialize(const hostpolicy_init_t &hostpolicy_init, c } host_contract.get_runtime_property = &get_runtime_property; + host_contract.set_runtime_property = &set_runtime_property; pal::stringstream_t ptr_stream; ptr_stream << "0x" << std::hex << (size_t)(&host_contract); if (!coreclr_properties.add(_STRINGIFY(HOST_PROPERTY_RUNTIME_CONTRACT), ptr_stream.str().c_str())) diff --git a/src/native/eventpipe/ds-process-protocol.c b/src/native/eventpipe/ds-process-protocol.c index a40403a03f7897..4fdd912449abf6 100644 --- a/src/native/eventpipe/ds-process-protocol.c +++ b/src/native/eventpipe/ds-process-protocol.c @@ -89,6 +89,12 @@ process_protocol_helper_disable_perfmap ( DiagnosticsIpcMessage *message, DiagnosticsIpcStream *stream); +static +bool +process_protocol_helper_apply_startup_hook ( + DiagnosticsIpcMessage *message, + DiagnosticsIpcStream *stream); + static bool process_protocol_helper_unknown_command ( @@ -872,6 +878,87 @@ process_protocol_helper_disable_perfmap ( ep_exit_error_handler (); } +DiagnosticsApplyStartupHookPayload * +ds_apply_startup_hook_payload_alloc (void) +{ + return ep_rt_object_alloc (DiagnosticsApplyStartupHookPayload); +} + +void +ds_apply_startup_hook_payload_free (DiagnosticsApplyStartupHookPayload *payload) +{ + ep_return_void_if_nok (payload != NULL); + ep_rt_byte_array_free (payload->incoming_buffer); + ep_rt_object_free (payload); +} + +static +uint8_t * +apply_startup_hook_command_try_parse_payload ( + uint8_t *buffer, + uint16_t buffer_len) +{ + EP_ASSERT (buffer != NULL); + + uint8_t * buffer_cursor = buffer; + uint32_t buffer_cursor_len = buffer_len; + + DiagnosticsApplyStartupHookPayload *instance = ds_apply_startup_hook_payload_alloc (); + ep_raise_error_if_nok (instance != NULL); + + instance->incoming_buffer = buffer; + + if (!ds_ipc_message_try_parse_string_utf16_t (&buffer_cursor, &buffer_cursor_len, &instance->startup_hook_path)) + ep_raise_error (); + +ep_on_exit: + return (uint8_t *)instance; + +ep_on_error: + ds_apply_startup_hook_payload_free (instance); + instance = NULL; + ep_exit_error_handler (); +} + +static +bool +process_protocol_helper_apply_startup_hook ( + DiagnosticsIpcMessage *message, + DiagnosticsIpcStream *stream) +{ + EP_ASSERT (message != NULL); + EP_ASSERT (stream != NULL); + + if (!stream) + return false; + + bool result = false; + DiagnosticsApplyStartupHookPayload *payload = (DiagnosticsApplyStartupHookPayload *)ds_ipc_message_try_parse_payload (message, apply_startup_hook_command_try_parse_payload); + if (!payload) { + ds_ipc_message_send_error (stream, DS_IPC_E_BAD_ENCODING); + ep_raise_error (); + } + + ds_ipc_result_t ipc_result; + ipc_result = ds_rt_apply_startup_hook (payload->startup_hook_path); + if (ipc_result != DS_IPC_S_OK) { + ds_ipc_message_send_error (stream, ipc_result); + ep_raise_error (); + } else { + ds_ipc_message_send_success (stream, ipc_result); + } + + result = true; + +ep_on_exit: + ds_ipc_stream_free (stream); + return result; + +ep_on_error: + EP_ASSERT (!result); + ep_exit_error_handler (); +} + static bool process_protocol_helper_unknown_command ( @@ -916,6 +1003,9 @@ ds_process_protocol_helper_handle_ipc_message ( case DS_PROCESS_COMMANDID_DISABLE_PERFMAP: result = process_protocol_helper_disable_perfmap (message, stream); break; + case DS_PROCESS_COMMANDID_APPLY_STARTUP_HOOK: + result = process_protocol_helper_apply_startup_hook (message, stream); + break; default: result = process_protocol_helper_unknown_command (message, stream); break; diff --git a/src/native/eventpipe/ds-process-protocol.h b/src/native/eventpipe/ds-process-protocol.h index 04430bbb72b85e..dc9b1250618c52 100644 --- a/src/native/eventpipe/ds-process-protocol.h +++ b/src/native/eventpipe/ds-process-protocol.h @@ -190,6 +190,32 @@ ds_enable_perfmap_payload_alloc (void); void ds_enable_perfmap_payload_free (DiagnosticsEnablePerfmapPayload *payload); +/* +* DiagnosticsApplyStartupHookPayload +*/ + +#if defined(DS_INLINE_GETTER_SETTER) || defined(DS_IMPL_PROCESS_PROTOCOL_GETTER_SETTER) +struct _DiagnosticsApplyStartupHookPayload { +#else +struct _DiagnosticsApplyStartupHookPayload_Internal { +#endif + uint8_t * incoming_buffer; + + const ep_char16_t *startup_hook_path; +}; + +#if !defined(DS_INLINE_GETTER_SETTER) && !defined(DS_IMPL_PROCESS_PROTOCOL_GETTER_SETTER) +struct _DiagnosticsApplyStartupHookPayload { + uint8_t _internal [sizeof (struct _DiagnosticsApplyStartupHookPayload_Internal)]; +}; +#endif + +DiagnosticsApplyStartupHookPayload * +ds_apply_startup_hook_payload_alloc (void); + +void +ds_apply_startup_hook_payload_free (DiagnosticsApplyStartupHookPayload *payload); + /* * DiagnosticsProcessProtocolHelper. */ diff --git a/src/native/eventpipe/ds-rt.h b/src/native/eventpipe/ds-rt.h index f4a54b3d52df5d..ff33cf71b99869 100644 --- a/src/native/eventpipe/ds-rt.h +++ b/src/native/eventpipe/ds-rt.h @@ -118,6 +118,10 @@ static uint32_t ds_rt_disable_perfmap (void); +static +uint32_t +ds_rt_apply_startup_hook (const ep_char16_t *startup_hook_path); + /* * DiagnosticServer. */ diff --git a/src/native/eventpipe/ds-types.h b/src/native/eventpipe/ds-types.h index 85363f499e4046..16675059cf6cf4 100644 --- a/src/native/eventpipe/ds-types.h +++ b/src/native/eventpipe/ds-types.h @@ -24,6 +24,7 @@ typedef struct _DiagnosticsGenerateCoreDumpResponsePayload DiagnosticsGenerateCo typedef struct _DiagnosticsSetEnvironmentVariablePayload DiagnosticsSetEnvironmentVariablePayload; typedef struct _DiagnosticsGetEnvironmentVariablePayload DiagnosticsGetEnvironmentVariablePayload; typedef struct _DiagnosticsEnablePerfmapPayload DiagnosticsEnablePerfmapPayload; +typedef struct _DiagnosticsApplyStartupHookPayload DiagnosticsApplyStartupHookPayload; typedef struct _DiagnosticsIpcHeader DiagnosticsIpcHeader; typedef struct _DiagnosticsIpcMessage DiagnosticsIpcMessage; typedef struct _DiagnosticsListenPort DiagnosticsListenPort; @@ -74,7 +75,8 @@ typedef enum { DS_PROCESS_COMMANDID_SET_ENV_VAR = 0x03, DS_PROCESS_COMMANDID_GET_PROCESS_INFO_2 = 0x04, DS_PROCESS_COMMANDID_ENABLE_PERFMAP = 0x05, - DS_PROCESS_COMMANDID_DISABLE_PERFMAP = 0x06 + DS_PROCESS_COMMANDID_DISABLE_PERFMAP = 0x06, + DS_PROCESS_COMMANDID_APPLY_STARTUP_HOOK = 0x07 // future } DiagnosticsProcessCommandId; From bba1a15ca853ce79e4050aa8669927832f32938c Mon Sep 17 00:00:00 2001 From: Justin Anderson Date: Fri, 26 May 2023 14:42:35 -0700 Subject: [PATCH 02/23] Fix macro quoting --- src/coreclr/dlls/mscoree/exports.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/dlls/mscoree/exports.cpp b/src/coreclr/dlls/mscoree/exports.cpp index 53b047223d666f..86205c2a7228f9 100644 --- a/src/coreclr/dlls/mscoree/exports.cpp +++ b/src/coreclr/dlls/mscoree/exports.cpp @@ -247,10 +247,10 @@ static void CreateAppDomainProperties( // If one of the properties that was passed for intialization is the diganostic startup // hook AND the diagnostic startup hook was set in the host information, replace the // value by prefering the host information value. - if (u16_strcmp(configPropertyKeys[propertyIndex], TEXT(HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS)) == 0 && + if (u16_strcmp(configPropertyKeys[propertyIndex], _T(HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS)) == 0 && nullptr != diagnostic_startup_hooks_value) { - appDomainPropertyKeys[propertyIndex] = TEXT(HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS); + appDomainPropertyKeys[propertyIndex] = _T(HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS); appDomainPropertyValues[propertyIndex] = diagnostic_startup_hooks_value; if (!diagnostic_startup_hooks_handled) @@ -272,7 +272,7 @@ static void CreateAppDomainProperties( // Append diagnostic startup hook property if it has not been handled yet if (!diagnostic_startup_hooks_handled) { - appDomainPropertyKeys[propertyIndex] = TEXT(HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS); + appDomainPropertyKeys[propertyIndex] = _T(HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS); appDomainPropertyValues[propertyIndex] = diagnostic_startup_hooks_value; propertyIndex++; } From 5db28576ca34f0156673b93031697087fad64fa0 Mon Sep 17 00:00:00 2001 From: Justin Anderson Date: Tue, 30 May 2023 10:07:14 -0700 Subject: [PATCH 03/23] Revert "Fix macro quoting" --- src/coreclr/dlls/mscoree/exports.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/dlls/mscoree/exports.cpp b/src/coreclr/dlls/mscoree/exports.cpp index 86205c2a7228f9..53b047223d666f 100644 --- a/src/coreclr/dlls/mscoree/exports.cpp +++ b/src/coreclr/dlls/mscoree/exports.cpp @@ -247,10 +247,10 @@ static void CreateAppDomainProperties( // If one of the properties that was passed for intialization is the diganostic startup // hook AND the diagnostic startup hook was set in the host information, replace the // value by prefering the host information value. - if (u16_strcmp(configPropertyKeys[propertyIndex], _T(HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS)) == 0 && + if (u16_strcmp(configPropertyKeys[propertyIndex], TEXT(HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS)) == 0 && nullptr != diagnostic_startup_hooks_value) { - appDomainPropertyKeys[propertyIndex] = _T(HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS); + appDomainPropertyKeys[propertyIndex] = TEXT(HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS); appDomainPropertyValues[propertyIndex] = diagnostic_startup_hooks_value; if (!diagnostic_startup_hooks_handled) @@ -272,7 +272,7 @@ static void CreateAppDomainProperties( // Append diagnostic startup hook property if it has not been handled yet if (!diagnostic_startup_hooks_handled) { - appDomainPropertyKeys[propertyIndex] = _T(HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS); + appDomainPropertyKeys[propertyIndex] = TEXT(HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS); appDomainPropertyValues[propertyIndex] = diagnostic_startup_hooks_value; propertyIndex++; } From b6fc5edae1a8cd2fc80f6c4ec20d247ba4c6027d Mon Sep 17 00:00:00 2001 From: Justin Anderson Date: Tue, 30 May 2023 10:10:08 -0700 Subject: [PATCH 04/23] Revert "ApplyStartupHook diagnostic IPC command" --- src/coreclr/dlls/mscoree/exports.cpp | 108 +----------------- src/coreclr/inc/hostinformation.h | 1 - src/coreclr/vm/hostinformation.cpp | 8 -- src/native/corehost/host_runtime_contract.h | 8 -- .../hostpolicy/hostpolicy_context.cpp | 27 ----- 5 files changed, 3 insertions(+), 149 deletions(-) diff --git a/src/coreclr/dlls/mscoree/exports.cpp b/src/coreclr/dlls/mscoree/exports.cpp index 53b047223d666f..d14471f3b6bb8d 100644 --- a/src/coreclr/dlls/mscoree/exports.cpp +++ b/src/coreclr/dlls/mscoree/exports.cpp @@ -147,8 +147,6 @@ static void ConvertConfigPropertiesToUnicode( LPCWSTR* propertyValuesW = new (nothrow) LPCWSTR[propertyCount]; ASSERTE_ALL_BUILDS(propertyValuesW != nullptr); - LPCWSTR diagnostic_startup_hooks_value = nullptr; - for (int propertyIndex = 0; propertyIndex < propertyCount; ++propertyIndex) { propertyKeysW[propertyIndex] = StringToUnicode(propertyKeys[propertyIndex]); @@ -191,99 +189,12 @@ static void ConvertConfigPropertiesToUnicode( if (hostContractLocal->pinvoke_override != nullptr) *pinvokeOverride = hostContractLocal->pinvoke_override; } - else if (strcmp(propertyKeys[propertyIndex], HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS) == 0) - { - diagnostic_startup_hooks_value = propertyValuesW[propertyIndex]; - } - } - - if (nullptr != diagnostic_startup_hooks_value) - { - HostInformation::SetProperty(HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS, diagnostic_startup_hooks_value); } *propertyKeysWRef = propertyKeysW; *propertyValuesWRef = propertyValuesW; } -static void CreateAppDomainProperties( - LPCWSTR* configPropertyKeys, - LPCWSTR* configPropertyValues, - int configPropertyCount, - NewArrayHolder& appDomainPropertyKeys, - NewArrayHolder& appDomainPropertyValues, - int& appDomainPropertyCount, - ConstWStringArrayHolder& allocatedStrings) -{ - // Dynamically create list of properties to provide to app domain creation - // Some properties may have been dynamically specified through other means, - // such as the diagnostic IPC channel setting startup hooks. - appDomainPropertyCount = configPropertyCount; - - // Check if the diagnostic startup hook was specified in the host - bool diagnostic_startup_hooks_handled = true; - const WCHAR* diagnostic_startup_hooks_value = nullptr; - SString diagnostic_startup_hooks_value_str; - if (HostInformation::GetProperty(HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS, diagnostic_startup_hooks_value_str)) - { - diagnostic_startup_hooks_handled = false; - diagnostic_startup_hooks_value = diagnostic_startup_hooks_value_str.GetCopyOfUnicodeString(); - appDomainPropertyCount++; - } - - // These holders will not take ownership of the strings but provide a wrapper - // in order to allow using them as a single indexable array. The owners are still - // responsible for deleting the strings. - appDomainPropertyKeys = new (nothrow) LPCWSTR[appDomainPropertyCount]; - ASSERTE_ALL_BUILDS(appDomainPropertyKeys != nullptr); - - appDomainPropertyValues = new (nothrow) LPCWSTR[appDomainPropertyCount]; - ASSERTE_ALL_BUILDS(appDomainPropertyValues != nullptr); - - // Add properties that were explicitly passed for initialization - int propertyIndex = 0; - while (propertyIndex < configPropertyCount) - { - // If one of the properties that was passed for intialization is the diganostic startup - // hook AND the diagnostic startup hook was set in the host information, replace the - // value by prefering the host information value. - if (u16_strcmp(configPropertyKeys[propertyIndex], TEXT(HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS)) == 0 && - nullptr != diagnostic_startup_hooks_value) - { - appDomainPropertyKeys[propertyIndex] = TEXT(HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS); - appDomainPropertyValues[propertyIndex] = diagnostic_startup_hooks_value; - - if (!diagnostic_startup_hooks_handled) - { - // Skip dynamically adding the diagnostic startup hook to the array holder since - // it already exists in the properties passed for initialization. - diagnostic_startup_hooks_handled = true; - appDomainPropertyCount--; - } - } - else - { - appDomainPropertyKeys[propertyIndex] = configPropertyKeys[propertyIndex]; - appDomainPropertyValues[propertyIndex] = configPropertyValues[propertyIndex]; - } - propertyIndex++; - } - - // Append diagnostic startup hook property if it has not been handled yet - if (!diagnostic_startup_hooks_handled) - { - appDomainPropertyKeys[propertyIndex] = TEXT(HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS); - appDomainPropertyValues[propertyIndex] = diagnostic_startup_hooks_value; - propertyIndex++; - } - - allocatedStrings.Set(new (nothrow) LPCWSTR[1], 1); - ASSERTE_ALL_BUILDS(allocatedStrings != nullptr); - allocatedStrings[0] = diagnostic_startup_hooks_value; - - ASSERTE_ALL_BUILDS(propertyIndex == appDomainPropertyCount); -} - coreclr_error_writer_callback_fn g_errorWriter = nullptr; // @@ -409,27 +320,14 @@ int coreclr_initialize( hr = host->Start(); IfFailRet(hr); - NewArrayHolder appDomainPropertyKeys; - NewArrayHolder appDomainPropertyValues; - int appDomainPropertyCount; - ConstWStringArrayHolder allocatedStrings; - CreateAppDomainProperties( - propertyKeysW, - propertyValuesW, - propertyCount, - appDomainPropertyKeys, - appDomainPropertyValues, - appDomainPropertyCount, - allocatedStrings); - hr = host->CreateAppDomainWithManager( appDomainFriendlyNameW, APPDOMAIN_SECURITY_DEFAULT, NULL, // Name of the assembly that contains the AppDomainManager implementation NULL, // The AppDomainManager implementation type name - appDomainPropertyCount, - appDomainPropertyKeys, - appDomainPropertyValues, + propertyCount, + propertyKeysW, + propertyValuesW, (DWORD *)domainId); if (SUCCEEDED(hr)) diff --git a/src/coreclr/inc/hostinformation.h b/src/coreclr/inc/hostinformation.h index 5ff2e352fef680..d57b4729d30e68 100644 --- a/src/coreclr/inc/hostinformation.h +++ b/src/coreclr/inc/hostinformation.h @@ -11,7 +11,6 @@ class HostInformation public: static void SetContract(_In_ host_runtime_contract* hostContract); static bool GetProperty(_In_z_ const char* name, SString& value); - static bool SetProperty(_In_z_ const char* name, const SString& value); }; #endif // _HOSTINFORMATION_H_ diff --git a/src/coreclr/vm/hostinformation.cpp b/src/coreclr/vm/hostinformation.cpp index f5215ca11a382f..b440f6f2168ab7 100644 --- a/src/coreclr/vm/hostinformation.cpp +++ b/src/coreclr/vm/hostinformation.cpp @@ -40,11 +40,3 @@ bool HostInformation::GetProperty(_In_z_ const char* name, SString& value) return lenActual > 0 && lenActual <= len; } - -bool HostInformation::SetProperty(_In_z_ const char* name, const SString& value) -{ - if (s_hostContract == nullptr || s_hostContract->set_runtime_property == nullptr) - return false; - - return s_hostContract->set_runtime_property(name, value.GetUTF8(), s_hostContract->context); -} diff --git a/src/native/corehost/host_runtime_contract.h b/src/native/corehost/host_runtime_contract.h index 384820dd1841e4..11d98c3494de47 100644 --- a/src/native/corehost/host_runtime_contract.h +++ b/src/native/corehost/host_runtime_contract.h @@ -23,7 +23,6 @@ #define HOST_PROPERTY_PINVOKE_OVERRIDE "PINVOKE_OVERRIDE" #define HOST_PROPERTY_PLATFORM_RESOURCE_ROOTS "PLATFORM_RESOURCE_ROOTS" #define HOST_PROPERTY_TRUSTED_PLATFORM_ASSEMBLIES "TRUSTED_PLATFORM_ASSEMBLIES" -#define HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS "DIAGNOSTIC_STARTUP_HOOKS" struct host_runtime_contract { @@ -40,13 +39,6 @@ struct host_runtime_contract size_t value_buffer_size, void* contract_context); - // Set the value of a runtime property. - // Returns true if able to set successfully, false otherwise. - bool(HOST_CONTRACT_CALLTYPE* set_runtime_property)( - const char* key, - const char* value, - void* contract_context); - // Probe an app bundle for `path`. Sets its location (`offset`, `size`) in the bundle if found. // Returns true if found, false otherwise. bool(HOST_CONTRACT_CALLTYPE* bundle_probe)( diff --git a/src/native/corehost/hostpolicy/hostpolicy_context.cpp b/src/native/corehost/hostpolicy/hostpolicy_context.cpp index bced955e1c6feb..4c93ea300d7c52 100644 --- a/src/native/corehost/hostpolicy/hostpolicy_context.cpp +++ b/src/native/corehost/hostpolicy/hostpolicy_context.cpp @@ -133,32 +133,6 @@ namespace return -1; } - - bool HOST_CONTRACT_CALLTYPE set_runtime_property( - const char* key, - const char* value, - void* contract_context) - { - hostpolicy_context_t* context = static_cast(contract_context); - - pal::string_t key_str; - if (!pal::clr_palstring(key, &key_str)) - return false; - - if (nullptr != value) - { - pal::string_t value_str; - if (!pal::clr_palstring(value, &value_str)) - return false; - - context->coreclr_properties.add(key_str.c_str(), value_str.c_str()); - } - else - { - context->coreclr_properties.remove(key_str.c_str()); - } - return true; - } } bool hostpolicy_context_t::should_read_rid_fallback_graph(const hostpolicy_init_t &init) @@ -404,7 +378,6 @@ int hostpolicy_context_t::initialize(const hostpolicy_init_t &hostpolicy_init, c } host_contract.get_runtime_property = &get_runtime_property; - host_contract.set_runtime_property = &set_runtime_property; pal::stringstream_t ptr_stream; ptr_stream << "0x" << std::hex << (size_t)(&host_contract); if (!coreclr_properties.add(_STRINGIFY(HOST_PROPERTY_RUNTIME_CONTRACT), ptr_stream.str().c_str())) From a4604612012a6b990cbe55877e46060c3dbb0e59 Mon Sep 17 00:00:00 2001 From: Justin Anderson Date: Tue, 30 May 2023 14:11:49 -0700 Subject: [PATCH 05/23] Pass diagnostic startup hooks directly to StartupHookProvider --- .../src/System/StartupHookProvider.CoreCLR.cs | 4 +- src/coreclr/vm/assembly.cpp | 49 ++++++++++++++++++- src/coreclr/vm/assembly.hpp | 6 +++ src/coreclr/vm/corelib.h | 2 +- .../vm/eventing/eventpipe/ds-rt-coreclr.h | 16 ++---- src/coreclr/vm/metasig.h | 1 + .../src/System/StartupHookProvider.cs | 7 ++- 7 files changed, 63 insertions(+), 22 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/StartupHookProvider.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/StartupHookProvider.CoreCLR.cs index d277c177e1ef16..7e869219decca1 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/StartupHookProvider.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/StartupHookProvider.CoreCLR.cs @@ -12,7 +12,7 @@ namespace System { internal static partial class StartupHookProvider { - private static void ManagedStartup() + private static unsafe void ManagedStartup(char* pDiagnosticStartupHooks) { #if FEATURE_PERFTRACING if (EventSource.IsSupported) @@ -20,7 +20,7 @@ private static void ManagedStartup() #endif if (IsSupported) - ProcessStartupHooks(); + ProcessStartupHooks(pDiagnosticStartupHooks == null ? string.Empty : new string(pDiagnosticStartupHooks)); } } } diff --git a/src/coreclr/vm/assembly.cpp b/src/coreclr/vm/assembly.cpp index 7561e7d9e50559..63d44631f5337f 100644 --- a/src/coreclr/vm/assembly.cpp +++ b/src/coreclr/vm/assembly.cpp @@ -52,6 +52,7 @@ //#define STRICT_JITLOCK_ENTRY_LEAK_DETECTION //#define STRICT_CLSINITLOCK_ENTRY_LEAK_DETECTION +LPCWSTR Assembly::s_wszDiagnosticStartupHookPaths = nullptr; #ifndef DACCESS_COMPILE @@ -1122,7 +1123,47 @@ bool Assembly::IgnoresAccessChecksTo(Assembly *pAccessedAssembly) return GetFriendAssemblyInfo()->IgnoresAccessChecksTo(pAccessedAssembly); } +void Assembly::AddDiagnosticStartupHookPath(LPCWSTR wszPath) +{ + LPCWSTR wszDiagnosticStartupHookPathsLocal = s_wszDiagnosticStartupHookPaths; + + size_t cchPath = u16_strlen(wszPath); + size_t cchDiagnosticStartupHookPathsNew = cchPath; + size_t cchDiagnosticStartupHookPathsLocal = 0; + if (nullptr != wszDiagnosticStartupHookPathsLocal) + { + cchDiagnosticStartupHookPathsLocal = u16_strlen(wszDiagnosticStartupHookPathsLocal); + // Add 1 for the path separator + cchDiagnosticStartupHookPathsNew += cchDiagnosticStartupHookPathsLocal + 1; + } + size_t currentSize = cchDiagnosticStartupHookPathsNew + 1; + LPWSTR wszDiagnosticStartupHookPathsNew = new WCHAR[currentSize]; + LPWSTR wszCurrent = wszDiagnosticStartupHookPathsNew; + + u16_strcpy_s(wszCurrent, currentSize, wszPath); + wszCurrent += cchPath; + currentSize -= cchPath; + + if (cchDiagnosticStartupHookPathsLocal > 0) + { + u16_strcpy_s(wszCurrent, currentSize, PATH_SEPARATOR_STR_W); + wszCurrent += 1; + currentSize -= 1; + + u16_strcpy_s(wszCurrent, currentSize, wszDiagnosticStartupHookPathsLocal); + wszCurrent += cchDiagnosticStartupHookPathsLocal; + currentSize -= cchDiagnosticStartupHookPathsLocal; + } + + // Expect null terminating character + _ASSERTE(currentSize == 1); + _ASSERTE(wszCurrent[0] == W('\0')); + + s_wszDiagnosticStartupHookPaths = wszDiagnosticStartupHookPathsNew; + + delete [] wszDiagnosticStartupHookPathsLocal; +} enum CorEntryPointType { @@ -1376,7 +1417,7 @@ static void RunMainPost() } } -static void RunManagedStartup() +void Assembly::RunManagedStartup() { CONTRACTL { @@ -1388,7 +1429,11 @@ static void RunManagedStartup() CONTRACTL_END; MethodDescCallSite managedStartup(METHOD__STARTUP_HOOK_PROVIDER__MANAGED_STARTUP); - managedStartup.Call(NULL); + + ARG_SLOT args[1]; + args[0] = PtrToArgSlot(s_wszDiagnosticStartupHookPaths); + + managedStartup.Call(args); } INT32 Assembly::ExecuteMainMethod(PTRARRAYREF *stringArgs, BOOL waitForOtherThreads) diff --git a/src/coreclr/vm/assembly.hpp b/src/coreclr/vm/assembly.hpp index 43e95cfe0902bd..ab70b0c4a9df42 100644 --- a/src/coreclr/vm/assembly.hpp +++ b/src/coreclr/vm/assembly.hpp @@ -380,6 +380,8 @@ class Assembly } #endif + static void AddDiagnosticStartupHookPath(LPCWSTR wszPath); + protected: #ifdef FEATURE_COMINTEROP @@ -425,6 +427,9 @@ class Assembly #ifndef DACCESS_COMPILE ReleaseHolder GetFriendAssemblyInfo(); #endif + + void RunManagedStartup(); + public: void UpdateCachedFriendAssemblyInfo(); private: @@ -464,6 +469,7 @@ class Assembly IsInstrumentedStatus m_isInstrumentedStatus; #endif // FEATURE_READYTORUN + static LPCWSTR s_wszDiagnosticStartupHookPaths; }; #ifndef DACCESS_COMPILE diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index ed41cec4fa948a..90cf7a9241d91d 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -812,7 +812,7 @@ DEFINE_FIELD_U(rgiLastFrameFromForeignExceptionStackTrace, StackFrame DEFINE_FIELD_U(iFrameCount, StackFrameHelper, iFrameCount) DEFINE_CLASS(STARTUP_HOOK_PROVIDER, System, StartupHookProvider) -DEFINE_METHOD(STARTUP_HOOK_PROVIDER, MANAGED_STARTUP, ManagedStartup, SM_RetVoid) +DEFINE_METHOD(STARTUP_HOOK_PROVIDER, MANAGED_STARTUP, ManagedStartup, SM_PtrChar_RetVoid) DEFINE_CLASS(STREAM, IO, Stream) DEFINE_METHOD(STREAM, BEGIN_READ, BeginRead, IM_ArrByte_Int_Int_AsyncCallback_Object_RetIAsyncResult) diff --git a/src/coreclr/vm/eventing/eventpipe/ds-rt-coreclr.h b/src/coreclr/vm/eventing/eventpipe/ds-rt-coreclr.h index 9d7b39e12a39f7..7ea2201b9787b7 100644 --- a/src/coreclr/vm/eventing/eventpipe/ds-rt-coreclr.h +++ b/src/coreclr/vm/eventing/eventpipe/ds-rt-coreclr.h @@ -329,6 +329,8 @@ ds_rt_disable_perfmap (void) #endif // FEATURE_PERFMAP } +static ep_char16_t * _ds_rt_coreclr_diagnostic_startup_hook_paths = NULL; + static uint32_t ds_rt_apply_startup_hook (const ep_char16_t *startup_hook_path) @@ -343,19 +345,7 @@ ds_rt_apply_startup_hook (const ep_char16_t *startup_hook_path) } else { - // Prepend the new startup hook to the existing list of diagnostic startup hooks - SString new_paths(reinterpret_cast(startup_hook_path)); - - SString old_paths; - if (HostInformation::GetProperty(HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS, old_paths) && - !old_paths.IsEmpty()) - { - new_paths.Append(PATH_SEPARATOR_STR_W); - new_paths.Append(old_paths); - } - - if (!HostInformation::SetProperty(HOST_PROPERTY_DIAGNOSTIC_STARTUP_HOOKS, new_paths)) - return DS_IPC_E_FAIL; + Assembly::AddDiagnosticStartupHookPath(reinterpret_cast(startup_hook_path)); } return DS_IPC_S_OK; diff --git a/src/coreclr/vm/metasig.h b/src/coreclr/vm/metasig.h index 913280c839a161..6adf044a0170df 100644 --- a/src/coreclr/vm/metasig.h +++ b/src/coreclr/vm/metasig.h @@ -338,6 +338,7 @@ DEFINE_METASIG(SM(IntPtr_Bool_RetVoid, I F, v)) DEFINE_METASIG(SM(IntPtr_UInt_IntPtr_RetVoid, I K I, v)) DEFINE_METASIG(SM(IntPtr_RetUInt, I, K)) DEFINE_METASIG(SM(PtrChar_RetInt, P(u), i)) +DEFINE_METASIG(SM(PtrChar_RetVoid, P(u), v)) DEFINE_METASIG(SM(IntPtr_IntPtr_RetIntPtr, I I, I)) DEFINE_METASIG(SM(IntPtr_IntPtr_Int_RetIntPtr, I I i, I)) DEFINE_METASIG(SM(PtrVoid_PtrVoid_RetVoid, P(v) P(v), v)) diff --git a/src/libraries/System.Private.CoreLib/src/System/StartupHookProvider.cs b/src/libraries/System.Private.CoreLib/src/System/StartupHookProvider.cs index d21640f7b2c402..8e849d9722443c 100644 --- a/src/libraries/System.Private.CoreLib/src/System/StartupHookProvider.cs +++ b/src/libraries/System.Private.CoreLib/src/System/StartupHookProvider.cs @@ -28,17 +28,16 @@ private struct StartupHookNameOrPath // Parse a string specifying a list of assemblies and types // containing a startup hook, and call each hook in turn. - private static void ProcessStartupHooks() + private static void ProcessStartupHooks(string diagnosticStartupHooks) { if (!IsSupported) return; List startupHookParts = new(); - string? diagnosticStartupHooksVariable = AppContext.GetData("DIAGNOSTIC_STARTUP_HOOKS") as string; - if (null != diagnosticStartupHooksVariable) + if (!string.IsNullOrEmpty(diagnosticStartupHooks)) { - startupHookParts.AddRange(diagnosticStartupHooksVariable.Split(Path.PathSeparator)); + startupHookParts.AddRange(diagnosticStartupHooks.Split(Path.PathSeparator)); } string? startupHooksVariable = AppContext.GetData("STARTUP_HOOKS") as string; From 0126f6a5b4b8c8e7fbbe6c09c5db05aa7258da12 Mon Sep 17 00:00:00 2001 From: Justin Anderson Date: Tue, 30 May 2023 16:10:19 -0700 Subject: [PATCH 06/23] Prevent allocation of List if no startup hooks exist --- .../src/System/StartupHookProvider.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/StartupHookProvider.cs b/src/libraries/System.Private.CoreLib/src/System/StartupHookProvider.cs index 8e849d9722443c..72232ba42d3adc 100644 --- a/src/libraries/System.Private.CoreLib/src/System/StartupHookProvider.cs +++ b/src/libraries/System.Private.CoreLib/src/System/StartupHookProvider.cs @@ -33,6 +33,10 @@ private static void ProcessStartupHooks(string diagnosticStartupHooks) if (!IsSupported) return; + string? startupHooksVariable = AppContext.GetData("STARTUP_HOOKS") as string; + if (null == startupHooksVariable && string.IsNullOrEmpty(diagnosticStartupHooks)) + return; + List startupHookParts = new(); if (!string.IsNullOrEmpty(diagnosticStartupHooks)) @@ -40,17 +44,11 @@ private static void ProcessStartupHooks(string diagnosticStartupHooks) startupHookParts.AddRange(diagnosticStartupHooks.Split(Path.PathSeparator)); } - string? startupHooksVariable = AppContext.GetData("STARTUP_HOOKS") as string; if (null != startupHooksVariable) { startupHookParts.AddRange(startupHooksVariable.Split(Path.PathSeparator)); } - if (startupHookParts.Count == 0) - { - return; - } - ReadOnlySpan disallowedSimpleAssemblyNameChars = stackalloc char[4] { Path.DirectorySeparatorChar, From 6e216ec85bf28959d1eda460c57c2509a28dc09c Mon Sep 17 00:00:00 2001 From: Justin Anderson Date: Tue, 30 May 2023 16:10:36 -0700 Subject: [PATCH 07/23] Simplify string creation --- .../src/System/StartupHookProvider.CoreCLR.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/StartupHookProvider.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/StartupHookProvider.CoreCLR.cs index 7e869219decca1..8afaddc63b45e0 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/StartupHookProvider.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/StartupHookProvider.CoreCLR.cs @@ -20,7 +20,7 @@ private static unsafe void ManagedStartup(char* pDiagnosticStartupHooks) #endif if (IsSupported) - ProcessStartupHooks(pDiagnosticStartupHooks == null ? string.Empty : new string(pDiagnosticStartupHooks)); + ProcessStartupHooks(new string(pDiagnosticStartupHooks)); } } } From bace87457f8545b7469cb427f591039dd05fb78a Mon Sep 17 00:00:00 2001 From: Justin Anderson Date: Tue, 30 May 2023 16:10:58 -0700 Subject: [PATCH 08/23] Update suppression --- .../src/ILLink/ILLink.Suppressions.LibraryBuild.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Suppressions.LibraryBuild.xml b/src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Suppressions.LibraryBuild.xml index 1dbf49097f8f5c..386aa1169c2dbf 100644 --- a/src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Suppressions.LibraryBuild.xml +++ b/src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Suppressions.LibraryBuild.xml @@ -19,7 +19,7 @@ ILLink IL2026 member - M:System.StartupHookProvider.ProcessStartupHooks() + M:System.StartupHookProvider.ProcessStartupHooks(string) This warning is left in the product so developers get an ILLink warning when trimming an app with System.StartupHookProvider.IsSupported=true. From 71c64044131939d435ae12c1ffbed749ce4b6021 Mon Sep 17 00:00:00 2001 From: Justin Anderson Date: Wed, 31 May 2023 09:30:46 -0700 Subject: [PATCH 09/23] Fix suppression again --- .../src/ILLink/ILLink.Suppressions.LibraryBuild.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Suppressions.LibraryBuild.xml b/src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Suppressions.LibraryBuild.xml index 386aa1169c2dbf..278ed933f07043 100644 --- a/src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Suppressions.LibraryBuild.xml +++ b/src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Suppressions.LibraryBuild.xml @@ -19,7 +19,7 @@ ILLink IL2026 member - M:System.StartupHookProvider.ProcessStartupHooks(string) + M:System.StartupHookProvider.ProcessStartupHooks(System.String) This warning is left in the product so developers get an ILLink warning when trimming an app with System.StartupHookProvider.IsSupported=true. From 030a89df06d5ef390857d694a86335610937d380 Mon Sep 17 00:00:00 2001 From: Justin Anderson Date: Wed, 31 May 2023 09:54:10 -0700 Subject: [PATCH 10/23] Fix StartupHookTests tests --- .../Loader/StartupHooks/StartupHookTests.cs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/tests/Loader/StartupHooks/StartupHookTests.cs b/src/tests/Loader/StartupHooks/StartupHookTests.cs index 9bab54da74b162..971818d54ceb59 100644 --- a/src/tests/Loader/StartupHooks/StartupHookTests.cs +++ b/src/tests/Loader/StartupHooks/StartupHookTests.cs @@ -14,7 +14,7 @@ public unsafe class StartupHookTests private static Type s_startupHookProvider = typeof(object).Assembly.GetType("System.StartupHookProvider", throwOnError: true); - private static delegate* ProcessStartupHooks = (delegate*)s_startupHookProvider.GetMethod("ProcessStartupHooks", BindingFlags.NonPublic | BindingFlags.Static).MethodHandle.GetFunctionPointer(); + private static delegate* ProcessStartupHooks = (delegate*)s_startupHookProvider.GetMethod("ProcessStartupHooks", BindingFlags.NonPublic | BindingFlags.Static).MethodHandle.GetFunctionPointer(); private static bool IsUnsupportedPlatform = // these platforms need special setup for startup hooks @@ -38,7 +38,7 @@ public static void ValidHookName() hook.CallCount = 0; Assert.Equal(0, hook.CallCount); - ProcessStartupHooks(); + ProcessStartupHooks(string.Empty); Assert.Equal(1, hook.CallCount); } @@ -54,7 +54,7 @@ public static void ValidHookPath() hook.CallCount = 0; Assert.Equal(0, hook.CallCount); - ProcessStartupHooks(); + ProcessStartupHooks(string.Empty); Assert.Equal(1, hook.CallCount); } @@ -73,7 +73,7 @@ public static void MultipleValidHooksAndSeparators() Assert.Equal(0, hook1.CallCount); Assert.Equal(0, hook2.CallCount); - ProcessStartupHooks(); + ProcessStartupHooks(string.Empty); Assert.Equal(1, hook1.CallCount); Assert.Equal(1, hook2.CallCount); } @@ -89,7 +89,7 @@ public static void MissingAssembly(bool useAssemblyName) AppContext.SetData(StartupHookKey, $"{Hook.Basic.Value}{Path.PathSeparator}{hook}"); Hook.Basic.CallCount = 0; - var ex = Assert.Throws(() => ProcessStartupHooks()); + var ex = Assert.Throws(() => ProcessStartupHooks(string.Empty)); Assert.Equal($"Startup hook assembly '{hook}' failed to load. See inner exception for details.", ex.Message); Assert.IsType(ex.InnerException); @@ -109,7 +109,7 @@ public static void InvalidAssembly() AppContext.SetData(StartupHookKey, $"{Hook.Basic.Value}{Path.PathSeparator}{hook}"); Hook.Basic.CallCount = 0; - var ex = Assert.Throws(() => ProcessStartupHooks()); + var ex = Assert.Throws(() => ProcessStartupHooks(string.Empty)); Assert.Equal($"Startup hook assembly '{hook}' failed to load. See inner exception for details.", ex.Message); var innerEx = ex.InnerException; Assert.IsType(ex.InnerException); @@ -142,7 +142,7 @@ public static void InvalidSimpleAssemblyName(string name, bool failsSimpleNameCh AppContext.SetData(StartupHookKey, $"{Hook.Basic.Value}{Path.PathSeparator}{name}"); Hook.Basic.CallCount = 0; - var ex = Assert.Throws(() => ProcessStartupHooks()); + var ex = Assert.Throws(() => ProcessStartupHooks(string.Empty)); Assert.StartsWith($"The startup hook simple assembly name '{name}' is invalid.", ex.Message); if (failsSimpleNameCheck) { @@ -167,7 +167,7 @@ public static void MissingStartupHookType() var asm = typeof(StartupHookTests).Assembly; string hook = asm.Location; AppContext.SetData(StartupHookKey, hook); - var ex = Assert.Throws(() => ProcessStartupHooks()); + var ex = Assert.Throws(() => ProcessStartupHooks(string.Empty)); Assert.StartsWith($"Could not load type 'StartupHook' from assembly '{asm.GetName().Name}", ex.Message); } @@ -177,7 +177,7 @@ public static void MissingInitializeMethod() Console.WriteLine($"Running {nameof(MissingInitializeMethod)}..."); AppContext.SetData(StartupHookKey, Hook.NoInitializeMethod.Value); - var ex = Assert.Throws(() => ProcessStartupHooks()); + var ex = Assert.Throws(() => ProcessStartupHooks(string.Empty)); Assert.Equal($"Method 'StartupHook.Initialize' not found.", ex.Message); } @@ -196,7 +196,7 @@ public static void IncorrectInitializeSignature(Hook hook) Console.WriteLine($"Running {nameof(IncorrectInitializeSignature)}({hook.Name})..."); AppContext.SetData(StartupHookKey, hook.Value); - var ex = Assert.Throws(() => ProcessStartupHooks()); + var ex = Assert.Throws(() => ProcessStartupHooks(string.Empty)); Assert.Equal($"The signature of the startup hook 'StartupHook.Initialize' in assembly '{hook.Value}' was invalid. It must be 'public static void Initialize()'.", ex.Message); } } From bc11b8f310eb0bee152e5ed622ea8a4b59d4f5d1 Mon Sep 17 00:00:00 2001 From: Justin Anderson Date: Wed, 31 May 2023 17:23:17 -0700 Subject: [PATCH 11/23] Fix ProcessStartupHooks invocation on Mono --- src/mono/mono/metadata/object.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/mono/mono/metadata/object.c b/src/mono/mono/metadata/object.c index fab53233699b6a..bba7df0968a0ac 100644 --- a/src/mono/mono/metadata/object.c +++ b/src/mono/mono/metadata/object.c @@ -8135,7 +8135,11 @@ mono_runtime_run_startup_hooks (void) mono_error_cleanup (error); if (!method) return; - mono_runtime_invoke_checked (method, NULL, NULL, error); + + gpointer args [1]; + args[0] = mono_string_empty_wrapper (); + + mono_runtime_invoke_checked (method, NULL, args, error); // runtime hooks design doc says not to catch exceptions from the hooks mono_error_raise_exception_deprecated (error); } From 9c970ce45eefaa287d1b47736567469a6e38eeaa Mon Sep 17 00:00:00 2001 From: Justin Anderson Date: Wed, 31 May 2023 23:06:34 -0700 Subject: [PATCH 12/23] Use non-deprecated functions --- src/mono/mono/metadata/object.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mono/mono/metadata/object.c b/src/mono/mono/metadata/object.c index bba7df0968a0ac..8604114fe520f5 100644 --- a/src/mono/mono/metadata/object.c +++ b/src/mono/mono/metadata/object.c @@ -8137,7 +8137,7 @@ mono_runtime_run_startup_hooks (void) return; gpointer args [1]; - args[0] = mono_string_empty_wrapper (); + args[0] = mono_string_empty_internal (mono_domain_get ()); mono_runtime_invoke_checked (method, NULL, args, error); // runtime hooks design doc says not to catch exceptions from the hooks From 11240022a72543101e560f0b0ebb03ba8be6e26d Mon Sep 17 00:00:00 2001 From: Justin Anderson Date: Thu, 1 Jun 2023 09:05:43 -0700 Subject: [PATCH 13/23] Use static global but local to assembly.cpp --- src/coreclr/vm/assembly.cpp | 4 ++-- src/coreclr/vm/assembly.hpp | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/coreclr/vm/assembly.cpp b/src/coreclr/vm/assembly.cpp index 63d44631f5337f..d196eee3fd9504 100644 --- a/src/coreclr/vm/assembly.cpp +++ b/src/coreclr/vm/assembly.cpp @@ -52,7 +52,7 @@ //#define STRICT_JITLOCK_ENTRY_LEAK_DETECTION //#define STRICT_CLSINITLOCK_ENTRY_LEAK_DETECTION -LPCWSTR Assembly::s_wszDiagnosticStartupHookPaths = nullptr; +LPCWSTR s_wszDiagnosticStartupHookPaths = nullptr; #ifndef DACCESS_COMPILE @@ -1417,7 +1417,7 @@ static void RunMainPost() } } -void Assembly::RunManagedStartup() +void RunManagedStartup() { CONTRACTL { diff --git a/src/coreclr/vm/assembly.hpp b/src/coreclr/vm/assembly.hpp index ab70b0c4a9df42..245ec91802bf7f 100644 --- a/src/coreclr/vm/assembly.hpp +++ b/src/coreclr/vm/assembly.hpp @@ -428,7 +428,6 @@ class Assembly ReleaseHolder GetFriendAssemblyInfo(); #endif - void RunManagedStartup(); public: void UpdateCachedFriendAssemblyInfo(); @@ -469,7 +468,6 @@ class Assembly IsInstrumentedStatus m_isInstrumentedStatus; #endif // FEATURE_READYTORUN - static LPCWSTR s_wszDiagnosticStartupHookPaths; }; #ifndef DACCESS_COMPILE From 52f3a6817db4736f32b496a6387f1f2ff65d6e52 Mon Sep 17 00:00:00 2001 From: Justin Anderson Date: Thu, 1 Jun 2023 09:24:10 -0700 Subject: [PATCH 14/23] Add managed tests for diagnostic startup hooks --- .../Loader/StartupHooks/StartupHookTests.cs | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/tests/Loader/StartupHooks/StartupHookTests.cs b/src/tests/Loader/StartupHooks/StartupHookTests.cs index 971818d54ceb59..2e86cdeb1a330e 100644 --- a/src/tests/Loader/StartupHooks/StartupHookTests.cs +++ b/src/tests/Loader/StartupHooks/StartupHookTests.cs @@ -78,6 +78,46 @@ public static void MultipleValidHooksAndSeparators() Assert.Equal(1, hook2.CallCount); } + [Fact] + public static void MultipleValidDiagnosticHooksAndSeparators() + { + Console.WriteLine($"Running {nameof(MultipleValidDiagnosticHooksAndSeparators)}..."); + + Hook hook1 = Hook.Basic; + Hook hook2 = Hook.PrivateInitialize; + // Use multiple diagnostic hooks with an empty entry and leading/trailing separators + string diagnosticStartupHooks = $"{Path.PathSeparator}{hook1.Value}{Path.PathSeparator}{Path.PathSeparator}{hook2.Value}{Path.PathSeparator}"; + + AppContext.SetData(StartupHookKey, null); + hook1.CallCount = 0; + hook2.CallCount = 0; + + Assert.Equal(0, hook1.CallCount); + Assert.Equal(0, hook2.CallCount); + ProcessStartupHooks(diagnosticStartupHooks); + Assert.Equal(1, hook1.CallCount); + Assert.Equal(1, hook2.CallCount); + } + + [Fact] + public static void MultipleValidDiagnosticAndStandardHooks() + { + Console.WriteLine($"Running {nameof(MultipleValidDiagnosticAndStandardHooks)}..."); + + Hook hook1 = Hook.Basic; + Hook hook2 = Hook.PrivateInitialize; + + AppContext.SetData(StartupHookKey, host2.Value); + hook1.CallCount = 0; + hook2.CallCount = 0; + + Assert.Equal(0, hook1.CallCount); + Assert.Equal(0, hook2.CallCount); + ProcessStartupHooks(host1.Value); + Assert.Equal(1, hook1.CallCount); + Assert.Equal(1, hook2.CallCount); + } + [Theory] [InlineData(true)] [InlineData(false)] From d498905b65c87150efd6e6cacbd36a6fe7feac54 Mon Sep 17 00:00:00 2001 From: Justin Anderson Date: Thu, 1 Jun 2023 09:26:11 -0700 Subject: [PATCH 15/23] Remove extra white space --- src/coreclr/vm/assembly.hpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/coreclr/vm/assembly.hpp b/src/coreclr/vm/assembly.hpp index 245ec91802bf7f..c70c85861b54fd 100644 --- a/src/coreclr/vm/assembly.hpp +++ b/src/coreclr/vm/assembly.hpp @@ -427,8 +427,6 @@ class Assembly #ifndef DACCESS_COMPILE ReleaseHolder GetFriendAssemblyInfo(); #endif - - public: void UpdateCachedFriendAssemblyInfo(); private: From d049f67e79a50e8a1d90dcaa0327f53ac32be903 Mon Sep 17 00:00:00 2001 From: Justin Anderson Date: Thu, 1 Jun 2023 10:33:00 -0700 Subject: [PATCH 16/23] Fix tests --- src/tests/Loader/StartupHooks/StartupHookTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests/Loader/StartupHooks/StartupHookTests.cs b/src/tests/Loader/StartupHooks/StartupHookTests.cs index 2e86cdeb1a330e..e0801f335222f3 100644 --- a/src/tests/Loader/StartupHooks/StartupHookTests.cs +++ b/src/tests/Loader/StartupHooks/StartupHookTests.cs @@ -107,13 +107,13 @@ public static void MultipleValidDiagnosticAndStandardHooks() Hook hook1 = Hook.Basic; Hook hook2 = Hook.PrivateInitialize; - AppContext.SetData(StartupHookKey, host2.Value); + AppContext.SetData(StartupHookKey, hook2.Value); hook1.CallCount = 0; hook2.CallCount = 0; Assert.Equal(0, hook1.CallCount); Assert.Equal(0, hook2.CallCount); - ProcessStartupHooks(host1.Value); + ProcessStartupHooks(hook1.Value); Assert.Equal(1, hook1.CallCount); Assert.Equal(1, hook2.CallCount); } From d970ae6f701ccce430b0615e40b4fda2d0a9655a Mon Sep 17 00:00:00 2001 From: Justin Anderson Date: Thu, 1 Jun 2023 14:34:11 -0700 Subject: [PATCH 17/23] Add event pipe test --- .../ApplyStartupHookValidation.cs | 116 ++++++++++++++++++ .../eventpipe/applystartuphook/Hook.cs | 59 +++++++++ .../applystartuphook/applystartuphook.csproj | 25 ++++ .../eventpipe/applystartuphook/hooks/Basic.cs | 21 ++++ .../applystartuphook/hooks/Basic.csproj | 9 ++ .../tracing/eventpipe/common/IpcUtils.cs | 1 + .../DiagnosticsClient/DiagnosticsClient.cs | 23 ++++ .../DiagnosticsIpc/IpcCommands.cs | 3 +- 8 files changed, 256 insertions(+), 1 deletion(-) create mode 100644 src/tests/tracing/eventpipe/applystartuphook/ApplyStartupHookValidation.cs create mode 100644 src/tests/tracing/eventpipe/applystartuphook/Hook.cs create mode 100644 src/tests/tracing/eventpipe/applystartuphook/applystartuphook.csproj create mode 100644 src/tests/tracing/eventpipe/applystartuphook/hooks/Basic.cs create mode 100644 src/tests/tracing/eventpipe/applystartuphook/hooks/Basic.csproj diff --git a/src/tests/tracing/eventpipe/applystartuphook/ApplyStartupHookValidation.cs b/src/tests/tracing/eventpipe/applystartuphook/ApplyStartupHookValidation.cs new file mode 100644 index 00000000000000..cacd7ee777dc18 --- /dev/null +++ b/src/tests/tracing/eventpipe/applystartuphook/ApplyStartupHookValidation.cs @@ -0,0 +1,116 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Diagnostics.Tracing; +using System.Diagnostics; +using System.Linq; +using System.Threading.Tasks; +using System.Collections.Generic; +using System.Reflection; +using Microsoft.Diagnostics.Tools.RuntimeClient; +using Tracing.Tests.Common; +using System.Text; +using System.Threading; +using System.IO; +using Microsoft.Diagnostics.NETCore.Client; +using Microsoft.Diagnostics.Tracing; + +namespace Tracing.Tests.ApplyStartupHookValidation +{ + public class ApplyStartupHookValidation + { + public static async Task TEST_ApplyStartupHookAtStartupSuspension() + { + bool fSuccess = true; + string serverName = ReverseServer.MakeServerAddress(); + Logger.logger.Log($"Server name is '{serverName}'"); + Task subprocessTask = Utils.RunSubprocess( + currentAssembly: Assembly.GetExecutingAssembly(), + environment: new Dictionary + { + { Utils.DiagnosticPortsEnvKey, serverName } + }, + duringExecution: async (_) => + { + ReverseServer server = new ReverseServer(serverName); + using (Stream stream = await server.AcceptAsync()) + { + IpcAdvertise advertise = IpcAdvertise.Parse(stream); + Console.WriteLine($"IpcAdvertise: {advertise}"); + + while (!Debugger.IsAttached) + { + Thread.Sleep(1000); + } + + int processId = (int)advertise.ProcessId; + + // While we are paused in startup, apply startup hook + DiagnosticsClient client = new DiagnosticsClient(processId); + + string startupHookPath = Hook.Basic.AssemblyPath; + Console.WriteLine($"Applying startup hook during startup suspension: {startupHookPath}"); + client.ApplyStartupHook(startupHookPath); + } + + using (Stream stream = await server.AcceptAsync()) + { + IpcAdvertise advertise = IpcAdvertise.Parse(stream); + Console.WriteLine($"IpcAdvertise: {advertise}"); + + Logger.logger.Log($"Send ResumeRuntime Diagnostics IPC Command"); + // send ResumeRuntime command (0x04=ProcessCommandSet, 0x01=ResumeRuntime commandid) + IpcMessage message = new(0x04,0x01); + Logger.logger.Log($"Sent: {message.ToString()}"); + IpcMessage response = IpcClient.SendMessage(stream, message); + Logger.logger.Log($"received: {response.ToString()}"); + } + } + ); + + fSuccess &= await subprocessTask; + + return fSuccess; + } + + public static async Task Main(string[] args) + { + if (args.Length >= 1) + { + Console.Out.WriteLine("Subprocess started! Waiting for input..."); + var input = Console.In.ReadLine(); // will block until data is sent across stdin + Console.Out.WriteLine($"Received '{input}'"); + + // Validate the startup hook was executed + int callCount = Hook.Basic.CallCount; + Console.Out.WriteLine($"Startup hook call count: {callCount}"); + return callCount > 0 ? 0 : -1; + } + + bool fSuccess = true; + if (!IpcTraceTest.EnsureCleanEnvironment()) + return -1; + IEnumerable tests = typeof(ApplyStartupHookValidation).GetMethods().Where(mi => mi.Name.StartsWith("TEST_")); + foreach (var test in tests) + { + Logger.logger.Log($"::== Running test: {test.Name}"); + bool result = true; + try + { + result = await (Task)test.Invoke(null, new object[] {}); + } + catch (Exception e) + { + result = false; + Logger.logger.Log(e.ToString()); + } + fSuccess &= result; + Logger.logger.Log($"Test passed: {result}"); + Logger.logger.Log($""); + + } + return fSuccess ? 100 : -1; + } + } +} diff --git a/src/tests/tracing/eventpipe/applystartuphook/Hook.cs b/src/tests/tracing/eventpipe/applystartuphook/Hook.cs new file mode 100644 index 00000000000000..bd49fc6c39a007 --- /dev/null +++ b/src/tests/tracing/eventpipe/applystartuphook/Hook.cs @@ -0,0 +1,59 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.IO; +using System.Reflection; + +public class Hook +{ + public static Hook Basic = new Hook(nameof(Basic)); + + public Hook(string name) + { + Name = name; + AssemblyPath = Path.Combine(AppContext.BaseDirectory, "hooks", $"{name}.dll"); + } + + public string Name { get; } + + public string AssemblyPath { get; } + + public unsafe int CallCount + { + get + { + if (TryGetCallCountProperty(out PropertyInfo callCount)) + { + delegate* getCallCount = (delegate*)callCount.GetMethod.MethodHandle.GetFunctionPointer(); + return getCallCount(); + } + + return 0; + } + } + + private bool TryGetCallCountProperty(out PropertyInfo callCount) + { + callCount = null; + Assembly asm = null; + foreach(Assembly loaded in AppDomain.CurrentDomain.GetAssemblies()) + { + if (loaded.GetName().Name == Name && loaded.Location == AssemblyPath) + { + asm = loaded; + break; + } + } + + if (asm == null) + return false; + + Type hook = asm.GetType("StartupHook"); + if (hook == null) + return false; + + callCount = hook.GetProperty(nameof(CallCount), BindingFlags.NonPublic | BindingFlags.Static); + return callCount != null; + } +} \ No newline at end of file diff --git a/src/tests/tracing/eventpipe/applystartuphook/applystartuphook.csproj b/src/tests/tracing/eventpipe/applystartuphook/applystartuphook.csproj new file mode 100644 index 00000000000000..a626c361cde0a9 --- /dev/null +++ b/src/tests/tracing/eventpipe/applystartuphook/applystartuphook.csproj @@ -0,0 +1,25 @@ + + + .NETCoreApp + exe + true + true + true + + true + true + + + + + + + + + + false + Content + Always + + + diff --git a/src/tests/tracing/eventpipe/applystartuphook/hooks/Basic.cs b/src/tests/tracing/eventpipe/applystartuphook/hooks/Basic.cs new file mode 100644 index 00000000000000..3603c747737d1c --- /dev/null +++ b/src/tests/tracing/eventpipe/applystartuphook/hooks/Basic.cs @@ -0,0 +1,21 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; + +internal class StartupHook +{ + private static int CallCount { get; set; } + + public static void Initialize() + { + // Normal success case with a simple startup hook. + Initialize(123); + } + + public static void Initialize(int input) + { + CallCount++; + Console.WriteLine($"-- Hello from startup hook with overload! Call count: {CallCount}"); + } +} diff --git a/src/tests/tracing/eventpipe/applystartuphook/hooks/Basic.csproj b/src/tests/tracing/eventpipe/applystartuphook/hooks/Basic.csproj new file mode 100644 index 00000000000000..7caea741aa659e --- /dev/null +++ b/src/tests/tracing/eventpipe/applystartuphook/hooks/Basic.csproj @@ -0,0 +1,9 @@ + + + Library + BuildOnly + + + + + diff --git a/src/tests/tracing/eventpipe/common/IpcUtils.cs b/src/tests/tracing/eventpipe/common/IpcUtils.cs index e2130f4525fe8e..9165a1348537e5 100644 --- a/src/tests/tracing/eventpipe/common/IpcUtils.cs +++ b/src/tests/tracing/eventpipe/common/IpcUtils.cs @@ -137,6 +137,7 @@ public static async Task RunSubprocess(Assembly currentAssembly, Dictionar Logger.logger.Log("Subprocess didn't exit in 5 seconds!"); } Logger.logger.Log($"SubProcess exited - Exit code: {process.ExitCode}"); + fSuccess &= process.ExitCode == 0; } catch (Exception e) { diff --git a/src/tests/tracing/eventpipe/common/Microsoft.Diagnostics.NETCore.Client/DiagnosticsClient/DiagnosticsClient.cs b/src/tests/tracing/eventpipe/common/Microsoft.Diagnostics.NETCore.Client/DiagnosticsClient/DiagnosticsClient.cs index 2d58ef8626d115..42ef1cb0da98ce 100644 --- a/src/tests/tracing/eventpipe/common/Microsoft.Diagnostics.NETCore.Client/DiagnosticsClient/DiagnosticsClient.cs +++ b/src/tests/tracing/eventpipe/common/Microsoft.Diagnostics.NETCore.Client/DiagnosticsClient/DiagnosticsClient.cs @@ -296,6 +296,20 @@ internal async Task> GetProcessEnvironmentAsync(Cance return await helper.ReadEnvironmentAsync(response.Continuation, token).ConfigureAwait(false); } + public void ApplyStartupHook(string startupHookPath) + { + IpcMessage message = CreateApplyStartupHookMessage(startupHookPath); + IpcMessage response = IpcClient.SendMessage(_endpoint, message); + ValidateResponseMessage(response, nameof(ApplyStartupHook)); + } + + internal async Task ApplyStartupHookAsync(string startupHookPath, CancellationToken token) + { + IpcMessage message = CreateApplyStartupHookMessage(startupHookPath); + IpcMessage response = await IpcClient.SendMessageAsync(_endpoint, message, token).ConfigureAwait(false); + ValidateResponseMessage(response, nameof(ApplyStartupHookAsync)); + } + /// /// Get all the active processes that can be attached to. /// @@ -541,6 +555,15 @@ private static IpcMessage CreateWriteDumpMessage(DumpCommandId command, DumpType return new IpcMessage(DiagnosticsServerCommandSet.Dump, (byte)command, payload); } + private static IpcMessage CreateApplyStartupHookMessage(string startupHookPath) + { + if (string.IsNullOrEmpty(startupHookPath)) + throw new ArgumentException($"{nameof(startupHookPath)} required"); + + byte[] serializedConfiguration = SerializePayload(startupHookPath); + return new IpcMessage(DiagnosticsServerCommandSet.Process, (byte)ProcessCommandId.ApplyStartupHook, serializedConfiguration); + } + private static ProcessInfo GetProcessInfoFromResponse(IpcResponse response, string operationName) { ValidateResponseMessage(response.Message, operationName); diff --git a/src/tests/tracing/eventpipe/common/Microsoft.Diagnostics.NETCore.Client/DiagnosticsIpc/IpcCommands.cs b/src/tests/tracing/eventpipe/common/Microsoft.Diagnostics.NETCore.Client/DiagnosticsIpc/IpcCommands.cs index 241acaee04eca8..d084272a481cba 100644 --- a/src/tests/tracing/eventpipe/common/Microsoft.Diagnostics.NETCore.Client/DiagnosticsIpc/IpcCommands.cs +++ b/src/tests/tracing/eventpipe/common/Microsoft.Diagnostics.NETCore.Client/DiagnosticsIpc/IpcCommands.cs @@ -50,6 +50,7 @@ internal enum ProcessCommandId : byte ResumeRuntime = 0x01, GetProcessEnvironment = 0x02, SetEnvironmentVariable = 0x03, - GetProcessInfo2 = 0x04 + GetProcessInfo2 = 0x04, + ApplyStartupHook = 0x07 } } From 941c8f4b5420e4a4a2207424aac513683052d9d4 Mon Sep 17 00:00:00 2001 From: Justin Anderson Date: Thu, 1 Jun 2023 16:56:08 -0700 Subject: [PATCH 18/23] Remove debug code --- .../eventpipe/applystartuphook/ApplyStartupHookValidation.cs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/tests/tracing/eventpipe/applystartuphook/ApplyStartupHookValidation.cs b/src/tests/tracing/eventpipe/applystartuphook/ApplyStartupHookValidation.cs index cacd7ee777dc18..17e7b84396a50a 100644 --- a/src/tests/tracing/eventpipe/applystartuphook/ApplyStartupHookValidation.cs +++ b/src/tests/tracing/eventpipe/applystartuphook/ApplyStartupHookValidation.cs @@ -39,11 +39,6 @@ public static async Task TEST_ApplyStartupHookAtStartupSuspension() IpcAdvertise advertise = IpcAdvertise.Parse(stream); Console.WriteLine($"IpcAdvertise: {advertise}"); - while (!Debugger.IsAttached) - { - Thread.Sleep(1000); - } - int processId = (int)advertise.ProcessId; // While we are paused in startup, apply startup hook From e3a28b57fc3a9483297c72a16753bcc3f1ffa351 Mon Sep 17 00:00:00 2001 From: Justin Anderson Date: Fri, 2 Jun 2023 09:00:59 -0700 Subject: [PATCH 19/23] Fix project path casing --- .../tracing/eventpipe/applystartuphook/applystartuphook.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/tracing/eventpipe/applystartuphook/applystartuphook.csproj b/src/tests/tracing/eventpipe/applystartuphook/applystartuphook.csproj index a626c361cde0a9..f9b81636b6231f 100644 --- a/src/tests/tracing/eventpipe/applystartuphook/applystartuphook.csproj +++ b/src/tests/tracing/eventpipe/applystartuphook/applystartuphook.csproj @@ -16,7 +16,7 @@ - + false Content Always From cfa89982137a6024863bab6a65ae7a2f17d54c9a Mon Sep 17 00:00:00 2001 From: Justin Anderson Date: Fri, 2 Jun 2023 11:49:35 -0700 Subject: [PATCH 20/23] Exclude from AOT; verify failure on Mono; small cleanup --- .../ApplyStartupHookValidation.cs | 33 +++++++++++++------ .../applystartuphook/applystartuphook.csproj | 3 +- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/src/tests/tracing/eventpipe/applystartuphook/ApplyStartupHookValidation.cs b/src/tests/tracing/eventpipe/applystartuphook/ApplyStartupHookValidation.cs index 17e7b84396a50a..71ec0981572d2a 100644 --- a/src/tests/tracing/eventpipe/applystartuphook/ApplyStartupHookValidation.cs +++ b/src/tests/tracing/eventpipe/applystartuphook/ApplyStartupHookValidation.cs @@ -2,19 +2,20 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.Diagnostics.Tracing; using System.Diagnostics; -using System.Linq; -using System.Threading.Tasks; +using System.Diagnostics.Tracing; using System.Collections.Generic; +using System.IO; +using System.Linq; using System.Reflection; -using Microsoft.Diagnostics.Tools.RuntimeClient; -using Tracing.Tests.Common; using System.Text; using System.Threading; -using System.IO; +using System.Threading.Tasks; using Microsoft.Diagnostics.NETCore.Client; +using Microsoft.Diagnostics.Tools.RuntimeClient; using Microsoft.Diagnostics.Tracing; +using TestLibrary; +using Tracing.Tests.Common; namespace Tracing.Tests.ApplyStartupHookValidation { @@ -37,7 +38,7 @@ public static async Task TEST_ApplyStartupHookAtStartupSuspension() using (Stream stream = await server.AcceptAsync()) { IpcAdvertise advertise = IpcAdvertise.Parse(stream); - Console.WriteLine($"IpcAdvertise: {advertise}"); + Logger.logger.Log($"IpcAdvertise: {advertise}"); int processId = (int)advertise.ProcessId; @@ -45,14 +46,26 @@ public static async Task TEST_ApplyStartupHookAtStartupSuspension() DiagnosticsClient client = new DiagnosticsClient(processId); string startupHookPath = Hook.Basic.AssemblyPath; - Console.WriteLine($"Applying startup hook during startup suspension: {startupHookPath}"); - client.ApplyStartupHook(startupHookPath); + Logger.logger.Log($"Applying startup hook during startup suspension: {startupHookPath}"); + try + { + client.ApplyStartupHook(startupHookPath); + + if (TestLibrary.Utilities.IsMonoRuntime) + { + throw new InvalidOperationException($"{nameof(DiagnosticsClient.ApplyStartupHook)} unexceptedly succeeded for target Mono runtime."); + } + } + catch (ServerErrorException) when (TestLibrary.Utilities.IsMonoRuntime) + { + Logger.logger.Log($"{nameof(DiagnosticsClient.ApplyStartupHook)} expectedly failed for target Mono runtime."); + } } using (Stream stream = await server.AcceptAsync()) { IpcAdvertise advertise = IpcAdvertise.Parse(stream); - Console.WriteLine($"IpcAdvertise: {advertise}"); + Logger.logger.Log($"IpcAdvertise: {advertise}"); Logger.logger.Log($"Send ResumeRuntime Diagnostics IPC Command"); // send ResumeRuntime command (0x04=ProcessCommandSet, 0x01=ResumeRuntime commandid) diff --git a/src/tests/tracing/eventpipe/applystartuphook/applystartuphook.csproj b/src/tests/tracing/eventpipe/applystartuphook/applystartuphook.csproj index f9b81636b6231f..fa6016c37df7bd 100644 --- a/src/tests/tracing/eventpipe/applystartuphook/applystartuphook.csproj +++ b/src/tests/tracing/eventpipe/applystartuphook/applystartuphook.csproj @@ -7,13 +7,14 @@ true true - true + true + From 9a32a94d63a650a99e3d7df5885e315293eb2e32 Mon Sep 17 00:00:00 2001 From: Justin Anderson Date: Fri, 2 Jun 2023 13:26:41 -0700 Subject: [PATCH 21/23] Skip test for mono --- .../ApplyStartupHookValidation.cs | 15 +-------------- .../applystartuphook/applystartuphook.csproj | 2 +- 2 files changed, 2 insertions(+), 15 deletions(-) diff --git a/src/tests/tracing/eventpipe/applystartuphook/ApplyStartupHookValidation.cs b/src/tests/tracing/eventpipe/applystartuphook/ApplyStartupHookValidation.cs index 71ec0981572d2a..45ddb28614bf2b 100644 --- a/src/tests/tracing/eventpipe/applystartuphook/ApplyStartupHookValidation.cs +++ b/src/tests/tracing/eventpipe/applystartuphook/ApplyStartupHookValidation.cs @@ -14,7 +14,6 @@ using Microsoft.Diagnostics.NETCore.Client; using Microsoft.Diagnostics.Tools.RuntimeClient; using Microsoft.Diagnostics.Tracing; -using TestLibrary; using Tracing.Tests.Common; namespace Tracing.Tests.ApplyStartupHookValidation @@ -47,19 +46,7 @@ public static async Task TEST_ApplyStartupHookAtStartupSuspension() string startupHookPath = Hook.Basic.AssemblyPath; Logger.logger.Log($"Applying startup hook during startup suspension: {startupHookPath}"); - try - { - client.ApplyStartupHook(startupHookPath); - - if (TestLibrary.Utilities.IsMonoRuntime) - { - throw new InvalidOperationException($"{nameof(DiagnosticsClient.ApplyStartupHook)} unexceptedly succeeded for target Mono runtime."); - } - } - catch (ServerErrorException) when (TestLibrary.Utilities.IsMonoRuntime) - { - Logger.logger.Log($"{nameof(DiagnosticsClient.ApplyStartupHook)} expectedly failed for target Mono runtime."); - } + client.ApplyStartupHook(startupHookPath); } using (Stream stream = await server.AcceptAsync()) diff --git a/src/tests/tracing/eventpipe/applystartuphook/applystartuphook.csproj b/src/tests/tracing/eventpipe/applystartuphook/applystartuphook.csproj index fa6016c37df7bd..54aa82d39fd6b6 100644 --- a/src/tests/tracing/eventpipe/applystartuphook/applystartuphook.csproj +++ b/src/tests/tracing/eventpipe/applystartuphook/applystartuphook.csproj @@ -8,13 +8,13 @@ true true + true - From 8e8b7221fcca3484dd2f17d13e791104f73da82e Mon Sep 17 00:00:00 2001 From: Justin Anderson Date: Mon, 5 Jun 2023 09:06:23 -0700 Subject: [PATCH 22/23] More test logging --- .../applystartuphook/ApplyStartupHookValidation.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/tests/tracing/eventpipe/applystartuphook/ApplyStartupHookValidation.cs b/src/tests/tracing/eventpipe/applystartuphook/ApplyStartupHookValidation.cs index 45ddb28614bf2b..3ca5d6436e541c 100644 --- a/src/tests/tracing/eventpipe/applystartuphook/ApplyStartupHookValidation.cs +++ b/src/tests/tracing/eventpipe/applystartuphook/ApplyStartupHookValidation.cs @@ -34,8 +34,11 @@ public static async Task TEST_ApplyStartupHookAtStartupSuspension() duringExecution: async (_) => { ReverseServer server = new ReverseServer(serverName); + Logger.logger.Log("Waiting to accept diagnostic connection."); using (Stream stream = await server.AcceptAsync()) { + Logger.logger.Log("Accepted diagnostic connection."); + IpcAdvertise advertise = IpcAdvertise.Parse(stream); Logger.logger.Log($"IpcAdvertise: {advertise}"); @@ -47,10 +50,14 @@ public static async Task TEST_ApplyStartupHookAtStartupSuspension() string startupHookPath = Hook.Basic.AssemblyPath; Logger.logger.Log($"Applying startup hook during startup suspension: {startupHookPath}"); client.ApplyStartupHook(startupHookPath); + Logger.logger.Log("Finished startup hook application"); } + Logger.logger.Log("Waiting to accept diagnostic connection."); using (Stream stream = await server.AcceptAsync()) { + Logger.logger.Log("Accepted diagnostic connection."); + IpcAdvertise advertise = IpcAdvertise.Parse(stream); Logger.logger.Log($"IpcAdvertise: {advertise}"); From e6504fe999e6a158a9e3fcba8dabdac0f68b915b Mon Sep 17 00:00:00 2001 From: Justin Anderson Date: Mon, 5 Jun 2023 14:28:34 -0700 Subject: [PATCH 23/23] Use reverse server stream --- .../ApplyStartupHookValidation.cs | 26 ++++++++++++------- .../DiagnosticsClient/DiagnosticsClient.cs | 2 +- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/tests/tracing/eventpipe/applystartuphook/ApplyStartupHookValidation.cs b/src/tests/tracing/eventpipe/applystartuphook/ApplyStartupHookValidation.cs index 3ca5d6436e541c..c62579fbdd3790 100644 --- a/src/tests/tracing/eventpipe/applystartuphook/ApplyStartupHookValidation.cs +++ b/src/tests/tracing/eventpipe/applystartuphook/ApplyStartupHookValidation.cs @@ -11,10 +11,10 @@ using System.Text; using System.Threading; using System.Threading.Tasks; -using Microsoft.Diagnostics.NETCore.Client; using Microsoft.Diagnostics.Tools.RuntimeClient; using Microsoft.Diagnostics.Tracing; using Tracing.Tests.Common; +using DiagnosticsClient = Microsoft.Diagnostics.NETCore.Client.DiagnosticsClient; namespace Tracing.Tests.ApplyStartupHookValidation { @@ -42,15 +42,12 @@ public static async Task TEST_ApplyStartupHookAtStartupSuspension() IpcAdvertise advertise = IpcAdvertise.Parse(stream); Logger.logger.Log($"IpcAdvertise: {advertise}"); - int processId = (int)advertise.ProcessId; - - // While we are paused in startup, apply startup hook - DiagnosticsClient client = new DiagnosticsClient(processId); - string startupHookPath = Hook.Basic.AssemblyPath; - Logger.logger.Log($"Applying startup hook during startup suspension: {startupHookPath}"); - client.ApplyStartupHook(startupHookPath); - Logger.logger.Log("Finished startup hook application"); + Logger.logger.Log($"Send ApplyStartupHook Diagnostic IPC: {startupHookPath}"); + IpcMessage message = CreateApplyStartupHookMessage(startupHookPath); + Logger.logger.Log($"Sent: {message.ToString()}"); + IpcMessage response = IpcClient.SendMessage(stream, message); + Logger.logger.Log($"Received: {response.ToString()}"); } Logger.logger.Log("Waiting to accept diagnostic connection."); @@ -66,7 +63,7 @@ public static async Task TEST_ApplyStartupHookAtStartupSuspension() IpcMessage message = new(0x04,0x01); Logger.logger.Log($"Sent: {message.ToString()}"); IpcMessage response = IpcClient.SendMessage(stream, message); - Logger.logger.Log($"received: {response.ToString()}"); + Logger.logger.Log($"Received: {response.ToString()}"); } } ); @@ -76,6 +73,15 @@ public static async Task TEST_ApplyStartupHookAtStartupSuspension() return fSuccess; } + private static IpcMessage CreateApplyStartupHookMessage(string startupHookPath) + { + if (string.IsNullOrEmpty(startupHookPath)) + throw new ArgumentException($"{nameof(startupHookPath)} required"); + + byte[] serializedConfiguration = DiagnosticsClient.SerializePayload(startupHookPath); + return new IpcMessage(0x04, 0x07, serializedConfiguration); + } + public static async Task Main(string[] args) { if (args.Length >= 1) diff --git a/src/tests/tracing/eventpipe/common/Microsoft.Diagnostics.NETCore.Client/DiagnosticsClient/DiagnosticsClient.cs b/src/tests/tracing/eventpipe/common/Microsoft.Diagnostics.NETCore.Client/DiagnosticsClient/DiagnosticsClient.cs index 42ef1cb0da98ce..20d8ccfb5d2f14 100644 --- a/src/tests/tracing/eventpipe/common/Microsoft.Diagnostics.NETCore.Client/DiagnosticsClient/DiagnosticsClient.cs +++ b/src/tests/tracing/eventpipe/common/Microsoft.Diagnostics.NETCore.Client/DiagnosticsClient/DiagnosticsClient.cs @@ -378,7 +378,7 @@ private async Task TryGetProcessInfo2Async(CancellationToken token) return TryGetProcessInfo2FromResponse(response2, nameof(GetProcessInfoAsync)); } - private static byte[] SerializePayload(T arg) + public static byte[] SerializePayload(T arg) { using (var stream = new MemoryStream()) using (var writer = new BinaryWriter(stream))