From 22ebf5e60895d9d0db2549f6889d46e72522db2a Mon Sep 17 00:00:00 2001 From: xsvm <2479667226@qq.com> Date: Thu, 8 Oct 2026 03:00:47 +0800 Subject: [PATCH] fix(relinker): classify rewrite reference sites with the instruction decoder ClassifyInstruction assumed a ModRM byte right after every opcode that was not a near branch. A length-changing rewrite therefore adjusted immediates such as MOVABS imm64 that looked like a RIP-relative ModRM, and missed RIP-relative operands of VEX and 0F 38 / 0F 3A instructions and the rel8 targets of LOOP and JRCXZ. Take the RIP-relative displacement and branch target from DecodeInstruction instead. --- .../codegen/x86/X64OpcodeConstants.hpp | 1 - .../src/x86/X64InstructionRewriter.cpp | 74 ++----------------- .../codegen/tests/Amd64OnlyConverterTests.cpp | 14 ++++ 3 files changed, 22 insertions(+), 67 deletions(-) diff --git a/core/relinker/codegen/include/codegen/x86/X64OpcodeConstants.hpp b/core/relinker/codegen/include/codegen/x86/X64OpcodeConstants.hpp index ba63d071c..969a1fd30 100644 --- a/core/relinker/codegen/include/codegen/x86/X64OpcodeConstants.hpp +++ b/core/relinker/codegen/include/codegen/x86/X64OpcodeConstants.hpp @@ -215,7 +215,6 @@ inline constexpr std::uint8_t SibBaseDisp32 = 0x05; inline constexpr std::size_t Disp8Size = 1; inline constexpr std::size_t Disp32Size = 4; -inline constexpr std::size_t Rel32InstructionLength = 5; inline constexpr std::size_t Vex2PrefixLength = 2; inline constexpr std::size_t Vex3PrefixLength = 3; diff --git a/core/relinker/codegen/src/x86/X64InstructionRewriter.cpp b/core/relinker/codegen/src/x86/X64InstructionRewriter.cpp index 0d012bea4..a3899c3dd 100644 --- a/core/relinker/codegen/src/x86/X64InstructionRewriter.cpp +++ b/core/relinker/codegen/src/x86/X64InstructionRewriter.cpp @@ -14,78 +14,20 @@ X64InstructionRewriter::ReferenceSite X64InstructionRewriter::ClassifyInstructio const std::uint8_t* data, std::size_t length ) { - std::size_t pos = 0; + const auto info = X64InstructionDecoder{}.DecodeInstruction(data, length); - while (pos < length) { - const std::uint8_t b = data[pos]; - if ((b >= RexMin && b <= RexMax) || - b == PrefixLock || b == PrefixRepne || b == PrefixRep || - b == PrefixSegCs || b == PrefixSegSs || b == PrefixSegDs || - b == PrefixSegEs || b == PrefixSegFs || b == PrefixSegGs || - b == PrefixOperandSize || b == PrefixAddressSize) { - pos += 1; - continue; - } - break; + if (info.HasRipRelativeDisp) { + return {ReferenceKind::RipRelativeDisp32, info.RipRelativeDispOffset}; } - if (pos >= length) { + if (!info.HasBranchTarget) { return {ReferenceKind::None, 0}; } - std::uint8_t opcode = data[pos]; - const std::size_t opcodeOffset = pos; - pos += 1; - bool twoByteOpcode = false; - - if (opcode == TwoByteOpcodeEscape) { - if (pos >= length) { - return {ReferenceKind::None, 0}; - } - twoByteOpcode = true; - opcode = data[pos]; - pos += 1; - } - - if (!twoByteOpcode) { - if (opcode == OneByteCallRel32 || opcode == OneByteJmpRel32) { - if (length >= opcodeOffset + Rel32InstructionLength) { - return {ReferenceKind::Rel32Branch, pos}; - } - return {ReferenceKind::None, 0}; - } - - if (opcode == OneByteJmpRel8 || (opcode >= OneByteJccRel8Min && opcode <= OneByteJccRel8Max)) { - if (length >= pos + Disp8Size) { - return {ReferenceKind::Rel8Branch, pos}; - } - return {ReferenceKind::None, 0}; - } - } else { - if (opcode >= TwoByteJccRel32Min && opcode <= TwoByteJccRel32Max) { - if (length >= pos + Disp32Size) { - return {ReferenceKind::Rel32Branch, pos}; - } - return {ReferenceKind::None, 0}; - } - } - - if (pos >= length) { - return {ReferenceKind::None, 0}; - } - - const std::uint8_t modrm = data[pos]; - const std::uint8_t mod = static_cast((modrm >> ModRmModShift) & ModRmModMask); - const std::uint8_t rm = static_cast(modrm & ModRmRmMask); - - if (mod == ModRmModIndirect && rm == ModRmRmRipRelative) { - const std::size_t dispOffset = pos + 1; - if (length >= dispOffset + Disp32Size) { - return {ReferenceKind::RipRelativeDisp32, dispOffset}; - } - } - - return {ReferenceKind::None, 0}; + const bool rel32 = info.IsTwoByteOpcode || info.Opcode == OneByteCallRel32 || info.Opcode == OneByteJmpRel32; + return rel32 + ? ReferenceSite{ReferenceKind::Rel32Branch, info.Length - Disp32Size} + : ReferenceSite{ReferenceKind::Rel8Branch, info.Length - Disp8Size}; } std::int32_t X64InstructionRewriter::ReadInt32(const std::uint8_t* data) { diff --git a/core/relinker/codegen/tests/Amd64OnlyConverterTests.cpp b/core/relinker/codegen/tests/Amd64OnlyConverterTests.cpp index f94034c19..69ec74251 100644 --- a/core/relinker/codegen/tests/Amd64OnlyConverterTests.cpp +++ b/core/relinker/codegen/tests/Amd64OnlyConverterTests.cpp @@ -723,6 +723,19 @@ void rewriterStrayRex() { require(rewritten.Bytes == expected, "Branch with a stray REX was not adjusted by a length-changing rewrite"); } +void rewriterReferenceSites() { + const std::vector> cases = { + {{0x90, 0x48, 0xB8, 0x05, 0xF0, 0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0xC3}, 0, + {0x66, 0x90, 0x48, 0xB8, 0x05, 0xF0, 0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0xC3}}, + {{0xC5, 0xF9, 0x6F, 0x05, 0x01, 0x00, 0x00, 0x00, 0x90, 0xC3}, 8, + {0xC5, 0xF9, 0x6F, 0x05, 0x02, 0x00, 0x00, 0x00, 0x66, 0x90, 0xC3}}, + {{0x66, 0x0F, 0x38, 0x00, 0x05, 0x01, 0x00, 0x00, 0x00, 0x90, 0xC3}, 9, + {0x66, 0x0F, 0x38, 0x00, 0x05, 0x02, 0x00, 0x00, 0x00, 0x66, 0x90, 0xC3}}, + {{0xE2, 0x01, 0x90, 0xC3}, 2, {0xE2, 0x02, 0x66, 0x90, 0xC3}}}; + for (const auto& [code, offset, expected] : cases) + require(Codegen::X64InstructionRewriter{}.Rewrite(code, {offset, {0x66, 0x90}}).Bytes == expected, "Length-changing rewrite adjusted an immediate or missed a VEX, three-byte or LOOP reference"); +} + void converterFailureOffsets() { const auto converter = Codegen::MakeAmd64OnlyConverter(); const auto file = segmentFixture(); @@ -1323,6 +1336,7 @@ int main() { converterReciprocal(); converterStrayRex(); rewriterStrayRex(); + rewriterReferenceSites(); converterFailureOffsets(); linuxPlacement(); linuxRipRelativePlacement();