Merge pull request #8063 from AasishDairelSahayaGrinspan/fix/relinker-gs-memory-only-traps

fix(relinker): trap only GS memory accesses in Windows and macOS output

core/relinker/CMakeLists.txt

@@ -481,7 +481,7 @@ if(BUILD_TESTING)
         foreach(opcode IN ITEMS add or adc sbb and sub xor cmp)
             list(APPEND tlsCoverageGroups alu_${opcode})
         endforeach()
-        list(APPEND tlsCoverageGroups gs_traps)
+        list(APPEND tlsCoverageGroups gs_traps gs_traps_implicit)
         foreach(group IN LISTS tlsCoverageGroups)
             add_test(NAME tls_function_coverage_${group} COMMAND ${Python3_EXECUTABLE}
                 ${CMAKE_CURRENT_SOURCE_DIR}/relinker/tests/test_tls_function_coverage.py $<TARGET_FILE:relinker> ${group})

core/relinker/codegen/include/codegen/CodegenTypes.hpp

@@ -68,6 +68,7 @@ struct DecodedInstructionInfo {
     bool AddressSize32;
     std::size_t RipRelativeDispOffset;
     bool HasModRm;
+    bool HasMemoryOperand;
     std::uint8_t ModRmByte;
     bool HasBranchTarget;
     std::int64_t BranchDisp;

core/relinker/codegen/include/codegen/x86/X64OpcodeConstants.hpp

@@ -146,6 +146,7 @@ inline constexpr std::uint8_t TwoByteMovdMovq = 0x7E;
 inline constexpr std::uint8_t TwoByteMovqMmxXmm = 0x6F;
 inline constexpr std::uint8_t TwoByteMovdqu = 0x7F;
 inline constexpr std::uint8_t TwoByteGrp7 = 0x01;
+inline constexpr std::uint8_t TwoByteMaskmov = 0xF7;
 inline constexpr std::uint8_t TwoByteShiftImm8Min = 0x70;
 inline constexpr std::uint8_t TwoByteShiftImm8Max = 0x73;
 inline constexpr std::uint8_t ThreeByteEscape38 = 0x38;

core/relinker/codegen/src/x86/X64InstructionDecoder.cpp

@@ -356,6 +356,7 @@ DecodedInstructionInfo X64InstructionDecoder::DecodeInstruction(
 
     std::size_t pos = 0;
     bool operandSizeOverride = false;
+    bool repPrefix = false;
 
     while (pos < info.Length) {
         std::uint8_t b = data[pos];
@@ -374,6 +375,8 @@ DecodedInstructionInfo X64InstructionDecoder::DecodeInstruction(
                 info.AddressSize32 = true;
             if (b == PrefixOperandSize)
                 operandSizeOverride = true;
+            if (b == PrefixRep)
+                repPrefix = true;
             info.RexPrefix = 0;
             ++pos;
             continue;
@@ -393,6 +396,8 @@ DecodedInstructionInfo X64InstructionDecoder::DecodeInstruction(
         info.ModRmRegField = (modrm >> ModRmRegShift) & ModRmRegMask;
         const std::uint8_t mod = registerOnly ? ModRmModRegister : (modrm >> ModRmModShift) & ModRmModMask;
         const std::uint8_t rm = modrm & ModRmRmMask;
+        if (mod != ModRmModRegister)
+            info.HasMemoryOperand = true;
         if (mod == ModRmModIndirect && rm == ModRmRmRipRelative) {
             info.HasRipRelativeDisp = true;
             info.RipRelativeDispOffset = modrmPos + 1;
@@ -404,9 +409,17 @@ DecodedInstructionInfo X64InstructionDecoder::DecodeInstruction(
 
     const bool xop = op == XopPrefix && pos < info.Length && (data[pos] & Vex3MapMask) >= XopMapMin;
     if (op == OneByteVex2 || op == OneByteVex3 || op == EvexPrefix || xop) {
-        pos += (op == OneByteVex2 ? 1 : (op == OneByteVex3 || xop ? 2 : EvexPrefixLength - 1)) + 1;
-        if (pos < info.Length)
+        const bool vex = op == OneByteVex2 || op == OneByteVex3;
+        const std::size_t vectorOpcodeOffset = pos + (op == OneByteVex2 ? 1 : (op == OneByteVex3 || xop ? 2 : EvexPrefixLength - 1));
+        const std::uint8_t vexMap = op == OneByteVex2 ? 1 : (data[pos] & (op == EvexPrefix ? EvexMapMask : Vex3MapMask));
+        const std::uint8_t vectorOpcode = vectorOpcodeOffset < info.Length ? data[vectorOpcodeOffset] : 0;
+        pos = vectorOpcodeOffset + 1;
+        if (pos < info.Length) {
             readModRm(pos, false);
+            if (vex && vexMap == 1 && vectorOpcode == TwoByteMaskmov &&
+                ((info.ModRmByte >> ModRmModShift) & ModRmModMask) == ModRmModRegister)
+                info.HasMemoryOperand = true;
+        }
         info.FlowKind = ControlFlowKind::Sequential;
         return info;
     }
@@ -480,6 +493,8 @@ DecodedInstructionInfo X64InstructionDecoder::DecodeInstruction(
             info.ModRmRegField = reg;
             std::uint8_t mod = (modrm >> ModRmModShift) & ModRmModMask;
             std::uint8_t rm = modrm & ModRmRmMask;
+            if (mod != ModRmModRegister)
+                info.HasMemoryOperand = true;
             if (reg == 2) {
                 if (mod == ModRmModIndirect && rm == ModRmRmRipRelative) {
                     info.FlowKind = ControlFlowKind::Call;
@@ -592,6 +607,21 @@ DecodedInstructionInfo X64InstructionDecoder::DecodeInstruction(
     if (hasModRm && pos < info.Length)
         readModRm(pos, twoByteOpcode && op >= TwoByteMovCrDrMin && op <= TwoByteMovCrDrMax);
 
+    if (info.HasModRm && twoByteOpcode) {
+        const std::uint8_t mod = (info.ModRmByte >> ModRmModShift) & ModRmModMask;
+        const std::uint8_t reg = (info.ModRmByte >> ModRmRegShift) & ModRmRegMask;
+        if (op == TwoByteMaskmov && mod == ModRmModRegister)
+            info.HasMemoryOperand = true;
+        else if (op == TwoByteGrp7 && (info.ModRmByte == 0xC8 || info.ModRmByte == 0xFA || info.ModRmByte == 0xFC))
+            info.HasMemoryOperand = true;
+        else if (op == TwoByteGrp15 && repPrefix && reg == 6 && mod == ModRmModRegister)
+            info.HasMemoryOperand = true;
+    }
+
+    if (!twoByteOpcode && ((op >= 0x6c && op <= 0x6f) || (op >= 0xa0 && op <= 0xa7) ||
+                           (op >= 0xaa && op <= 0xaf) || op == 0xd7))
+        info.HasMemoryOperand = true;
+
     info.FlowKind = ControlFlowKind::Sequential;
     return info;
 }

core/relinker/codegen/tests/DecoderLengthTests.cpp

@@ -117,6 +117,32 @@ int main() {
         const auto threeDNowInfo = decoder.DecodeInstruction(threeDNow.data(), threeDNow.size());
         require(threeDNowInfo.HasModRm && threeDNowInfo.ModRmByte == 0x0d, "3DNow! ModRM was not reported");
         require(threeDNowInfo.HasRipRelativeDisp && threeDNowInfo.RipRelativeDispOffset == 3, "3DNow! RIP-relative displacement was not reported");
+        for (const auto& testCase : std::vector<std::pair<const char*, bool>>{
+            {"6590", false},
+            {"654889c8", false},
+            {"65488b0c2578030000", true},
+            {"65a4", true},
+            {"650fa2", false},
+            {"650ff7c1", true},
+            {"65660ff7c1", true},
+            {"65c5f9f7c1", true},
+            {"65c4e179f7c1", true},
+            {"650f01c8", true},
+            {"650f01fa", true},
+            {"650f01fc", true},
+            {"650f01c9", false},
+            {"650f01fd", false},
+            {"65f30faef0", true},
+            {"f20faef0", false},
+            {"660faef0", false},
+            {"f30faee8", false},
+            {"ff10", true},
+            {"ffe0", false},
+        }) {
+            const auto bytes = parse(testCase.first);
+            require(decoder.DecodeInstruction(bytes.data(), bytes.size()).HasMemoryOperand == testCase.second,
+                    "Wrong memory operand presence for " + std::string(testCase.first));
+        }
         std::cout << "Decoder length tests passed\n";
     } catch (const std::exception& error) {
         std::cerr << error.what() << '\n';

core/relinker/elfpatcher/src/macos/MacOsImage.cpp

@@ -274,7 +274,8 @@ TlsScan ScanTls(const std::vector<std::uint8_t>& source, const std::vector<Domai
             }
             if (info.SegmentPrefix == 0 || (insidePrevious && info.SegmentPrefix != 0x64)) continue;
             if (info.SegmentPrefix == 0x65) {
-                scan.GsTraps.push_back({rva, info.Length});
+                if (info.HasMemoryOperand)
+                    scan.GsTraps.push_back({rva, info.Length});
                 continue;
             }
             const auto position = info.OpcodeOffset;

core/relinker/elfpatcher/src/windows/WindowsTlsBuilder.cpp

@@ -150,7 +150,8 @@ PeDirectory WindowsTlsBuilder::Build(const std::vector<std::uint8_t>& source, co
             }
 
             if (info.SegmentPrefix == 0x65 && !insidePrevious) {
-                gsTraps.emplace_back(rva, info.Length);
+                if (info.HasMemoryOperand)
+                    gsTraps.emplace_back(rva, info.Length);
                 continue;
             }
             if (info.SegmentPrefix != 0 && !(insidePrevious && info.SegmentPrefix != 0x64)) {

core/relinker/relinker/tests/test_macos_gs_traps.py

@@ -15,6 +15,22 @@ FORMS = {
     "gs-store": bytes.fromhex("65 48 89 04 25 30 00 00 00"),
     "gs-alu": bytes.fromhex("65 48 33 04 25 28 00 00 00"),
     "gs-short-register-address": bytes.fromhex("65 48 8b 00"),
+    "gs-maskmovdqu": bytes.fromhex("65 66 0f f7 c1"),
+    "gs-maskmovq": bytes.fromhex("65 0f f7 c1"),
+    "gs-vmaskmovdqu": bytes.fromhex("65 c5 f9 f7 c1"),
+    "gs-monitor": bytes.fromhex("65 0f 01 c8"),
+    "gs-monitorx": bytes.fromhex("65 0f 01 fa"),
+    "gs-clzero": bytes.fromhex("65 0f 01 fc"),
+    "gs-umonitor": bytes.fromhex("65 f3 0f ae f0"),
+}
+
+NON_MEMORY_FORMS = {
+    "gs-nop": bytes.fromhex("65 90"),
+    "gs-register-move": bytes.fromhex("65 48 89 c8"),
+    "gs-cpuid": bytes.fromhex("65 0f a2"),
+    "gs-mwait": bytes.fromhex("65 0f 01 c9"),
+    "gs-rdpru": bytes.fromhex("65 0f 01 fd"),
+    "gs-umwait": bytes.fromhex("65 f2 0f ae f0"),
 }
 
 
@@ -38,12 +54,20 @@ def check_trapped(name, code, instruction):
         raise SystemExit(f"{name}: the GS access was left in the code")
 
 
+def check_untouched(name, code, instruction):
+    if instruction + b"\xc3" not in code:
+        raise SystemExit(f"{name}: a non-memory GS instruction was replaced")
+
+
 def main():
     relinker = Path(sys.argv[1]).resolve()
     with tempfile.TemporaryDirectory(prefix="anyps5-macos-gs-") as directory:
         for name, instruction in FORMS.items():
             image = make_image("register", "unwind", body=instruction + b"\xc3")
             check_trapped(name, convert(relinker, directory, name, image), instruction)
+        for name, instruction in NON_MEMORY_FORMS.items():
+            image = make_image("register", "unwind", body=instruction + b"\xc3")
+            check_untouched(name, convert(relinker, directory, name, image), instruction)
         instruction = FORMS["gs-load-rcx"]
         image = make_image("register", "unwind", body=instruction + b"\xc3")
         struct.pack_into("<I", image, 176, 0)

core/relinker/relinker/tests/test_tls_function_coverage.py

@@ -530,7 +530,7 @@ def add_alias(image, size, begin=0x1200):
 
 def main():
     groups = ("metadata", "padding", "register_loads", "displacement_bounds", "alu_execution",
-              "register_address_loads", "moved_branch", "rejected", "gs_traps")
+              "register_address_loads", "moved_branch", "rejected", "gs_traps", "gs_traps_implicit")
     groups += tuple(f"displacement_r{register}" for register in range(16) if register != 4)
     groups += tuple(f"alu_{name}" for name in ALU_READ)
     parser = argparse.ArgumentParser(description="Run Windows guest TLS function coverage fixtures")
@@ -776,14 +776,9 @@ def main():
             conflicting[232:344] = unwind[232:344]
             struct.pack_into("<H", conflicting, 56, 5)
             convert("conflicting-function-extents", conflicting, "Code analysis: conflicting function ranges")
-        if args.group in ("all", "gs_traps"):
-            for name, instruction in {
-                "gs-load-rcx": bytes.fromhex("65 48 8b 0c 25 78 03 00 00"),
-                "gs-load-r13": bytes.fromhex("65 4c 8b 2c 25 30 00 00 00"),
-                "gs-store": bytes.fromhex("65 48 89 04 25 30 00 00 00"),
-                "gs-alu": bytes.fromhex("65 48 33 04 25 28 00 00 00"),
-                "gs-short-register-address": bytes.fromhex("65 48 8b 00"),
-            }.items():
+
+        def check_gs_traps(cases):
+            for name, instruction, trapped in cases:
                 source = work / (name + ".elf")
                 output = source.with_suffix(".exe")
                 source.write_bytes(make_image("register", "unwind", body=instruction + b"\xc3"))
@@ -791,7 +786,23 @@ def main():
                                         capture_output=True, text=True, timeout=30)
                 assert result.returncode == 0, (name, result.stdout, result.stderr)
                 patched = pe_bytes_at(output.read_bytes(), 0x11240, len(instruction) + 1)
-                assert patched == b"\x0f\x0b" + b"\x90" * (len(instruction) - 2) + b"\xc3", (name, patched.hex())
+                if trapped:
+                    expected = b"\x0f\x0b" + b"\x90" * (len(instruction) - 2) + b"\xc3"
+                else:
+                    expected = instruction + b"\xc3"
+                assert patched == expected, (name, patched.hex())
+
+        if args.group in ("all", "gs_traps"):
+            check_gs_traps([
+                ("gs-load-rcx", bytes.fromhex("65 48 8b 0c 25 78 03 00 00"), True),
+                ("gs-load-r13", bytes.fromhex("65 4c 8b 2c 25 30 00 00 00"), True),
+                ("gs-store", bytes.fromhex("65 48 89 04 25 30 00 00 00"), True),
+                ("gs-alu", bytes.fromhex("65 48 33 04 25 28 00 00 00"), True),
+                ("gs-short-register-address", bytes.fromhex("65 48 8b 00"), True),
+                ("gs-nop", bytes.fromhex("65 90"), False),
+                ("gs-register-move", bytes.fromhex("65 48 89 c8"), False),
+                ("gs-cpuid", bytes.fromhex("65 0f a2"), False),
+            ])
             instruction = bytes.fromhex("65 48 8b 0c 25 78 03 00 00")
             image = make_image("register", "unwind", body=instruction + b"\xc3")
             struct.pack_into("<I", image, 176, 0)
@@ -803,6 +814,19 @@ def main():
             assert result.returncode == 0, ("gs-without-pt-tls", result.stdout, result.stderr)
             patched = pe_bytes_at(output.read_bytes(), 0x11240, len(instruction) + 1)
             assert patched == b"\x0f\x0b" + b"\x90" * (len(instruction) - 2) + b"\xc3", ("gs-without-pt-tls", patched.hex())
+        if args.group in ("all", "gs_traps_implicit"):
+            check_gs_traps([
+                ("gs-maskmovdqu", bytes.fromhex("65 66 0f f7 c1"), True),
+                ("gs-maskmovq", bytes.fromhex("65 0f f7 c1"), True),
+                ("gs-vmaskmovdqu", bytes.fromhex("65 c5 f9 f7 c1"), True),
+                ("gs-monitor", bytes.fromhex("65 0f 01 c8"), True),
+                ("gs-monitorx", bytes.fromhex("65 0f 01 fa"), True),
+                ("gs-clzero", bytes.fromhex("65 0f 01 fc"), True),
+                ("gs-umonitor", bytes.fromhex("65 f3 0f ae f0"), True),
+                ("gs-mwait", bytes.fromhex("65 0f 01 c9"), False),
+                ("gs-rdpru", bytes.fromhex("65 0f 01 fd"), False),
+                ("gs-umwait", bytes.fromhex("65 f2 0f ae f0"), False),
+            ])
     print(f"TLS function coverage integration tests passed: {args.group}")
 
 

docs/dev/debt/relinker.md

@@ -24,7 +24,7 @@
 - In the [code analysis](../../../core/relinker/relinker/src/analysis/CodeInstructionCollector.cpp) of the Windows and macOS outputs for such images, a relocation or `lea` target outside every unwind function is followed as code only when its linear decoding, up to the first jump, return or trap, the next unwind function or 256 instructions, holds an `fs:[disp32]` access, or when it is 16-byte aligned, starts less than 256 bytes after an unwind function and ends in a jump, return or trap. Every other such target is taken as data and counted in one message, and the Windows TLS rewrite fails instead of overwriting one. A TLS access of another form, such as `mov rax, fs:[rax]`, or a `gs` instruction in code reached only through such a target is therefore not rewritten or trapped. Other fs and gs instructions do not tell code from data there: string bytes decode as these prefixes, and in two measured titles about 2,500 of about 5,500 such targets decode to one before their first jump.
 - In executables and guest modules with a `PT_SCE_DYNLIBDATA` segment, the [code analysis](../../../core/relinker/relinker/src/analysis/CodeInstructionCollector.cpp) of the Windows and macOS outputs follows code only from the end of `PT_INTERP`, or of the `/libexec/ld-elf.so.1` string that begins the segment when no `PT_INTERP` covers it, as in OpenOrbis modules, to the end of the PLT after the last function of the unwind table, or to the end of that function when the PLT comes before the code, as in the `libSceFios2.prx` and `libc.prx` that CUSA02636 bundles (PLT at 0xc8); the pipeline's scans of executables above stop at this same end, also when the PLT comes before the code. A guest module with PLT relocations and no PLT before or after its unwind functions, or with a PLT that overlaps one, fails as the executable does. Code after that end is not analyzed, such as the `DT_FINI` routine without an unwind entry that follows the last unwind function in both modules and that the Windows and macOS outputs call when the program exits: a TLS access in it is not rewritten, the ELF entry point, `DT_INIT`, `DT_FINI`, a branch or a pointer into it is ignored, and a function symbol or init/fini array entry pointing into it fails the conversion.
 - A relocation against a dynamic symbol entry that is all zero, like the null symbol, takes the value 0 plus its addend, as the gABI defines for `STN_UNDEF` ([pipeline](../../../core/relinker/relinker/src/pipeline/RelinkerPipeline.cpp)). The Linux output imports such an entry as an unnamed weak symbol, which the host loader resolves to 0; the Windows and macOS outputs write the value into the slot ([relocation builder](../../../core/relinker/elfpatcher/src/windows/WindowsRelocationBuilder.cpp)). What the console's loader does with such an entry is not measured. OliSe Player (PPSA01153) has seven of them, and its code calls through those slots only when they are not 0 ([#4437](https://github.com/boykopovar/AnyPS5/issues/4437)).
-- In macOS output, a guest instruction with a GS segment prefix, including one with no memory operand (`65 90`, `65 48 89 c8`), which does not touch GS, becomes `UD2` and NOP padding of its own length ([macOS image writer](../../../core/relinker/elfpatcher/src/macos/MacOsImage.cpp)): the host owns GS, which holds the pthread TSD base the guest TLS stubs read, and the guest GS base that FreeBSD's `sysarch(AMD64_SET_GSBASE)` would set is not emulated. A converted module runs until such an instruction is reached, which then faults. Prospero Win (PPSA99995) has six in its Wine `ntdll`.
+- In macOS output, a guest instruction that addresses memory through GS becomes `UD2` and NOP padding of its own length ([macOS image writer](../../../core/relinker/elfpatcher/src/macos/MacOsImage.cpp)): the host owns GS, which holds the pthread TSD base the guest TLS stubs read, and the guest GS base that FreeBSD's `sysarch(AMD64_SET_GSBASE)` would set is not emulated. A converted module runs until such an instruction is reached, which then faults. Prospero Win (PPSA99995) has six in its Wine `ntdll`.
 - The Linux placement of the `--to-intel` stubs in the [ELF patcher](../../../core/relinker/elfpatcher/src/linux/LinuxElfPatcher.cpp) is covered only by a synthetic test.
 - `--to-rosetta` lowers what Rosetta lacks on top of `--to-intel` ([matcher](../../../core/relinker/codegen/src/x86/Amd64OnlyInstructionMatcher.cpp)). AVX2, FMA, BMI1/2, F16C and MOVBE were observed to execute on an M5 Max without being reported through CPUID; this is not a guarantee for other Rosetta hosts. ANDN and RDRAND fault on an M1 running macOS 15.3.1 and execute on an M1 running macOS 26.6.2, so on the M1 the difference follows the macOS version; on that host BEXTR, BLSI, BLSMSK, BLSR, TZCNT, LZCNT, POPCNT, BZHI, MULX, PDEP, PEXT, RORX, SARX, SHLX, SHRX, MOVBE, FMA, AVX2 and F16C execute as well, its CPUID leaf 1 reports none of MOVBE, FMA, AVX or F16C, and RDSEED, RDPID, CLWB, ADCX and ADOX raise SIGILL. They also execute on an M5 running macOS 26.4, where the rest of BMI1 and BMI2, LZCNT, PREFETCHW, CLFLUSHOPT, AVX, AVX2, FMA, F16C and MOVBE execute as well although CPUID leaf 7 reads zero and leaves 1 and 0x80000001 report none of them, and where RDSEED, CLWB, RDPID, ADCX and ADOX raise an illegal instruction. ANDN is lowered, but RDRAND and other BMI instructions still depend on the host. RDSEED becomes RDRAND and CLWB a NOP. RDPID returns processor 0, and RDTSCP runs unchanged with ECX set to 0 after it, because Rosetta's TSC_AUX is the constant 0xAAAAAAAB (also measured on an M5, macOS 27.0.1). An RDPID or RDTSCP site with no movable instruction after it for the jump is kept; a kept RDTSCP reports 0xAAAAAAAB. ADCX/ADOX fault under Rosetta and are not converted: compilers do not emit them, and code that checks CPUID sees no ADX. The conversions are covered by the converter tests. ANDN lowering is also executed by an x86-64 test build under Rosetta on the M1; the other conversions were originally checked on an M5 Max, outside this repository's CI.
 - `--to-intel` replaces the approximate legacy SSE and VEX.128 register forms of RSQRTPS/RCPPS and VRSQRTPS/VRCPPS with [correctly rounded](../../../core/relinker/codegen/src/x86/ReciprocalLowering.cpp) 1/sqrt(x) and 1/x, not with the AMD approximation: its tables are not public. Intel's approximation of 1/sqrt(1.0) is 0x3F7FF000, and two Newton-Raphson steps on it settle on 0.99999994 instead of 1.0; a renormalised identity quaternion then turns into a NaN axis in PPSA21564, which the console does not show. Refined results now reach the exact value; unrefined ones can still differ from the console in the low bits. Each site costs an out-of-line stub (two jumps, a spill below the red zone, SQRTPS/DIVPS). Legacy SSE forms preserve the destination's upper YMM bits; VEX forms clear them. The 256-bit, scalar and memory forms keep the native instruction, as does a site shorter than 5 bytes with no movable instruction after it for the jump (5 sites in PPSA21564, counted before a following RIP-relative instruction could move into the stub; not recounted).