Address CR feedback

Commit migrated from https://github.com/dotnet/coreclr/commit/aac02c9ceb7f41ffed1353759e079f063fc953f9

diff --git a/src/coreclr/src/inc/corprof.idl b/src/coreclr/src/inc/corprof.idl
index b126cf2..9af1cd9 100644 (file)
--- a/src/coreclr/src/inc/corprof.idl
+++ b/src/coreclr/src/inc/corprof.idl
@@ -2387,18 +2387,25 @@ interface ICorProfilerCallback7 : ICorProfilerCallback6
 ]
 interface ICorProfilerCallback8 : ICorProfilerCallback7
 {
-    // This event is triggered whenever a metadata less method is jit compiled.
+    // This event is triggered whenever a dynamic method is jit compiled.
     // These include various IL Stubs and LCG Methods.
     // The goal is to provide profiler writers with enough information to identify
     // it to users as beyond unknown code addresses.
     // Note: FunctionID's provided here cannot be used to resolve to their metadata
-    //       tokens since LCG methods have no metadata.
+    //       tokens since dynamic methods have no metadata.
     //
-    // Documentation Note: ilHeader is only valid during the callback
-
-    HRESULT DynamicMethodJITCompilationStarted(FunctionID functionId, LPCBYTE ilHeader);
-
-    HRESULT DynamicMethodJITCompilationFinished(FunctionID functionId);
+    // Documentation Note: pILHeader is only valid during the callback
+
+    HRESULT DynamicMethodJITCompilationStarted(
+        [in] FunctionID functionId,
+        [in] BOOL       fIsSafeToBlock,
+        [in] LPCBYTE    pILHeader,
+        [in] ULONG      cbILHeader);
+
+    HRESULT DynamicMethodJITCompilationFinished(
+        [in] FunctionID functionId,
+        [in] HRESULT    hrStatus,
+        [in] BOOL       fIsSafeToBlock);
 }
 
 
@@ -3833,23 +3840,23 @@ interface ICorProfilerInfo8 : ICorProfilerInfo7
     /*
     * Maps a managed code instruction pointer to a FunctionID.
     *
-    * GetFunctionFromIP2 fails for methods without metadata, this method works for
-    * both metadata and metadata-less methods and is a superset of GetFunctionFromIP2
+    * GetFunctionFromIP2 fails for dynamic methods, this method works for
+    * both dynamic and non-dynamic methods. It is a superset of GetFunctionFromIP2
     */
     HRESULT GetFunctionFromIP3([in] LPCBYTE ip,
                                [out] FunctionID *functionId,
                                [out] ReJITID * pReJitId);
 
     /*
-    * Retrieves informaiton about metadata-less methods
+    * Retrieves informaiton about dynamic methods
     *
-    * Certain methods like IL Stubs or DynamicMethod do not have
+    * Certain methods like IL Stubs or LCG do not have
     * associated metadata that can be retrieved using the IMetaDataImport APIs.
     *
     * Such methods can be encountered by profilers through instruction pointers
-    * or by listening to ICorProfilerCallback::JITCompilationStartedNoMetadata
+    * or by listening to ICorProfilerCallback::DynamicMethodJITCompilationStarted
     *
-    * This API can be used to retrieve information about metadata less methods
+    * This API can be used to retrieve information about dynamic methods
     * including a friendly name if available.
     */
     HRESULT GetDynamicFunctionInfo( [in]  FunctionID              functionId,

diff --git a/src/coreclr/src/pal/prebuilt/inc/corprof.h b/src/coreclr/src/pal/prebuilt/inc/corprof.h
index 56b1fbe..6778fb5 100644 (file)
--- a/src/coreclr/src/pal/prebuilt/inc/corprof.h
+++ b/src/coreclr/src/pal/prebuilt/inc/corprof.h
@@ -5,7 +5,7 @@
 /* this ALWAYS GENERATED file contains the definitions for the interfaces */
 
 
- /* File created by MIDL compiler version 8.00.0613 */
+ /* File created by MIDL compiler version 8.01.0620 */
 /* @@MIDL_FILE_HEADING(  ) */
 
 #pragma warning( disable: 4049 )  /* more than 64k source lines */
@@ -5843,11 +5843,15 @@ EXTERN_C const IID IID_ICorProfilerCallback8;
     {
     public:
         virtual HRESULT STDMETHODCALLTYPE DynamicMethodJITCompilationStarted( 
-            FunctionID functionId,
-            LPCBYTE ilHeader) = 0;
+            /* [in] */ FunctionID functionId,
+            /* [in] */ BOOL fIsSafeToBlock,
+            /* [in] */ LPCBYTE pILHeader,
+            /* [in] */ ULONG cbILHeader) = 0;
         
         virtual HRESULT STDMETHODCALLTYPE DynamicMethodJITCompilationFinished( 
-            FunctionID functionId) = 0;
+            /* [in] */ FunctionID functionId,
+            /* [in] */ HRESULT hrStatus,
+            /* [in] */ BOOL fIsSafeToBlock) = 0;
         
     };
     
@@ -6282,12 +6286,16 @@ EXTERN_C const IID IID_ICorProfilerCallback8;
         
         HRESULT ( STDMETHODCALLTYPE *DynamicMethodJITCompilationStarted )( 
             ICorProfilerCallback8 * This,
-            FunctionID functionId,
-            LPCBYTE ilHeader);
+            /* [in] */ FunctionID functionId,
+            /* [in] */ BOOL fIsSafeToBlock,
+            /* [in] */ LPCBYTE pILHeader,
+            /* [in] */ ULONG cbILHeader);
         
         HRESULT ( STDMETHODCALLTYPE *DynamicMethodJITCompilationFinished )( 
             ICorProfilerCallback8 * This,
-            FunctionID functionId);
+            /* [in] */ FunctionID functionId,
+            /* [in] */ HRESULT hrStatus,
+            /* [in] */ BOOL fIsSafeToBlock);
         
         END_INTERFACE
     } ICorProfilerCallback8Vtbl;
@@ -6586,11 +6594,11 @@ EXTERN_C const IID IID_ICorProfilerCallback8;
     ( (This)->lpVtbl -> ModuleInMemorySymbolsUpdated(This,moduleId) ) 
 
 
-#define ICorProfilerCallback8_DynamicMethodJITCompilationStarted(This,functionId,ilHeader)     \
-    ( (This)->lpVtbl -> DynamicMethodJITCompilationStarted(This,functionId,ilHeader) ) 
+#define ICorProfilerCallback8_DynamicMethodJITCompilationStarted(This,functionId,fIsSafeToBlock,pILHeader,cbILHeader)  \
+    ( (This)->lpVtbl -> DynamicMethodJITCompilationStarted(This,functionId,fIsSafeToBlock,pILHeader,cbILHeader) ) 
 
-#define ICorProfilerCallback8_DynamicMethodJITCompilationFinished(This,functionId)     \
-    ( (This)->lpVtbl -> DynamicMethodJITCompilationFinished(This,functionId) ) 
+#define ICorProfilerCallback8_DynamicMethodJITCompilationFinished(This,functionId,hrStatus,fIsSafeToBlock)     \
+    ( (This)->lpVtbl -> DynamicMethodJITCompilationFinished(This,functionId,hrStatus,fIsSafeToBlock) ) 
 
 #endif /* COBJMACROS */
 

diff --git a/src/coreclr/src/vm/eetoprofinterfaceimpl.cpp b/src/coreclr/src/vm/eetoprofinterfaceimpl.cpp
index 16da50e..1ceed3d 100644 (file)
--- a/src/coreclr/src/vm/eetoprofinterfaceimpl.cpp
+++ b/src/coreclr/src/vm/eetoprofinterfaceimpl.cpp
@@ -3230,7 +3230,9 @@ HRESULT EEToProfInterfaceImpl::JITCompilationStarted(FunctionID functionId,
     }
 }
 
-HRESULT EEToProfInterfaceImpl::DynamicMethodJITCompilationFinished(FunctionID functionId)
+HRESULT EEToProfInterfaceImpl::DynamicMethodJITCompilationFinished(FunctionID functionId,
+                                                                   HRESULT hrStatus,
+                                                                   BOOL fIsSafeToBlock)
 {
     CONTRACTL
     {
@@ -3252,21 +3254,23 @@ HRESULT EEToProfInterfaceImpl::DynamicMethodJITCompilationFinished(FunctionID fu
 
     _ASSERTE(functionId);
 
-    // Should only be called on profilers that support ICorProfilerCallback8
     if (m_pCallback8 == NULL)
     {
-        return E_FAIL;
+        return S_OK;
     }
 
     {
         // All callbacks are really NOTHROW, but that's enforced partially by the profiler,
         // whose try/catch blocks aren't visible to the contract system        
         PERMANENT_CONTRACT_VIOLATION(ThrowsViolation, ReasonProfilerCallout);
-        return m_pCallback8->DynamicMethodJITCompilationFinished(functionId);
+        return m_pCallback8->DynamicMethodJITCompilationFinished(functionId, hrStatus, fIsSafeToBlock);
     }
 }
 
-HRESULT EEToProfInterfaceImpl::DynamicMethodJITCompilationStarted(FunctionID functionId, LPCBYTE ilHeader)
+HRESULT EEToProfInterfaceImpl::DynamicMethodJITCompilationStarted(FunctionID functionId,
+                                                                  BOOL fIsSafeToBlock,
+                                                                  LPCBYTE pILHeader,
+                                                                  ULONG cbILHeader)
 {
     CONTRACTL
     {
@@ -3288,17 +3292,21 @@ HRESULT EEToProfInterfaceImpl::DynamicMethodJITCompilationStarted(FunctionID fun
 
     _ASSERTE(functionId);
 
-    // Should only be called on profilers that support ICorProfilerCallback8
+    // Currently DynamicMethodJITCompilationStarted is always called with fIsSafeToBlock==TRUE.  If this ever changes,
+    // it's safe to remove this assert, but this should serve as a trigger to change our
+    // public documentation to state that this callback is no longer called in preemptive mode all the time.
+    _ASSERTE(fIsSafeToBlock);
+
     if (m_pCallback8 == NULL)
     {
-        return E_FAIL;
+        return S_OK;
     }
 
     {
         // All callbacks are really NOTHROW, but that's enforced partially by the profiler,
         // whose try/catch blocks aren't visible to the contract system        
         PERMANENT_CONTRACT_VIOLATION(ThrowsViolation, ReasonProfilerCallout);
-        return m_pCallback8->DynamicMethodJITCompilationStarted(functionId, ilHeader);
+        return m_pCallback8->DynamicMethodJITCompilationStarted(functionId, fIsSafeToBlock, pILHeader, cbILHeader);
     }
 }
 

diff --git a/src/coreclr/src/vm/eetoprofinterfaceimpl.h b/src/coreclr/src/vm/eetoprofinterfaceimpl.h
index e7ec34c..76797fc 100644 (file)
--- a/src/coreclr/src/vm/eetoprofinterfaceimpl.h
+++ b/src/coreclr/src/vm/eetoprofinterfaceimpl.h
@@ -173,10 +173,14 @@ public:
 
     HRESULT DynamicMethodJITCompilationStarted(
         FunctionID  functionId,
-        LPCBYTE     ilHeader);
+        BOOL        fIsSafeToBlock,
+        LPCBYTE     pILHeader,
+        ULONG       cbILHeader);
 
     HRESULT DynamicMethodJITCompilationFinished(
-        FunctionID functionId);
+        FunctionID  functionId,
+        HRESULT     hrStatus,
+        BOOL        fIsSafeToBlock);
     
     HRESULT JITCachedFunctionSearchStarted(
         /* [in] */  FunctionID functionId,

diff --git a/src/coreclr/src/vm/prestub.cpp b/src/coreclr/src/vm/prestub.cpp
index ff897fa..572065c 100644 (file)
--- a/src/coreclr/src/vm/prestub.cpp
+++ b/src/coreclr/src/vm/prestub.cpp
@@ -443,7 +443,7 @@ PCODE MethodDesc::MakeJitWorker(COR_ILMETHOD_DECODER* ILHeader, DWORD flags, DWO
                 }
                 else
                 {
-                    g_profControlBlock.pProfInterface->DynamicMethodJITCompilationStarted((FunctionID) this, (LPCBYTE)ILHeader);
+                    g_profControlBlock.pProfInterface->DynamicMethodJITCompilationStarted((FunctionID) this, TRUE, (LPCBYTE)ILHeader, ILHeader->GetSize());
                 }
                 END_PIN_PROFILER();
             }
@@ -599,7 +599,7 @@ GotNewCode:
                 }
                 else
                 {
-                    g_profControlBlock.pProfInterface->DynamicMethodJITCompilationFinished((FunctionID) this);
+                    g_profControlBlock.pProfInterface->DynamicMethodJITCompilationFinished((FunctionID) this, pEntry->m_hrResultCode, TRUE);
                 }
                 END_PIN_PROFILER();
             }