[mono] Disable unsupported apply update scenarios (#55698)

lambdageek · web-flow · commit 2e93fab1fded · 2021-07-15T10:46:47.000-04:00
* Disallow mixing ApplyChanges and debugger apply changes

* Don't allow debugger thread to start if ApplyChanges has been called

* Don't allow ApplyChanges managed API call if debugger is attached

* MONO_COMPONENT_API mono_error_set_not_supported
diff --git 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);
 }
 	
@@ -10203,6 +10203,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/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
@@ -54,7 +54,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);
@@ -1349,13 +1349,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
@@ -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
@@ -5783,7 +5783,15 @@ 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);
+#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);
 }
diff --git 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);
 
@@ -821,8 +826,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
@@ -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);
 	}
@@ -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 ();
+}
diff --git 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);
 

Original file line number	Diff line number	Diff line change
`@@ -15,7 +15,7 @@ static bool`
`15`	`15`	`hot_reload_stub_available (void);`
`16`	`16`
`17`	`17`	`static void`
`18`		`-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);`
	`18`	`+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);`
`19`	`19`
`20`	`20`	`static MonoComponentHotReload *`
`21`	`21`	`component_hot_reload_stub_init (void);`
`@@ -147,7 +147,7 @@ hot_reload_stub_relative_delta_index (MonoImage *image_dmeta, int token)`
`147`	`147`	`}`
`148`	`148`
`149`	`149`	`void`
`150`		`-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)`
	`150`	`+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)`
`151`	`151`	`{`
`152`	`152`	`mono_error_set_not_supported (error, "Hot reload not supported in this runtime.");`
`153`	`153`	`}`
Original file line number	Diff line number	Diff line change
`@@ -73,9 +73,9 @@ mono_image_relative_delta_index (MonoImage *image_dmeta, int token)`
`73`	`73`	`}`
`74`	`74`
`75`	`75`	`void`
`76`		`-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)`
	`76`	`+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)`
`77`	`77`	`{`
`78`		`- mono_component_hot_reload ()->apply_changes (base_image, dmeta, dmeta_len, dil, dil_len, dpdb, dpdb_len, error);`
	`78`	`+ mono_component_hot_reload ()->apply_changes (origin, base_image, dmeta, dmeta_len, dil, dil_len, dpdb, dpdb_len, error);`
`79`	`79`	`if (is_ok (error)) {`
`80`	`80`	`mono_component_debugger ()->send_enc_delta (base_image, dmeta, dmeta_len, dpdb, dpdb_len);`
`81`	`81`	`}`
`@@ -135,3 +135,9 @@ mono_metadata_update_has_modified_rows (const MonoTableInfo *table)`
`135`	`135`	`{`
`136`	`136`	`return mono_component_hot_reload ()->has_modified_rows (table);`
`137`	`137`	`}`
	`138`	`+`
	`139`	`+gboolean`
	`140`	`+mono_metadata_has_updates_api (void)`
	`141`	`+{`
	`142`	`+ return mono_metadata_has_updates ();`
	`143`	`+}`