Trace ptr_or_offset in jl_genericmemoryref_t

[qinsoon]

See #249 (comment).

[udesou]

Did that actually fix the issue? I remember implementing something like that and that it didn't work. I believe what I did to reproduce the issue was running make test-atomics. If that test succeeds with your PR then that's a good indication that your fix works.
With max copying that is.

[udesou]

I think these tests would always fail: https://github.com/mmtk/julia/blob/a3d4e393dc871b8dbe1f7d105ca9d0a9d98864b6/test/atomics.jl#L877

[qinsoon]

Did that actually fix the issue? I remember implementing something like that and that it didn't work. I believe what I did to reproduce the issue was running make test-atomics. If that test succeeds with your PR then that's a good indication that your fix works. With max copying that is.

I will try with atomics tests. I am not sure if this will fix the problem. But currently we are not tracing ptr_or_offset, which seems wrong.

[udesou]

@d-netto Is it possible that we're not tracing all jl_genericmemoryref_t objects with that check? I remember I had some code to simply print out if those objects were scanned when running the atomics test but for some reason nothing was being printed out.

[qinsoon]

Just more info for the discussion. In the runtime code, we can see that jl_genericmemoryref_t may be used as a value (rather than as a pointer to heap objects) -- the ref in jl_array_t is an example, there are a few functions that return jl_genericmemoryref_t as a value -- so objects of this type may be stored outside the heap, like stacks, global memory, native heap, etc. We should be fine with stacks. But if it appears in global memory, or native heap, or anywhere else, we would have an issue -- though I don't know if that happens or not.

[d-netto]

Sorry about the delay. Will take a look soon.

[qinsoon]

I think these tests would always fail: https://github.com/mmtk/julia/blob/a3d4e393dc871b8dbe1f7d105ca9d0a9d98864b6/test/atomics.jl#L877

The test still failed. Will look more into it tomorrow.

[qinsoon]

I added a hack that deals with Ref{GenericMemoryRef{T}}, and the atomics tests now can pass. However, I don't think it is a good solution. See: #249 (comment)

[qinsoon]

Just as a note: in our dev branch mmtk-support-moving-upstream, the tracing for ptr_or_offset in GenericMemoryRef is wrong. The tracing code assumes ptr_or_offset is an internal pointer of mem, however, ptr_or_offset and mem may actually point to different objects. unsafe_wrap exposes this bug. The following code is a minimal example.

n = 15
a = zeros(Bool, n)
# a gets moved, but it is correct.
GC.gc()
# ptr_or_offset and mem for a8 do not point to the same object
a8 = unsafe_wrap(Array, Ptr{UInt8}(pointer(a)), length(a); own=false) # unsafely observe the actual bit patterns in `a`
# a8 gets moved, and its ptr_or_offset field does not get updated correctly by GC
GC.gc() 

println(a) # a is an array of 0s.
println(a8) # a8's pointer is dangling, and points to corrupted memory.

See 3587825 and fb3f50d.

[qinsoon]

fb3f50d changed the location of the forwarding pointer from -64 (bits) to 0. So at least it won't overwrite the object type. It still overwrites the object payload, and causes errors for size calculation. Currently we only trace internal pointers for a few limited number of types, so we are fine at the moment. But anyway, it is still a hack.

mmtk/mmtk-core#1331 needs to be resolved, then we can properly use find_object_from_internal_pointer during tracing.