Remove 'random' usage in eventpipe and unify all usages of the minipa…

…l guid API instead of having three separate implementation of the exact same function body
dotnet · jkoritzinsky · Oct 31, 2024 · Oct 16, 2024 · Oct 16, 2024 · Oct 16, 2024
commit 2b4779be9c758f5c22d8d9c660ae7c42c4a4a4da
@@ -783,46 +783,6 @@ void ep_rt_aot_os_environment_get_utf16 (dn_vector_ptr_t *env_array)
 #endif
 }
 
-void ep_rt_aot_create_activity_id (uint8_t *activity_id, uint32_t activity_id_len)
-{
-    // We call CoCreateGuid for windows, and use a random generator for non-windows
-    STATIC_CONTRACT_NOTHROW;
-    EP_ASSERT (activity_id != NULL);
-    EP_ASSERT (activity_id_len == EP_ACTIVITY_ID_SIZE);
-#ifdef HOST_WIN32
-    CoCreateGuid (reinterpret_cast<GUID *>(activity_id));
-#else
-    if(minipal_get_cryptographically_secure_random_bytes(activity_id, activity_id_len)==-1)
-    {
-        *activity_id=0;
-        return;
-    }
-
-    const uint16_t version_mask = 0xF000;
-    const uint16_t random_guid_version = 0x4000;
-    const uint8_t clock_seq_hi_and_reserved_mask = 0xC0;
-    const uint8_t clock_seq_hi_and_reserved_value = 0x80;
-
-    // Modify bits indicating the type of the GUID
-    uint8_t *activity_id_c = activity_id + sizeof (uint32_t) + sizeof (uint16_t);
-    uint8_t *activity_id_d = activity_id + sizeof (uint32_t) + sizeof (uint16_t) + sizeof (uint16_t);
-
-    uint16_t c;
-    memcpy (&c, activity_id_c, sizeof (c));
-
-    uint8_t d;
-    memcpy (&d, activity_id_d, sizeof (d));
-
-    // time_hi_and_version
-    c = ((c & ~version_mask) | random_guid_version);
-    // clock_seq_hi_and_reserved
-    d = ((d & ~clock_seq_hi_and_reserved_mask) | clock_seq_hi_and_reserved_value);
-
-    memcpy (activity_id_c, &c, sizeof (c));
-    memcpy (activity_id_d, &d, sizeof (d));
-#endif
-}
-
 ep_rt_thread_handle_t ep_rt_aot_thread_get_handle (void)
 {
     return ThreadStore::GetCurrentThreadIfAvailable();

@@ -660,17 +660,6 @@ ep_rt_process_shutdown (void)
     return false;
 }
 
-static
-inline
-void
-ep_rt_create_activity_id (
-    uint8_t *activity_id,
-    uint32_t activity_id_len)
-{
-    extern void ep_rt_aot_create_activity_id (uint8_t *activity_id, uint32_t activity_id_len);
-    ep_rt_aot_create_activity_id(activity_id, activity_id_len);
-}
-
 static
 inline
 bool

@@ -198,7 +198,7 @@ EXTERN_C int QCALLTYPE EventPipeInternal_EventActivityIdControl(uint32_t control
 
         case ActivityControlCode::EVENT_ACTIVITY_CONTROL_CREATE_ID:
 
-            ep_rt_create_activity_id(reinterpret_cast<uint8_t *>(pActivityId), EP_ACTIVITY_ID_SIZE);
+            ep_thread_create_activity_id(reinterpret_cast<uint8_t *>(pActivityId), EP_ACTIVITY_ID_SIZE);
             break;
 
         case ActivityControlCode::EVENT_ACTIVITY_CONTROL_GET_SET_ID:
@@ -212,7 +212,7 @@ EXTERN_C int QCALLTYPE EventPipeInternal_EventActivityIdControl(uint32_t control
         case ActivityControlCode::EVENT_ACTIVITY_CONTROL_CREATE_SET_ID:
 
             ep_rt_thread_get_activity_id (activityIdHandle, reinterpret_cast<uint8_t *>(pActivityId), EP_ACTIVITY_ID_SIZE);
-            ep_rt_create_activity_id(reinterpret_cast<uint8_t *>(&currentActivityId), EP_ACTIVITY_ID_SIZE);
+            ep_thread_create_activity_id(reinterpret_cast<uint8_t *>(&currentActivityId), EP_ACTIVITY_ID_SIZE);
             ep_rt_thread_set_activity_id (activityIdHandle, reinterpret_cast<uint8_t *>(&currentActivityId), EP_ACTIVITY_ID_SIZE);
             break;
 

diff --git a/src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.h b/src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.h
@@ -738,20 +738,6 @@ ep_rt_process_shutdown (void)
 	return (bool)g_fEEShutDown;
 }
 
-static
-inline
-void
-ep_rt_create_activity_id (
-	uint8_t *activity_id,
-	uint32_t activity_id_len)
-{
-	STATIC_CONTRACT_NOTHROW;
-	EP_ASSERT (activity_id != NULL);
-	EP_ASSERT (activity_id_len == EP_ACTIVITY_ID_SIZE);
-
-	minipal_guid_v4_create (reinterpret_cast<minipal_guid_t *>(activity_id));
-}
-
 static
 inline
 bool

@@ -13,7 +13,6 @@
 #include <mono/metadata/profiler.h>
 #include <mono/mini/mini-runtime.h>
 #include <minipal/getexepath.h>
-#include <minipal/random.h>
 #include <runtime_version.h>
 #include <clretwallmain.h>
 
@@ -59,14 +58,6 @@ EVENTPIPE_TRACE_CONTEXT MICROSOFT_DOTNETRUNTIME_MONO_PROFILER_PROVIDER_DOTNET_Co
 void
 ep_rt_mono_thread_exited (void);
 
-bool
-ep_rt_mono_rand_try_get_bytes (
-	uint8_t *buffer,
-	size_t buffer_size)
-{
-	return minipal_get_cryptographically_secure_random_bytes(buffer, buffer_size);
-}
-
 char *
 ep_rt_mono_get_managed_cmd_line (void)
 {

@@ -69,7 +69,6 @@ extern void ep_rt_mono_provider_config_init (EventPipeProviderConfiguration *pro
 extern void ep_rt_mono_init_providers_and_events (void);
 extern bool ep_rt_mono_providers_validate_all_disabled (void);
 extern bool ep_rt_mono_sample_profiler_write_sampling_event_for_threads (ep_rt_thread_handle_t sampling_thread, EventPipeEvent *sampling_event);
-extern bool ep_rt_mono_rand_try_get_bytes (uint8_t *buffer,size_t buffer_size);
 extern void ep_rt_mono_execute_rundown (dn_vector_ptr_t *execution_checkpoints);
 extern int64_t ep_rt_mono_perf_counter_query (void);
 extern int64_t ep_rt_mono_perf_frequency_query (void);
@@ -772,42 +771,6 @@ ep_rt_process_shutdown (void)
 	return ep_rt_process_detach ();
 }
 
-static
-inline
-void
-ep_rt_create_activity_id (
-	uint8_t *activity_id,
-	uint32_t activity_id_len)
-{
-	EP_ASSERT (activity_id != NULL);
-	EP_ASSERT (activity_id_len == EP_ACTIVITY_ID_SIZE);
-
-	ep_rt_mono_rand_try_get_bytes ((guchar *)activity_id, EP_ACTIVITY_ID_SIZE);
-
-	const uint16_t version_mask = 0xF000;
-	const uint16_t random_guid_version = 0x4000;
-	const uint8_t clock_seq_hi_and_reserved_mask = 0xC0;
-	const uint8_t clock_seq_hi_and_reserved_value = 0x80;
-
-	// Modify bits indicating the type of the GUID
-	uint8_t *activity_id_c = activity_id + sizeof (uint32_t) + sizeof (uint16_t);
-	uint8_t *activity_id_d = activity_id + sizeof (uint32_t) + sizeof (uint16_t) + sizeof (uint16_t);
-
-	uint16_t c;
-	memcpy (&c, activity_id_c, sizeof (c));
-
-	uint8_t d;
-	memcpy (&d, activity_id_d, sizeof (d));
-
-	// time_hi_and_version
-	c = ((c & ~version_mask) | random_guid_version);
-	// clock_seq_hi_and_reserved
-	d = ((d & ~clock_seq_hi_and_reserved_mask) | clock_seq_hi_and_reserved_value);
-
-	memcpy (activity_id_c, &c, sizeof (c));
-	memcpy (activity_id_d, &d, sizeof (d));
-}
-
 static
 inline
 bool

diff --git a/src/native/eventpipe/ds-protocol.c b/src/native/eventpipe/ds-protocol.c
@@ -80,7 +80,7 @@ ds_ipc_advertise_cookie_v1_get (void)
 void
 ds_ipc_advertise_cookie_v1_init (void)
 {
-	ep_rt_create_activity_id ((uint8_t *)&_ds_ipc_advertise_cooike_v1, EP_GUID_SIZE);
+	ep_thread_create_activity_id ((uint8_t *)&_ds_ipc_advertise_cooike_v1, EP_GUID_SIZE);
 }
 
 /**

diff --git a/src/native/eventpipe/ep-rt.h b/src/native/eventpipe/ep-rt.h
@@ -270,12 +270,6 @@ static
 bool
 ep_rt_process_shutdown (void);
 
-static
-void
-ep_rt_create_activity_id (
-	uint8_t *activity_id,
-	uint32_t activity_id_len);
-
 static
 bool
 ep_rt_is_running (void);

diff --git a/src/native/eventpipe/ep-thread.h b/src/native/eventpipe/ep-thread.h
@@ -114,7 +114,10 @@ ep_thread_create_activity_id (
 	uint8_t *activity_id,
 	uint32_t activity_id_len)
 {
-	ep_rt_create_activity_id (activity_id, activity_id_len);
+	EP_ASSERT (activity_id != NULL);
+	EP_ASSERT (activity_id_len == EP_ACTIVITY_ID_SIZE);
+
+	minipal_guid_v4_create (reinterpret_cast<minipal_guid_t *>(activity_id));
 }
 
 static

diff --git a/src/native/eventpipe/ep-types.h b/src/native/eventpipe/ep-types.h
@@ -19,6 +19,7 @@
 #include <containers/dn-list.h>
 #include <containers/dn-queue.h>
 #include <containers/dn-umap-t.h>
+#include <minipal/guid.h>
 
 /*
  * EventFilterDescriptor.
-Original file line number
+Diff line change
@@ Expand Up / @@ -80,7 +80,7 @@ ds_ipc_advertise_cookie_v1_get (void) @@
     void
     ds_ipc_advertise_cookie_v1_init (void)
     {
-    	ep_rt_create_activity_id ((uint8_t *)&_ds_ipc_advertise_cooike_v1, EP_GUID_SIZE);
+    	ep_thread_create_activity_id ((uint8_t *)&_ds_ipc_advertise_cooike_v1, EP_GUID_SIZE);
     }
     /**
@@ Expand Down @@