Skip to content

Commit c08fe22

Browse files
matteoldanimatteoldani
committed
[GR-17321] Intercept memory reads.
PullRequest: graal/14778
2 parents 3904a91 + 773f3c5 commit c08fe22

File tree

11 files changed

+172
-56
lines changed

11 files changed

+172
-56
lines changed

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/asm/amd64/AMD64Assembler.java‎

Lines changed: 57 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
import jdk.graal.compiler.asm.BranchTargetOutOfBoundsException;
5656
import jdk.graal.compiler.asm.Label;
5757
import jdk.graal.compiler.asm.amd64.AVXKind.AVXSize;
58+
import jdk.graal.compiler.core.amd64.MemoryReadInterceptor;
5859
import jdk.graal.compiler.core.common.GraalOptions;
5960
import jdk.graal.compiler.core.common.NumUtil;
6061
import jdk.graal.compiler.core.common.Stride;
@@ -73,7 +74,7 @@
7374
/**
7475
* This class implements an assembler that can encode most X86 instructions.
7576
*/
76-
public class AMD64Assembler extends AMD64BaseAssembler {
77+
public class AMD64Assembler extends AMD64BaseAssembler implements MemoryReadInterceptor {
7778

7879
public static class Options {
7980
// @formatter:off
@@ -474,13 +475,15 @@ public void emit(AMD64Assembler asm, OperandSize size, Register dst, Register sr
474475
public void emit(AMD64Assembler asm, OperandSize size, Register dst, AMD64Address src) {
475476
assert verify(asm, size, dst, null);
476477
assert !isSSEInstruction();
478+
asm.interceptMemorySrcOperands(src);
477479
emitOpcode(asm, size, getRXB(dst, src), dst.encoding, 0);
478480
asm.emitOperandHelper(dst, src, 0);
479481
}
480482

481483
public void emit(AMD64Assembler asm, OperandSize size, Register dst, AMD64Address src, boolean force4Byte) {
482484
assert verify(asm, size, dst, null);
483485
assert !isSSEInstruction();
486+
asm.interceptMemorySrcOperands(src);
484487
emitOpcode(asm, size, getRXB(dst, src), dst.encoding, 0);
485488
asm.emitOperandHelper(dst, src, force4Byte, 0);
486489
}
@@ -589,29 +592,35 @@ public void emit(AMD64Assembler asm, OperandSize size, AMD64Address dst) {
589592
*/
590593
public static class AMD64MIOp extends AMD64ImmOp {
591594
// @formatter:off
592-
public static final AMD64MIOp BT = new AMD64MIOp("BT", true, P_0F, 0xBA, 4, OpAssertion.WordOrLargerAssertion);
593-
public static final AMD64MIOp BTR = new AMD64MIOp("BTR", true, P_0F, 0xBA, 6, OpAssertion.WordOrLargerAssertion);
594-
public static final AMD64MIOp MOVB = new AMD64MIOp("MOVB", true, 0xC6, 0, OpAssertion.ByteAssertion);
595-
public static final AMD64MIOp MOV = new AMD64MIOp("MOV", false, 0xC7, 0, OpAssertion.WordOrLargerAssertion);
596-
public static final AMD64MIOp SAR = new AMD64MIOp("SAR", true, 0xC1, 7, OpAssertion.WordOrLargerAssertion);
597-
public static final AMD64MIOp SHL = new AMD64MIOp("SHL", true, 0xC1, 4, OpAssertion.WordOrLargerAssertion);
598-
public static final AMD64MIOp SHR = new AMD64MIOp("SHR", true, 0xC1, 5, OpAssertion.WordOrLargerAssertion);
599-
public static final AMD64MIOp TEST = new AMD64MIOp("TEST", false, 0xF7, 0);
595+
public static final AMD64MIOp BT = new AMD64MIOp("BT", true, P_0F, 0xBA, 4, true, OpAssertion.WordOrLargerAssertion);
596+
public static final AMD64MIOp BTR = new AMD64MIOp("BTR", true, P_0F, 0xBA, 6, true, OpAssertion.WordOrLargerAssertion);
597+
public static final AMD64MIOp MOVB = new AMD64MIOp("MOVB", true, 0xC6, 0, false, OpAssertion.ByteAssertion);
598+
public static final AMD64MIOp MOV = new AMD64MIOp("MOV", false, 0xC7, 0, false, OpAssertion.WordOrLargerAssertion);
599+
public static final AMD64MIOp SAR = new AMD64MIOp("SAR", true, 0xC1, 7, true, OpAssertion.WordOrLargerAssertion);
600+
public static final AMD64MIOp SHL = new AMD64MIOp("SHL", true, 0xC1, 4, true, OpAssertion.WordOrLargerAssertion);
601+
public static final AMD64MIOp SHR = new AMD64MIOp("SHR", true, 0xC1, 5, true, OpAssertion.WordOrLargerAssertion);
602+
public static final AMD64MIOp TEST = new AMD64MIOp("TEST", false, 0xF7, 0, true);
600603
// @formatter:on
601604

602605
private final int ext;
606+
/**
607+
* Defines if the Op reads from memory and makes the result observable by the user (e.g.
608+
* spilling to a register or in a flag).
609+
*/
610+
private final boolean isMemRead;
603611

604-
protected AMD64MIOp(String opcode, boolean immIsByte, int op, int ext) {
605-
this(opcode, immIsByte, op, ext, OpAssertion.WordOrLargerAssertion);
612+
protected AMD64MIOp(String opcode, boolean immIsByte, int op, int ext, boolean isMemRead) {
613+
this(opcode, immIsByte, op, ext, isMemRead, OpAssertion.WordOrLargerAssertion);
606614
}
607615

608-
protected AMD64MIOp(String opcode, boolean immIsByte, int op, int ext, OpAssertion assertion) {
609-
this(opcode, immIsByte, 0, op, ext, assertion);
616+
protected AMD64MIOp(String opcode, boolean immIsByte, int op, int ext, boolean isMemRead, OpAssertion assertion) {
617+
this(opcode, immIsByte, 0, op, ext, isMemRead, assertion);
610618
}
611619

612-
protected AMD64MIOp(String opcode, boolean immIsByte, int prefix, int op, int ext, OpAssertion assertion) {
620+
protected AMD64MIOp(String opcode, boolean immIsByte, int prefix, int op, int ext, boolean isMemRead, OpAssertion assertion) {
613621
super(opcode, immIsByte, prefix, op, assertion);
614622
this.ext = ext;
623+
this.isMemRead = isMemRead;
615624
}
616625

617626
public final void emit(AMD64Assembler asm, OperandSize size, Register dst, int imm) {
@@ -631,22 +640,29 @@ public final void emit(AMD64Assembler asm, OperandSize size, Register dst, int i
631640
}
632641
}
633642

634-
public final void emit(AMD64Assembler asm, OperandSize size, AMD64Address dst, int imm) {
635-
emit(asm, size, dst, imm, false);
643+
public final void emit(AMD64Assembler asm, OperandSize size, AMD64Address address, int imm) {
644+
emit(asm, size, address, imm, false);
636645
}
637646

638-
public final void emit(AMD64Assembler asm, OperandSize size, AMD64Address dst, int imm, boolean annotateImm) {
647+
public final void emit(AMD64Assembler asm, OperandSize size, AMD64Address address, int imm, boolean annotateImm) {
639648
assert verify(asm, size, null, null);
649+
if (isMemRead) {
650+
asm.interceptMemorySrcOperands(address);
651+
}
640652
int insnPos = asm.position();
641-
emitOpcode(asm, size, getRXB(null, dst), 0, 0);
642-
asm.emitOperandHelper(ext, dst, immediateSize(size));
653+
emitOpcode(asm, size, getRXB(null, address), 0, 0);
654+
asm.emitOperandHelper(ext, address, immediateSize(size));
643655
int immPos = asm.position();
644656
emitImmediate(asm, size, imm);
645657
int nextInsnPos = asm.position();
646658
if (annotateImm && asm.codePatchingAnnotationConsumer != null) {
647659
asm.codePatchingAnnotationConsumer.accept(new OperandDataAnnotation(insnPos, immPos, nextInsnPos - immPos, nextInsnPos));
648660
}
649661
}
662+
663+
public boolean isMemRead() {
664+
return isMemRead;
665+
}
650666
}
651667

652668
/**
@@ -721,6 +737,7 @@ public void emit(AMD64Assembler asm, OperandSize size, Register dst, Register sr
721737

722738
public void emit(AMD64Assembler asm, OperandSize size, Register dst, AMD64Address src, int imm) {
723739
assert verify(asm, size, dst, null);
740+
asm.interceptMemorySrcOperands(src);
724741
emitOpcode(asm, size, getRXB(dst, src), dst.encoding, 0);
725742
asm.emitOperandHelper(dst, src, immediateSize(size));
726743
emitImmediate(asm, size, imm);
@@ -883,6 +900,7 @@ public final void emit(AMD64Assembler asm, OperandSize size, Register dst, Regis
883900
public final void emit(AMD64Assembler asm, OperandSize size, Register dst, AMD64Address src) {
884901
assert verify(asm, size, dst, null);
885902
assert isSSEInstruction();
903+
asm.interceptMemorySrcOperands(src);
886904
// MOVSS/SD are not RVM instruction when the dst is an address
887905
Register nds = (this == MOVSS || this == MOVSD) ? Register.None : preferredNDS.getNds(dst, src);
888906
asm.simdPrefix(dst, nds, src, size, prefix1, prefix2, size == OperandSize.QWORD);
@@ -972,6 +990,7 @@ public final void emit(AMD64Assembler asm, OperandSize size, Register dst, Regis
972990
public final void emit(AMD64Assembler asm, OperandSize size, Register dst, AMD64Address src, int imm) {
973991
assert verify(asm, size, dst, null);
974992
assert isSSEInstruction();
993+
asm.interceptMemorySrcOperands(src);
975994
asm.simdPrefix(dst, preferredNDS.getNds(dst, src), src, size, prefix1, prefix2, w);
976995
asm.emitByte(op);
977996
asm.emitOperandHelper(dst, src, immediateSize(size));
@@ -1090,12 +1109,12 @@ public static final class AMD64BinaryArithmetic {
10901109
private AMD64BinaryArithmetic(String opcode, int code) {
10911110
int baseOp = code << 3;
10921111

1093-
byteImmOp = new AMD64MIOp(opcode, true, 0, 0x80, code, OpAssertion.ByteAssertion);
1112+
byteImmOp = new AMD64MIOp(opcode, true, 0, 0x80, code, false, OpAssertion.ByteAssertion);
10941113
byteMrOp = new AMD64MROp(opcode, 0, baseOp, OpAssertion.ByteAssertion);
10951114
byteRmOp = new AMD64RMOp(opcode, 0, baseOp | 0x02, OpAssertion.ByteAssertion);
10961115

1097-
immOp = new AMD64MIOp(opcode, false, 0, 0x81, code, OpAssertion.WordOrLargerAssertion);
1098-
immSxOp = new AMD64MIOp(opcode, true, 0, 0x83, code, OpAssertion.WordOrLargerAssertion);
1116+
immOp = new AMD64MIOp(opcode, false, 0, 0x81, code, false, OpAssertion.WordOrLargerAssertion);
1117+
immSxOp = new AMD64MIOp(opcode, true, 0, 0x83, code, false, OpAssertion.WordOrLargerAssertion);
10991118
mrOp = new AMD64MROp(opcode, 0, baseOp | 0x01, OpAssertion.WordOrLargerAssertion);
11001119
rmOp = new AMD64RMOp(opcode, 0, baseOp | 0x03, OpAssertion.WordOrLargerAssertion);
11011120
}
@@ -1148,7 +1167,7 @@ public static final class AMD64Shift {
11481167
private AMD64Shift(String opcode, int code) {
11491168
m1Op = new AMD64MOp(opcode, 0, 0xD1, code, OpAssertion.WordOrLargerAssertion);
11501169
mcOp = new AMD64MOp(opcode, 0, 0xD3, code, OpAssertion.WordOrLargerAssertion);
1151-
miOp = new AMD64MIOp(opcode, true, 0, 0xC1, code, OpAssertion.WordOrLargerAssertion);
1170+
miOp = new AMD64MIOp(opcode, true, 0, 0xC1, code, true, OpAssertion.WordOrLargerAssertion);
11521171
}
11531172
}
11541173

@@ -1502,6 +1521,7 @@ protected final void emitVexOrEvex(AMD64Assembler asm, Register dst, Register nd
15021521

15031522
protected final void emitVexOrEvex(AMD64Assembler asm, Register dst, Register nds, AMD64Address src, Register opmask, AVXSize size, int actualPP, int actualMMMMM, int actualW,
15041523
int actualWEvex, int z, int b) {
1524+
asm.interceptMemorySrcOperands(src);
15051525
if (isEvex) {
15061526
checkEvex(asm, size, dst, opmask, z, nds, null, b);
15071527
asm.evexPrefix(dst, opmask, nds, src, size, actualPP, actualMMMMM, actualWEvex, z, b);
@@ -4202,6 +4222,7 @@ public final void cmovl(ConditionFlag cc, Register dst, Register src) {
42024222
}
42034223

42044224
public final void cmovl(ConditionFlag cc, Register dst, AMD64Address src) {
4225+
interceptMemorySrcOperands(src);
42054226
prefix(src, dst);
42064227
emitByte(0x0F);
42074228
emitByte(0x40 | cc.getValue());
@@ -4216,6 +4237,7 @@ public final void cmovq(ConditionFlag cc, Register dst, Register src) {
42164237
}
42174238

42184239
public final void cmovq(ConditionFlag cc, Register dst, AMD64Address src) {
4240+
interceptMemorySrcOperands(src);
42194241
prefixq(src, dst);
42204242
emitByte(0x0F);
42214243
emitByte(0x40 | cc.getValue());
@@ -4244,6 +4266,7 @@ public final void fincstp() {
42444266
}
42454267

42464268
public final void fldd(AMD64Address src) {
4269+
interceptMemorySrcOperands(src);
42474270
emitByte(0xDD);
42484271
emitOperandHelper(0, src, 0);
42494272
}
@@ -4259,6 +4282,7 @@ public final void fldln2() {
42594282
}
42604283

42614284
public final void flds(AMD64Address src) {
4285+
interceptMemorySrcOperands(src);
42624286
emitByte(0xD9);
42634287
emitOperandHelper(0, src, 0);
42644288
}
@@ -4290,11 +4314,13 @@ public final void fstp(int i) {
42904314
}
42914315

42924316
public final void fstpd(AMD64Address src) {
4317+
interceptMemorySrcOperands(src);
42934318
emitByte(0xDD);
42944319
emitOperandHelper(3, src, 0);
42954320
}
42964321

42974322
public final void fstps(AMD64Address src) {
4323+
interceptMemorySrcOperands(src);
42984324
emitByte(0xD9);
42994325
emitOperandHelper(3, src, 0);
43004326
}
@@ -4351,13 +4377,13 @@ public final void leave() {
43514377
emitByte(0xC9);
43524378
}
43534379

4354-
public final void lfence() {
4380+
public void lfence() {
43554381
emitByte(0x0f);
43564382
emitByte(0xae);
43574383
emitByte(0xe8);
43584384
}
43594385

4360-
public final void lock() {
4386+
public void lock() {
43614387
emitByte(0xF0);
43624388
}
43634389

@@ -4408,6 +4434,7 @@ public final void movlhps(Register dst, Register src) {
44084434
*/
44094435
public final void movlpd(Register dst, AMD64Address src) {
44104436
assert inRC(XMM, dst);
4437+
interceptMemorySrcOperands(src);
44114438
simdPrefix(dst, dst, src, OperandSize.PD, P_0F, false);
44124439
emitByte(0x12);
44134440
emitOperandHelper(dst, src, 0);
@@ -4424,6 +4451,7 @@ public final void movq(Register dst, AMD64Address src, boolean force4BytesDispla
44244451
// An alternative instruction would be 66 REX.W 0F 6E /r. We prefer the REX.W free
44254452
// format, because it would allow us to emit 2-bytes-prefixed vex-encoding instruction
44264453
// when applicable.
4454+
interceptMemorySrcOperands(src);
44274455
simdPrefix(dst, Register.None, src, OperandSize.SS, P_0F, false);
44284456
emitByte(0x7E);
44294457
emitOperandHelper(dst, src, force4BytesDisplacement, 0);
@@ -4868,7 +4896,7 @@ public final void cmpwImm16(AMD64Address dst, int imm16) {
48684896
* adr if so; otherwise, the value at adr is loaded into X86.rax,. The ZF is set if the compared
48694897
* values were equal, and cleared otherwise.
48704898
*/
4871-
public final void cmpxchgb(Register reg, AMD64Address adr) { // cmpxchg
4899+
public final void cmpxchgb(AMD64Address adr, Register reg) { // cmpxchg
48724900
AMD64MROp.CMPXCHGB.emit(this, OperandSize.BYTE, adr, reg);
48734901
}
48744902

@@ -4877,7 +4905,7 @@ public final void cmpxchgb(Register reg, AMD64Address adr) { // cmpxchg
48774905
* into adr if so; otherwise, the value at adr is loaded into X86.rax,. The ZF is set if the
48784906
* compared values were equal, and cleared otherwise.
48794907
*/
4880-
public final void cmpxchgl(Register reg, AMD64Address adr) { // cmpxchg
4908+
public final void cmpxchgl(AMD64Address adr, Register reg) { // cmpxchg
48814909
AMD64MROp.CMPXCHG.emit(this, OperandSize.DWORD, adr, reg);
48824910
}
48834911

@@ -4890,7 +4918,7 @@ public final void cmpxchgq(Register reg, AMD64Address adr) {
48904918
* into adr if so; otherwise, the value at adr is loaded into X86.rax,. The ZF is set if the
48914919
* compared values were equal, and cleared otherwise.
48924920
*/
4893-
public final void cmpxchgw(Register reg, AMD64Address adr) { // cmpxchg
4921+
public final void cmpxchgw(AMD64Address adr, Register reg) { // cmpxchg
48944922
AMD64MROp.CMPXCHG.emit(this, OperandSize.WORD, adr, reg);
48954923
}
48964924

@@ -6282,4 +6310,5 @@ public final void evpternlogq(Register dst, int imm8, Register src1, Register sr
62826310
public final void evpxorq(Register dst, Register mask, Register nds, AMD64Address src) {
62836311
VexRVMOp.EVPXORQ.emit(this, AVXSize.ZMM, dst, nds, src, mask);
62846312
}
6313+
62856314
}

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/asm/amd64/AMD64MacroAssembler.java‎

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,7 @@ public int call(PostCallAction postCallAction, InvokeTarget callTarget) {
503503

504504
// This should guarantee that the alignment in AMD64Assembler.jcc methods will be not triggered.
505505
private void alignFusedPair(Label branchTarget, boolean isShortJmp, int prevOpInBytes) {
506-
assert prevOpInBytes < 26 : "Fused pair may be longer than 0x20 bytes.";
506+
GraalError.guarantee(prevOpInBytes < 26, "Fused pair may be longer than 0x20 bytes.");
507507
if (branchTarget == null) {
508508
mitigateJCCErratum(prevOpInBytes + 6);
509509
} else if (isShortJmp) {
@@ -542,15 +542,26 @@ private int applyMIOpAndJcc(AMD64MIOp op, OperandSize size, Register src, int im
542542
return beforeJcc;
543543
}
544544

545-
private int applyMIOpAndJcc(AMD64MIOp op, OperandSize size, AMD64Address src, int imm32, ConditionFlag cc, Label branchTarget, boolean isShortJmp, boolean annotateImm,
545+
private int applyMIOpAndJcc(AMD64MIOp op, OperandSize size, AMD64Address address, int imm32, ConditionFlag cc, Label branchTarget, boolean isShortJmp, boolean annotateImm,
546546
IntConsumer applyBeforeFusedPair) {
547-
final int bytesToEmit = getPrefixInBytes(size, src) + OPCODE_IN_BYTES + addressInBytes(src) + op.immediateSize(size);
547+
int bytesToEmit = getPrefixInBytes(size, address) + OPCODE_IN_BYTES + addressInBytes(address) + op.immediateSize(size);
548+
// Address is "source" only if the op reads from memory.
549+
if (op.isMemRead()) {
550+
/**
551+
* The extra bytes introduced by MemoryReadInterceptor are also included in the fused
552+
* pair size, which may lead to imprecision. However, this does not affect the
553+
* correctness of the Intel JCC erratum, as it ensures that both the instrumented logic
554+
* and the fused pair remain within the 32-byte boundary. If the total size exceeds 32
555+
* bytes, the assertion in alignFusedPair will detect it.
556+
*/
557+
bytesToEmit += extraSourceAddressBytes(address);
558+
}
548559
alignFusedPair(branchTarget, isShortJmp, bytesToEmit);
549560
final int beforeFusedPair = position();
550561
if (applyBeforeFusedPair != null) {
551562
applyBeforeFusedPair.accept(beforeFusedPair);
552563
}
553-
op.emit(this, size, src, imm32, annotateImm);
564+
op.emit(this, size, address, imm32, annotateImm);
554565
final int beforeJcc = position();
555566
assert beforeFusedPair + bytesToEmit == beforeJcc : Assertions.errorMessage(beforeFusedPair, bytesToEmit, position());
556567
jcc(cc, branchTarget, isShortJmp);
@@ -571,7 +582,14 @@ private int applyRMOpAndJcc(AMD64RMOp op, OperandSize size, Register src1, Regis
571582
}
572583

573584
private int applyRMOpAndJcc(AMD64RMOp op, OperandSize size, Register src1, AMD64Address src2, ConditionFlag cc, Label branchTarget, boolean isShortJmp, IntConsumer applyBeforeFusedPair) {
574-
final int bytesToEmit = getPrefixInBytes(size, src1, op.dstIsByte, src2) + OPCODE_IN_BYTES + addressInBytes(src2);
585+
/**
586+
* The extra bytes introduced by MemoryReadInterceptor are also included in the fused pair
587+
* size, which may lead to imprecision. However, this does not affect the correctness of the
588+
* Intel JCC erratum, as it ensures that both the instrumented logic and the fused pair
589+
* remain within the 32-byte boundary. If the total size exceeds 32 bytes, the assertion in
590+
* alignFusedPair will detect it.
591+
*/
592+
final int bytesToEmit = getPrefixInBytes(size, src1, op.dstIsByte, src2) + OPCODE_IN_BYTES + addressInBytes(src2) + extraSourceAddressBytes(src2);
575593
alignFusedPair(branchTarget, isShortJmp, bytesToEmit);
576594
final int beforeFusedPair = position();
577595
if (applyBeforeFusedPair != null) {
@@ -699,11 +717,11 @@ public final int cmpqAndJcc(Register src1, AMD64Address src2, ConditionFlag cc,
699717

700718
public final int cmpAndJcc(OperandSize size, Register src1, Supplier<AMD64Address> src2, ConditionFlag cc, Label branchTarget) {
701719
AMD64Address placeHolder = getPlaceholder(position());
720+
AMD64Address src2AsAddress = src2.get();
702721
final AMD64RMOp op = AMD64BinaryArithmetic.CMP.getRMOpcode(size);
703-
final int bytesToEmit = getPrefixInBytes(size, src1, op.dstIsByte, placeHolder) + OPCODE_IN_BYTES + addressInBytes(placeHolder);
722+
final int bytesToEmit = getPrefixInBytes(size, src1, op.dstIsByte, placeHolder) + OPCODE_IN_BYTES + addressInBytes(placeHolder) + extraSourceAddressBytes(src2AsAddress);
704723
alignFusedPair(branchTarget, false, bytesToEmit);
705724
final int beforeFusedPair = position();
706-
AMD64Address src2AsAddress = src2.get();
707725
op.emit(this, size, src1, src2AsAddress);
708726
int beforeJcc = position();
709727
assert beforeFusedPair + bytesToEmit == beforeJcc : Assertions.errorMessage(beforeFusedPair, bytesToEmit, position());

0 commit comments

Comments
 (0)