Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/9.0-staging] Ignore modopts/modreqs for UnsafeAccessor field targets #109709

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/coreclr/vm/prestub.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1497,7 +1497,7 @@ namespace

TokenPairList list { nullptr };
MetaSig::CompareState state{ &list };
state.IgnoreCustomModifiers = false;
state.IgnoreCustomModifiers = true;
if (!DoesFieldMatchUnsafeAccessorDeclaration(cxt, pField, state))
continue;

Expand Down
2 changes: 1 addition & 1 deletion src/mono/mono/metadata/class.c
Original file line number Diff line number Diff line change
Expand Up @@ -2580,7 +2580,7 @@ mono_class_get_field_from_name_full (MonoClass *klass, const char *name, MonoTyp
MonoClassField *gfield = mono_metadata_get_corresponding_field_from_generic_type_definition (field);
g_assert (gfield != NULL);
MonoType *field_type = gfield->type;
if (!mono_metadata_type_equal_full (type, field_type, TRUE))
if (!mono_metadata_type_equal_full (type, field_type, MONO_TYPE_EQ_FLAGS_SIG_ONLY))
continue;
}
return field;
Expand Down
14 changes: 7 additions & 7 deletions src/mono/mono/metadata/marshal-lightweight.c
Original file line number Diff line number Diff line change
Expand Up @@ -2305,8 +2305,8 @@ emit_unsafe_accessor_field_wrapper (MonoMethodBuilder *mb, gboolean inflate_gene
}

MonoClassField *target_field = mono_class_get_field_from_name_full (target_class, member_name, NULL);
if (target_field == NULL || !mono_metadata_type_equal_full (target_field->type, m_class_get_byval_arg (mono_class_from_mono_type_internal (ret_type)), TRUE)) {
mono_mb_emit_exception_full (mb, "System", "MissingFieldException",
if (target_field == NULL || !mono_metadata_type_equal_full (target_field->type, m_class_get_byval_arg (mono_class_from_mono_type_internal (ret_type)), MONO_TYPE_EQ_FLAGS_SIG_ONLY | MONO_TYPE_EQ_FLAG_IGNORE_CMODS)) {
mono_mb_emit_exception_full (mb, "System", "MissingFieldException",
g_strdup_printf("No '%s' in '%s'. Or the type of '%s' doesn't match", member_name, m_class_get_name (target_class), member_name));
return;
}
Expand Down Expand Up @@ -2403,7 +2403,7 @@ inflate_method (MonoClass *klass, MonoMethod *method, MonoMethod *accessor_metho
if ((context.class_inst != NULL) || (context.method_inst != NULL))
result = mono_class_inflate_generic_method_checked (method, &context, error);
mono_error_assert_ok (error);

return result;
}

Expand All @@ -2425,13 +2425,13 @@ emit_unsafe_accessor_ctor_wrapper (MonoMethodBuilder *mb, gboolean inflate_gener
mono_mb_emit_exception_full (mb, "System", "BadImageFormatException", "Invalid usage of UnsafeAccessorAttribute.");
return;
}

MonoClass *target_class = mono_class_from_mono_type_internal (target_type);

ERROR_DECL(find_method_error);

MonoMethodSignature *member_sig = ctor_sig_from_accessor_sig (mb, sig);

MonoClass *in_class = target_class;

MonoMethod *target_method = mono_unsafe_accessor_find_ctor (in_class, member_sig, target_class, find_method_error);
Expand Down Expand Up @@ -2506,7 +2506,7 @@ emit_unsafe_accessor_method_wrapper (MonoMethodBuilder *mb, gboolean inflate_gen
emit_missing_method_error (mb, find_method_error, member_name);
return;
}

g_assert (target_method->klass == target_class);

emit_unsafe_accessor_ldargs (mb, sig, !hasthis ? 1 : 0);
Expand Down Expand Up @@ -2733,7 +2733,7 @@ emit_swift_lowered_struct_load (MonoMethodBuilder *mb, MonoMethodSignature *csig
}
}

/* Swift struct lowering handling causes csig to have additional arguments.
/* Swift struct lowering handling causes csig to have additional arguments.
* This function returns the index of the argument in the csig that corresponds to the argument in the original signature.
*/
static int
Expand Down
8 changes: 7 additions & 1 deletion src/mono/mono/metadata/metadata-internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -1021,8 +1021,14 @@ mono_type_stack_size_internal (MonoType *t, int *align, gboolean allow_open);

MONO_API void mono_type_get_desc (GString *res, MonoType *type, mono_bool include_namespace);

enum {
MONO_TYPE_EQ_FLAGS_NONE = 0,
MONO_TYPE_EQ_FLAGS_SIG_ONLY = 1,
MONO_TYPE_EQ_FLAG_IGNORE_CMODS = 2,
};

gboolean
mono_metadata_type_equal_full (MonoType *t1, MonoType *t2, gboolean signature_only);
mono_metadata_type_equal_full (MonoType *t1, MonoType *t2, int flags);

MonoMarshalSpec *
mono_metadata_parse_marshal_spec_full (MonoImage *image, MonoImage *parent_image, const char *ptr);
Expand Down
22 changes: 8 additions & 14 deletions src/mono/mono/metadata/metadata.c
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,6 @@ typedef struct {
MonoGenericContext context;
} MonoInflatedMethodSignature;

enum {
MONO_TYPE_EQ_FLAGS_SIG_ONLY = 1,
MONO_TYPE_EQ_FLAG_IGNORE_CMODS = 2,
};

static gboolean do_mono_metadata_parse_type (MonoType *type, MonoImage *m, MonoGenericContainer *container, gboolean transient,
const char *ptr, const char **rptr, MonoError *error);

Expand Down Expand Up @@ -2936,7 +2931,7 @@ aggregate_modifiers_equal (gconstpointer ka, gconstpointer kb)
for (int i = 0; i < amods1->count; ++i) {
if (amods1->modifiers [i].required != amods2->modifiers [i].required)
return FALSE;
if (!mono_metadata_type_equal_full (amods1->modifiers [i].type, amods2->modifiers [i].type, TRUE))
if (!mono_metadata_type_equal_full (amods1->modifiers [i].type, amods2->modifiers [i].type, MONO_TYPE_EQ_FLAGS_SIG_ONLY))
return FALSE;
}
return TRUE;
Expand Down Expand Up @@ -5936,24 +5931,23 @@ do_mono_metadata_type_equal (MonoType *t1, MonoType *t2, int equiv_flags)
gboolean
mono_metadata_type_equal (MonoType *t1, MonoType *t2)
{
return do_mono_metadata_type_equal (t1, t2, 0);
return do_mono_metadata_type_equal (t1, t2, MONO_TYPE_EQ_FLAGS_NONE);
}

/**
* mono_metadata_type_equal_full:
* \param t1 a type
* \param t2 another type
* \param signature_only if signature only comparison should be made
* \param flags flags used to modify comparison logic
*
* Determine if \p t1 and \p t2 are signature compatible if \p signature_only is TRUE, otherwise
* behaves the same way as mono_metadata_type_equal.
* The function mono_metadata_type_equal(a, b) is just a shortcut for mono_metadata_type_equal_full(a, b, FALSE).
* \returns TRUE if \p t1 and \p t2 are equal taking \p signature_only into account.
* Determine if \p t1 and \p t2 are compatible based on the supplied flags.
* The function mono_metadata_type_equal(a, b) is just a shortcut for mono_metadata_type_equal_full(a, b, MONO_TYPE_EQ_FLAGS_NONE).
* \returns TRUE if \p t1 and \p t2 are equal.
*/
gboolean
mono_metadata_type_equal_full (MonoType *t1, MonoType *t2, gboolean signature_only)
mono_metadata_type_equal_full (MonoType *t1, MonoType *t2, int flags)
{
return do_mono_metadata_type_equal (t1, t2, signature_only ? MONO_TYPE_EQ_FLAGS_SIG_ONLY : 0);
return do_mono_metadata_type_equal (t1, t2, flags);
}

enum {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,30 @@ public static void Verify_AccessAllFields_CorElementType()
extern static ref delegate*<void> GetFPtr(ref AllFields f);
}

// Contains fields that have modopts/modreqs
struct FieldsWithModifiers
{
private static volatile int s_vInt;
private volatile int _vInt;
}

[Fact]
public static void Verify_AccessFieldsWithModifiers()
{
Console.WriteLine($"Running {nameof(Verify_AccessFieldsWithModifiers)}");

FieldsWithModifiers fieldsWithModifiers = default;

GetStaticVolatileInt(ref fieldsWithModifiers) = default;
GetVolatileInt(ref fieldsWithModifiers) = default;

[UnsafeAccessor(UnsafeAccessorKind.StaticField, Name="s_vInt")]
extern static ref int GetStaticVolatileInt(ref FieldsWithModifiers f);

[UnsafeAccessor(UnsafeAccessorKind.Field, Name="_vInt")]
extern static ref int GetVolatileInt(ref FieldsWithModifiers f);
}

[Fact]
public static void Verify_AccessStaticMethodClass()
{
Expand Down
Loading