[mini] Add missing gsharedvt write barrier
authorLudovic Henry <ludovic@xamarin.com>
Tue, 19 Apr 2016 19:47:33 +0000 (21:47 +0200)
committerLudovic Henry <ludovic@xamarin.com>
Tue, 19 Apr 2016 19:47:33 +0000 (21:47 +0200)
The following test would trigger a SIGSEGV during a GC collection:

```
object _Lock;
C5.HashedLinkedList<byte[]> _Set;
List<byte[]> _Candidates;

void Run ()
{
    _Lock = new object ();
    _Set = new C5.HashedLinkedList<byte[]>(new Core.CommonUtils.ByteArrayComparer());

    BuildList ();

    for (var i = 0; i < 10; ++i) {
     Task.Run (() => WorkOnSet ());
}
}

void BuildList() {
    _Candidates = new List<byte[]>();

    Random r = new Random();
for(var i = 0; i < 20; ++i) {
    var key = new byte[16];
r.NextBytes(key);
_Candidates.Add(key);
}
}

void WorkOnSet() {
    Random r = new Random();
for (;;) {
    byte[] src = _Candidates [r.Next (_Candidates.Count)];
byte[] key = new byte[src.Length];

        for (int i = 0; i < src.Length; ++i)
    key [i] = src [i];

        lock(_Lock) {
    if(r.Next (2) != 0)
    _Set.Add(key);
else
    _Set.Remove(key);
}
}
}
```

The binary protocol just before the crash would be as follow:

```
collection_requested generation 0 requested_size 4096 force false
collection_begin index 14 generation 0
  copy from 0x3150658 to 0x47e0e40 vtable 0x189f1780 size 24
  gray_enqueue queue 0xa4a2d0 cursor 0x3b68178 value 0x47e0e40
  gray_dequeue queue 0xa4a2d0 cursor 0x3b68178 value 0x47e0e40
  scan_begin obj 0x47e0e40 vtable 0x189f1780 size 24
  scan_process_reference obj 0x47e0e40 ptr 0x47e0e48 value 0x3150618
  scan_process_reference obj 0x47e0e40 ptr 0x47e0e4c value 0x3150638
  scan_process_reference obj 0x47e0e40 ptr 0x47e0e50 value 0x0       <----- here the value is 0x0
  scan_process_reference obj 0x47e0db0 ptr 0x47e0dc0 value 0x47e0e40
collection_end 14 generation 0 scanned 0 unique 0 0.00%
 wbarrier ptr 0x3af8710 value 0x47e0e40 value_vtable 0x189f1780
collection_requested generation 0 requested_size 4096 force false
collection_begin index 15 generation 0
collection_end 15 generation 0 scanned 0 unique 0 0.00%
collection_requested generation 0 requested_size 4096 force false
collection_begin index 16 generation 0
  card_scan start 0x47e0df8 size 520
  scan_begin obj 0x47e0e40 vtable 0x189f1780 size 24
  scan_process_reference obj 0x47e0e40 ptr 0x47e0e48 value 0x47b4d40
  scan_process_reference obj 0x47e0e40 ptr 0x47e0e4c value 0x47b03c0
  scan_process_reference obj 0x47e0e40 ptr 0x47e0e50 value 0x3100800 <----- here the value is 0x3100800, but no WB in between
```

Object at 0x47e0e40 is a C5.HashSet<T>.Bucket with T a C5.KeyValuePair<byte[],C5.HashedLinkedList<byte[]>.Node>. The memory layout of C5.HashSet<T>.Bucket would be as follow:

class Bucket {
 item: offset 8 size 8
 overflow: offset 16 size 4
 hashval: offset 20 size 4
}

So the bug would manifest itself as a missing write barrier when writing to Bucket.overflow, leading to the premature sweeping of a Bucket object. The Bucket.overflow value would then point to the middle of an object, triggering a SIGSEGV when trying to access its vtable.

mono/mini/method-to-ir.c

index 8bf63dcfbfe1b221dbc800d6924d91ea3a40e20e..c0139f0f9a884c38b5ca829c260ae23d66b5b685 100644 (file)
@@ -11365,7 +11365,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
                                } else
 #endif
                                {
-                                       MonoInst *store;
+                                       MonoInst *store, *wbarrier_ptr_ins = NULL;
 
                                        MONO_EMIT_NULL_CHECK (cfg, sp [0]->dreg);
 
@@ -11379,6 +11379,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
                                                EMIT_NEW_BIALU_IMM (cfg, ins, OP_PSUB_IMM, offset_ins->dreg, offset_ins->dreg, 1);
                                                dreg = alloc_ireg_mp (cfg);
                                                EMIT_NEW_BIALU (cfg, ins, OP_PADD, dreg, sp [0]->dreg, offset_ins->dreg);
+                                               wbarrier_ptr_ins = ins;
                                                /* The decomposition will call mini_emit_stobj () which will emit a wbarrier if needed */
                                                EMIT_NEW_STORE_MEMBASE_TYPE (cfg, store, field->type, dreg, 0, sp [1]->dreg);
                                        } else {
@@ -11387,15 +11388,20 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
                                        if (sp [0]->opcode != OP_LDADDR)
                                                store->flags |= MONO_INST_FAULT;
 
-                               if (cfg->gen_write_barriers && mini_type_to_stind (cfg, field->type) == CEE_STIND_REF && !(sp [1]->opcode == OP_PCONST && sp [1]->inst_c0 == 0)) {
-                                       /* insert call to write barrier */
-                                       MonoInst *ptr;
-                                       int dreg;
+                                       if (cfg->gen_write_barriers && mini_type_to_stind (cfg, field->type) == CEE_STIND_REF && !(sp [1]->opcode == OP_PCONST && sp [1]->inst_c0 == 0)) {
+                                               if (mini_is_gsharedvt_klass (klass)) {
+                                                       g_assert (wbarrier_ptr_ins);
+                                                       emit_write_barrier (cfg, wbarrier_ptr_ins, sp [1]);
+                                               } else {
+                                                       /* insert call to write barrier */
+                                                       MonoInst *ptr;
+                                                       int dreg;
 
-                                       dreg = alloc_ireg_mp (cfg);
-                                       EMIT_NEW_BIALU_IMM (cfg, ptr, OP_PADD_IMM, dreg, sp [0]->dreg, foffset);
-                                       emit_write_barrier (cfg, ptr, sp [1]);
-                               }
+                                                       dreg = alloc_ireg_mp (cfg);
+                                                       EMIT_NEW_BIALU_IMM (cfg, ptr, OP_PADD_IMM, dreg, sp [0]->dreg, foffset);
+                                                       emit_write_barrier (cfg, ptr, sp [1]);
+                                               }
+                                       }
 
                                        store->flags |= ins_flag;
                                }