Skip to content

Commit

Permalink
Intercept memory reads. Fixed cmpxch signature semantic
Browse files Browse the repository at this point in the history
  • Loading branch information
@matteoldani
matteoldani committed Jan 31, 2025
1 parent e4117f0 commit 773f3c5
Show file tree
Hide file tree
Showing 11 changed files with 172 additions and 56 deletions.
85 changes: 57 additions & 28 deletions compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/asm/amd64/AMD64Assembler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
import jdk.graal.compiler.asm.BranchTargetOutOfBoundsException;
import jdk.graal.compiler.asm.Label;
import jdk.graal.compiler.asm.amd64.AVXKind.AVXSize;
import jdk.graal.compiler.core.amd64.MemoryReadInterceptor;
import jdk.graal.compiler.core.common.GraalOptions;
import jdk.graal.compiler.core.common.NumUtil;
import jdk.graal.compiler.core.common.Stride;
Expand All @@ -73,7 +74,7 @@
/**
* This class implements an assembler that can encode most X86 instructions.
*/
public class AMD64Assembler extends AMD64BaseAssembler {
public class AMD64Assembler extends AMD64BaseAssembler implements MemoryReadInterceptor {

public static class Options {
// @formatter:off
Expand Down Expand Up @@ -474,13 +475,15 @@ public void emit(AMD64Assembler asm, OperandSize size, Register dst, Register sr
public void emit(AMD64Assembler asm, OperandSize size, Register dst, AMD64Address src) {
assert verify(asm, size, dst, null);
assert !isSSEInstruction();
asm.interceptMemorySrcOperands(src);
emitOpcode(asm, size, getRXB(dst, src), dst.encoding, 0);
asm.emitOperandHelper(dst, src, 0);
}

public void emit(AMD64Assembler asm, OperandSize size, Register dst, AMD64Address src, boolean force4Byte) {
assert verify(asm, size, dst, null);
assert !isSSEInstruction();
asm.interceptMemorySrcOperands(src);
emitOpcode(asm, size, getRXB(dst, src), dst.encoding, 0);
asm.emitOperandHelper(dst, src, force4Byte, 0);
}
Expand Down Expand Up @@ -589,29 +592,35 @@ public void emit(AMD64Assembler asm, OperandSize size, AMD64Address dst) {
*/
public static class AMD64MIOp extends AMD64ImmOp {
// @formatter:off
public static final AMD64MIOp BT = new AMD64MIOp("BT", true, P_0F, 0xBA, 4, OpAssertion.WordOrLargerAssertion);
public static final AMD64MIOp BTR = new AMD64MIOp("BTR", true, P_0F, 0xBA, 6, OpAssertion.WordOrLargerAssertion);
public static final AMD64MIOp MOVB = new AMD64MIOp("MOVB", true, 0xC6, 0, OpAssertion.ByteAssertion);
public static final AMD64MIOp MOV = new AMD64MIOp("MOV", false, 0xC7, 0, OpAssertion.WordOrLargerAssertion);
public static final AMD64MIOp SAR = new AMD64MIOp("SAR", true, 0xC1, 7, OpAssertion.WordOrLargerAssertion);
public static final AMD64MIOp SHL = new AMD64MIOp("SHL", true, 0xC1, 4, OpAssertion.WordOrLargerAssertion);
public static final AMD64MIOp SHR = new AMD64MIOp("SHR", true, 0xC1, 5, OpAssertion.WordOrLargerAssertion);
public static final AMD64MIOp TEST = new AMD64MIOp("TEST", false, 0xF7, 0);
public static final AMD64MIOp BT = new AMD64MIOp("BT", true, P_0F, 0xBA, 4, true, OpAssertion.WordOrLargerAssertion);
public static final AMD64MIOp BTR = new AMD64MIOp("BTR", true, P_0F, 0xBA, 6, true, OpAssertion.WordOrLargerAssertion);
public static final AMD64MIOp MOVB = new AMD64MIOp("MOVB", true, 0xC6, 0, false, OpAssertion.ByteAssertion);
public static final AMD64MIOp MOV = new AMD64MIOp("MOV", false, 0xC7, 0, false, OpAssertion.WordOrLargerAssertion);
public static final AMD64MIOp SAR = new AMD64MIOp("SAR", true, 0xC1, 7, true, OpAssertion.WordOrLargerAssertion);
public static final AMD64MIOp SHL = new AMD64MIOp("SHL", true, 0xC1, 4, true, OpAssertion.WordOrLargerAssertion);
public static final AMD64MIOp SHR = new AMD64MIOp("SHR", true, 0xC1, 5, true, OpAssertion.WordOrLargerAssertion);
public static final AMD64MIOp TEST = new AMD64MIOp("TEST", false, 0xF7, 0, true);
// @formatter:on

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

protected AMD64MIOp(String opcode, boolean immIsByte, int op, int ext) {
this(opcode, immIsByte, op, ext, OpAssertion.WordOrLargerAssertion);
protected AMD64MIOp(String opcode, boolean immIsByte, int op, int ext, boolean isMemRead) {
this(opcode, immIsByte, op, ext, isMemRead, OpAssertion.WordOrLargerAssertion);
}

protected AMD64MIOp(String opcode, boolean immIsByte, int op, int ext, OpAssertion assertion) {
this(opcode, immIsByte, 0, op, ext, assertion);
protected AMD64MIOp(String opcode, boolean immIsByte, int op, int ext, boolean isMemRead, OpAssertion assertion) {
this(opcode, immIsByte, 0, op, ext, isMemRead, assertion);
}

protected AMD64MIOp(String opcode, boolean immIsByte, int prefix, int op, int ext, OpAssertion assertion) {
protected AMD64MIOp(String opcode, boolean immIsByte, int prefix, int op, int ext, boolean isMemRead, OpAssertion assertion) {
super(opcode, immIsByte, prefix, op, assertion);
this.ext = ext;
this.isMemRead = isMemRead;
}

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

public final void emit(AMD64Assembler asm, OperandSize size, AMD64Address dst, int imm) {
emit(asm, size, dst, imm, false);
public final void emit(AMD64Assembler asm, OperandSize size, AMD64Address address, int imm) {
emit(asm, size, address, imm, false);
}

public final void emit(AMD64Assembler asm, OperandSize size, AMD64Address dst, int imm, boolean annotateImm) {
public final void emit(AMD64Assembler asm, OperandSize size, AMD64Address address, int imm, boolean annotateImm) {
assert verify(asm, size, null, null);
if (isMemRead) {
asm.interceptMemorySrcOperands(address);
}
int insnPos = asm.position();
emitOpcode(asm, size, getRXB(null, dst), 0, 0);
asm.emitOperandHelper(ext, dst, immediateSize(size));
emitOpcode(asm, size, getRXB(null, address), 0, 0);
asm.emitOperandHelper(ext, address, immediateSize(size));
int immPos = asm.position();
emitImmediate(asm, size, imm);
int nextInsnPos = asm.position();
if (annotateImm && asm.codePatchingAnnotationConsumer != null) {
asm.codePatchingAnnotationConsumer.accept(new OperandDataAnnotation(insnPos, immPos, nextInsnPos - immPos, nextInsnPos));
}
}

public boolean isMemRead() {
return isMemRead;
}
}

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

public void emit(AMD64Assembler asm, OperandSize size, Register dst, AMD64Address src, int imm) {
assert verify(asm, size, dst, null);
asm.interceptMemorySrcOperands(src);
emitOpcode(asm, size, getRXB(dst, src), dst.encoding, 0);
asm.emitOperandHelper(dst, src, immediateSize(size));
emitImmediate(asm, size, imm);
Expand Down Expand Up @@ -883,6 +900,7 @@ public final void emit(AMD64Assembler asm, OperandSize size, Register dst, Regis
public final void emit(AMD64Assembler asm, OperandSize size, Register dst, AMD64Address src) {
assert verify(asm, size, dst, null);
assert isSSEInstruction();
asm.interceptMemorySrcOperands(src);
// MOVSS/SD are not RVM instruction when the dst is an address
Register nds = (this == MOVSS || this == MOVSD) ? Register.None : preferredNDS.getNds(dst, src);
asm.simdPrefix(dst, nds, src, size, prefix1, prefix2, size == OperandSize.QWORD);
Expand Down Expand Up @@ -972,6 +990,7 @@ public final void emit(AMD64Assembler asm, OperandSize size, Register dst, Regis
public final void emit(AMD64Assembler asm, OperandSize size, Register dst, AMD64Address src, int imm) {
assert verify(asm, size, dst, null);
assert isSSEInstruction();
asm.interceptMemorySrcOperands(src);
asm.simdPrefix(dst, preferredNDS.getNds(dst, src), src, size, prefix1, prefix2, w);
asm.emitByte(op);
asm.emitOperandHelper(dst, src, immediateSize(size));
Expand Down Expand Up @@ -1090,12 +1109,12 @@ public static final class AMD64BinaryArithmetic {
private AMD64BinaryArithmetic(String opcode, int code) {
int baseOp = code << 3;

byteImmOp = new AMD64MIOp(opcode, true, 0, 0x80, code, OpAssertion.ByteAssertion);
byteImmOp = new AMD64MIOp(opcode, true, 0, 0x80, code, false, OpAssertion.ByteAssertion);
byteMrOp = new AMD64MROp(opcode, 0, baseOp, OpAssertion.ByteAssertion);
byteRmOp = new AMD64RMOp(opcode, 0, baseOp | 0x02, OpAssertion.ByteAssertion);

immOp = new AMD64MIOp(opcode, false, 0, 0x81, code, OpAssertion.WordOrLargerAssertion);
immSxOp = new AMD64MIOp(opcode, true, 0, 0x83, code, OpAssertion.WordOrLargerAssertion);
immOp = new AMD64MIOp(opcode, false, 0, 0x81, code, false, OpAssertion.WordOrLargerAssertion);
immSxOp = new AMD64MIOp(opcode, true, 0, 0x83, code, false, OpAssertion.WordOrLargerAssertion);
mrOp = new AMD64MROp(opcode, 0, baseOp | 0x01, OpAssertion.WordOrLargerAssertion);
rmOp = new AMD64RMOp(opcode, 0, baseOp | 0x03, OpAssertion.WordOrLargerAssertion);
}
Expand Down Expand Up @@ -1148,7 +1167,7 @@ public static final class AMD64Shift {
private AMD64Shift(String opcode, int code) {
m1Op = new AMD64MOp(opcode, 0, 0xD1, code, OpAssertion.WordOrLargerAssertion);
mcOp = new AMD64MOp(opcode, 0, 0xD3, code, OpAssertion.WordOrLargerAssertion);
miOp = new AMD64MIOp(opcode, true, 0, 0xC1, code, OpAssertion.WordOrLargerAssertion);
miOp = new AMD64MIOp(opcode, true, 0, 0xC1, code, true, OpAssertion.WordOrLargerAssertion);
}
}

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

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

public final void cmovl(ConditionFlag cc, Register dst, AMD64Address src) {
interceptMemorySrcOperands(src);
prefix(src, dst);
emitByte(0x0F);
emitByte(0x40 | cc.getValue());
Expand All @@ -4216,6 +4237,7 @@ public final void cmovq(ConditionFlag cc, Register dst, Register src) {
}

public final void cmovq(ConditionFlag cc, Register dst, AMD64Address src) {
interceptMemorySrcOperands(src);
prefixq(src, dst);
emitByte(0x0F);
emitByte(0x40 | cc.getValue());
Expand Down Expand Up @@ -4244,6 +4266,7 @@ public final void fincstp() {
}

public final void fldd(AMD64Address src) {
interceptMemorySrcOperands(src);
emitByte(0xDD);
emitOperandHelper(0, src, 0);
}
Expand All @@ -4259,6 +4282,7 @@ public final void fldln2() {
}

public final void flds(AMD64Address src) {
interceptMemorySrcOperands(src);
emitByte(0xD9);
emitOperandHelper(0, src, 0);
}
Expand Down Expand Up @@ -4290,11 +4314,13 @@ public final void fstp(int i) {
}

public final void fstpd(AMD64Address src) {
interceptMemorySrcOperands(src);
emitByte(0xDD);
emitOperandHelper(3, src, 0);
}

public final void fstps(AMD64Address src) {
interceptMemorySrcOperands(src);
emitByte(0xD9);
emitOperandHelper(3, src, 0);
}
Expand Down Expand Up @@ -4351,13 +4377,13 @@ public final void leave() {
emitByte(0xC9);
}

public final void lfence() {
public void lfence() {
emitByte(0x0f);
emitByte(0xae);
emitByte(0xe8);
}

public final void lock() {
public void lock() {
emitByte(0xF0);
}

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

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

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

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

}
32 changes: 25 additions & 7 deletions compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/asm/amd64/AMD64MacroAssembler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ public int call(PostCallAction postCallAction, InvokeTarget callTarget) {

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

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

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

public final int cmpAndJcc(OperandSize size, Register src1, Supplier<AMD64Address> src2, ConditionFlag cc, Label branchTarget) {
AMD64Address placeHolder = getPlaceholder(position());
AMD64Address src2AsAddress = src2.get();
final AMD64RMOp op = AMD64BinaryArithmetic.CMP.getRMOpcode(size);
final int bytesToEmit = getPrefixInBytes(size, src1, op.dstIsByte, placeHolder) + OPCODE_IN_BYTES + addressInBytes(placeHolder);
final int bytesToEmit = getPrefixInBytes(size, src1, op.dstIsByte, placeHolder) + OPCODE_IN_BYTES + addressInBytes(placeHolder) + extraSourceAddressBytes(src2AsAddress);
alignFusedPair(branchTarget, false, bytesToEmit);
final int beforeFusedPair = position();
AMD64Address src2AsAddress = src2.get();
op.emit(this, size, src1, src2AsAddress);
int beforeJcc = position();
assert beforeFusedPair + bytesToEmit == beforeJcc : Assertions.errorMessage(beforeFusedPair, bytesToEmit, position());
Expand Down
Loading

0 comments on commit 773f3c5

Please sign in to comment.