Re: Merging gc state tests for two dependent loads

[Yude Lin]

> Are you observing some sort of failure?

No. In this case I haven't observed a failure. It only triggers a fault in a protection mechanism (which should be changed depending on if this really is a bug).

> I wonder if what you're seeing is that the loads are hoisted above the
> gc state test but the results of the chain of loads is used only after
> the gc state test. If that's the case, then AFIU, there's no bug
> i.e. it's ok to "speculatively" load from the objects assuming they
> haven't moved as long as the resulting value are not used until it's
> proven they haven't moved..

The chain of loads result is used after the gc state tests.
But still this changed what I thought was the invariant that lrb guarantees.
Let me use an example to show what might be the problem.

Thread1:                                                           Thread2:
 inside a lock {
                                                                            objB_ts.refC = NULL; // Suppose objB_ts is the to-space reference of objB
                                                                            cond = true; // cond was initialized to false
                                                                          }
inside a lock {
  if (cond == true) {
 objB = objA.refB;
 objC = objB.refC; // expects objC == NULL, but if objB is the from-space reference, then not necessarily NULL
    // gc state tests come after
    // objC will be corrected to point to to-space object, but we expects NULL
  }
}

So if another thread (Thread2) somehow got a to-space reference of objB and made some update.
Thread1 should see this update. Instead if we let thread1 loads from the from-space objB, the
update will be lost. The lrb cannot restore the update. I cannot think of a simpler example but I think
this behaviour breaks many assumptions and could cause other problems?

Yude
------------------------------------------------------------------
From:Roland Westrelin <rwestrel at redhat.com>
Send Time:2022年3月21日(星期一) 20:23
To:林育德(御德) <yude.lyd at alibaba-inc.com>; shenandoah-dev <shenandoah-dev at openjdk.java.net>
Subject:Re: Merging gc state tests for two dependent loads


> Shenandoah will merge two gc state tests (from two LRBs) if they satisfy certain control flow constraint (in ShenandoahBarrierC2Support::merge_back_to_back_tests). I think there could be an issue if the two loads the two LRBs guard are dependent.
>
> To reproduce, I ran java -XX:-UseCompressedOops -XX:+UseShenandoahGC -XX:CompileCommand=print,*ThreadLocal.set -jar SPECjvm2008.jar -coe -ict -ikv -i 2 -bt 8 derby
>
> The C2 outcome for ThreadLocal.set has:
>
> ..
>   0: mov 0x268(%r15),%rsi ; Thread t = Thread.currentThread(); // get oop handle
>   1: mov (%rsi),%r12 ; Thread t = Thread.currentThread(); // get oop out of oop handle
>   2: mov 0x58(%r12),%rbx ; ThreadLocalMap map = getMap(t);
>   3: nopl 0x0(%rax)
>   4: testb $0x1,0x20(%r15) ; gc state test is too late here
>   5: jne 0x00007f0a3508f6e7
> ..
>
> There is no barrier between line 1 and line 2. There was a barrier, but it was merged with the barrier for line 2, (the gc state tests are merged) and the merged test is placed at line 4. The problem is line 2 could visit from-space object and get outdated data.

I wonder if what you're seeing is that the loads are hoisted above the
gc state test but the results of the chain of loads is used only after
the gc state test. If that's the case, then AFIU, there's no bug
i.e. it's ok to "speculatively" load from the objects assuming they
haven't moved as long as the resulting value are not used until it's
proven they haven't moved..

Are you observing some sort of failure?

Roland.