Issues with generic memory and generic memory ref

[qinsoon]

0f8b533 and mmtk/julia@31cea77 started allocating generic memory into Immix space without pinning (such types used to be allocated with pinning, or into the non moving space). We have seen correctness issues that seem related with generic memory ref.

[d-netto]

Could you elaborate on the correctness issues? Maybe post a reproducer and failure report?

[qinsoon]

@udesou Do you know if we can reproduce the bug easily?

[qinsoon]

#252 should fix one of the problems. jl_genericmemoryref_t has two fields, ptr_or_offset and mem. We trace this type based on its datatype layout. However, in its layout, ptr_or_offset is listed as pointer_void and will not be traced (dt->layout->npointers is 1 for this type).

    jl_datatype_t *memoryref_datatype =
        jl_new_datatype(jl_symbol("GenericMemoryRef"), core, jl_memoryref_supertype, tv,
                        jl_perm_symsvec(2, "ptr_or_offset", "mem"),
                        jl_svec(2, pointer_void, memory_datatype),
                        jl_emptysvec, 0, 0, 2);

We have to make sure ptr_or_offset is properly traced by GC.

[qinsoon]

There is another related issue that is revealed by make test-atomics.

The test uses a type Ref{GenericMemoryRef{T}}. This type gets materialized as RefValue, with GenericMemoryRef that is embedded into the type. From GC's point of view, we see an object of type RefValue, but its layout is the same as GenericMemoryRef. Our special tracing code for ptr_or_offset in GenericMemoryRef won't run, as the type of the object is RefValue rather than GenericMemoryRef. 3966399 in #252 specifically checks this case, and make sure we scan ptr_or_offset for this case.

However, this is very hacky. There can be other corner cases that trigger the same bug, and are not captured by our special case check. I don't suggest we use the above approach unless the Julia devs think it is sufficient.

The problem is that there are some fields that are actually pointers, or internal pointers, or a union of internal pointers and integers, but those are not listed in the jl_datatype_layout as pointers. Julia's GC does not trace those fields, given that the objects that are referenced by those fields are kept alive already. However, for moving GCs, we have to trace those field, and update the reference in case the objects get moved. We currently treat those as special cases -- when we scan certain types, we specifically trace those fields in an adhoc way. However, this is not general. When we embed a type with such pointer fields as values into another type, it is hard/impossible to make sure that those fields can still be traced in the adhoc way.

An ideal solution would be to record those fields in jl_datatype_layout, similar to how pointers are recorded. We can call them as 'maybe_derived_pointers` or something else that makes sense. When a GC scan an object, they should also trace 'maybe_derived_pointers', knowing those fields could be pointers/internal pointers. But this requires a larger change, and I feel we should consult with Julia devs before putting more efforts into it.

Currently we have to special-case ptr_or_offset in jl_genericmemoryref_t, including jl_genericmemoryref_t in jl_array_t, the ptr field in jl_genericmemory_t. With the above solution, we will have a general way to deal with those fields.

[qinsoon]

Array is a similar case as Ref, which also embeds a GenericMemoryRef. If we deal with GenericMemoryRef properly, we can remove the code that treats Array as the special case.

[udesou]

Should we close this after #252?

[qinsoon]

Yeah. #252 and mmtk/julia#92 should fix this issue.