From 2078f22ff07f83ce8fd4f78becba196eff9aa73d Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 17 Mar 2021 22:26:07 -0400 Subject: [PATCH] [mbr] Return -1 if relative index is not in given generation Logically after an update is applied, tokens refer to rows in tables that are a concatenation of all the added rows from every update. (ie: if the baseline table 0x23 had 2 rows and then generation 1 an added 3 more rows, an index like 0x05 refers to table 0x23 row 3 from gen 1). The way the lookup works is that the EnC map table records the logical tokens of the update. So for table 0x23 the 3 additions in gen 1 will have entries like encmap row 5 = 0x23000003, row 6 = 0x23000004, row 7 = 0x23000005, and then rows for the next table and so on. The relative index in gen1 of token 0x23000005 then, is the distance of its row (ie row 7) from the first token for the table (ie row 5), plus 1 (since tokens are 1-based). So in this case token 0x23000005 corresponds to the 7-5+1 = 3rd row of table 0x23 in the gen1 dmeta image. The problem is that previously we returns this index-base+1 computation even in cases where we walked either to the end of the encmap table or past the last row for the table we care about. In the case where we walked to the end of the encmap table, the index-base+1 could sometimes return a value that looked like a valid token. The upshot is that we looked up an incorrect AssemblyRef (0x23 table) from the wrong generation. The updated code returns -1 for all cases where we didn't find an encmap row for our given token. Example: https://gist.github.com/lambdageek/cd7ea06bf5004ba884e7979f28623da5 --- src/mono/mono/metadata/metadata-update.c | 37 +++++++++++++++--------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/src/mono/mono/metadata/metadata-update.c b/src/mono/mono/metadata/metadata-update.c index 510e7456ef7224..0e9cf557ec9129 100644 --- a/src/mono/mono/metadata/metadata-update.c +++ b/src/mono/mono/metadata/metadata-update.c @@ -406,8 +406,10 @@ mono_image_append_delta (MonoImage *base, MonoImage *delta) return; } g_assert (((MonoImage*)base->delta_image_last->data)->generation < delta->generation); - /* FIXME: g_list_append returns the previous end of the list, not the newly appended element! */ - base->delta_image_last = g_list_append (base->delta_image_last, delta); + /* g_list_append returns the given list, not the newly appended */ + GList *l = g_list_append (base->delta_image_last, delta); + g_assert (l != NULL && l->next != NULL && l->next->next == NULL); + base->delta_image_last = l->next; } /** @@ -579,6 +581,7 @@ mono_image_effective_table_slow (const MonoTableInfo **t, int *idx) * * 1. Keep a table of inv */ + int g = 0; do { g_assertf (list, "couldn't find idx=0x%08x in assembly=%s", *idx, dmeta && dmeta->name ? dmeta->name : "unknown image"); @@ -586,9 +589,10 @@ mono_image_effective_table_slow (const MonoTableInfo **t, int *idx) list = list->next; table = &dmeta->tables [tbl_index]; ridx = mono_image_relative_delta_index (dmeta, mono_metadata_make_token (tbl_index, *idx + 1)) - 1; + g++; } while (ridx < 0 || ridx >= table->rows); - mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "effective table for %s: 0x%08x -> 0x%08x (gen %d)", mono_meta_table_name (tbl_index), *idx, ridx, metadata_update_local_generation (base, dmeta)); + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "effective table for %s: 0x%08x -> 0x%08x (rows = 0x%08x) (gen %d, g %d)", mono_meta_table_name (tbl_index), *idx, ridx, table->rows, metadata_update_local_generation (base, dmeta), g); *t = table; *idx = ridx; @@ -597,6 +601,8 @@ mono_image_effective_table_slow (const MonoTableInfo **t, int *idx) /* * The ENCMAP table contains the base of the relative offset. * + * Returns -1 if the token does not resolve in this generation's ENCMAP. + * * Example: * Say you have a base image with a METHOD table having 5 entries. The minimal * delta image adds another one, so it would be indexed with token @@ -639,17 +645,22 @@ mono_image_relative_delta_index (MonoImage *image_dmeta, int token) } if (mono_metadata_token_table (map_entry) == table) { -#if 0 - g_assert (mono_metadata_token_index (map_entry) == index); -#endif - if (mono_metadata_token_index (map_entry) != index) - if (mono_trace_is_traced (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE)) - g_print ("warning: map_entry=0x%08x != index=0x%08x. is this a problem?\n", map_entry, index); + if (mono_metadata_token_index (map_entry) == index) { + /* token resolves to this generation */ + int return_val = index_map - delta_info->enc_recs [table] + 1; + g_assert (return_val > 0 && return_val <= image_dmeta->tables[table].rows); + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "relative index for token 0x%08x -> table 0x%02x row 0x%08x", token, table, return_val); + return return_val; + } else { + /* otherwise the last entry in the encmap is for this table, but is still less than the index - the index is in the next generation */ + g_assert (mono_metadata_token_index (map_entry) < index && index_map == encmap->rows); + return -1; + } + } else { + /* otherwise there are no more encmap entries for this table, and we didn't see the index, so there index is in the next generation */ + g_assert (mono_metadata_token_table (map_entry) > table); + return -1; } - - int return_val = index_map - delta_info->enc_recs [table] + 1; - g_assert (return_val > 0); - return return_val; } static DeltaInfo*