- Notifications
You must be signed in to change notification settings - Fork5.2k
[mono] generic wrapper methods for unsafe accessors#101732
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Merged
Uh oh!
There was an error while loading.Please reload this page.
Merged
Changes from1 commit
Commits
Show all changes
31 commits Select commitHold shift + click to select a range
81285fb WIP HACK: always AOT wrappers, even for generics, not the actual acce…
lambdageek3d59833 cleanup
lambdageekfbd3b89 checkpoint generic wrapper methods
lambdageek34381f7 fix msvc build
lambdageekecaff9e hack
lambdageek06be3b4 fix build
lambdageek430f822 clean WIP printfs
lambdageek5789ab7 use generic method owner caches
lambdageek374c1e7 lookup unsafe accessor wrapper instances in aot-runtime
lambdageekdb8cbad cleanup marshaling - dont' use ctx as a flag
lambdageekf252a47 handle some generic field accessors
lambdageeke424b64 issues.targets: enable some unsafe accessor AOT tests
lambdageek03c72f7 WIP: looks like methods are working too?
lambdageek6fcefc6 [metadata] add ability to inflate wrapper data
lambdageekccbd0ba [marshal] refactor unsafe accessor; opt into inflate_wrapper_data
lambdageek0c824fc Merge remote-tracking branch 'origin/main' into hack-generic-wrappers
lambdageek56516b2 comment out printfs
lambdageek769ae55 inflate MonoMethod wrapper data; impl ctor generic unsafe accessors
lambdageek9688f2b fix windows build
lambdageek70aaa2e [aot] handle case of partial generic sharing for unsafe accessor
lambdageeke814387 [aot] for unsafe accessor wrappers with no name, record a length 0
lambdageek03208db [aot-runtime] try to fix gsharedvt lookup
lambdageekc244dd2 better comments; remove fied FIXMEs
lambdageek2ff358b update HelloWorld sample to support either normal AOT or FullAOT
lambdageek25f9e99 revert HelloWorld sample code changes
lambdageek0374f2b rename helper methods
lambdageek9786200 apply suggestions from code review
lambdageekceb5345 DRY. compute inflate_generic_data in one place
lambdageek342fe1d Just do one loop for inflating generic wrapper data
lambdageeka667f53 better comments
lambdageek0bf0f61 DRY. move common AOT code to mini.c
lambdageekFile filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
[metadata] add ability to inflate wrapper data
When we create generic wrappers (or wrappers in a generic class),if the wrapper data needs to refer to a field, method, or parametertype of the definition, that data might need to be inflated if thecontaining class is inflated (or the generic wrapper method isinflated).Add a new function to opt into inflation:```c get_marshal_cb ()->mb_inflate_wrapper_data (mb);```Add a new function to be called after mono_mb_emit_op (or any othercall that calls mono_mb_add_data):```cmono_mb_emit_op (mb, CEE_LDFLDA, field); mono_mb_set_wrapper_data_kind (mb, MONO_MB_ILGEN_WRAPPER_DATA_FIELD);```Note: mono_mb_set_wrapper_data_kind asserts if you haven't called mb_inflate_wrapper_data.TODO: add more wrapper data kinds for MonoMethod* and MonoClass* etc
- Loading branch information
Uh oh!
There was an error while loading.Please reload this page.
commit6fcefc69f5920d8e82514d7a74fc30837b5810ef
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -41,6 +41,7 @@ | ||
| #include <mono/metadata/gc-internals.h> | ||
| #include <mono/metadata/mono-debug.h> | ||
| #include <mono/metadata/metadata-update.h> | ||
| #include <mono/metadata/method-builder-ilgen.h> | ||
| #include <mono/utils/mono-string.h> | ||
| #include <mono/utils/mono-error-internals.h> | ||
| #include <mono/utils/mono-logger-internals.h> | ||
| @@ -1263,6 +1264,17 @@ mono_class_inflate_generic_method_full_checked (MonoMethod *method, MonoClass *k | ||
| resw->method_data = (void **)g_malloc (sizeof (gpointer) * (len + 1)); | ||
| memcpy (resw->method_data, mw->method_data, sizeof (gpointer) * (len + 1)); | ||
| if (mw->inflate_wrapper_data) { | ||
| mono_mb_inflate_generic_wrapper_data (context, (gpointer*)resw->method_data, error); | ||
| if (!is_ok (error)) { | ||
| g_free (resw->method_data); | ||
| goto fail; | ||
| } | ||
| // we can't set inflate_wrapper_data to 0 on the result, it's possible it | ||
| // will need to be inflated again (for example in the method_inst == | ||
| // generic_container->context.method_inst case, below) | ||
| resw->inflate_wrapper_data = 1; | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Nit: Consistency between MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more.
| ||
| } | ||
| } | ||
| if (iresult->context.method_inst) { | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -33,6 +33,25 @@ struct _MonoMethodBuilder { | ||
| const gchar **param_names; | ||
| MonoBitSet *volatile_args; | ||
| MonoBitSet *volatile_locals; | ||
| gboolean inflate_wrapper_data; | ||
| GList *wrapper_data_inflate_info; | ||
| }; | ||
| typedef struct MonoMethodBuilderInflateWrapperData | ||
| { | ||
| uint16_t idx; | ||
| uint16_t kind; // MonoILGenWrapperDataKind | ||
| } MonoMethodBuilderInflateWrapperData; | ||
| enum MonoILGenWrapperDataKind | ||
| { | ||
| MONO_MB_ILGEN_WRAPPER_DATA_NONE = 0, | ||
| MONO_MB_ILGEN_WRAPPER_DATA_FIELD, | ||
| }; | ||
| // index in MonoMethodWrapper:wrapper_data where the inflate info will be stored | ||
| // notes: | ||
| // - idx 1 is the wrapper info (see mono_method_get_wrapper_info) | ||
lambdageek marked this conversation as resolved. OutdatedShow resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
| #define MONO_MB_ILGEN_INFLATE_WRAPPER_INFO_IDX 2 | ||
| #endif | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -52,7 +52,7 @@ new_base_ilgen (MonoClass *klass, MonoWrapperType type, gboolean dynamic) | ||
| mb->init_locals = TRUE; | ||
| mb->dynamic = dynamic; | ||
| /* placeholder for the wrapper always at index 1, see mono_marshal_set_wrapper_info */ | ||
| mono_mb_add_data (mb, NULL); | ||
| return mb; | ||
| @@ -61,9 +61,15 @@ new_base_ilgen (MonoClass *klass, MonoWrapperType type, gboolean dynamic) | ||
| static void | ||
| free_ilgen (MonoMethodBuilder *mb) | ||
| { | ||
| if (mb->wrapper_data_inflate_info) { | ||
| for (GList *p = mb->wrapper_data_inflate_info; p && p->data; p = p->next) { | ||
| g_free (p->data); | ||
| } | ||
| g_list_free (mb->wrapper_data_inflate_info); | ||
| mb->wrapper_data_inflate_info = NULL; | ||
| } | ||
| for (GList *l = mb->locals_list; l; l = l->next) { | ||
| /* Allocated in mono_mb_add_local () */ | ||
| g_free (l->data); | ||
| } | ||
| @@ -172,14 +178,44 @@ create_method_ilgen (MonoMethodBuilder *mb, MonoMethodSignature *signature, int | ||
| i = g_list_length ((GList *)mw->method_data); | ||
| if (i) { | ||
| MonoMethodBuilderInflateWrapperData *inflate_data = NULL; | ||
| if (mb->inflate_wrapper_data) { | ||
| int count = g_list_length (mb->wrapper_data_inflate_info); | ||
| inflate_data = (MonoMethodBuilderInflateWrapperData *) mb_alloc0 (mb, sizeof(MonoMethodBuilderInflateWrapperData) * (count + 1)); | ||
| int j = 0; | ||
| for (GList *p = mb->wrapper_data_inflate_info; p && p->data; p = p->next) { | ||
| inflate_data[j++] = *(MonoMethodBuilderInflateWrapperData*)p->data; | ||
| } | ||
| // trailing idx 0 element to mark the end | ||
| inflate_data[j].idx = 0; | ||
| inflate_data[j].kind = MONO_MB_ILGEN_WRAPPER_DATA_NONE; | ||
| mw->inflate_wrapper_data = 1; | ||
| } | ||
| GList *tmp; | ||
| void **data; | ||
| l = g_list_reverse ((GList *)mw->method_data); | ||
| int data_count = i + (inflate_data ? 2 : 1); | ||
| data = (void **)mb_alloc0 (mb, sizeof (gpointer) * data_count); | ||
| /* store the size in the first element */ | ||
| data [0] = GUINT_TO_POINTER (i); | ||
| i = 1; | ||
| // manually peel off the first 1 or 2 iterations of the loop since they're special | ||
| tmp = l; | ||
| g_assert (tmp); | ||
| // wrapper info is in slot 1 | ||
| g_assert (tmp->data == NULL); | ||
| data [i++] = NULL; | ||
| tmp = tmp->next; | ||
| // inflate data is in slot 2 | ||
| if (inflate_data) { | ||
| g_assert (tmp); | ||
| g_assert (MONO_MB_ILGEN_INFLATE_WRAPPER_INFO_IDX == i); | ||
| g_assert (tmp->data == NULL); | ||
| data[i++] = inflate_data; | ||
| tmp = tmp->next; | ||
| } | ||
| for (;tmp; tmp = tmp->next) { | ||
| data [i++] = tmp->data; | ||
| } | ||
| g_list_free (l); | ||
| @@ -664,3 +700,62 @@ mono_mb_set_param_names (MonoMethodBuilder *mb, const char **param_names) | ||
| { | ||
| mb->param_names = param_names; | ||
| } | ||
| void | ||
| mono_mb_set_wrapper_data_kind (MonoMethodBuilder *mb, uint16_t wrapper_data_kind) | ||
| { | ||
| g_assert (mb->inflate_wrapper_data); | ||
| MonoMethodWrapper *mw = (MonoMethodWrapper*)mb->method; | ||
| // index of the data added by most recent mono_mb_add_data | ||
| int idx = g_list_length((GList *)mw->method_data); | ||
| g_assert (idx > 0 && idx <= UINT16_MAX); | ||
| MonoMethodBuilderInflateWrapperData *info = g_new (MonoMethodBuilderInflateWrapperData, 1); | ||
| info->idx = (uint16_t)idx; | ||
| info->kind = wrapper_data_kind; | ||
| mb->wrapper_data_inflate_info = g_list_prepend (mb->wrapper_data_inflate_info, info); | ||
| } | ||
| gboolean | ||
| mono_mb_inflate_generic_wrapper_data (MonoGenericContext *context, gpointer *method_data, MonoError *error) | ||
| { | ||
| MonoMethodBuilderInflateWrapperData* inflate_info = (MonoMethodBuilderInflateWrapperData*)method_data[MONO_MB_ILGEN_INFLATE_WRAPPER_INFO_IDX]; | ||
| MonoMethodBuilderInflateWrapperData* p = inflate_info; | ||
| int count = 0; | ||
| while (p->idx != 0) {p++; count++;} | ||
lambdageek marked this conversation as resolved. OutdatedShow resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
| for (int i = 0; i < count; i++) { | ||
| int idx = inflate_info[i].idx; | ||
| uint16_t kind = inflate_info[i].kind; | ||
| gpointer *pdata = &method_data[idx]; | ||
| switch (kind) { | ||
| case MONO_MB_ILGEN_WRAPPER_DATA_NONE: | ||
| continue; | ||
| case MONO_MB_ILGEN_WRAPPER_DATA_FIELD: { | ||
| MonoClassField *field = (MonoClassField*)*pdata; | ||
| MonoType *inflated_type = mono_class_inflate_generic_type_checked (m_class_get_byval_arg (m_field_get_parent (field)), context, error); | ||
| if (!is_ok (error)) | ||
| return FALSE; | ||
| MonoClass *inflated_class = mono_class_from_mono_type_internal (inflated_type); | ||
| // TODO: EnC metadata-update | ||
| g_assert (!m_field_is_from_update (field)); | ||
| int i = GPTRDIFF_TO_INT (field - m_class_get_fields (m_field_get_parent (field))); | ||
| gpointer dummy = NULL; | ||
| mono_metadata_free_type (inflated_type); | ||
| mono_class_get_fields_internal (inflated_class, &dummy); | ||
fanyang-mono marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
| g_assert (m_class_get_fields (inflated_class)); | ||
| MonoClassField *inflated_field = &m_class_get_fields (inflated_class) [i]; | ||
| *pdata = inflated_field; | ||
| break; | ||
| } | ||
| default: | ||
| g_assert_not_reached(); | ||
| } | ||
| } | ||
| return TRUE; | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.