541338 - Stack pinning and scanning must consider interior pointers

Status: VERIFIED FIXED

(Tamarin Graveyard :: Garbage Collection (mmGC), defect, P2)

Description

[Lars T Hansen]

Two related issues:

- Stack pinning ignores AVMFEATURE_INTERIOR_POINTERS but should not

- It is probably useful to have AVMFEATURE_INTERIOR_STACK_POINTERS to turn on
  interior-pointer recognition during pinning / stack scanning, because interior
  pointers are vastly more likely here than during scanning of the heap.

It has been reported (Srikanth Paladugu) that the RVCT 3.1 compiler plays tricks that makes this necessary, his example is this:

Say for example I have two classes
 
  Class B {
     ...
  }
 
  Class A {
     B bptr;
     ...
  }
 
The pseudocode generated for

  L1:
      A* p = new(gc) A;

is 
 
  Call new; (register r0 will have the return value)
  Call A::A
 
The pseuodocode generated for A::A is

  r0 = r0+0x10    (where 0x10 is the offset of bptr in class A)
  Call B::B
  r0 = r0 – 0x10  (r0 will contain the return value of constructor 
                   which is B’s this)
 
So now when in constructor B, if a call to GC causes a sweep, we really don’t have the actual reference to A, but a reference to B on the stack.  Reference to A can be achieved by a simple offset from B.  The memory for A is swept even before it gets assigned to p at L1.

Comment 1

[Lars T Hansen]

Update: Scanning stack segments from the GC must also consider interior pointers, that's actually crucial.  We should not require AVMFEATURE_INTERIOR_POINTERS to be enabled for that to work.

Comment 2

[Lars T Hansen]

Needed for Garuda.

Comment 3

[Lars T Hansen]

Conclusion after discussion with Tommy is that we'll just do this, we won't introduce a feature for it.  There's a slightly increased chance of false retention from stack data - notably doubles - but we really have to live with that.  Also, we can re-enable this optimization in our own JIT.

Comment 4

[Lars T Hansen]

Attachment: Recognize interior ptrs during pinning, stack scanning (TR-ARGO)

For pinning, we simply recognize interior pointers.

For stack scanning, we mark the stack segment as "has interior pointers" when pushing it onto the mark stack.  Then GC::MarkItem tests this bit.  This adds a test and branch inside MarkItem, but in the testing I've done so far I've not seen any fallout from that.  I'll do a little more benchmarking but I'm not nervous about it.

A fallout from this patch is that MMGC_INTERIOR_PTRS is finessed: the #ifdef only appears in the GCWorkItem constructor, where it sets the bit for "has interior pointers" on all items.

Comment 5

[Lars T Hansen]

tamarin-redux-argo changeset: 3608:8a7dab3933d3

Comment 6

[Chris Peyer]

Engineering work item.  Marking verified fixed.