FEX-Emu/FEX
1
0
Fork 0
mirror of https://github.com/FEX-Emu/FEX.git synced 2025-01-07 14:10:23 +00:00
Code Issues Actions 6 Packages Projects Releases Wiki Activity

Arm64: Fix undefined behaviour in Push operation

Browse Source 
Arm64 store with writeback when source register is the same register as
the address is undefined behaviour.
Depending on hardware details this can do a whole bunch of things.

This situation happens when the x86 code does `push rsp` which is quite
common for applications to do. We would then convert this to a `str x8, [x8, #-8]!`
Which results in undefined behaviour.

Now that redundant loads are optimized this showed up as an issue. Adds
a unit test to ensure we don't hit this again.
This commit is contained in:
Ryan Houdek 2023-09-07 17:38:39 -07:00
parent 22ca46a227
commit 636f8aa4a7
2 changed files with 39 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
FEXCore/Source/Interface/Core/JIT/Arm64/MemoryOps.cpp
View File

@ -1601,7 +1601,7 @@ DEF_OP(VBroadcastFromMem) {
DEF_OP(Push) {
const auto Op = IROp->C<IR::IROp_Push>();
const auto ValueSize = Op->ValueSize;
const auto Src = GetReg(Op->Value.ID());
auto Src = GetReg(Op->Value.ID());
const auto AddrSrc = GetReg(Op->Addr.ID());
const auto Dst = GetReg(Node);
@ -1617,6 +1617,22 @@ DEF_OP(Push) {
}
}
if (Src == AddrSrc) {
// If the data source is the address source then we need to do some additional work.
// This is because it is undefined behaviour to do a writeback on store operation where dest == src.
// In the case of writeback where the source is the address there are multiple behaviours.
// - SIGILL - Apple Silicon Behaviour
// - Stores original value - Cortex behaviour
// - Stores value after pre-index adjust adjust - Vixl simulator behaviour.
// - Undefined value stored
// - Undefined behaviour(!)
// In this path Src can end up overlapping both AddrSrc and Dst.
// Move the data to a temporary and store from there instead.
mov(TMP1, Src.X());
Src = TMP1;
}
if (NeedsMoveAfterwards) {
switch (ValueSize) {
case 1: {

22
unittests/ASM/FEX_bugs/Push.asm Normal file
View File

@ -0,0 +1,22 @@
%ifdef CONFIG
{
"RegData": {
"RAX": "0xe0000010",
"RSP": "0xe0000008"
}
}
%endif
; FEX had a bug where a `push rsp` would generate an Arm64 instruction with undefined behaviour.
; `push rsp` -> `str x8, [x8, #-8]!`
; This instruction has constrained undefined behaviour.
; On Cortex it stores the original value.
; On Apple Silicon it raises a SIGILL.
; It can also store undefined data or have undefined behaviour.
; Test to ensure we don't generate undefined behaviour.
mov rsp, 0xe0000010
push rsp
mov rax, [rsp]
hlt