From bfbe869468e19ccc95e689c03526121064c82146 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 14 Jul 2021 20:17:19 -0400 Subject: [PATCH 1/5] Disallow mixing ApplyChanges and debugger apply changes --- src/mono/mono/component/debugger-agent.c | 2 +- src/mono/mono/component/hot_reload-stub.c | 4 ++-- src/mono/mono/component/hot_reload.c | 15 +++++++++++++-- src/mono/mono/component/hot_reload.h | 2 +- src/mono/mono/metadata/icall.c | 2 +- src/mono/mono/metadata/metadata-internals.h | 7 ++++++- src/mono/mono/metadata/metadata-update.c | 4 ++-- 7 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/mono/mono/component/debugger-agent.c b/src/mono/mono/component/debugger-agent.c index bec98edb3c2f3..19abca151d8b6 100644 --- a/src/mono/mono/component/debugger-agent.c +++ b/src/mono/mono/component/debugger-agent.c @@ -6575,7 +6575,7 @@ module_apply_changes (MonoImage *image, MonoArray *dmeta, MonoArray *dil, MonoAr int32_t dil_len = mono_array_length_internal (dil); gpointer dpdb_bytes = !dpdb ? NULL : (gpointer)mono_array_addr_internal (dpdb, char, 0); int32_t dpdb_len = !dpdb ? 0 : mono_array_length_internal (dpdb); - mono_image_load_enc_delta (image, dmeta_bytes, dmeta_len, dil_bytes, dil_len, dpdb_bytes, dpdb_len, error); + mono_image_load_enc_delta (MONO_ENC_DELTA_DBG, image, dmeta_bytes, dmeta_len, dil_bytes, dil_len, dpdb_bytes, dpdb_len, error); return is_ok (error); } diff --git a/src/mono/mono/component/hot_reload-stub.c b/src/mono/mono/component/hot_reload-stub.c index 0ebf79a50c8f6..35b8fe38a56e4 100644 --- a/src/mono/mono/component/hot_reload-stub.c +++ b/src/mono/mono/component/hot_reload-stub.c @@ -15,7 +15,7 @@ static bool hot_reload_stub_available (void); static void -hot_reload_stub_apply_changes (MonoImage *base_image, gconstpointer dmeta, uint32_t dmeta_len, gconstpointer dil, uint32_t dil_len, gconstpointer dpdb_bytes_orig, uint32_t dpdb_length, MonoError *error); +hot_reload_stub_apply_changes (int origin, MonoImage *base_image, gconstpointer dmeta, uint32_t dmeta_len, gconstpointer dil, uint32_t dil_len, gconstpointer dpdb_bytes_orig, uint32_t dpdb_length, MonoError *error); static MonoComponentHotReload * component_hot_reload_stub_init (void); @@ -147,7 +147,7 @@ hot_reload_stub_relative_delta_index (MonoImage *image_dmeta, int token) } void -hot_reload_stub_apply_changes (MonoImage *base_image, gconstpointer dmeta, uint32_t dmeta_len, gconstpointer dil, uint32_t dil_len, gconstpointer dpdb_bytes_orig, uint32_t dpdb_length, MonoError *error) +hot_reload_stub_apply_changes (int origin, MonoImage *base_image, gconstpointer dmeta, uint32_t dmeta_len, gconstpointer dil, uint32_t dil_len, gconstpointer dpdb_bytes_orig, uint32_t dpdb_length, MonoError *error) { mono_error_set_not_supported (error, "Hot reload not supported in this runtime."); } diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 32820888b2638..7fdac9222cbac 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -53,7 +53,7 @@ static void hot_reload_effective_table_slow (const MonoTableInfo **t, int *idx); static void -hot_reload_apply_changes (MonoImage *base_image, gconstpointer dmeta, uint32_t dmeta_len, gconstpointer dil, uint32_t dil_len, gconstpointer dpdb_bytes_orig, uint32_t dpdb_length, MonoError *error); +hot_reload_apply_changes (int origin, MonoImage *base_image, gconstpointer dmeta, uint32_t dmeta_len, gconstpointer dil, uint32_t dil_len, gconstpointer dpdb_bytes_orig, uint32_t dpdb_length, MonoError *error); static int hot_reload_relative_delta_index (MonoImage *image_dmeta, int token); @@ -1346,13 +1346,24 @@ apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t gen * LOCKING: Takes the publish_lock */ void -hot_reload_apply_changes (MonoImage *image_base, gconstpointer dmeta_bytes, uint32_t dmeta_length, gconstpointer dil_bytes_orig, uint32_t dil_length, gconstpointer dpdb_bytes_orig, uint32_t dpdb_length, MonoError *error) +hot_reload_apply_changes (int origin, MonoImage *image_base, gconstpointer dmeta_bytes, uint32_t dmeta_length, gconstpointer dil_bytes_orig, uint32_t dil_length, gconstpointer dpdb_bytes_orig, uint32_t dpdb_length, MonoError *error) { if (!assembly_update_supported (image_base->assembly)) { mono_error_set_invalid_operation (error, "The assembly can not be edited or changed."); return; } + static int first_origin = -1; + + if (first_origin < 0) { + first_origin = origin; + } + + if (first_origin != origin) { + mono_error_set_not_supported (error, "Applying deltas through the debugger and System.Reflection.Metadata.MetadataUpdater.ApplyUpdate simultaneously is not supported"); + return; + } + const char *basename = image_base->filename; if (mono_trace_is_traced (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE)) { diff --git a/src/mono/mono/component/hot_reload.h b/src/mono/mono/component/hot_reload.h index 26ca0089a69ef..37dc1401bbd11 100644 --- a/src/mono/mono/component/hot_reload.h +++ b/src/mono/mono/component/hot_reload.h @@ -23,7 +23,7 @@ typedef struct _MonoComponentHotReload { void (*cleanup_on_close) (MonoImage *image); void (*effective_table_slow) (const MonoTableInfo **t, int *idx); int (*relative_delta_index) (MonoImage *image_dmeta, int token); - void (*apply_changes) (MonoImage *base_image, gconstpointer dmeta, uint32_t dmeta_len, gconstpointer dil, uint32_t dil_len, gconstpointer dpdb_bytes_orig, uint32_t dpdb_length, MonoError *error); + void (*apply_changes) (int origin, MonoImage *base_image, gconstpointer dmeta, uint32_t dmeta_len, gconstpointer dil, uint32_t dil_len, gconstpointer dpdb_bytes_orig, uint32_t dpdb_length, MonoError *error); void (*image_close_except_pools_all) (MonoImage *base_image); void (*image_close_all) (MonoImage *base_image); gpointer (*get_updated_method_rva) (MonoImage *base_image, uint32_t idx); diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index 240479fb90a56..e4bc465d6becc 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -5783,7 +5783,7 @@ ves_icall_AssemblyExtensions_ApplyUpdate (MonoAssembly *assm, MonoImage *image_base = assm->image; g_assert (image_base); - mono_image_load_enc_delta (image_base, dmeta_bytes, dmeta_len, dil_bytes, dil_len, dpdb_bytes, dpdb_len, error); + mono_image_load_enc_delta (MONO_ENC_DELTA_API, image_base, dmeta_bytes, dmeta_len, dil_bytes, dil_len, dpdb_bytes, dpdb_len, error); mono_error_set_pending_exception (error); } diff --git a/src/mono/mono/metadata/metadata-internals.h b/src/mono/mono/metadata/metadata-internals.h index 9a0d030a9f171..9383b4f5ed7aa 100644 --- a/src/mono/mono/metadata/metadata-internals.h +++ b/src/mono/mono/metadata/metadata-internals.h @@ -821,8 +821,13 @@ mono_image_effective_table (const MonoTableInfo **t, int *idx) int mono_image_relative_delta_index (MonoImage *image_dmeta, int token); +enum MonoEnCDeltaOrigin { + MONO_ENC_DELTA_API = 0, + MONO_ENC_DELTA_DBG = 1, +}; + MONO_COMPONENT_API void -mono_image_load_enc_delta (MonoImage *base_image, gconstpointer dmeta, uint32_t dmeta_len, gconstpointer dil, uint32_t dil_len, gconstpointer dpdb, uint32_t dpdb_len, MonoError *error); +mono_image_load_enc_delta (int delta_origin, MonoImage *base_image, gconstpointer dmeta, uint32_t dmeta_len, gconstpointer dil, uint32_t dil_len, gconstpointer dpdb, uint32_t dpdb_len, MonoError *error); gboolean mono_image_load_cli_header (MonoImage *image, MonoCLIImageInfo *iinfo); diff --git a/src/mono/mono/metadata/metadata-update.c b/src/mono/mono/metadata/metadata-update.c index 71520b8aa1e67..f171e03f88394 100644 --- a/src/mono/mono/metadata/metadata-update.c +++ b/src/mono/mono/metadata/metadata-update.c @@ -73,9 +73,9 @@ mono_image_relative_delta_index (MonoImage *image_dmeta, int token) } void -mono_image_load_enc_delta (MonoImage *base_image, gconstpointer dmeta, uint32_t dmeta_len, gconstpointer dil, uint32_t dil_len, gconstpointer dpdb, uint32_t dpdb_len, MonoError *error) +mono_image_load_enc_delta (int origin, MonoImage *base_image, gconstpointer dmeta, uint32_t dmeta_len, gconstpointer dil, uint32_t dil_len, gconstpointer dpdb, uint32_t dpdb_len, MonoError *error) { - mono_component_hot_reload ()->apply_changes (base_image, dmeta, dmeta_len, dil, dil_len, dpdb, dpdb_len, error); + mono_component_hot_reload ()->apply_changes (origin, base_image, dmeta, dmeta_len, dil, dil_len, dpdb, dpdb_len, error); if (is_ok (error)) { mono_component_debugger ()->send_enc_delta (base_image, dmeta, dmeta_len, dpdb, dpdb_len); } From 4b87b14151a279db2eeab145f8e6595f4507aba1 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 14 Jul 2021 20:39:21 -0400 Subject: [PATCH 2/5] Don't allow debugger thread to start if ApplyChanges has been called --- src/mono/mono/component/debugger-agent.c | 12 ++++++++++++ src/mono/mono/metadata/metadata-internals.h | 5 +++++ src/mono/mono/metadata/metadata-update.c | 6 ++++++ 3 files changed, 23 insertions(+) diff --git a/src/mono/mono/component/debugger-agent.c b/src/mono/mono/component/debugger-agent.c index 19abca151d8b6..fa3e93a17756b 100644 --- a/src/mono/mono/component/debugger-agent.c +++ b/src/mono/mono/component/debugger-agent.c @@ -10199,6 +10199,18 @@ debugger_thread (void *arg) mono_set_is_debugger_attached (TRUE); } +#ifndef HOST_WASM + if (!attach_failed) { + if (mono_metadata_has_updates_api ()) { + PRINT_DEBUG_MSG (1, "[dbg] Cannot attach after System.Reflection.Metadata.MetadataUpdater.ApplyChanges has been called.\n"); + attach_failed = TRUE; + command_set = (CommandSet)0; + command = 0; + dispose_vm (); + } + } +#endif + while (!attach_failed) { res = transport_recv (header, HEADER_LENGTH); diff --git a/src/mono/mono/metadata/metadata-internals.h b/src/mono/mono/metadata/metadata-internals.h index 9383b4f5ed7aa..2aea7227de725 100644 --- a/src/mono/mono/metadata/metadata-internals.h +++ b/src/mono/mono/metadata/metadata-internals.h @@ -802,6 +802,11 @@ mono_metadata_has_updates (void) return mono_metadata_update_data_private.has_updates != 0; } +/* components can't call the inline function directly since the private data isn't exported */ +MONO_COMPONENT_API +gboolean +mono_metadata_has_updates_api (void); + void mono_image_effective_table_slow (const MonoTableInfo **t, int *idx); diff --git a/src/mono/mono/metadata/metadata-update.c b/src/mono/mono/metadata/metadata-update.c index f171e03f88394..99a2d99a5d7c3 100644 --- a/src/mono/mono/metadata/metadata-update.c +++ b/src/mono/mono/metadata/metadata-update.c @@ -135,3 +135,9 @@ mono_metadata_update_has_modified_rows (const MonoTableInfo *table) { return mono_component_hot_reload ()->has_modified_rows (table); } + +gboolean +mono_metadata_has_updates_api (void) +{ + return mono_metadata_has_updates (); +} From 716c0fb278f57cf57a599fb4378ff063a9dafb8f Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 14 Jul 2021 20:42:31 -0400 Subject: [PATCH 3/5] Don't allow ApplyChanges managed API call if debugger is attached --- src/mono/mono/metadata/icall.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index e4bc465d6becc..b4f4d07502be5 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -5783,6 +5783,14 @@ ves_icall_AssemblyExtensions_ApplyUpdate (MonoAssembly *assm, MonoImage *image_base = assm->image; g_assert (image_base); +#ifndef HOST_WASM + if (mono_is_debugger_attached ()) { + mono_error_set_not_supported (error, "Cannot use System.Reflection.Metadata.MetadataUpdater.ApplyChanges while debugger is attached"); + mono_error_set_pending_exception (error); + return; + } +#endif + mono_image_load_enc_delta (MONO_ENC_DELTA_API, image_base, dmeta_bytes, dmeta_len, dil_bytes, dil_len, dpdb_bytes, dpdb_len, error); mono_error_set_pending_exception (error); From 72bdc9f4937d973d8d3d9ee362ad8cb4911cdd12 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 14 Jul 2021 22:12:29 -0400 Subject: [PATCH 4/5] MONO_COMPONENT_API mono_error_set_not_supported --- src/mono/mono/utils/mono-error-internals.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/mono/mono/utils/mono-error-internals.h b/src/mono/mono/utils/mono-error-internals.h index 2256c6b5adb6f..a26558689ae1c 100644 --- a/src/mono/mono/utils/mono-error-internals.h +++ b/src/mono/mono/utils/mono-error-internals.h @@ -205,6 +205,7 @@ mono_error_set_execution_engine (MonoError *error, const char *msg_format, ...) void mono_error_set_not_implemented (MonoError *error, const char *msg_format, ...) MONO_ATTR_FORMAT_PRINTF(2,3); +MONO_COMPONENT_API void mono_error_set_not_supported (MonoError *error, const char *msg_format, ...) MONO_ATTR_FORMAT_PRINTF(2,3); From f093634b952e33e7f6d63e0dcb6163b762340f88 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 14 Jul 2021 22:53:37 -0400 Subject: [PATCH 5/5] empty