diff --git a/src/coreclr/inc/clrconfigvalues.h b/src/coreclr/inc/clrconfigvalues.h index 236dc0d22a5d1d..c14b1d06987288 100644 --- a/src/coreclr/inc/clrconfigvalues.h +++ b/src/coreclr/inc/clrconfigvalues.h @@ -386,7 +386,7 @@ CONFIG_DWORD_INFO(INTERNAL_MD_MiniMDBreak, W("MD_MiniMDBreak"), 0, "ASSERT when CONFIG_DWORD_INFO(INTERNAL_MD_PreSaveBreak, W("MD_PreSaveBreak"), 0, "ASSERT when calling CMiniMdRw::PreSave") CONFIG_DWORD_INFO(INTERNAL_MD_RegMetaBreak, W("MD_RegMetaBreak"), 0, "ASSERT when creating RegMeta class") CONFIG_DWORD_INFO(INTERNAL_MD_RegMetaDump, W("MD_RegMetaDump"), 0, "Dump MD in 4 functions (?)") -RETAIL_CONFIG_STRING_INFO_EX(EXTERNAL_DOTNET_MODIFIABLE_ASSEMBLIES, W("MODIFIABLE_ASSEMBLIES"), "Enables hot reload on debug built assemblies with the 'debug' keyword", CLRConfig::LookupOptions::TrimWhiteSpaceFromStringValue); +RETAIL_CONFIG_STRING_INFO_EX(EXTERNAL_DOTNET_MODIFIABLE_ASSEMBLIES, W("MODIFIABLE_ASSEMBLIES"), "Enables hot reload on debug built assemblies with the 'debug' keyword, 'none' explicitly disables it even when debugger is attached", CLRConfig::LookupOptions::TrimWhiteSpaceFromStringValue); // Metadata - mscordbi only - this flag is only intended to mitigate potential issues in bug fix 458597. RETAIL_CONFIG_DWORD_INFO(EXTERNAL_MD_PreserveDebuggerMetadataMemory, W("MD_PreserveDebuggerMetadataMemory"), 0, "Save all versions of metadata memory in the debugger when debuggee metadata is updated") diff --git a/src/coreclr/vm/assemblynative.cpp b/src/coreclr/vm/assemblynative.cpp index 923517a3aa4837..7e7b098dc76d84 100644 --- a/src/coreclr/vm/assemblynative.cpp +++ b/src/coreclr/vm/assemblynative.cpp @@ -1419,7 +1419,8 @@ extern "C" BOOL QCALLTYPE AssemblyNative_IsApplyUpdateSupported() BEGIN_QCALL; #ifdef FEATURE_METADATA_UPDATER - result = CORDebuggerAttached() || g_pConfig->DebugAssembliesModifiable(); + result = (g_pConfig->ModifiableAssemblies() != MODIFIABLE_ASSM_NONE) && + (CORDebuggerAttached() || g_pConfig->ModifiableAssemblies() == MODIFIABLE_ASSM_DEBUG); #endif END_QCALL; diff --git a/src/coreclr/vm/ceeload.cpp b/src/coreclr/vm/ceeload.cpp index 275e0107761e68..97aaf7b88c5b82 100644 --- a/src/coreclr/vm/ceeload.cpp +++ b/src/coreclr/vm/ceeload.cpp @@ -523,7 +523,9 @@ void Module::SetDebuggerInfoBits(DebuggerAssemblyControlFlags newBits) #ifdef DEBUGGING_SUPPORTED if (IsEditAndContinueCapable()) { - BOOL setEnC = (newBits & DACF_ENC_ENABLED) != 0 || (g_pConfig->DebugAssembliesModifiable() && AreJITOptimizationsDisabled()); + BOOL setEnC = (g_pConfig->ModifiableAssemblies() != MODIFIABLE_ASSM_NONE) && + ((newBits & DACF_ENC_ENABLED) != 0 || + (g_pConfig->ModifiableAssemblies() == MODIFIABLE_ASSM_DEBUG && AreJITOptimizationsDisabled())); if (setEnC) { EnableEditAndContinue(); diff --git a/src/coreclr/vm/eeconfig.cpp b/src/coreclr/vm/eeconfig.cpp index 19b593098f535c..a6b9d47b3e36d6 100644 --- a/src/coreclr/vm/eeconfig.cpp +++ b/src/coreclr/vm/eeconfig.cpp @@ -116,6 +116,7 @@ HRESULT EEConfig::Init() INDEBUG(fStressLog = true;) fDebuggable = false; + modifiableAssemblies = MODIFIABLE_ASSM_UNSET; #ifdef _DEBUG fExpandAllOnLoad = false; @@ -443,7 +444,16 @@ HRESULT EEConfig::sync() NewArrayHolder wszModifiableAssemblies; IfFailRet(CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_DOTNET_MODIFIABLE_ASSEMBLIES, &wszModifiableAssemblies)); if (wszModifiableAssemblies) - fDebugAssembliesModifiable = _wcsicmp(wszModifiableAssemblies, W("debug")) == 0; + { + if (_wcsicmp(wszModifiableAssemblies, W("debug")) == 0) + { + modifiableAssemblies = MODIFIABLE_ASSM_DEBUG; + } + else if (_wcsicmp(wszModifiableAssemblies, W("none")) == 0) + { + modifiableAssemblies = MODIFIABLE_ASSM_NONE; + } + } } pReadyToRunExcludeList = NULL; diff --git a/src/coreclr/vm/eeconfig.h b/src/coreclr/vm/eeconfig.h index 1e960142dcd038..23c4c3d99ed3d9 100644 --- a/src/coreclr/vm/eeconfig.h +++ b/src/coreclr/vm/eeconfig.h @@ -43,6 +43,15 @@ enum { OPT_BLENDED, OPT_RANDOM, OPT_DEFAULT = OPT_BLENDED }; +enum ClrModifiableAssemblies { + /* modifiable assemblies are implicitly disabled */ + MODIFIABLE_ASSM_UNSET = 0, + /* modifiable assemblies are explicitly disabled */ + MODIFIABLE_ASSM_NONE = 1, + /* assemblies with the Debug flag are modifiable */ + MODIFIABLE_ASSM_DEBUG = 2, +}; + enum ParseCtl { parseAll, // parse entire config file stopAfterRuntimeSection // stop after ... section @@ -404,8 +413,8 @@ class EEConfig // Loader bool ExcludeReadyToRun(LPCUTF8 assemblyName) const; - bool StressLog() const { LIMITED_METHOD_CONTRACT; return fStressLog; } - bool DebugAssembliesModifiable() const { LIMITED_METHOD_CONTRACT; return fDebugAssembliesModifiable; } + bool StressLog() const { LIMITED_METHOD_CONTRACT; return fStressLog; } + ClrModifiableAssemblies ModifiableAssemblies() const { LIMITED_METHOD_CONTRACT; return modifiableAssemblies; } // Optimizations to improve working set @@ -567,7 +576,7 @@ class EEConfig AssemblyNamesList * pReadyToRunExcludeList; bool fStressLog; - bool fDebugAssembliesModifiable; + ClrModifiableAssemblies modifiableAssemblies; #ifdef _DEBUG // interop logging diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index c8576441586296..9d4396a70914a5 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -6,6 +6,7 @@ using System.Reflection; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; +using Microsoft.DotNet.RemoteExecutor; using Xunit; namespace System.Reflection.Metadata @@ -1157,5 +1158,24 @@ void TestIncreaseMetadataRowSize() Assert.Equal("x800", pars[0].Name); }); } + + [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsRemoteExecutorSupportedAndFeatureCapable), nameof(ApplyUpdateUtil.IsNotMonoRuntime))] + void TestDisableMetadataUpdate() + { + RemoteInvokeOptions options = null; + ApplyUpdateUtil.AddRemoteInvokeOptions(ref options); + RemoteExecutor.Invoke(static () => { Assert.True(MetadataUpdater.IsSupported); }, options).Dispose(); + + options.StartInfo.EnvironmentVariables[ApplyUpdateUtil.DotNetModifiableAssembliesSwitch] = ApplyUpdateUtil.DotNetModifiableAssembliesDisabledValue; + RemoteExecutor.Invoke(static () => + { + Assert.False(MetadataUpdater.IsSupported); + var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.ClassWithCustomAttributeUpdates).Assembly; + var metadataDelta = new byte[20]; + var ilDelta = new byte[20]; + Assert.Throws(() => + MetadataUpdater.ApplyUpdate(assm, metadataDelta, ilDelta, ReadOnlySpan.Empty)); + }, options).Dispose(); + } } } diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateUtil.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateUtil.cs index b74144dceffd59..6a8f37fb1c62d5 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateUtil.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateUtil.cs @@ -11,6 +11,7 @@ namespace System.Reflection.Metadata public class ApplyUpdateUtil { internal const string DotNetModifiableAssembliesSwitch = "DOTNET_MODIFIABLE_ASSEMBLIES"; internal const string DotNetModifiableAssembliesValue = "debug"; + internal const string DotNetModifiableAssembliesDisabledValue = "none"; /// Whether ApplyUpdate is supported by the environment, test configuration, and runtime. /// @@ -21,6 +22,9 @@ public class ApplyUpdateUtil { public static bool IsSupported => (IsModifiableAssembliesSet || IsRemoteExecutorSupported) && (!IsMonoRuntime || IsSupportedMonoConfiguration); + /// true if RemoteExecutor is available and the runtime supports metadata updates. + public static bool IsRemoteExecutorSupportedAndFeatureCapable => IsSupported && IsRemoteExecutorSupported; + /// true if the current runtime was not launched with the appropriate settings for applying /// updates (DOTNET_MODIFIABLE_ASSEMBLIES unset), but we can use the remote executor to /// launch a child process that has the right setting. diff --git a/src/mono/mono/component/hot_reload-stub.c b/src/mono/mono/component/hot_reload-stub.c index 73fc1fba50f381..f4d1b8dd1ec587 100644 --- a/src/mono/mono/component/hot_reload-stub.c +++ b/src/mono/mono/component/hot_reload-stub.c @@ -190,7 +190,7 @@ gboolean hot_reload_stub_update_enabled (int *modifiable_assemblies_out) { if (modifiable_assemblies_out) - *modifiable_assemblies_out = MONO_MODIFIABLE_ASSM_NONE; + *modifiable_assemblies_out = MONO_MODIFIABLE_ASSM_UNSET; return false; } diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index e5614bb0788aec..d5ee8b6c9bd876 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -344,13 +344,13 @@ gboolean hot_reload_update_enabled (int *modifiable_assemblies_out) { static gboolean inited = FALSE; - static int modifiable = MONO_MODIFIABLE_ASSM_NONE; + static int modifiable = MONO_MODIFIABLE_ASSM_UNSET; - gboolean result = FALSE; + static gboolean result = FALSE; if (!inited) { modifiable = hot_reload_update_enabled_slow_check (NULL); inited = TRUE; - result = (modifiable != MONO_MODIFIABLE_ASSM_NONE); + result = (modifiable == MONO_MODIFIABLE_ASSM_DEBUG); if (result) { mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Metadata update enabled for debuggable assemblies"); } @@ -362,16 +362,20 @@ hot_reload_update_enabled (int *modifiable_assemblies_out) /** * checks the DOTNET_MODIFIABLE_ASSEMBLIES environment value. if it is recognized returns the - * aporporiate MONO_MODIFIABLE_ASSM_ enum value. if it is unset or uncrecognized returns - * MONO_MODIFIABLE_ASSM_NONE and writes the value to \c env_invalid_val_out, if it is non-NULL; + * appropriate MONO_MODIFIABLE_ASSM_ enum value. if it is unset or unrecognized returns + * MONO_MODIFIABLE_ASSM_UNSET and writes the value to \c env_invalid_val_out, if it is non-NULL; */ static int hot_reload_update_enabled_slow_check (char **env_invalid_val_out) { - int modifiable = MONO_MODIFIABLE_ASSM_NONE; + int modifiable = MONO_MODIFIABLE_ASSM_UNSET; char *val = g_getenv (DOTNET_MODIFIABLE_ASSEMBLIES); if (val && !g_strcasecmp (val, "debug")) { modifiable = MONO_MODIFIABLE_ASSM_DEBUG; + g_free (val); + } else if (val && !g_strcasecmp (val, "none")) { + modifiable = MONO_MODIFIABLE_ASSM_NONE; + g_free (val); } else { /* unset or unrecognized value */ if (env_invalid_val_out != NULL) { @@ -389,10 +393,18 @@ assembly_update_supported (MonoImage *image_base, MonoError *error) int modifiable = 0; char *invalid_env_val = NULL; modifiable = hot_reload_update_enabled_slow_check (&invalid_env_val); - if (modifiable == MONO_MODIFIABLE_ASSM_NONE) { - mono_error_set_invalid_operation (error, "The assembly '%s' cannot be edited or changed, because environment variable DOTNET_MODIFIABLE_ASSEMBLIES is set to '%s', not 'Debug'", image_base->name, invalid_env_val); + if (modifiable == MONO_MODIFIABLE_ASSM_UNSET) { + if (invalid_env_val == NULL) + { + mono_error_set_invalid_operation (error, "The assembly '%s' cannot be edited or changed, because environment variable DOTNET_MODIFIABLE_ASSEMBLIES is not set", image_base->name); + } else { + mono_error_set_invalid_operation (error, "The assembly '%s' cannot be edited or changed, because environment variable DOTNET_MODIFIABLE_ASSEMBLIES is set to '%s', not 'Debug'", image_base->name, invalid_env_val); + } g_free (invalid_env_val); return FALSE; + } else if (modifiable == MONO_MODIFIABLE_ASSM_NONE) { + mono_error_set_invalid_operation (error, "The assembly '%s' cannot be edited or changed, because environment variable DOTNET_MODIFIABLE_ASSEMBLIES is set to 'none', not 'Debug'", image_base->name); + return FALSE; } else if (!mono_assembly_is_jit_optimizer_disabled (image_base->assembly)) { mono_error_set_invalid_operation (error, "The assembly '%s' cannot be edited or changed, because it does not have a System.Diagnostics.DebuggableAttribute with the DebuggingModes.DisableOptimizations flag (editing Release build assemblies is not supported)", image_base->name); return FALSE; @@ -953,11 +965,11 @@ delta_info_initialize_mutants (const MonoImage *base, const BaselineInfo *base_i { const char *src = src_base + src_offset; char *dst = dst_base + dst_offset; - + /* copy src to dst, via a temporary to adjust for size differences */ /* FIXME: unaligned access, endianness */ guint32 tmp; - + switch (src_col_size) { case 1: tmp = *(guint8*)src; @@ -971,7 +983,7 @@ delta_info_initialize_mutants (const MonoImage *base, const BaselineInfo *base_i default: g_assert_not_reached (); } - + /* FIXME: unaligned access, endianness */ switch (dst_col_size) { case 1: @@ -1436,7 +1448,7 @@ delta_info_mutate_row (MonoImage *image_dmeta, DeltaInfo *cur_delta, guint32 log /* The complication here is that we want the mutant table to look like the table in * the baseline image with respect to column widths, but the delta tables are generally coming in - * uncompressed (4-byte columns). And we have already adjusted the baseline image column widths + * uncompressed (4-byte columns). And we have already adjusted the baseline image column widths * so we can use memcpy here. */ diff --git a/src/mono/mono/metadata/metadata-update.h b/src/mono/mono/metadata/metadata-update.h index 4e45c60439b5c3..d46ad4b4a4e779 100644 --- a/src/mono/mono/metadata/metadata-update.h +++ b/src/mono/mono/metadata/metadata-update.h @@ -14,10 +14,12 @@ void mono_metadata_update_init (void); enum MonoModifiableAssemblies { - /* modifiable assemblies are disabled */ - MONO_MODIFIABLE_ASSM_NONE = 0, + /* modifiable assemblies are implicitly disabled */ + MONO_MODIFIABLE_ASSM_UNSET = 0, + /* modifiable assemblies are explicitly disabled */ + MONO_MODIFIABLE_ASSM_NONE = 1, /* assemblies with the Debug flag are modifiable */ - MONO_MODIFIABLE_ASSM_DEBUG = 1, + MONO_MODIFIABLE_ASSM_DEBUG = 2, }; typedef MonoStreamHeader* (*MetadataHeapGetterFunc) (MonoImage*);