Skip to content

Commit

Permalink
[Debugger] Debugger failure when field content is assigned by an unsa…
Browse files Browse the repository at this point in the history 
…fe value (#12394)

* [Debugger] Debugger failure when a method is called in watches and any field of the Class has the type of the field different of the value of the field because the assign was done in an unsafe way like this:
Now the datatypes check in the fields are only executed when it's not inside an invoke method.
_pinnable = Unsafe.As<Pinnable<T>>(array);
A unit test that reproduces the issue was created either.
Fixes #12374
  • Loading branch information
@thaystg
thaystg committed Jan 15, 2019
1 parent 3e3a2f5 commit 41eb902
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 25 deletions.
2 changes: 1 addition & 1 deletion mcs/class/Mono.Debugger.Soft/Makefile
View file
Expand Up @@ -34,7 +34,7 @@ $(test_output_dir)/dtest-excfilter.exe: Test/dtest-excfilter.il | $(test_output_
$(ILASM) -out:$@ /exe /debug Test/dtest-excfilter.il

$(test_output_dir)/dtest-app.exe: Test/dtest-app.cs $(TEST_HELPERS_SOURCES) | $(test_output_dir)
$(CSCOMPILE) -r:$(topdir)/class/lib/$(PROFILE)/mscorlib.dll -r:$(topdir)/class/lib/$(PROFILE)/System.Core.dll -r:$(topdir)/class/lib/$(PROFILE)/System.dll -sourcelink:Test/sourcelink.json -out:$@ -unsafe $(PLATFORM_DEBUG_FLAGS) -optimize- Test/dtest-app.cs $(TEST_HELPERS_SOURCES)
$(CSCOMPILE) -r:$(topdir)/class/lib/$(PROFILE)/mscorlib.dll -r:$(topdir)/class/lib/$(PROFILE)/System.Core.dll -r:$(topdir)/class/lib/$(PROFILE)/System.dll -r:$(topdir)/class/lib/$(PROFILE)/System.Runtime.CompilerServices.Unsafe.dll -sourcelink:Test/sourcelink.json -out:$@ -unsafe $(PLATFORM_DEBUG_FLAGS) -optimize- Test/dtest-app.cs $(TEST_HELPERS_SOURCES)

TEST_HELPERS_SOURCES = \
../test-helpers/NetworkHelpers.cs \
Expand Down
23 changes: 23 additions & 0 deletions mcs/class/Mono.Debugger.Soft/Test/dtest-app.cs
View file
Expand Up @@ -193,6 +193,19 @@ public class GClass<T> {
}
}

public struct MySpan<T> {
internal class Pinnable<J> {
public J Data;
}
Pinnable<T> _pinnable;
public MySpan(T[] array) {
_pinnable = Unsafe.As<Pinnable<T>>(array);
}
public override string ToString() {
return "abc";
}
}

public struct GStruct<T> {
public T i;

Expand Down Expand Up @@ -474,6 +487,7 @@ public static void wait_one ()
new Tests ().evaluate_method ();
Bug59649 ();
elapsed_time();
field_with_unsafe_cast_value();
inspect_enumerator_in_generic_struct();
return 3;
}
Expand Down Expand Up @@ -657,6 +671,15 @@ public static void ss7_3 ()
}
}

[MethodImplAttribute (MethodImplOptions.NoInlining)]
public static void field_with_unsafe_cast_value() {
var arr = new char[3];
arr[0] = 'a';
arr[1] = 'b';
arr[2] = 'c';
MySpan<char> bytes = new MySpan<char>(arr);
}

[MethodImplAttribute (MethodImplOptions.NoInlining)]
public static void ss_nested () {
ss_nested_1 (ss_nested_2 ());
Expand Down
21 changes: 21 additions & 0 deletions mcs/class/Mono.Debugger.Soft/Test/dtest.cs
View file
Expand Up @@ -4807,5 +4807,26 @@ public void Bug59649 ()
AssertValue (1, mirror["i"]);
AssertValue (2.0, mirror["d"]);
}

[Test]
public void FieldWithUnsafeCastValue() {
Event e = run_until("field_with_unsafe_cast_value");
var req = create_step(e);
req.Enable();
e = step_once();
e = step_over();
e = step_over();
e = step_over();
e = step_over();
e = step_over();
var frame = e.Thread.GetFrames () [0];
var ginst = frame.Method.GetLocal ("bytes");
Value variable = frame.GetValue (ginst);
StructMirror thisObj = (StructMirror)variable;
TypeMirror thisType = thisObj.Type;
variable = thisObj.InvokeMethod(e.Thread, thisType.GetMethod("ToString"), null);
AssertValue ("abc", variable);

}
} // class DebuggerTests
} // namespace
50 changes: 26 additions & 24 deletions mono/mini/debugger-agent.c
View file
Expand Up @@ -5383,10 +5383,10 @@ obj_is_of_type (MonoObject *obj, MonoType *t)
}

static ErrorCode
decode_value (MonoType *t, MonoDomain *domain, gpointer void_addr, gpointer void_buf, guint8 **endbuf, guint8 *limit);
decode_value (MonoType *t, MonoDomain *domain, gpointer void_addr, gpointer void_buf, guint8 **endbuf, guint8 *limit, gboolean check_field_datatype);

static ErrorCode
decode_vtype (MonoType *t, MonoDomain *domain, gpointer void_addr, gpointer void_buf, guint8 **endbuf, guint8 *limit)
decode_vtype (MonoType *t, MonoDomain *domain, gpointer void_addr, gpointer void_buf, guint8 **endbuf, guint8 *limit, gboolean check_field_datatype)
{
guint8 *addr = (guint8*)void_addr;
guint8 *buf = (guint8*)void_buf;
Expand Down Expand Up @@ -5421,7 +5421,7 @@ decode_vtype (MonoType *t, MonoDomain *domain, gpointer void_addr, gpointer void
continue;
if (mono_field_is_deleted (f))
continue;
err = decode_value (f->type, domain, mono_vtype_get_field_addr (addr, f), buf, &buf, limit);
err = decode_value (f->type, domain, mono_vtype_get_field_addr (addr, f), buf, &buf, limit, check_field_datatype);
if (err != ERR_NONE)
return err;
nfields --;
Expand All @@ -5434,7 +5434,7 @@ decode_vtype (MonoType *t, MonoDomain *domain, gpointer void_addr, gpointer void
}

static ErrorCode
decode_value_internal (MonoType *t, int type, MonoDomain *domain, guint8 *addr, guint8 *buf, guint8 **endbuf, guint8 *limit)
decode_value_internal (MonoType *t, int type, MonoDomain *domain, guint8 *addr, guint8 *buf, guint8 **endbuf, guint8 *limit, gboolean check_field_datatype)
{
ErrorCode err;

Expand Down Expand Up @@ -5524,7 +5524,7 @@ decode_value_internal (MonoType *t, int type, MonoDomain *domain, guint8 *addr,
}
memcpy (addr, mono_object_unbox_internal (obj), mono_class_value_size (obj->vtable->klass, NULL));
} else {
err = decode_vtype (t, domain, addr, buf, &buf, limit);
err = decode_vtype (t, domain, addr, buf, &buf, limit, check_field_datatype);
if (err != ERR_NONE)
return err;
}
Expand All @@ -5543,8 +5543,10 @@ decode_value_internal (MonoType *t, int type, MonoDomain *domain, guint8 *addr,

if (obj) {
if (!obj_is_of_type (obj, t)) {
DEBUG_PRINTF (1, "Expected type '%s', got '%s'\n", mono_type_full_name (t), m_class_get_name (obj->vtable->klass));
return ERR_INVALID_ARGUMENT;
if (check_field_datatype) { //if it's not executing a invoke method check the datatypes.
DEBUG_PRINTF (1, "Expected type '%s', got '%s'\n", mono_type_full_name (t), m_class_get_name (obj->vtable->klass));
return ERR_INVALID_ARGUMENT;
}
}
}
if (obj && obj->vtable->domain != domain)
Expand Down Expand Up @@ -5581,7 +5583,7 @@ decode_value_internal (MonoType *t, int type, MonoDomain *domain, guint8 *addr,
g_assert (vtype_buf);

buf = buf2;
err = decode_vtype (NULL, domain, vtype_buf, buf, &buf, limit);
err = decode_vtype (NULL, domain, vtype_buf, buf, &buf, limit, check_field_datatype);
if (err != ERR_NONE) {
g_free (vtype_buf);
return err;
Expand All @@ -5598,7 +5600,7 @@ decode_value_internal (MonoType *t, int type, MonoDomain *domain, guint8 *addr,
} else if ((t->type == MONO_TYPE_GENERICINST) &&
mono_metadata_generic_class_is_valuetype (t->data.generic_class) &&
m_class_is_enumtype (t->data.generic_class->container_class)){
err = decode_vtype (t, domain, addr, buf, &buf, limit);
err = decode_vtype (t, domain, addr, buf, &buf, limit, check_field_datatype);
if (err != ERR_NONE)
return err;
} else {
Expand All @@ -5613,7 +5615,7 @@ decode_value_internal (MonoType *t, int type, MonoDomain *domain, guint8 *addr,
}

static ErrorCode
decode_value (MonoType *t, MonoDomain *domain, gpointer void_addr, gpointer void_buf, guint8 **endbuf, guint8 *limit)
decode_value (MonoType *t, MonoDomain *domain, gpointer void_addr, gpointer void_buf, guint8 **endbuf, guint8 *limit, gboolean check_field_datatype)
{
guint8 *addr = (guint8*)void_addr;
guint8 *buf = (guint8*)void_buf;
Expand All @@ -5629,7 +5631,7 @@ decode_value (MonoType *t, MonoDomain *domain, gpointer void_addr, gpointer void
/*
* First try decoding it as a Nullable`1
*/
err = decode_value_internal (t, type, domain, addr, buf, endbuf, limit);
err = decode_value_internal (t, type, domain, addr, buf, endbuf, limit, check_field_datatype);
if (err == ERR_NONE)
return err;

Expand All @@ -5638,7 +5640,7 @@ decode_value (MonoType *t, MonoDomain *domain, gpointer void_addr, gpointer void
*/
if (targ->type == type) {
nullable_buf = (guint8 *)g_malloc (mono_class_instance_size (mono_class_from_mono_type_internal (targ)));
err = decode_value_internal (targ, type, domain, nullable_buf, buf, endbuf, limit);
err = decode_value_internal (targ, type, domain, nullable_buf, buf, endbuf, limit, check_field_datatype);
if (err != ERR_NONE) {
g_free (nullable_buf);
return err;
Expand All @@ -5659,7 +5661,7 @@ decode_value (MonoType *t, MonoDomain *domain, gpointer void_addr, gpointer void
}
}

return decode_value_internal (t, type, domain, addr, buf, endbuf, limit);
return decode_value_internal (t, type, domain, addr, buf, endbuf, limit, check_field_datatype);
}

static void
Expand Down Expand Up @@ -6078,12 +6080,12 @@ do_invoke_method (DebuggerTlsData *tls, Buffer *buf, InvokeData *invoke, guint8
memset (this_buf, 0, mono_class_instance_size (m->klass));
p = tmp_p;
} else {
err = decode_value (m_class_get_byval_arg (m->klass), domain, this_buf, p, &p, end);
err = decode_value (m_class_get_byval_arg (m->klass), domain, this_buf, p, &p, end, FALSE);
if (err != ERR_NONE)
return err;
}
} else {
err = decode_value (m_class_get_byval_arg (m->klass), domain, this_buf, p, &p, end);
err = decode_value (m_class_get_byval_arg (m->klass), domain, this_buf, p, &p, end, FALSE);
if (err != ERR_NONE)
return err;
}
Expand Down Expand Up @@ -6147,7 +6149,7 @@ do_invoke_method (DebuggerTlsData *tls, Buffer *buf, InvokeData *invoke, guint8
args = (gpointer *)g_alloca (nargs * sizeof (gpointer));
for (i = 0; i < nargs; ++i) {
if (MONO_TYPE_IS_REFERENCE (sig->params [i])) {
err = decode_value (sig->params [i], domain, (guint8*)&args [i], p, &p, end);
err = decode_value (sig->params [i], domain, (guint8*)&args [i], p, &p, end, TRUE);
if (err != ERR_NONE)
break;
if (args [i] && ((MonoObject*)args [i])->vtable->domain != domain)
Expand All @@ -6161,7 +6163,7 @@ do_invoke_method (DebuggerTlsData *tls, Buffer *buf, InvokeData *invoke, guint8
} else {
MonoClass *arg_class = mono_class_from_mono_type_internal (sig->params [i]);
arg_buf [i] = (guint8 *)g_alloca (mono_class_instance_size (arg_class));
err = decode_value (sig->params [i], domain, arg_buf [i], p, &p, end);
err = decode_value (sig->params [i], domain, arg_buf [i], p, &p, end, TRUE);
if (err != ERR_NONE)
break;
if (mono_class_is_nullable (arg_class)) {
Expand Down Expand Up @@ -7227,7 +7229,7 @@ domain_commands (int command, guint8 *p, guint8 *end, Buffer *buf)
o = mono_object_new_checked (domain, klass, error);
mono_error_assert_ok (error);

err = decode_value (m_class_get_byval_arg (klass), domain, (guint8 *)mono_object_unbox_internal (o), p, &p, end);
err = decode_value (m_class_get_byval_arg (klass), domain, (guint8 *)mono_object_unbox_internal (o), p, &p, end, TRUE);
if (err != ERR_NONE)
return err;

Expand Down Expand Up @@ -7907,7 +7909,7 @@ type_commands_internal (int command, MonoClass *klass, MonoDomain *domain, guint
if (!is_ok (error))
return ERR_INVALID_FIELDID;
val = (guint8 *)g_malloc (mono_class_instance_size (mono_class_from_mono_type_internal (f->type)));
err = decode_value (f->type, domain, val, p, &p, end);
err = decode_value (f->type, domain, val, p, &p, end, TRUE);
if (err != ERR_NONE) {
g_free (val);
return err;
Expand Down Expand Up @@ -8905,7 +8907,7 @@ frame_commands (int command, guint8 *p, guint8 *end, Buffer *buf)
val_buf = (guint8 *)g_alloca (sizeof (MonoObject*));
else
val_buf = (guint8 *)g_alloca (mono_class_instance_size (mono_class_from_mono_type_internal (t)));
err = decode_value (t, frame->de.domain, val_buf, p, &p, end);
err = decode_value (t, frame->de.domain, val_buf, p, &p, end, TRUE);
if (err != ERR_NONE)
return err;

Expand Down Expand Up @@ -8939,7 +8941,7 @@ frame_commands (int command, guint8 *p, guint8 *end, Buffer *buf)
g_assert (MONO_TYPE_ISSTRUCT (t));

val_buf = (guint8 *)g_alloca (mono_class_instance_size (mono_class_from_mono_type_internal (t)));
err = decode_value (t, frame->de.domain, val_buf, p, &p, end);
err = decode_value (t, frame->de.domain, val_buf, p, &p, end, TRUE);
if (err != ERR_NONE)
return err;

Expand Down Expand Up @@ -9015,7 +9017,7 @@ array_commands (int command, guint8 *p, guint8 *end, Buffer *buf)
for (i = index; i < index + len; ++i) {
elem = (gpointer*)((char*)arr->vector + (i * esize));

decode_value (m_class_get_byval_arg (m_class_get_element_class (arr->obj.vtable->klass)), arr->obj.vtable->domain, (guint8 *)elem, p, &p, end);
decode_value (m_class_get_byval_arg (m_class_get_element_class (arr->obj.vtable->klass)), arr->obj.vtable->domain, (guint8 *)elem, p, &p, end, TRUE);
}
break;
default:
Expand Down Expand Up @@ -9250,15 +9252,15 @@ object_commands (int command, guint8 *p, guint8 *end, Buffer *buf)
}

val = (guint8 *)g_malloc (mono_class_instance_size (mono_class_from_mono_type_internal (f->type)));
err = decode_value (f->type, obj->vtable->domain, val, p, &p, end);
err = decode_value (f->type, obj->vtable->domain, val, p, &p, end, TRUE);
if (err != ERR_NONE) {
g_free (val);
return err;
}
mono_field_static_set_value_internal (vtable, f, val);
g_free (val);
} else {
err = decode_value (f->type, obj->vtable->domain, (guint8*)obj + f->offset, p, &p, end);
err = decode_value (f->type, obj->vtable->domain, (guint8*)obj + f->offset, p, &p, end, TRUE);
if (err != ERR_NONE)
return err;
}
Expand Down

0 comments on commit 41eb902

Please sign in to comment.