Block debugger attach after any ApplyUpdate changes (#50200)

* Block debugger attach after any ApplyUpdate changes * Add better error message for ApplyUpdate while debugging * Code review feedback * Fix arm32/arm64 builds * Code review feedback * Code review feedback
dotnet · Mar 26, 2021 · e503be0 · e503be0
1 parent ec2fedc
commit e503be0
Show file tree

Hide file tree

Showing 11 changed files with 45 additions and 1 deletion.
diff --git a/src/coreclr/debug/daccess/dacdbiimpl.cpp b/src/coreclr/debug/daccess/dacdbiimpl.cpp
@@ -4253,6 +4253,16 @@ HRESULT DacDbiInterfaceImpl::IsModuleMapped(VMPTR_Module pModule, OUT BOOL *isMo
     return hr;
 }
 
+bool DacDbiInterfaceImpl::MetadataUpdatesApplied()
+{
+    DD_ENTER_MAY_THROW;
+#ifdef EnC_SUPPORTED
+    return g_metadataUpdatesApplied;
+#else
+    return false;
+#endif
+}
+
 // Helper to intialize a TargetBuffer from a MemoryRange
 //
 // Arguments:

diff --git a/src/coreclr/debug/daccess/dacdbiimpl.h b/src/coreclr/debug/daccess/dacdbiimpl.h
@@ -365,6 +365,8 @@ class DacDbiInterfaceImpl :
 
     HRESULT IsModuleMapped(VMPTR_Module pModule, OUT BOOL *isModuleMapped);
 
+    bool MetadataUpdatesApplied();
+
     // retrieves the list of COM interfaces implemented by vmObject, as it is known at
     // the time of the call (the list may change as new interface types become available
     // in the runtime)

diff --git a/src/coreclr/debug/di/process.cpp b/src/coreclr/debug/di/process.cpp
@@ -14019,6 +14019,13 @@ void CordbWin32EventThread::AttachProcess()
 
     EX_TRY
     {
+        // Don't allow attach if any metadata/IL updates have been applied
+        if (pProcess->GetDAC()->MetadataUpdatesApplied())
+        {
+            hr = CORDBG_E_ASSEMBLY_UPDATES_APPLIED;
+            goto LExit;
+        }
+
         // Mark interop-debugging
         if (m_actionData.attachData.IsInteropDebugging())
         {

diff --git a/src/coreclr/debug/inc/dacdbiinterface.h b/src/coreclr/debug/inc/dacdbiinterface.h
@@ -2736,6 +2736,9 @@ class IDacDbiInterface
     virtual
     HRESULT IsModuleMapped(VMPTR_Module pModule, OUT BOOL *isModuleMapped) = 0;
 
+    virtual
+    bool MetadataUpdatesApplied() = 0;
+
     // The following tag tells the DD-marshalling tool to stop scanning.
     // END_MARSHAL
 

diff --git a/src/coreclr/inc/corerror.xml b/src/coreclr/inc/corerror.xml
@@ -2189,6 +2189,12 @@
   <Comment>The delegate contains a delegate currently not supported by the API.</Comment>
 </HRESULT>
 
+<HRESULT NumericValue="0x80131c69">
+  <SymbolicName>CORDBG_E_ASSEMBLY_UPDATES_APPLIED</SymbolicName>
+  <Message>"The operation is not supported because assembly updates have been applied."</Message>
+  <Comment>The operation is not supported because assembly updates have been applied.</Comment>
+</HRESULT>
+
 <HRESULT NumericValue="0x80131d02">
 	<SymbolicName>PEFMT_E_64BIT</SymbolicName>
 	<Message>"File is PE32+."</Message>

diff --git a/src/coreclr/inc/dacvars.h b/src/coreclr/inc/dacvars.h
@@ -248,5 +248,9 @@ DEFINE_DACVAR(ULONG, TADDR, dac__g_MiniMetaDataBuffAddress, ::g_MiniMetaDataBuff
 
 DEFINE_DACVAR(ULONG, SIZE_T, dac__g_clrNotificationArguments, ::g_clrNotificationArguments)
 
+#ifdef EnC_SUPPORTED
+DEFINE_DACVAR(ULONG, bool, dac__g_metadataUpdatesApplied, ::g_metadataUpdatesApplied)
+#endif
+
 #undef DEFINE_DACVAR
 #undef DEFINE_DACVAR_NO_DUMP
diff --git a/src/coreclr/pal/prebuilt/inc/corerror.h b/src/coreclr/pal/prebuilt/inc/corerror.h
@@ -383,6 +383,7 @@
 #define CORDBG_E_DATA_TARGET_ERROR EMAKEHR(0x1c61)
 #define CORDBG_E_NO_IMAGE_AVAILABLE EMAKEHR(0x1c64)
 #define CORDBG_E_UNSUPPORTED_DELEGATE EMAKEHR(0x1c68)
+#define CORDBG_E_ASSEMBLY_UPDATES_APPLIED EMAKEHR(0x1c69)
 #define PEFMT_E_64BIT EMAKEHR(0x1d02)
 #define PEFMT_E_32BIT EMAKEHR(0x1d0b)
 #define CLDB_E_INTERNALERROR EMAKEHR(0x1fff)

diff --git a/src/coreclr/vm/assemblynative.cpp b/src/coreclr/vm/assemblynative.cpp
@@ -1435,7 +1435,7 @@ void QCALLTYPE AssemblyNative::ApplyUpdate(
     {
         if (CORDebuggerAttached())
         {
-            COMPlusThrow(kNotSupportedException);
+            COMPlusThrow(kNotSupportedException, W("NotSupported_DebuggerAttached"));
         }
         Module* pModule = assembly->GetDomainAssembly()->GetModule();
         if (!pModule->IsEditAndContinueEnabled())
@@ -1447,6 +1447,7 @@ void QCALLTYPE AssemblyNative::ApplyUpdate(
         {
             COMPlusThrow(kInvalidOperationException, W("InvalidOperation_EditFailed"));
         }
+        g_metadataUpdatesApplied = true;
     }
 #else
     COMPlusThrow(kNotImplementedException);

diff --git a/src/coreclr/vm/vars.cpp b/src/coreclr/vm/vars.cpp
@@ -180,6 +180,10 @@ bool g_fEEInit = false;
 // code:IsAtProcessExit to read this.
 GVAL_IMPL(bool, g_fProcessDetach);
 
+#ifdef EnC_SUPPORTED
+GVAL_IMPL_INIT(bool, g_metadataUpdatesApplied, false);
+#endif
+
 GVAL_IMPL_INIT(DWORD, g_fEEShutDown, 0);
 
 #ifndef TARGET_UNIX

diff --git a/src/coreclr/vm/vars.hpp b/src/coreclr/vm/vars.hpp
@@ -472,6 +472,9 @@ EXTERN DWORD g_fFastExitProcess;
 EXTERN BOOL g_fFatalErrorOccurredOnGCThread;
 EXTERN Volatile<LONG> g_fForbidEnterEE;
 GVAL_DECL(bool, g_fProcessDetach);
+#ifdef EnC_SUPPORTED
+GVAL_DECL(bool, g_metadataUpdatesApplied);
+#endif
 EXTERN bool g_fManagedAttach;
 EXTERN bool g_fNoExceptions;
 

diff --git a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx
@@ -3682,6 +3682,9 @@
   <data name="InvalidOperation_EditFailed" xml:space="preserve">
     <value>The assembly update failed.</value>
   </data>
+  <data name="NotSupported_DebuggerAttached" xml:space="preserve">
+    <value>Assembly updates cannot be applied while a debugger is attached.</value>
+  </data>
   <data name="NotSupported_MethodBodyReplacement" xml:space="preserve">
     <value>Method body replacement not supported in this runtime.</value>
   </data>