RFR: 8351997: AArch64: Interpreter volatile reference stores with G1 are not sequentially consistent

[Aleksey Shipilev]

On Wed, 28 May 2025 14:05:41 GMT, Erik Österlund <eosterlund at openjdk.org> wrote:

> I was wondering why haven't we caught this in jcstress. jcstress runs in int/C1/C2 modes specifically to catch issues like these. I believe this slipped through because all of our seqcst tests, Dekker included, operate on primitives. So we never actually explore what happens with reference load/stores, and as this bug shows, there are _interesting_ interactions with GC barriers. I'll see how to amend jcstress to cover this...

Yeah, here it is:


@JCStressTest
@Outcome(id = {"null, A", "B, null", "B, A"}, expect = ACCEPTABLE, desc = "Trivial under sequential consistency")
@Outcome(id = "null, null",                   expect = FORBIDDEN,  desc = "Violates sequential consistency")
@State
public class RefDekkerTest {
    volatile Object a;
    volatile Object b;

    @Actor
    public void actor1(LL_Result r) {
        a = new String("A");
        r.r1 = b;
    }

    @Actor
    public void actor2(LL_Result r) {
        b = new String("B");
        r.r2 = a;
    }
}


...on Graviton 3:


% build/linux-aarch64-server-release/images/jdk/bin/java -jar jcstress.jar -t RefDekker -tb 1m -f 10 -jvmArgs "-Xint" -sc false

...... [FAILED] o.o.j.t.volatiles.RefDekkerTest

  Results across all configurations:

      RESULT      SAMPLES     FREQ      EXPECT  DESCRIPTION
        B, A      377,670    0.06%  Acceptable  Trivial under sequential consistency
     B, null  288,159,312   46.22%  Acceptable  Trivial under sequential consistency
     null, A  331,478,806   53.17%  Acceptable  Trivial under sequential consistency
  null, null    3,385,362    0.54%   Forbidden  Violates sequential consistency




Perhaps confusingly, this only reproduces when I supply `-jvmArgs "-Xint"`. A "normal" way for jcstress to separately compile/interpret methods is via compiler control. Which, I think, accidentally passes due to Erik's comment above: https://github.com/openjdk/jdk/pull/25483#issuecomment-2916521838 -- we still interpret in the mode that have extra fence.

In retrospect, I think conditionalizing barrier emit scheme on the presence of particular compilers is counter-productive, especially when the default behavior (C2 is enabled) is to emit the barriers. In this instance, this would have made this bug a nuisance (that we _would like_ to fix eventually) rather than a seqcst violation (that we _now have_ to fix for C1-only users) :)

Test starts to pass with the patch from this PR.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/25483#issuecomment-2916587396