Skip to content

Commit

Permalink
[mbr] Add support for editing property getters/setters (#51011)
Browse files Browse the repository at this point in the history 
* wip: allow Property table (0x17) EnCLog entries in pass1

* [mbr] Allow MONO_TABLE_PROPERTY updates in pass2

Note that modifications of getter or setter bodies add new rows to the
MethodSemantics table that point to an existing method def (with an updated IL)
and to an existing row in the property table.  So there is nothing to do.

* [mbr] Add pass1 check that new MethodSemantics are pointing at existing methods.

We only support modifications of getters/setters, not adding a new getter or
setter where there wasn't one previously.

(Adding new methods in general fails right now)

* address feedback
  • Loading branch information
@lambdageek
lambdageek authored Apr 14, 2021
1 parent 29b3105 commit aede50d
Showing 1 changed file with 77 additions and 9 deletions.
86 changes: 77 additions & 9 deletions src/mono/mono/metadata/metadata-update.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -814,11 +814,58 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, gconstpointer
int token_table = mono_metadata_token_table (log_token);
int token_index = mono_metadata_token_index (log_token);

if (token_table == MONO_TABLE_ASSEMBLYREF) {
switch (token_table) {
case MONO_TABLE_ASSEMBLYREF:
/* okay, supported */
} else if (token_table == MONO_TABLE_METHOD) {
break;
case MONO_TABLE_METHOD:
/* handled above */
} else {
break;
case MONO_TABLE_PROPERTY: {
/* modifying a property, ok */
if (token_index <= table_info_get_rows (&image_base->tables [token_table]))
break;
/* adding a property */
mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x we do not support adding new properties.", i, log_token);
mono_error_set_type_load_name (error, NULL, image_base->name, "EnC: we do not support adding new properties. token=0x%08x", log_token);
unsupported_edits = TRUE;
continue;
}
case MONO_TABLE_METHODSEMANTICS: {
if (token_index > table_info_get_rows (&image_base->tables [token_table])) {
/* new rows are fine, as long as they point at existing methods */
guint32 sema_cols [MONO_METHOD_SEMA_SIZE];
int mapped_token = mono_image_relative_delta_index (image_dmeta, mono_metadata_make_token (token_table, token_index));
g_assert (mapped_token != -1);
mono_metadata_decode_row (&image_dmeta->tables [MONO_TABLE_METHODSEMANTICS], mapped_token - 1, sema_cols, MONO_METHOD_SEMA_SIZE);

switch (sema_cols [MONO_METHOD_SEMA_SEMANTICS]) {
case METHOD_SEMANTIC_GETTER:
case METHOD_SEMANTIC_SETTER: {
int prop_method_index = sema_cols [MONO_METHOD_SEMA_METHOD];
/* ok, if it's pointing to an existing getter/setter */
if (prop_method_index < table_info_get_rows (&image_base->tables [MONO_TABLE_METHOD]))
break;
mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x adding new getter/setter method 0x%08x to a property is not supported", i, log_token, prop_method_index);
mono_error_set_type_load_name (error, NULL, image_base->name, "EnC: we do not support adding a new getter or setter to a property, token=0x%08x", log_token);
unsupported_edits = TRUE;
continue;
}
default:
mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x adding new non-getter/setter property or event methods is not supported.", i, log_token);
mono_error_set_type_load_name (error, NULL, image_base->name, "EnC: we do not support adding new non-getter/setter property or event methods. token=0x%08x", log_token);
unsupported_edits = TRUE;
continue;
}
} else {
mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x we do not support patching of existing table cols.", i, log_token);
mono_error_set_type_load_name (error, NULL, image_base->name, "EnC: we do not support patching of existing table cols. token=0x%08x", log_token);
unsupported_edits = TRUE;
continue;
}
}
default:
/* FIXME: this bounds check is wrong for cumulative updates - need to look at the DeltaInfo:count.prev_gen_rows */
if (token_index <= table_info_get_rows (&image_base->tables [token_table])) {
mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x we do not support patching of existing table cols.", i, log_token);
mono_error_set_type_load_name (error, NULL, image_base->name, "EnC: we do not support patching of existing table cols. token=0x%08x", log_token);
Expand Down Expand Up @@ -884,7 +931,8 @@ apply_enclog_pass2 (MonoImage *image_base, uint32_t generation, MonoImage *image
break;
}

if (token_table == MONO_TABLE_ASSEMBLYREF) {
switch (token_table) {
case MONO_TABLE_ASSEMBLYREF: {
g_assert (token_index > table_info_get_rows (&image_base->tables [token_table]));

if (assemblyref_updated)
Expand Down Expand Up @@ -916,7 +964,9 @@ apply_enclog_pass2 (MonoImage *image_base, uint32_t generation, MonoImage *image
mono_image_unlock (image_base);

g_free (old_array);
} else if (token_table == MONO_TABLE_METHOD) {
break;
}
case MONO_TABLE_METHOD: {
if (token_index > table_info_get_rows (&image_base->tables [token_table])) {
g_error ("EnC: new method added, should be caught by pass1");
}
Expand All @@ -935,17 +985,29 @@ apply_enclog_pass2 (MonoImage *image_base, uint32_t generation, MonoImage *image
/* rva points probably into image_base IL stream. can this ever happen? */
g_print ("TODO: this case is still a bit contrived. token=0x%08x with rva=0x%04x\n", log_token, rva);
}
} else if (token_table == MONO_TABLE_TYPEDEF) {
break;
}
case MONO_TABLE_TYPEDEF: {
/* TODO: throw? */
/* TODO: happens changing the class (adding field or method). we ignore it, but dragons are here */

/* existing entries are supposed to be patched */
g_assert (token_index <= table_info_get_rows (&image_base->tables [token_table]));
} else {
break;
}
case MONO_TABLE_PROPERTY: {
/* allow updates to existing properties. */
/* FIXME: use DeltaInfo:prev_gen_rows instead of image_base */
g_assert (token_index <= table_info_get_rows (&image_base->tables [token_table]));
/* assuming that property attributes and type haven't changed. */
break;
}
default: {
g_assert (token_index > table_info_get_rows (&image_base->tables [token_table]));
if (mono_trace_is_traced (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE))
g_print ("todo: do something about this table index: 0x%02x\n", token_table);
}
}
}
return TRUE;
}
Expand Down Expand Up @@ -1016,6 +1078,14 @@ mono_image_load_enc_delta (MonoImage *image_base, gconstpointer dmeta_bytes, uin
return;
}

/* Process EnCMap and compute number of added/modified rows from this
* delta. This enables computing row indexes relative to the delta.
* We use it in pass1 to bail out early if the EnCLog has unsupported
* edits.
*/
delta_info_init (image_dmeta, image_base);


if (!apply_enclog_pass1 (image_base, image_dmeta, dil_bytes, dil_length, error)) {
mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "Error on sanity-checking delta image to base=%s, due to: %s", basename, mono_error_get_message (error));
mono_metadata_update_cancel (generation);
Expand All @@ -1026,8 +1096,6 @@ mono_image_load_enc_delta (MonoImage *image_base, gconstpointer dmeta_bytes, uin
if (table_info_get_rows (table_enclog))
table_to_image_add (image_base);

delta_info_init (image_dmeta, image_base);

mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "base guid: %s", image_base->guid);
mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "dmeta guid: %s", image_dmeta->guid);

Expand Down

0 comments on commit aede50d

Please sign in to comment.