Correctly fixes prefix ordering in the frontend

This was sort of partially worked around.
The issue comes down to REX.W and Operand size prefix ordering on an
instruction.
When an instruction has both prefixes, then the one that has occured
later will overwrite the previous one.
Additionally in the case of 66h being reused as an escape prefix, we
needed to ignore it in this case.

The previous workaround was that if an instruction happened to have both
widening and narrowing, then widening would win(for MODRM args),
but then later on in the instruction decoding the literal constant
decoding would properly check which order the flags came in.
This resulted in literal constants working, but not modrm bytes

Instead replace the "last" flag handling with a two deep stack that we
can push and pop the flags correctly on to it, this ensures that the
flags stay around in the 66h ignoring mode (so widening doesn't get
obliterated).

This effects every instruction that supports REX.W and 66h Operand size
prefix, so it could be fixing some unknown bugs elsewhere.
Very specifically this one was found in OpenSSL, because it looks like
GCC is padding a mov instruction with 66h.

Instruction Examples:
Only difference is the prefix order being swapped
0x00000000   4                 66488b1f  mov rbx, qword [rdi]
0x00000004   4                 48668b1f  mov bx, word [rdi]
This commit is contained in:
Ryan Houdek committed 2020-07-15 00:10:20 -07:00
1 parent ac68dc40e5
commit 2536bfe42c
3 files changed
+50 -27

No files matched your search

+9 -20
View File
@@ -223,6 +223,8 @@ bool Decoder::NormalOp(FEXCore::X86Tables::X86InstInfo const *Info, uint16_t Op)
"Group Ops should have been decoded before this!");
uint8_t DestSize{};
bool HasWideningDisplacement = FEXCore::X86Tables::DecodeFlags::GetOpAddr(DecodeInst->Flags, 0) & FEXCore::X86Tables::DecodeFlags::FLAG_WIDENING_SIZE_LAST;
bool HasNarrowingDisplacement = FEXCore::X86Tables::DecodeFlags::GetOpAddr(DecodeInst->Flags, 0) & FEXCore::X86Tables::DecodeFlags::FLAG_OPERAND_SIZE_LAST;
// New instruction size decoding
{
@@ -242,7 +244,7 @@ bool Decoder::NormalOp(FEXCore::X86Tables::X86InstInfo const *Info, uint16_t Op)
DecodeInst->Flags |= DecodeFlags::GenSizeDstSize(DecodeFlags::SIZE_128BIT);
DestSize = 16;
}
else if (DecodeInst->Flags & DecodeFlags::FLAG_OPERAND_SIZE &&
else if (HasNarrowingDisplacement &&
(DstSizeFlag == FEXCore::X86Tables::InstFlags::SIZE_DEF ||
DstSizeFlag == FEXCore::X86Tables::InstFlags::SIZE_64BITDEF)) {
// See table 1-2. Operand-Size Overrides for this decoding
@@ -251,7 +253,7 @@ bool Decoder::NormalOp(FEXCore::X86Tables::X86InstInfo const *Info, uint16_t Op)
DestSize = 2;
}
else if (CTX->Config.Is64BitMode &&
(DecodeInst->Flags & DecodeFlags::FLAG_REX_WIDENING ||
(HasWideningDisplacement ||
DstSizeFlag == FEXCore::X86Tables::InstFlags::SIZE_64BIT ||
DstSizeFlag == FEXCore::X86Tables::InstFlags::SIZE_64BITDEF)) {
DecodeInst->Flags |= DecodeFlags::GenSizeDstSize(DecodeFlags::SIZE_64BIT);
@@ -272,7 +274,7 @@ bool Decoder::NormalOp(FEXCore::X86Tables::X86InstInfo const *Info, uint16_t Op)
else if (SrcSizeFlag == FEXCore::X86Tables::InstFlags::SIZE_128BIT) {
DecodeInst->Flags |= DecodeFlags::GenSizeSrcSize(DecodeFlags::SIZE_128BIT);
}
else if (DecodeInst->Flags & DecodeFlags::FLAG_OPERAND_SIZE &&
else if (HasNarrowingDisplacement &&
(SrcSizeFlag == FEXCore::X86Tables::InstFlags::SIZE_DEF ||
SrcSizeFlag == FEXCore::X86Tables::InstFlags::SIZE_64BITDEF)) {
// See table 1-2. Operand-Size Overrides for this decoding
@@ -280,7 +282,7 @@ bool Decoder::NormalOp(FEXCore::X86Tables::X86InstInfo const *Info, uint16_t Op)
DecodeInst->Flags |= DecodeFlags::GenSizeSrcSize(DecodeFlags::SIZE_16BIT);
}
else if (CTX->Config.Is64BitMode &&
(DecodeInst->Flags & DecodeFlags::FLAG_REX_WIDENING ||
(HasWideningDisplacement ||
SrcSizeFlag == FEXCore::X86Tables::InstFlags::SIZE_64BIT ||
SrcSizeFlag == FEXCore::X86Tables::InstFlags::SIZE_64BITDEF)) {
DecodeInst->Flags |= DecodeFlags::GenSizeSrcSize(DecodeFlags::SIZE_64BIT);
@@ -295,21 +297,7 @@ bool Decoder::NormalOp(FEXCore::X86Tables::X86InstInfo const *Info, uint16_t Op)
HasMODRM |= !!(Info->Flags & FEXCore::X86Tables::InstFlags::FLAGS_MODRM);
bool HasSIB = false;
bool HasWideningDisplacement = DecodeInst->Flags & DecodeFlags::FLAG_REX_WIDENING;
bool HasNarrowingDisplacement = DecodeInst->Flags & DecodeFlags::FLAG_OPERAND_SIZE;
if (HasWideningDisplacement && HasNarrowingDisplacement) {
// This is a fun edge case where widening displacement has a precedence over narrowing displacement
// If you have both then widening (REX.W) takes precedence over narrowing (66h)
// Found with a fun CALL instruction in libLLVM for `call __tls_get_addr`
// Instruction was: 66 66 48 e8 a7 ff ec ff
// 66: Operand size override
// 66: Operand Size override
// 48: REX.W
// E8: call
// a7 ff ec ff: Immediate offset
HasNarrowingDisplacement = false;
}
// This is used for ModRM register modification
// For both modrm.reg and modrm.rm(when mod == 0b11) when value is >= 0b100
// then it changes from expected registers to the high 8bits of the lower registers
@@ -650,7 +638,7 @@ bool Decoder::NormalOpHeader(FEXCore::X86Tables::X86InstInfo const *Info, uint16
// Widening displacement
if (Op & 0b1000) {
DecodeInst->Flags |= DecodeFlags::FLAG_REX_WIDENING;
DecodeInst->Flags = (DecodeInst->Flags & ~DecodeFlags::FLAG_OPADDR_MASK) | DecodeFlags::FLAG_WIDENING_SIZE_LAST;
DecodeFlags::PushOpAddr(&DecodeInst->Flags, DecodeFlags::FLAG_WIDENING_SIZE_LAST);
}
// XGPR_B bit set
@@ -782,6 +770,7 @@ bool Decoder::DecodeInstruction(uint64_t PC) {
// Remove prefix so it doesn't effect calculations.
// This is only an escape prefix rather tan modifier now
DecodeInst->Flags &= ~DecodeFlags::FLAG_OPERAND_SIZE;
DecodeFlags::PopOpAddrIf(&DecodeInst->Flags, DecodeFlags::FLAG_OPERAND_SIZE_LAST);
if (NormalOpHeader(&FEXCore::X86Tables::OpSizeModOps[EscapeOp], EscapeOp)) {
InstructionDecoded = true;
}
@@ -796,7 +785,7 @@ bool Decoder::DecodeInstruction(uint64_t PC) {
case 0x66: // Operand Size prefix
DecodeInst->Flags |= DecodeFlags::FLAG_OPERAND_SIZE;
DecodeInst->LastEscapePrefix = Op;
DecodeInst->Flags = (DecodeInst->Flags & ~DecodeFlags::FLAG_OPADDR_MASK) | DecodeFlags::FLAG_OPERAND_SIZE_LAST;
DecodeFlags::PushOpAddr(&DecodeInst->Flags, DecodeFlags::FLAG_OPERAND_SIZE_LAST);
break;
case 0x67: // Address Size override prefix
DecodeInst->Flags |= DecodeFlags::FLAG_ADDRESS_SIZE;
@@ -87,15 +87,15 @@ void OpDispatchBuilder::SyscallOp(OpcodeArgs) {
void OpDispatchBuilder::LEAOp(OpcodeArgs) {
if (CTX->Config.Is64BitMode) {
uint32_t DstSize = X86Tables::DecodeFlags::GetOpAddr(Op->Flags) == X86Tables::DecodeFlags::FLAG_OPERAND_SIZE_LAST ? 2 :
X86Tables::DecodeFlags::GetOpAddr(Op->Flags) == X86Tables::DecodeFlags::FLAG_WIDENING_SIZE_LAST ? 8 : 4;
uint32_t DstSize = X86Tables::DecodeFlags::GetOpAddr(Op->Flags, 0) == X86Tables::DecodeFlags::FLAG_OPERAND_SIZE_LAST ? 2 :
X86Tables::DecodeFlags::GetOpAddr(Op->Flags, 0) == X86Tables::DecodeFlags::FLAG_WIDENING_SIZE_LAST ? 8 : 4;
uint32_t SrcSize = (Op->Flags & X86Tables::DecodeFlags::FLAG_ADDRESS_SIZE) ? 4 : 8;
OrderedNode *Src = LoadSource_WithOpSize(GPRClass, Op, Op->Src[0], SrcSize, Op->Flags, -1, false);
StoreResult_WithOpSize(GPRClass, Op, Op->Dest, Src, DstSize, -1);
}
else {
uint32_t DstSize = X86Tables::DecodeFlags::GetOpAddr(Op->Flags) == X86Tables::DecodeFlags::FLAG_OPERAND_SIZE_LAST ? 2 : 4;
uint32_t DstSize = X86Tables::DecodeFlags::GetOpAddr(Op->Flags, 0) == X86Tables::DecodeFlags::FLAG_OPERAND_SIZE_LAST ? 2 : 4;
uint32_t SrcSize = (Op->Flags & X86Tables::DecodeFlags::FLAG_ADDRESS_SIZE) ? 2 : 4;
OrderedNode *Src = LoadSource_WithOpSize(GPRClass, Op, Op->Src[0], SrcSize, Op->Flags, -1, false);
+38 -4
View File
@@ -49,10 +49,13 @@ constexpr uint32_t SIZE_128BIT = 0b101;
constexpr uint32_t SIZE_256BIT = 0b110;
constexpr uint32_t FLAG_OPADDR_OFF = (FLAG_SIZE_SRC_OFF + 3);
constexpr uint32_t FLAG_OPADDR_MASK = (0b11 << FLAG_OPADDR_OFF);
constexpr uint32_t FLAG_OPADDR_STACKSIZE = 4; // Two level deep stack
constexpr uint32_t FLAG_OPADDR_FLAG_SIZE = 2;
constexpr uint32_t FLAG_OPADDR_MASK = (((1 << FLAG_OPADDR_STACKSIZE) - 1) << FLAG_OPADDR_OFF);
constexpr uint32_t FLAG_OPERAND_SIZE_LAST = (0b01 << FLAG_OPADDR_OFF);
constexpr uint32_t FLAG_WIDENING_SIZE_LAST = (0b10 << FLAG_OPADDR_OFF);
// 00 = NONE
constexpr uint32_t FLAG_OPERAND_SIZE_LAST = 0b01;
constexpr uint32_t FLAG_WIDENING_SIZE_LAST = 0b10;
inline uint32_t GetSizeDstFlags(uint32_t Flags) { return (Flags >> FLAG_SIZE_DST_OFF) & SIZE_MASK; }
inline uint32_t GetSizeSrcFlags(uint32_t Flags) { return (Flags >> FLAG_SIZE_SRC_OFF) & SIZE_MASK; }
@@ -60,7 +63,38 @@ inline uint32_t GetSizeSrcFlags(uint32_t Flags) { return (Flags >> FLAG_SIZE_SRC
inline uint32_t GenSizeDstSize(uint32_t Size) { return Size << FLAG_SIZE_DST_OFF; }
inline uint32_t GenSizeSrcSize(uint32_t Size) { return Size << FLAG_SIZE_SRC_OFF; }
inline uint32_t GetOpAddr(uint32_t Flags) { return Flags & FLAG_OPADDR_MASK; }
inline uint32_t GetOpAddr(uint32_t Flags, int Index) { return (((Flags & FLAG_OPADDR_MASK) >> FLAG_OPADDR_OFF) >> (Index * 2)) & ((1 << FLAG_OPADDR_FLAG_SIZE) - 1); }
inline void PushOpAddr(uint32_t *Flags, uint32_t Flag) {
uint32_t TmpFlags = *Flags;
uint32_t BottomOfStack = ((TmpFlags & FLAG_OPADDR_MASK) >> FLAG_OPADDR_OFF) & ((1 << FLAG_OPADDR_FLAG_SIZE) - 1);
TmpFlags &= ~(FLAG_OPADDR_MASK);
TmpFlags |=
(BottomOfStack << (FLAG_OPADDR_OFF + FLAG_OPADDR_FLAG_SIZE)) |
(Flag << FLAG_OPADDR_OFF);
*Flags = TmpFlags;
}
inline void PopOpAddrIf(uint32_t *Flags, uint32_t Flag) {
uint32_t TmpFlags = *Flags;
uint32_t BottomOfStack = ((TmpFlags & FLAG_OPADDR_MASK) >> FLAG_OPADDR_OFF) & ((1 << FLAG_OPADDR_FLAG_SIZE) - 1);
// Only pop the stack if the bottom flag is the one we care about
// Necessary for escape prefixes that overlap regular prefixes
if (BottomOfStack != Flag) {
return;
}
uint32_t TopOfStack = ((TmpFlags & FLAG_OPADDR_MASK) >> (FLAG_OPADDR_OFF + FLAG_OPADDR_FLAG_SIZE)) & ((1 << FLAG_OPADDR_FLAG_SIZE) - 1);
TmpFlags &= ~(FLAG_OPADDR_MASK);
TmpFlags |= (TopOfStack << FLAG_OPADDR_OFF);
*Flags = TmpFlags;
}
}
union DecodedOperand {