Skip to content

Commit 80b18b3

Browse files
axel22axel22
committed
[GR-10165] Unsafe.compareAndExchange regression on JDK 11.
PullRequest: graal/1609
2 parents 9ebd013 + 850f911 commit 80b18b3

File tree

2 files changed

+207
-2
lines changed

2 files changed

+207
-2
lines changed

‎compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/java/UnsafeCompareAndExchangeNode.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
package org.graalvm.compiler.nodes.java;
2424

2525
import jdk.vm.ci.meta.JavaKind;
26+
import org.graalvm.compiler.core.common.type.Stamp;
2627
import org.graalvm.compiler.graph.NodeClass;
2728
import org.graalvm.compiler.nodeinfo.NodeInfo;
2829
import org.graalvm.compiler.nodes.NodeView;
@@ -55,8 +56,7 @@ public final class UnsafeCompareAndExchangeNode extends AbstractMemoryCheckpoint
5556
private final LocationIdentity locationIdentity;
5657

5758
public UnsafeCompareAndExchangeNode(ValueNode object, ValueNode offset, ValueNode expected, ValueNode newValue, JavaKind valueKind, LocationIdentity locationIdentity) {
58-
super(TYPE, expected.stamp(NodeView.DEFAULT).meet(newValue.stamp(NodeView.DEFAULT)));
59-
assert expected.stamp(NodeView.DEFAULT).isCompatible(newValue.stamp(NodeView.DEFAULT));
59+
super(TYPE, meetInputs(expected.stamp(NodeView.DEFAULT), newValue.stamp(NodeView.DEFAULT)));
6060
this.object = object;
6161
this.offset = offset;
6262
this.expected = expected;
@@ -65,6 +65,11 @@ public UnsafeCompareAndExchangeNode(ValueNode object, ValueNode offset, ValueNod
6565
this.locationIdentity = locationIdentity;
6666
}
6767

68+
private static Stamp meetInputs(Stamp expected, Stamp newValue) {
69+
assert expected.isCompatible(newValue);
70+
return expected.unrestricted().meet(newValue.unrestricted());
71+
}
72+
6873
public ValueNode object() {
6974
return object;
7075
}

‎compiler/src/org.graalvm.compiler.replacements.jdk9.test/src/org/graalvm/compiler/replacements/jdk9/UnsafeReplacementsTest.java‎

Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,15 @@
2626
import jdk.vm.ci.code.TargetDescription;
2727
import jdk.vm.ci.meta.ResolvedJavaMethod;
2828
import org.graalvm.compiler.api.test.Graal;
29+
import org.graalvm.compiler.core.phases.HighTier;
30+
import org.graalvm.compiler.options.OptionValues;
2931
import org.graalvm.compiler.replacements.test.MethodSubstitutionTest;
3032
import org.graalvm.compiler.runtime.RuntimeProvider;
3133
import org.graalvm.compiler.test.AddExports;
3234
import org.junit.Test;
3335

36+
import java.lang.reflect.Field;
37+
3438
@AddExports("java.base/jdk.internal.misc")
3539
public class UnsafeReplacementsTest extends MethodSubstitutionTest {
3640

@@ -310,4 +314,200 @@ public void testGetAndSet() {
310314
test("unsafeGetAndSetLong");
311315
test("unsafeGetAndSetObject");
312316
}
317+
318+
public static void fieldInstance() {
319+
JdkInternalMiscUnsafeAccessTestBoolean.testFieldInstance();
320+
}
321+
322+
@Test
323+
public void testFieldInstance() {
324+
test(new OptionValues(getInitialOptions(), HighTier.Options.Inline, false), "fieldInstance");
325+
}
326+
327+
public static void array() {
328+
JdkInternalMiscUnsafeAccessTestBoolean.testArray();
329+
}
330+
331+
@Test
332+
public void testArray() {
333+
test(new OptionValues(getInitialOptions(), HighTier.Options.Inline, false), "array");
334+
}
335+
336+
public static void fieldStatic() {
337+
JdkInternalMiscUnsafeAccessTestBoolean.testFieldStatic();
338+
}
339+
340+
@Test
341+
public void testFieldStatic() {
342+
test(new OptionValues(getInitialOptions(), HighTier.Options.Inline, false), "fieldStatic");
343+
}
344+
345+
public static class JdkInternalMiscUnsafeAccessTestBoolean {
346+
static final int ITERATIONS = 100000;
347+
348+
static final int WEAK_ATTEMPTS = 10;
349+
350+
static final long V_OFFSET;
351+
352+
static final Object STATIC_V_BASE;
353+
354+
static final long STATIC_V_OFFSET;
355+
356+
static final int ARRAY_OFFSET;
357+
358+
static final int ARRAY_SHIFT;
359+
360+
static {
361+
try {
362+
Field staticVField = UnsafeReplacementsTest.JdkInternalMiscUnsafeAccessTestBoolean.class.getDeclaredField("staticV");
363+
STATIC_V_BASE = unsafe.staticFieldBase(staticVField);
364+
STATIC_V_OFFSET = unsafe.staticFieldOffset(staticVField);
365+
} catch (Exception e) {
366+
throw new RuntimeException(e);
367+
}
368+
369+
try {
370+
Field vField = UnsafeReplacementsTest.JdkInternalMiscUnsafeAccessTestBoolean.class.getDeclaredField("v");
371+
V_OFFSET = unsafe.objectFieldOffset(vField);
372+
} catch (Exception e) {
373+
throw new RuntimeException(e);
374+
}
375+
376+
ARRAY_OFFSET = unsafe.arrayBaseOffset(boolean[].class);
377+
int ascale = unsafe.arrayIndexScale(boolean[].class);
378+
ARRAY_SHIFT = 31 - Integer.numberOfLeadingZeros(ascale);
379+
}
380+
381+
static boolean staticV;
382+
383+
boolean v;
384+
385+
@BytecodeParserForceInline
386+
public static void testFieldInstance() {
387+
JdkInternalMiscUnsafeAccessTestBoolean t = new JdkInternalMiscUnsafeAccessTestBoolean();
388+
for (int c = 0; c < ITERATIONS; c++) {
389+
testAccess(t, V_OFFSET);
390+
}
391+
}
392+
393+
public static void testFieldStatic() {
394+
for (int c = 0; c < ITERATIONS; c++) {
395+
testAccess(STATIC_V_BASE, STATIC_V_OFFSET);
396+
}
397+
}
398+
399+
public static void testArray() {
400+
boolean[] array = new boolean[10];
401+
for (int c = 0; c < ITERATIONS; c++) {
402+
for (int i = 0; i < array.length; i++) {
403+
testAccess(array, (((long) i) << ARRAY_SHIFT) + ARRAY_OFFSET);
404+
}
405+
}
406+
}
407+
408+
public static void assertEquals(Object seen, Object expected, String message) {
409+
if (seen != expected) {
410+
throw new AssertionError(message + " - seen: " + seen + ", expected: " + expected);
411+
}
412+
}
413+
414+
// Checkstyle: stop
415+
@BytecodeParserForceInline
416+
public static void testAccess(Object base, long offset) {
417+
// Advanced compare
418+
{
419+
boolean r = unsafe.compareAndExchangeBoolean(base, offset, false, true);
420+
assertEquals(r, false, "success compareAndExchange boolean");
421+
boolean x = unsafe.getBoolean(base, offset);
422+
assertEquals(x, true, "success compareAndExchange boolean value");
423+
}
424+
425+
{
426+
boolean r = unsafe.compareAndExchangeBoolean(base, offset, false, false);
427+
assertEquals(r, true, "failing compareAndExchange boolean");
428+
boolean x = unsafe.getBoolean(base, offset);
429+
assertEquals(x, true, "failing compareAndExchange boolean value");
430+
}
431+
432+
{
433+
boolean r = unsafe.compareAndExchangeBooleanAcquire(base, offset, true, false);
434+
assertEquals(r, true, "success compareAndExchangeAcquire boolean");
435+
boolean x = unsafe.getBoolean(base, offset);
436+
assertEquals(x, false, "success compareAndExchangeAcquire boolean value");
437+
}
438+
439+
{
440+
boolean r = unsafe.compareAndExchangeBooleanAcquire(base, offset, true, false);
441+
assertEquals(r, false, "failing compareAndExchangeAcquire boolean");
442+
boolean x = unsafe.getBoolean(base, offset);
443+
assertEquals(x, false, "failing compareAndExchangeAcquire boolean value");
444+
}
445+
446+
{
447+
boolean r = unsafe.compareAndExchangeBooleanRelease(base, offset, false, true);
448+
assertEquals(r, false, "success compareAndExchangeRelease boolean");
449+
boolean x = unsafe.getBoolean(base, offset);
450+
assertEquals(x, true, "success compareAndExchangeRelease boolean value");
451+
}
452+
453+
{
454+
boolean r = unsafe.compareAndExchangeBooleanRelease(base, offset, false, false);
455+
assertEquals(r, true, "failing compareAndExchangeRelease boolean");
456+
boolean x = unsafe.getBoolean(base, offset);
457+
assertEquals(x, true, "failing compareAndExchangeRelease boolean value");
458+
}
459+
460+
{
461+
boolean success = false;
462+
for (int c = 0; c < WEAK_ATTEMPTS && !success; c++) {
463+
success = unsafe.weakCompareAndSetBooleanPlain(base, offset, true, false);
464+
}
465+
assertEquals(success, true, "weakCompareAndSetPlain boolean");
466+
boolean x = unsafe.getBoolean(base, offset);
467+
assertEquals(x, false, "weakCompareAndSetPlain boolean value");
468+
}
469+
470+
{
471+
boolean success = false;
472+
for (int c = 0; c < WEAK_ATTEMPTS && !success; c++) {
473+
success = unsafe.weakCompareAndSetBooleanAcquire(base, offset, false, true);
474+
}
475+
assertEquals(success, true, "weakCompareAndSetAcquire boolean");
476+
boolean x = unsafe.getBoolean(base, offset);
477+
assertEquals(x, true, "weakCompareAndSetAcquire boolean");
478+
}
479+
480+
{
481+
boolean success = false;
482+
for (int c = 0; c < WEAK_ATTEMPTS && !success; c++) {
483+
success = unsafe.weakCompareAndSetBooleanRelease(base, offset, true, false);
484+
}
485+
assertEquals(success, true, "weakCompareAndSetRelease boolean");
486+
boolean x = unsafe.getBoolean(base, offset);
487+
assertEquals(x, false, "weakCompareAndSetRelease boolean");
488+
}
489+
490+
{
491+
boolean success = false;
492+
for (int c = 0; c < WEAK_ATTEMPTS && !success; c++) {
493+
success = unsafe.weakCompareAndSetBoolean(base, offset, false, true);
494+
}
495+
assertEquals(success, true, "weakCompareAndSet boolean");
496+
boolean x = unsafe.getBoolean(base, offset);
497+
assertEquals(x, true, "weakCompareAndSet boolean");
498+
}
499+
500+
unsafe.putBoolean(base, offset, false);
501+
502+
// Compare set and get
503+
{
504+
boolean o = unsafe.getAndSetBoolean(base, offset, true);
505+
assertEquals(o, false, "getAndSet boolean");
506+
boolean x = unsafe.getBoolean(base, offset);
507+
assertEquals(x, true, "getAndSet boolean value");
508+
}
509+
510+
}
511+
// Checkstyle: resume
512+
}
313513
}

0 commit comments

Comments
 (0)