From 5c5695fb05c62f42b969cc33119399e6c8fcf04e Mon Sep 17 00:00:00 2001
From: Brian Robbins <brianrob@microsoft.com>
Date: Thu, 25 Jan 2018 20:28:48 -0800
Subject: Fix Windows-Specific EventPipe Bugs (#16025)

* Modify IsEnabled macros used on Windows.

* Fix arithmetic error due to order of operations.
---
 src/inc/eventtracebase.h  | 19 +++++++++++++------
 src/vm/sampleprofiler.cpp | 11 +++++------
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/src/inc/eventtracebase.h b/src/inc/eventtracebase.h
index 344296f016..d73d72a63f 100644
--- a/src/inc/eventtracebase.h
+++ b/src/inc/eventtracebase.h
@@ -72,6 +72,13 @@ enum EtwThreadFlags
 #ifndef FEATURE_REDHAWK
 
 #if defined(FEATURE_EVENT_TRACE)
+
+#if defined(FEATURE_PERFTRACING)
+#define EVENT_PIPE_ENABLED() (EventPipeHelper::Enabled())
+#else
+#define EVENT_PIPE_ENABLED() (FALSE)
+#endif
+
 #if  !defined(FEATURE_PAL)
 
 //
@@ -79,27 +86,27 @@ enum EtwThreadFlags
 //
 
 #define ETW_TRACING_INITIALIZED(RegHandle) \
-    (g_pEtwTracer && RegHandle)
+    ((g_pEtwTracer && RegHandle) || EVENT_PIPE_ENABLED())
 
 //
 // Use this macro to check if an event is enabled
 // if the fields in the event are not cheap to calculate
 //
 #define ETW_EVENT_ENABLED(Context, EventDescriptor) \
-    (MCGEN_ENABLE_CHECK(Context, EventDescriptor))
+    ((MCGEN_ENABLE_CHECK(Context, EventDescriptor)) || EVENT_PIPE_ENABLED())
 
 //
 // Use this macro to check if a category of events is enabled
 //
 
 #define ETW_CATEGORY_ENABLED(Context, Level, Keyword) \
-    (Context.IsEnabled && McGenEventProviderEnabled(&Context, Level, Keyword))
+    ((Context.IsEnabled && McGenEventProviderEnabled(&Context, Level, Keyword)) || EVENT_PIPE_ENABLED())
 
 
 // This macro only checks if a provider is enabled
 // It does not check the flags and keywords for which it is enabled
 #define ETW_PROVIDER_ENABLED(ProviderSymbol)                 \
-        ProviderSymbol##_Context.IsEnabled
+        ((ProviderSymbol##_Context.IsEnabled) || EVENT_PIPE_ENABLED())
 
 
 #else //defined(FEATURE_PAL)
@@ -183,7 +190,7 @@ struct ProfilingScanContext;
 // Use this macro to check if ETW is initialized and the event is enabled
 //
 #define ETW_TRACING_ENABLED(Context, EventDescriptor) \
-    (Context.IsEnabled && ETW_TRACING_INITIALIZED(Context.RegistrationHandle) && ETW_EVENT_ENABLED(Context, EventDescriptor))
+    ((Context.IsEnabled && ETW_TRACING_INITIALIZED(Context.RegistrationHandle) && ETW_EVENT_ENABLED(Context, EventDescriptor)) || EVENT_PIPE_ENABLED())
 
 //
 // Using KEYWORDZERO means when checking the events category ignore the keyword
@@ -194,7 +201,7 @@ struct ProfilingScanContext;
 // Use this macro to check if ETW is initialized and the category is enabled
 //
 #define ETW_TRACING_CATEGORY_ENABLED(Context, Level, Keyword) \
-    (ETW_TRACING_INITIALIZED(Context.RegistrationHandle) && ETW_CATEGORY_ENABLED(Context, Level, Keyword))
+    ((ETW_TRACING_INITIALIZED(Context.RegistrationHandle) && ETW_CATEGORY_ENABLED(Context, Level, Keyword)) || EVENT_PIPE_ENABLED())
 
     #define ETWOnStartup(StartEventName, EndEventName) \
         ETWTraceStartup trace##StartEventName##(Microsoft_Windows_DotNETRuntimePrivateHandle, &StartEventName, &StartupId, &EndEventName, &StartupId);
diff --git a/src/vm/sampleprofiler.cpp b/src/vm/sampleprofiler.cpp
index ade21669c4..4548620662 100644
--- a/src/vm/sampleprofiler.cpp
+++ b/src/vm/sampleprofiler.cpp
@@ -15,8 +15,7 @@
 #include <mmsystem.h>
 #endif //FEATURE_PAL
 
-// To avoid counting zeros in conversions
-#define MILLION * 1000000
+#define NUM_NANOSECONDS_IN_1_MS (1000000)
 
 Volatile<BOOL> SampleProfiler::s_profilingEnabled = false;
 Thread* SampleProfiler::s_pSamplingThread = NULL;
@@ -26,7 +25,7 @@ EventPipeEvent* SampleProfiler::s_pThreadTimeEvent = NULL;
 BYTE* SampleProfiler::s_pPayloadExternal = NULL;
 BYTE* SampleProfiler::s_pPayloadManaged = NULL;
 CLREventStatic SampleProfiler::s_threadShutdownEvent;
-unsigned long SampleProfiler::s_samplingRateInNs = 1 MILLION; // 1ms
+unsigned long SampleProfiler::s_samplingRateInNs = NUM_NANOSECONDS_IN_1_MS; // 1ms
 bool SampleProfiler::s_timePeriodIsSet = FALSE;
 
 #ifndef FEATURE_PAL
@@ -256,7 +255,7 @@ void SampleProfiler::PlatformSleep(unsigned long nanoseconds)
 #ifdef FEATURE_PAL
     PAL_nanosleep(nanoseconds);
 #else //FEATURE_PAL
-    ClrSleepEx(s_samplingRateInNs / 1 MILLION, FALSE);
+    ClrSleepEx(s_samplingRateInNs / NUM_NANOSECONDS_IN_1_MS, FALSE);
 #endif //FEATURE_PAL
 }
 
@@ -278,7 +277,7 @@ void SampleProfiler::SetTimeGranularity()
     // the OS is on-CPU, decreasing overall system performance and increasing power consumption
     if(s_timeBeginPeriodFn != NULL)
     {
-        if(((TimePeriodFnPtr) s_timeBeginPeriodFn)(s_samplingRateInNs / 1 MILLION) == TIMERR_NOERROR)
+        if(((TimePeriodFnPtr) s_timeBeginPeriodFn)(s_samplingRateInNs / NUM_NANOSECONDS_IN_1_MS) == TIMERR_NOERROR)
         {
             s_timePeriodIsSet = TRUE;
         }
@@ -300,7 +299,7 @@ void SampleProfiler::ResetTimeGranularity()
     // End the modifications we had to the timer period in Enable
     if(s_timeEndPeriodFn != NULL)
     {
-        if(((TimePeriodFnPtr) s_timeEndPeriodFn)(s_samplingRateInNs / 1 MILLION) == TIMERR_NOERROR)
+        if(((TimePeriodFnPtr) s_timeEndPeriodFn)(s_samplingRateInNs / NUM_NANOSECONDS_IN_1_MS) == TIMERR_NOERROR)
         {
             s_timePeriodIsSet = FALSE;
         }
-- 
cgit v1.2.3