Skip to content

Commit

Permalink
[sdb] Avoid stack overflows when a boxed vtype recursively references…
Browse files Browse the repository at this point in the history 
… itself using fields. Fixes #18914.
  • Loading branch information
@vargaz
vargaz committed May 13, 2014
1 parent aca422f commit 5f54be7
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 13 deletions.
8 changes: 6 additions & 2 deletions mcs/class/Mono.Debugger.Soft/Mono.Debugger.Soft/Connection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ struct ObjectRefInfo {

enum ValueTypeId {
VALUE_TYPE_ID_NULL = 0xf0,
VALUE_TYPE_ID_TYPE = 0xf1
VALUE_TYPE_ID_TYPE = 0xf1,
VALUE_TYPE_ID_PARENT_VTYPE = 0xf2
}

[Flags]
Expand Down Expand Up @@ -209,6 +210,7 @@ class ValueImpl {
public ValueImpl[] Fields; // for ElementType.ValueType
public bool IsEnum; // For ElementType.ValueType
public long Id; /* For VALUE_TYPE_ID_TYPE */
public int Index; /* For VALUE_TYPE_PARENT_VTYPE */
}

class ModuleInfo {
Expand Down Expand Up @@ -413,7 +415,7 @@ public abstract class Connection
* with newer runtimes, and vice versa.
*/
internal const int MAJOR_VERSION = 2;
internal const int MINOR_VERSION = 32;
internal const int MINOR_VERSION = 33;

enum WPSuspendPolicy {
NONE = 0,
Expand Down Expand Up @@ -848,6 +850,8 @@ public ValueImpl ReadValue () {
return new ValueImpl { Type = etype };
case (ElementType)ValueTypeId.VALUE_TYPE_ID_TYPE:
return new ValueImpl () { Type = etype, Id = ReadId () };
case (ElementType)ValueTypeId.VALUE_TYPE_ID_PARENT_VTYPE:
return new ValueImpl () { Type = etype, Index = ReadInt () };
default:
throw new NotImplementedException ("Unable to handle type " + etype);
}
Expand Down
4 changes: 4 additions & 0 deletions mcs/class/Mono.Debugger.Soft/Mono.Debugger.Soft/StructMirror.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ public Value this [String field] {
}
}

internal void SetFields (Value[] fields) {
this.fields = fields;
}

internal void SetField (int index, Value value) {
fields [index] = value;
}
Expand Down
24 changes: 22 additions & 2 deletions mcs/class/Mono.Debugger.Soft/Mono.Debugger.Soft/VirtualMachine.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,10 @@ internal EventRequest GetRequest (int id) {
}

internal Value DecodeValue (ValueImpl v) {
return DecodeValue (v, null);
}

internal Value DecodeValue (ValueImpl v, Dictionary<int, Value> parent_vtypes) {
if (v.Value != null)
return new PrimitiveValue (this, v.Value);

Expand All @@ -582,12 +586,21 @@ internal Value DecodeValue (ValueImpl v) {
case ElementType.Object:
return GetObject (v.Objid);
case ElementType.ValueType:
if (parent_vtypes == null)
parent_vtypes = new Dictionary<int, Value> ();
StructMirror vtype;
if (v.IsEnum)
return new EnumMirror (this, GetType (v.Klass), DecodeValues (v.Fields));
vtype = new EnumMirror (this, GetType (v.Klass), (Value[])null);
else
return new StructMirror (this, GetType (v.Klass), DecodeValues (v.Fields));
vtype = new StructMirror (this, GetType (v.Klass), (Value[])null);
parent_vtypes [parent_vtypes.Count] = vtype;
vtype.SetFields (DecodeValues (v.Fields, parent_vtypes));
parent_vtypes.Remove (parent_vtypes.Count - 1);
return vtype;
case (ElementType)ValueTypeId.VALUE_TYPE_ID_NULL:
return new PrimitiveValue (this, null);
case (ElementType)ValueTypeId.VALUE_TYPE_ID_PARENT_VTYPE:
return parent_vtypes [v.Index];
default:
throw new NotImplementedException ("" + v.Type);
}
Expand All @@ -600,6 +613,13 @@ internal Value[] DecodeValues (ValueImpl[] values) {
return res;
}

internal Value[] DecodeValues (ValueImpl[] values, Dictionary<int, Value> parent_vtypes) {
Value[] res = new Value [values.Length];
for (int i = 0; i < values.Length; ++i)
res [i] = DecodeValue (values [i], parent_vtypes);
return res;
}

internal ValueImpl EncodeValue (Value v) {
if (v is PrimitiveValue) {
object val = (v as PrimitiveValue).Value;
Expand Down
24 changes: 24 additions & 0 deletions mcs/class/Mono.Debugger.Soft/Test/dtest-app.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,18 @@ public struct NestedStruct {
public struct NestedInner {
}

public interface IRecStruct {
void foo (object o);
}

struct RecStruct : IRecStruct {
public object o;

public void foo (object o) {
this.o = o;
}
}

interface ITest
{
void Foo ();
Expand Down Expand Up @@ -617,6 +629,7 @@ public static void vtypes () {
t.vtypes1 (s, arr);
vtypes2 (s);
vtypes3 (s);
vtypes4 ();
}

[MethodImplAttribute (MethodImplOptions.NoInlining)]
Expand All @@ -637,6 +650,17 @@ public static void vtypes3 (AStruct s) {
AStruct.static_foo (5);
}

[MethodImplAttribute (MethodImplOptions.NoInlining)]
public static void vtypes4_2 (IRecStruct o) {
}

[MethodImplAttribute (MethodImplOptions.NoInlining)]
public static void vtypes4 () {
IRecStruct s = new RecStruct ();
s.foo (s);
vtypes4_2 (s);
}

[MethodImplAttribute (MethodImplOptions.NoInlining)]
public static void locals () {
string s = null;
Expand Down
6 changes: 6 additions & 0 deletions mcs/class/Mono.Debugger.Soft/Test/dtest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1463,6 +1463,12 @@ public void VTypes () {
Assert.AreEqual ("static_foo", (e as StepEvent).Method.Name);
obj = frame.GetThis ();
AssertValue (null, obj);

// vtypes which reference themselves recursively
e = run_until ("vtypes4_2");
frame = e.Thread.GetFrames () [0];

Assert.IsTrue (frame.GetArgument (0) is StructMirror);
}

[Test]
Expand Down
51 changes: 42 additions & 9 deletions mono/mini/debugger-agent.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ typedef struct {
#define HEADER_LENGTH 11

#define MAJOR_VERSION 2
#define MINOR_VERSION 32
#define MINOR_VERSION 33

typedef enum {
CMD_SET_VM = 1,
Expand Down Expand Up @@ -385,7 +385,8 @@ typedef enum {

typedef enum {
VALUE_TYPE_ID_NULL = 0xf0,
VALUE_TYPE_ID_TYPE = 0xf1
VALUE_TYPE_ID_TYPE = 0xf1,
VALUE_TYPE_ID_PARENT_VTYPE = 0xf2
} ValueTypeId;

typedef enum {
Expand Down Expand Up @@ -5534,9 +5535,10 @@ mono_debugger_agent_end_exception_filter (MonoException *exc, MonoContext *ctx,
*/
static void
buffer_add_value_full (Buffer *buf, MonoType *t, void *addr, MonoDomain *domain,
gboolean as_vtype)
gboolean as_vtype, GHashTable *parent_vtypes)
{
MonoObject *obj;
gboolean boxed_vtype = FALSE;

if (t->byref) {
if (!(*(void**)addr)) {
Expand Down Expand Up @@ -5626,6 +5628,7 @@ buffer_add_value_full (Buffer *buf, MonoType *t, void *addr, MonoDomain *domain,
if (obj->vtable->klass->valuetype) {
t = &obj->vtable->klass->byval_arg;
addr = mono_object_unbox (obj);
boxed_vtype = TRUE;
goto handle_vtype;
} else if (obj->vtable->klass->rank) {
buffer_add_byte (buf, obj->vtable->klass->byval_arg.type);
Expand All @@ -5643,6 +5646,28 @@ buffer_add_value_full (Buffer *buf, MonoType *t, void *addr, MonoDomain *domain,
gpointer iter;
MonoClassField *f;
MonoClass *klass = mono_class_from_mono_type (t);
int vtype_index;

if (boxed_vtype) {
/*
* Handle boxed vtypes recursively referencing themselves using fields.
*/
if (!parent_vtypes)
parent_vtypes = g_hash_table_new (NULL, NULL);
vtype_index = GPOINTER_TO_INT (g_hash_table_lookup (parent_vtypes, addr));
if (vtype_index) {
if (CHECK_PROTOCOL_VERSION (2, 33)) {
buffer_add_byte (buf, VALUE_TYPE_ID_PARENT_VTYPE);
buffer_add_int (buf, vtype_index - 1);
} else {
/* The client can't handle PARENT_VTYPE */
buffer_add_byte (buf, VALUE_TYPE_ID_NULL);
}
break;
} else {
g_hash_table_insert (parent_vtypes, addr, GINT_TO_POINTER (g_hash_table_size (parent_vtypes) + 1));
}
}

buffer_add_byte (buf, MONO_TYPE_VALUETYPE);
buffer_add_byte (buf, klass->enumtype);
Expand All @@ -5665,7 +5690,15 @@ buffer_add_value_full (Buffer *buf, MonoType *t, void *addr, MonoDomain *domain,
continue;
if (mono_field_is_deleted (f))
continue;
buffer_add_value_full (buf, f->type, (guint8*)addr + f->offset - sizeof (MonoObject), domain, FALSE);
buffer_add_value_full (buf, f->type, (guint8*)addr + f->offset - sizeof (MonoObject), domain, FALSE, parent_vtypes);
}

if (boxed_vtype) {
g_hash_table_remove (parent_vtypes, addr);
if (g_hash_table_size (parent_vtypes) == 0) {
g_hash_table_destroy (parent_vtypes);
parent_vtypes = NULL;
}
}
break;
}
Expand All @@ -5684,7 +5717,7 @@ buffer_add_value_full (Buffer *buf, MonoType *t, void *addr, MonoDomain *domain,
static void
buffer_add_value (Buffer *buf, MonoType *t, void *addr, MonoDomain *domain)
{
buffer_add_value_full (buf, t, addr, domain, FALSE);
buffer_add_value_full (buf, t, addr, domain, FALSE, NULL);
}

static gboolean
Expand Down Expand Up @@ -5960,15 +5993,15 @@ add_var (Buffer *buf, MonoDebugMethodJitInfo *jit, MonoType *t, MonoDebugVarInfo
case MONO_DEBUG_VAR_ADDRESS_MODE_REGISTER:
reg_val = mono_arch_context_get_int_reg (ctx, reg);

buffer_add_value_full (buf, t, &reg_val, domain, as_vtype);
buffer_add_value_full (buf, t, &reg_val, domain, as_vtype, NULL);
break;
case MONO_DEBUG_VAR_ADDRESS_MODE_REGOFFSET:
addr = (gpointer)mono_arch_context_get_int_reg (ctx, reg);
addr += (gint32)var->offset;

//printf ("[R%d+%d] = %p\n", reg, var->offset, addr);

buffer_add_value_full (buf, t, addr, domain, as_vtype);
buffer_add_value_full (buf, t, addr, domain, as_vtype, NULL);
break;
case MONO_DEBUG_VAR_ADDRESS_MODE_DEAD:
NOT_IMPLEMENTED;
Expand All @@ -5981,7 +6014,7 @@ add_var (Buffer *buf, MonoDebugMethodJitInfo *jit, MonoType *t, MonoDebugVarInfo

gaddr = *(gpointer*)addr;
g_assert (gaddr);
buffer_add_value_full (buf, t, gaddr, domain, as_vtype);
buffer_add_value_full (buf, t, gaddr, domain, as_vtype, NULL);
break;
case MONO_DEBUG_VAR_ADDRESS_MODE_GSHAREDVT_LOCAL: {
MonoDebugVarInfo *info_var = jit->gsharedvt_info_var;
Expand Down Expand Up @@ -6023,7 +6056,7 @@ add_var (Buffer *buf, MonoDebugMethodJitInfo *jit, MonoType *t, MonoDebugVarInfo

addr = locals + GPOINTER_TO_INT (info->entries [idx]);

buffer_add_value_full (buf, t, addr, domain, as_vtype);
buffer_add_value_full (buf, t, addr, domain, as_vtype, NULL);
break;
}

Expand Down

0 comments on commit 5f54be7

Please sign in to comment.