Skip to content

Commit 952f9c7

Browse files
GoeLinGoeLin
committed
8275645: [JVMCI] avoid unaligned volatile reads on AArch64
Backport-of: 4dec8fc4cc2b1762fba554d0401da8be0d6d1166
1 parent 869bb36 commit 952f9c7

File tree

6 files changed

+62
-22
lines changed

6 files changed

+62
-22
lines changed

‎src/hotspot/share/jvmci/jvmciCompilerToVM.cpp‎

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1832,7 +1832,20 @@ C2V_VMENTRY_NULL(jobjectArray, getDeclaredMethods, (JNIEnv* env, jobject, jobjec
18321832
return JVMCIENV->get_jobjectArray(methods);
18331833
C2V_END
18341834

1835-
C2V_VMENTRY_NULL(jobject, readFieldValue, (JNIEnv* env, jobject, jobject object, jobject expected_type, long displacement, jboolean is_volatile, jobject kind_object))
1835+
// Enforces volatile semantics for a non-volatile read.
1836+
class VolatileRead : public StackObj {
1837+
public:
1838+
VolatileRead() {
1839+
// Ensures a possibly volatile read is not reordered with a prior
1840+
// volatile write.
1841+
OrderAccess::storeload();
1842+
}
1843+
~VolatileRead() {
1844+
OrderAccess::acquire();
1845+
}
1846+
};
1847+
1848+
C2V_VMENTRY_NULL(jobject, readFieldValue, (JNIEnv* env, jobject, jobject object, jobject expected_type, long displacement, jobject kind_object))
18361849
if (object == NULL || kind_object == NULL) {
18371850
JVMCI_THROW_0(NullPointerException);
18381851
}
@@ -1916,15 +1929,25 @@ C2V_VMENTRY_NULL(jobject, readFieldValue, (JNIEnv* env, jobject, jobject object,
19161929
}
19171930

19181931
jlong value = 0;
1932+
1933+
// Treat all reads as volatile for simplicity as this function can be used
1934+
// both for reading Java fields declared as volatile as well as for constant
1935+
// folding Unsafe.get* methods with volatile semantics. This is done by
1936+
// performing the volatile barrier operations around a call to an
1937+
// oopDesc::<kind>_field method. The oopDesc::<kind>_field_acquire method
1938+
// cannot be used since it does not support unaligned reads on all platforms
1939+
// (e.g., an unaligned ldar on AArch64 causes a SIGBUS).
1940+
1941+
19191942
switch (basic_type) {
1920-
case T_BOOLEAN: value = is_volatile ? obj->bool_field_acquire(displacement) : obj->bool_field(displacement); break;
1921-
case T_BYTE: value = is_volatile ? obj->byte_field_acquire(displacement) : obj->byte_field(displacement); break;
1922-
case T_SHORT: value = is_volatile ? obj->short_field_acquire(displacement) : obj->short_field(displacement); break;
1923-
case T_CHAR: value = is_volatile ? obj->char_field_acquire(displacement) : obj->char_field(displacement); break;
1943+
case T_BOOLEAN: { VolatileRead vr; value = obj->bool_field(displacement); } break;
1944+
case T_BYTE: { VolatileRead vr; value = obj->byte_field(displacement); } break;
1945+
case T_SHORT: { VolatileRead vr; value = obj->short_field(displacement);} break;
1946+
case T_CHAR: { VolatileRead vr; value = obj->char_field(displacement); } break;
19241947
case T_FLOAT:
1925-
case T_INT: value = is_volatile ? obj->int_field_acquire(displacement) : obj->int_field(displacement); break;
1948+
case T_INT: { VolatileRead vr; value = obj->int_field(displacement); } break;
19261949
case T_DOUBLE:
1927-
case T_LONG: value = is_volatile ? obj->long_field_acquire(displacement) : obj->long_field(displacement); break;
1950+
case T_LONG: { VolatileRead vr; value = obj->long_field(displacement); } break;
19281951

19291952
case T_OBJECT: {
19301953
if (displacement == java_lang_Class::component_mirror_offset() && java_lang_Class::is_instance(obj()) &&
@@ -1934,7 +1957,9 @@ C2V_VMENTRY_NULL(jobject, readFieldValue, (JNIEnv* env, jobject, jobject object,
19341957
return JVMCIENV->get_jobject(JVMCIENV->get_JavaConstant_NULL_POINTER());
19351958
}
19361959

1937-
oop value = is_volatile ? obj->obj_field_acquire(displacement) : obj->obj_field(displacement);
1960+
oop value;
1961+
{ VolatileRead vr; value = obj->obj_field(displacement); }
1962+
19381963
if (value == NULL) {
19391964
return JVMCIENV->get_jobject(JVMCIENV->get_JavaConstant_NULL_POINTER());
19401965
} else {
@@ -2680,8 +2705,8 @@ JNINativeMethod CompilerToVM::methods[] = {
26802705
{CC "boxPrimitive", CC "(" OBJECT ")" OBJECTCONSTANT, FN_PTR(boxPrimitive)},
26812706
{CC "getDeclaredConstructors", CC "(" HS_RESOLVED_KLASS ")[" RESOLVED_METHOD, FN_PTR(getDeclaredConstructors)},
26822707
{CC "getDeclaredMethods", CC "(" HS_RESOLVED_KLASS ")[" RESOLVED_METHOD, FN_PTR(getDeclaredMethods)},
2683-
{CC "readFieldValue", CC "(" HS_RESOLVED_KLASS HS_RESOLVED_KLASS "JZLjdk/vm/ci/meta/JavaKind;)" JAVACONSTANT, FN_PTR(readFieldValue)},
2684-
{CC "readFieldValue", CC "(" OBJECTCONSTANT HS_RESOLVED_KLASS "JZLjdk/vm/ci/meta/JavaKind;)" JAVACONSTANT, FN_PTR(readFieldValue)},
2708+
{CC "readFieldValue", CC "(" HS_RESOLVED_KLASS HS_RESOLVED_KLASS "JLjdk/vm/ci/meta/JavaKind;)" JAVACONSTANT, FN_PTR(readFieldValue)},
2709+
{CC "readFieldValue", CC "(" OBJECTCONSTANT HS_RESOLVED_KLASS "JLjdk/vm/ci/meta/JavaKind;)" JAVACONSTANT, FN_PTR(readFieldValue)},
26852710
{CC "isInstance", CC "(" HS_RESOLVED_KLASS OBJECTCONSTANT ")Z", FN_PTR(isInstance)},
26862711
{CC "isAssignableFrom", CC "(" HS_RESOLVED_KLASS HS_RESOLVED_KLASS ")Z", FN_PTR(isAssignableFrom)},
26872712
{CC "isTrustedForIntrinsics", CC "(" HS_RESOLVED_KLASS ")Z", FN_PTR(isTrustedForIntrinsics)},

‎src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/CompilerToVM.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -786,14 +786,14 @@ HotSpotResolvedObjectTypeImpl getResolvedJavaType(long displacement, boolean com
786786
* object is exptected to be a subtype of {@code expectedType} and extra sanity checking is
787787
* performed on the offset and kind of the read being performed.
788788
*/
789-
native JavaConstant readFieldValue(HotSpotResolvedObjectTypeImpl object, HotSpotResolvedObjectTypeImpl expectedType, long offset, boolean isVolatile, JavaKind kind);
789+
native JavaConstant readFieldValue(HotSpotResolvedObjectTypeImpl object, HotSpotResolvedObjectTypeImpl expectedType, long offset, JavaKind kind);
790790

791791
/**
792792
* Reads the current value of an instance field. If {@code expectedType} is non-null, then the
793793
* object is exptected to be a subtype of {@code expectedType} and extra sanity checking is
794794
* performed on the offset and kind of the read being performed.
795795
*/
796-
native JavaConstant readFieldValue(HotSpotObjectConstantImpl object, HotSpotResolvedObjectTypeImpl expectedType, long offset, boolean isVolatile, JavaKind kind);
796+
native JavaConstant readFieldValue(HotSpotObjectConstantImpl object, HotSpotResolvedObjectTypeImpl expectedType, long offset, JavaKind kind);
797797

798798
/**
799799
* @see ResolvedJavaType#isInstance(JavaConstant)

‎src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotConstantReflectionProvider.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,11 +166,11 @@ public JavaConstant readFieldValue(ResolvedJavaField field, JavaConstant receive
166166
if (hotspotField.isStatic()) {
167167
HotSpotResolvedObjectTypeImpl holder = (HotSpotResolvedObjectTypeImpl) hotspotField.getDeclaringClass();
168168
if (holder.isInitialized()) {
169-
return runtime().compilerToVm.readFieldValue(holder, (HotSpotResolvedObjectTypeImpl) hotspotField.getDeclaringClass(), hotspotField.getOffset(), field.isVolatile(),
169+
return runtime().compilerToVm.readFieldValue(holder, (HotSpotResolvedObjectTypeImpl) hotspotField.getDeclaringClass(), hotspotField.getOffset(),
170170
hotspotField.getType().getJavaKind());
171171
}
172172
} else if (receiver instanceof HotSpotObjectConstantImpl) {
173-
return ((HotSpotObjectConstantImpl) receiver).readFieldValue(hotspotField, field.isVolatile());
173+
return ((HotSpotObjectConstantImpl) receiver).readFieldValue(hotspotField);
174174
} else if (receiver == null) {
175175
throw new NullPointerException("receiver is null");
176176
}

‎src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotMemoryAccessProviderImpl.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@ public JavaConstant readPrimitiveConstant(JavaKind kind, Constant baseConstant,
7777
throw new IllegalArgumentException(String.valueOf(bits));
7878
}
7979
}
80-
JavaConstant result = runtime().compilerToVm.readFieldValue((HotSpotObjectConstantImpl) baseConstant, null, initialDisplacement, true, readKind);
80+
HotSpotObjectConstantImpl baseObject = (HotSpotObjectConstantImpl) baseConstant;
81+
JavaConstant result = runtime().compilerToVm.readFieldValue(baseObject, null, initialDisplacement, readKind);
8182
if (result != null && kind != readKind) {
8283
return JavaConstant.forPrimitive(kind, result.asLong());
8384
}
@@ -108,7 +109,7 @@ public JavaConstant readPrimitiveConstant(JavaKind kind, Constant baseConstant,
108109
@Override
109110
public JavaConstant readObjectConstant(Constant base, long displacement) {
110111
if (base instanceof HotSpotObjectConstantImpl) {
111-
return runtime.getCompilerToVM().readFieldValue((HotSpotObjectConstantImpl) base, null, displacement, true, JavaKind.Object);
112+
return runtime.getCompilerToVM().readFieldValue((HotSpotObjectConstantImpl) base, null, displacement, JavaKind.Object);
112113
}
113114
if (base instanceof HotSpotMetaspaceConstant) {
114115
MetaspaceObject metaspaceObject = HotSpotMetaspaceConstantImpl.getMetaspaceObject(base);
@@ -130,7 +131,7 @@ public JavaConstant readObjectConstant(Constant base, long displacement) {
130131
public JavaConstant readNarrowOopConstant(Constant base, long displacement) {
131132
if (base instanceof HotSpotObjectConstantImpl) {
132133
assert runtime.getConfig().useCompressedOops;
133-
JavaConstant res = runtime.getCompilerToVM().readFieldValue((HotSpotObjectConstantImpl) base, null, displacement, true, JavaKind.Object);
134+
JavaConstant res = runtime.getCompilerToVM().readFieldValue((HotSpotObjectConstantImpl) base, null, displacement, JavaKind.Object);
134135
if (res != null) {
135136
return JavaConstant.NULL_POINTER.equals(res) ? HotSpotCompressedNullConstant.COMPRESSED_NULL : ((HotSpotObjectConstant) res).compress();
136137
}
@@ -147,7 +148,6 @@ private HotSpotResolvedObjectTypeImpl readKlass(Constant base, long displacement
147148
}
148149
}
149150

150-
151151
@Override
152152
public Constant readKlassPointerConstant(Constant base, long displacement) {
153153
HotSpotResolvedObjectTypeImpl klass = readKlass(base, displacement, false);

‎src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotObjectConstantImpl.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,15 +72,15 @@ private boolean isFullyInitializedConstantCallSite() {
7272
}
7373
// read ConstantCallSite.isFrozen as a volatile field
7474
HotSpotResolvedJavaField field = HotSpotMethodHandleAccessProvider.Internals.instance().constantCallSiteFrozenField;
75-
boolean isFrozen = readFieldValue(field, true /* volatile */).asBoolean();
75+
boolean isFrozen = readFieldValue(field).asBoolean();
7676
// isFrozen true implies fully-initialized
7777
return isFrozen;
7878
}
7979

8080
private HotSpotObjectConstantImpl readTarget() {
8181
// read CallSite.target as a volatile field
8282
HotSpotResolvedJavaField field = HotSpotMethodHandleAccessProvider.Internals.instance().callSiteTargetField;
83-
return (HotSpotObjectConstantImpl) readFieldValue(field, true /* volatile */);
83+
return (HotSpotObjectConstantImpl) readFieldValue(field);
8484
}
8585

8686
@Override
@@ -184,7 +184,7 @@ public String toString() {
184184
return (compressed ? "NarrowOop" : getJavaKind().getJavaName()) + "[" + runtime().reflection.formatString(this) + "]";
185185
}
186186

187-
public JavaConstant readFieldValue(HotSpotResolvedJavaField field, boolean isVolatile) {
187+
public JavaConstant readFieldValue(HotSpotResolvedJavaField field) {
188188
if (IS_IN_NATIVE_IMAGE && this instanceof DirectHotSpotObjectConstantImpl) {
189189
// cannot read fields from objects due to lack of
190190
// general reflection support in native image
@@ -193,7 +193,7 @@ public JavaConstant readFieldValue(HotSpotResolvedJavaField field, boolean isVol
193193
if (field.isStatic()) {
194194
return null;
195195
}
196-
return runtime().compilerToVm.readFieldValue(this, (HotSpotResolvedObjectTypeImpl) field.getDeclaringClass(), field.getOffset(), isVolatile, field.getType().getJavaKind());
196+
return runtime().compilerToVm.readFieldValue(this, (HotSpotResolvedObjectTypeImpl) field.getDeclaringClass(), field.getOffset(), field.getType().getJavaKind());
197197
}
198198

199199
public ResolvedJavaType asJavaType() {

‎test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.hotspot.test/src/jdk/vm/ci/hotspot/test/MemoryAccessProviderData.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,9 @@ public static Object[][] getPositivePrimitiveJavaKinds() {
7777
for (KindData k : PRIMITIVE_KIND_DATA) {
7878
result.add(new Object[] {k.kind, TEST_CONSTANT, k.instanceFieldOffset, k.instanceFieldValue, Math.max(8, k.kind.getBitCount())});
7979
result.add(new Object[] {k.kind, TEST_CLASS_CONSTANT, k.staticFieldOffset, k.staticFieldValue, Math.max(8, k.kind.getBitCount())});
80+
if (k.unalignedInstanceFieldValue != null) {
81+
result.add(new Object[] {k.kind, TEST_CONSTANT, k.instanceFieldOffset - 1, k.unalignedInstanceFieldValue, Math.max(8, k.kind.getBitCount())});
82+
}
8083
}
8184
return result.toArray(new Object[result.size()][]);
8285
}
@@ -170,6 +173,7 @@ static class KindData {
170173
final long staticFieldOffset;
171174
final JavaConstant instanceFieldValue;
172175
final JavaConstant staticFieldValue;
176+
final JavaConstant unalignedInstanceFieldValue;
173177
KindData(JavaKind kind, Object testObject) {
174178
this.kind = kind;
175179
try {
@@ -182,6 +186,17 @@ static class KindData {
182186
staticFieldOffset = UNSAFE.staticFieldOffset(staticField);
183187
instanceFieldValue = JavaConstant.forBoxedPrimitive(instanceField.get(testObject));
184188
staticFieldValue = JavaConstant.forBoxedPrimitive(staticField.get(null));
189+
if (kind == JavaKind.Long) {
190+
unalignedInstanceFieldValue = JavaConstant.forLong(UNSAFE.getLongUnaligned(testObject, instanceFieldOffset - 1));
191+
} else if (kind == JavaKind.Int) {
192+
unalignedInstanceFieldValue = JavaConstant.forInt(UNSAFE.getIntUnaligned(testObject, instanceFieldOffset - 1));
193+
} else if (kind == JavaKind.Char) {
194+
unalignedInstanceFieldValue = JavaConstant.forChar(UNSAFE.getCharUnaligned(testObject, instanceFieldOffset - 1));
195+
} else if (kind == JavaKind.Short) {
196+
unalignedInstanceFieldValue = JavaConstant.forShort(UNSAFE.getShortUnaligned(testObject, instanceFieldOffset - 1));
197+
} else {
198+
unalignedInstanceFieldValue = null;
199+
}
185200
} catch (Exception e) {
186201
throw new Error("TESTBUG for kind " + kind, e);
187202
}

0 commit comments

Comments
 (0)