Skip to content

Commit b69b81f

Browse files
rakachanrakachan
committed
[GR-41739] Add synchronization around the jsr/ret bytecodes target handling.
PullRequest: graal/12950
2 parents de8da6c + 537f8d7 commit b69b81f

File tree

2 files changed

+162
-30
lines changed

2 files changed

+162
-30
lines changed

‎espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/BytecodeNode.java

Lines changed: 130 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -416,8 +416,12 @@ public final class BytecodeNode extends AbstractInstrumentableBytecodeNode imple
416416
@CompilationFinal(dimensions = 1) //
417417
private final int[] stackOverflowErrorInfo;
418418

419+
/**
420+
* Outer array should be seen and used as a {@code @CompilationFinal volatile} array, while
421+
* inner arrays can be seen as {@code final} arrays.
422+
*/
419423
@CompilationFinal(dimensions = 2) //
420-
private int[][] jsrBci = null;
424+
private volatile int[][] jsrBci = null;
421425

422426
private final BytecodeStream bs;
423427

@@ -1033,31 +1037,81 @@ Object executeBodyFromBCI(VirtualFrame frame, int startBCI, int startTop, int st
10331037
continue loop;
10341038
}
10351039
case RET: {
1036-
int targetBCI = getLocalReturnAddress(frame, bs.readLocalIndex1(curBCI));
1040+
// Use final local variables to pass in lambdas.
1041+
final int retOpBci = curBCI;
1042+
final int targetBCI = getLocalReturnAddress(frame, bs.readLocalIndex1(curBCI));
10371043
livenessAnalysis.performPostBCI(frame, curBCI, skipLivenessActions);
1038-
if (jsrBci == null) {
1044+
1045+
// Safely obtain the known targets mappings.
1046+
int[][] knownTargets = jsrBci;
1047+
if (knownTargets == null) {
10391048
CompilerDirectives.transferToInterpreterAndInvalidate();
1040-
jsrBci = new int[bs.endBCI()][];
1049+
atomic(() -> {
1050+
// Double-checked locking.
1051+
if (jsrBci == null) {
1052+
jsrBci = new int[bs.endBCI()][];
1053+
}
1054+
});
1055+
knownTargets = jsrBci;
10411056
}
1042-
if (jsrBci[curBCI] == null) {
1057+
1058+
// Safely obtain the known targets for the current ret operation.
1059+
int[] knownRets = VolatileArrayAccess.volatileRead(knownTargets, retOpBci);
1060+
if (knownRets == null) {
10431061
CompilerDirectives.transferToInterpreterAndInvalidate();
1044-
jsrBci[curBCI] = new int[]{targetBCI};
1062+
atomic(() -> {
1063+
if (VolatileArrayAccess.volatileRead(jsrBci, retOpBci) != null) {
1064+
return;
1065+
}
1066+
/*
1067+
* Be very careful on updating the known target bcis, as if another
1068+
* thread reads the not fully initialized array, it may consider 0
1069+
* to be a valid RET target, completely breaking PE.
1070+
*/
1071+
int[] targets = new int[]{targetBCI};
1072+
// Also serves as a "final publication" barrier for the assignment
1073+
// above.
1074+
VolatileArrayAccess.volatileWrite(jsrBci, retOpBci, targets);
1075+
});
1076+
knownRets = VolatileArrayAccess.volatileRead(knownTargets, retOpBci);
10451077
}
1046-
for (int jsr : jsrBci[curBCI]) {
1078+
assert knownRets != null;
1079+
1080+
// Lookup in the known targets to transform the return address to a
1081+
// constant.
1082+
for (int jsr : knownRets) {
10471083
if (jsr == targetBCI) {
10481084
CompilerAsserts.partialEvaluationConstant(jsr);
1049-
targetBCI = jsr;
10501085
top += Bytecodes.stackEffectOf(RET);
1051-
nextStatementIndex = beforeJumpChecks(frame, curBCI, targetBCI, top, statementIndex, instrument, loopCount, skipLivenessActions);
1052-
curBCI = targetBCI;
1086+
nextStatementIndex = beforeJumpChecks(frame, curBCI, jsr, top, statementIndex, instrument, loopCount, skipLivenessActions);
1087+
curBCI = jsr;
10531088
continue loop;
10541089
}
10551090
}
1091+
1092+
// Lookup failed: Add the current target to the known targets.
10561093
CompilerDirectives.transferToInterpreterAndInvalidate();
1057-
jsrBci[curBCI] = Arrays.copyOf(jsrBci[curBCI], jsrBci[curBCI].length + 1);
1058-
jsrBci[curBCI][jsrBci[curBCI].length - 1] = targetBCI;
1094+
atomic(() -> {
1095+
int[] currentRets = VolatileArrayAccess.volatileRead(jsrBci, retOpBci);
1096+
for (int jsr : currentRets) {
1097+
if (jsr == targetBCI) {
1098+
// target has been added by another thread.
1099+
return;
1100+
}
1101+
}
1102+
int[] updatedTargets = Arrays.copyOf(currentRets, currentRets.length + 1);
1103+
/*
1104+
* Be very careful on updating the known target bcis, as if another
1105+
* thread reads the not fully initialized array, it may consider 0 to be
1106+
* a valid RET target, completely breaking PE.
1107+
*/
1108+
updatedTargets[updatedTargets.length - 1] = targetBCI;
1109+
// Also serves as a "final publication" barrier for the assignment
1110+
// above.
1111+
VolatileArrayAccess.volatileWrite(jsrBci, retOpBci, updatedTargets);
1112+
});
10591113
top += Bytecodes.stackEffectOf(RET);
1060-
nextStatementIndex = beforeJumpChecks(frame, curBCI, targetBCI, top, statementIndex, instrument, loopCount, skipLivenessActions);
1114+
nextStatementIndex = beforeJumpChecks(frame, retOpBci, targetBCI, top, statementIndex, instrument, loopCount, skipLivenessActions);
10611115
curBCI = targetBCI;
10621116
continue loop;
10631117
}
@@ -1224,31 +1278,82 @@ Object executeBodyFromBCI(VirtualFrame frame, int startBCI, int startTop, int st
12241278
case IINC: setLocalInt(frame, bs.readLocalIndex2(curBCI), getLocalInt(frame, bs.readLocalIndex2(curBCI)) + bs.readIncrement2(curBCI)); break;
12251279
// @formatter:on
12261280
case RET: {
1227-
int targetBCI = getLocalReturnAddress(frame, bs.readLocalIndex2(curBCI));
1281+
// Use final local variables to pass in lambdas.
1282+
final int retOpBci = curBCI;
1283+
final int targetBCI = getLocalReturnAddress(frame, bs.readLocalIndex1(curBCI));
12281284
livenessAnalysis.performPostBCI(frame, curBCI, skipLivenessActions);
1229-
if (jsrBci == null) {
1285+
1286+
// Safely obtain the known targets mappings.
1287+
int[][] knownTargets = jsrBci;
1288+
if (knownTargets == null) {
12301289
CompilerDirectives.transferToInterpreterAndInvalidate();
1231-
jsrBci = new int[bs.endBCI()][];
1290+
atomic(() -> {
1291+
// Double-checked locking.
1292+
if (jsrBci == null) {
1293+
jsrBci = new int[bs.endBCI()][];
1294+
}
1295+
});
1296+
knownTargets = jsrBci;
12321297
}
1233-
if (jsrBci[curBCI] == null) {
1298+
1299+
// Safely obtain the known targets for the current ret operation.
1300+
int[] knownRets = VolatileArrayAccess.volatileRead(knownTargets, retOpBci);
1301+
if (knownRets == null) {
12341302
CompilerDirectives.transferToInterpreterAndInvalidate();
1235-
jsrBci[curBCI] = new int[]{targetBCI};
1303+
atomic(() -> {
1304+
if (VolatileArrayAccess.volatileRead(jsrBci, retOpBci) != null) {
1305+
return;
1306+
}
1307+
/*
1308+
* Be very careful on updating the known target bcis, as if
1309+
* another thread reads the not fully initialized array, it
1310+
* may consider 0 to be a valid RET target, completely
1311+
* breaking PE.
1312+
*/
1313+
int[] targets = new int[]{targetBCI};
1314+
// Also serves as a "final publication" barrier for the
1315+
// assignment above.
1316+
VolatileArrayAccess.volatileWrite(jsrBci, retOpBci, targets);
1317+
});
1318+
knownRets = VolatileArrayAccess.volatileRead(knownTargets, retOpBci);
12361319
}
1237-
for (int jsr : jsrBci[curBCI]) {
1320+
assert knownRets != null;
1321+
1322+
// Lookup in the known targets to transform the return address to a
1323+
// constant.
1324+
for (int jsr : knownRets) {
12381325
if (jsr == targetBCI) {
12391326
CompilerAsserts.partialEvaluationConstant(jsr);
1240-
targetBCI = jsr;
12411327
top += Bytecodes.stackEffectOf(RET);
1242-
nextStatementIndex = beforeJumpChecks(frame, curBCI, targetBCI, top, statementIndex, instrument, loopCount, skipLivenessActions);
1243-
curBCI = targetBCI;
1328+
nextStatementIndex = beforeJumpChecks(frame, curBCI, jsr, top, statementIndex, instrument, loopCount, skipLivenessActions);
1329+
curBCI = jsr;
12441330
continue loop;
12451331
}
12461332
}
1333+
1334+
// Lookup failed: Add the current target to the known targets.
12471335
CompilerDirectives.transferToInterpreterAndInvalidate();
1248-
jsrBci[curBCI] = Arrays.copyOf(jsrBci[curBCI], jsrBci[curBCI].length + 1);
1249-
jsrBci[curBCI][jsrBci[curBCI].length - 1] = targetBCI;
1336+
atomic(() -> {
1337+
int[] currentRets = VolatileArrayAccess.volatileRead(jsrBci, retOpBci);
1338+
for (int jsr : currentRets) {
1339+
if (jsr == targetBCI) {
1340+
// target has been added by another thread.
1341+
return;
1342+
}
1343+
}
1344+
int[] updatedTargets = Arrays.copyOf(currentRets, currentRets.length + 1);
1345+
/*
1346+
* Be very careful on updating the known target bcis, as if
1347+
* another thread reads the not fully initialized array, it may
1348+
* consider 0 to be a valid RET target, completely breaking PE.
1349+
*/
1350+
updatedTargets[updatedTargets.length - 1] = targetBCI;
1351+
// Also serves as a "final publication" barrier for the
1352+
// assignment above.
1353+
VolatileArrayAccess.volatileWrite(jsrBci, retOpBci, updatedTargets);
1354+
});
12501355
top += Bytecodes.stackEffectOf(RET);
1251-
nextStatementIndex = beforeJumpChecks(frame, curBCI, targetBCI, top, statementIndex, instrument, loopCount, skipLivenessActions);
1356+
nextStatementIndex = beforeJumpChecks(frame, retOpBci, targetBCI, top, statementIndex, instrument, loopCount, skipLivenessActions);
12521357
curBCI = targetBCI;
12531358
continue loop;
12541359
}

‎espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/quick/VolatileArrayAccess.java

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,16 +30,43 @@
3030
public class VolatileArrayAccess {
3131
private static final Unsafe U = UnsafeAccess.get();
3232

33-
@SuppressWarnings("unused")
34-
private static long offsetFor(byte[] array, int index) {
35-
return Unsafe.ARRAY_BYTE_BASE_OFFSET + (index * Unsafe.ARRAY_BYTE_INDEX_SCALE);
36-
}
37-
3833
public static void volatileWrite(byte[] array, int index, byte value) {
3934
U.putByteVolatile(array, offsetFor(array, index), value);
4035
}
4136

4237
public static byte volatileRead(byte[] array, int index) {
4338
return U.getByteVolatile(array, offsetFor(array, index));
4439
}
40+
41+
public static void volatileWrite(int[] array, int index, int value) {
42+
U.putIntVolatile(array, offsetFor(array, index), value);
43+
}
44+
45+
public static int volatileRead(int[] array, int index) {
46+
return U.getIntVolatile(array, offsetFor(array, index));
47+
}
48+
49+
public static <T> void volatileWrite(T[] array, int index, Object value) {
50+
U.putObjectVolatile(array, offsetFor(array, index), value);
51+
}
52+
53+
@SuppressWarnings("unchecked")
54+
public static <T> T volatileRead(T[] array, int index) {
55+
return (T) U.getObjectVolatile(array, offsetFor(array, index));
56+
}
57+
58+
@SuppressWarnings("unused")
59+
private static long offsetFor(byte[] array, int index) {
60+
return Unsafe.ARRAY_BYTE_BASE_OFFSET + ((long) index * Unsafe.ARRAY_BYTE_INDEX_SCALE);
61+
}
62+
63+
@SuppressWarnings("unused")
64+
private static long offsetFor(int[] array, int index) {
65+
return Unsafe.ARRAY_INT_BASE_OFFSET + ((long) index * Unsafe.ARRAY_INT_INDEX_SCALE);
66+
}
67+
68+
@SuppressWarnings("unused")
69+
private static <T> long offsetFor(T[] array, int index) {
70+
return Unsafe.ARRAY_OBJECT_BASE_OFFSET + ((long) index * Unsafe.ARRAY_OBJECT_INDEX_SCALE);
71+
}
4572
}

0 commit comments

Comments
 (0)