From f2f3a59dbf9e9729006bf8713e04a362c961655d Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sun, 14 Feb 2021 16:59:03 +0100 Subject: [PATCH] JitArm64: Fix improper uses of offsetof Using a non-constant array index inside offsetof is not standards compliant, and is rejected by GCC 11. https://bugs.dolphin-emu.org/issues/12409#note-5 --- Source/Core/Core/PowerPC/JitArm64/Jit.cpp | 2 +- .../Core/PowerPC/JitArm64/JitArm64_Branch.cpp | 24 ++++++++--------- .../JitArm64/JitArm64_LoadStorePaired.cpp | 4 +-- .../PowerPC/JitArm64/JitArm64_RegCache.cpp | 26 ++++++++----------- .../Core/PowerPC/JitArm64/JitArm64_RegCache.h | 18 ++++++++++--- .../JitArm64/JitArm64_SystemRegisters.cpp | 14 +++++----- 6 files changed, 48 insertions(+), 40 deletions(-) diff --git a/Source/Core/Core/PowerPC/JitArm64/Jit.cpp b/Source/Core/Core/PowerPC/JitArm64/Jit.cpp index 43397d0355..27420cbb09 100644 --- a/Source/Core/Core/PowerPC/JitArm64/Jit.cpp +++ b/Source/Core/Core/PowerPC/JitArm64/Jit.cpp @@ -666,7 +666,7 @@ void JitArm64::DoJit(u32 em_address, JitBlock* b, u32 nextPC) int gqr = *code_block.m_gqr_used.begin(); if (!code_block.m_gqr_modified[gqr] && !GQR(gqr)) { - LDR(IndexType::Unsigned, W0, PPC_REG, PPCSTATE_OFF(spr[SPR_GQR0]) + gqr * 4); + LDR(IndexType::Unsigned, W0, PPC_REG, PPCSTATE_OFF_SPR(SPR_GQR0 + gqr)); FixupBranch no_fail = CBZ(W0); FixupBranch fail = B(); SwitchToFarCode(); diff --git a/Source/Core/Core/PowerPC/JitArm64/JitArm64_Branch.cpp b/Source/Core/Core/PowerPC/JitArm64/JitArm64_Branch.cpp index 0a1393a42f..7f7cb9a19b 100644 --- a/Source/Core/Core/PowerPC/JitArm64/JitArm64_Branch.cpp +++ b/Source/Core/Core/PowerPC/JitArm64/JitArm64_Branch.cpp @@ -57,14 +57,14 @@ void JitArm64::rfi(UGeckoInstruction inst) ANDI2R(WC, WC, (~mask) & clearMSR13, WA); // rD = Masked MSR - LDR(IndexType::Unsigned, WA, PPC_REG, PPCSTATE_OFF(spr[SPR_SRR1])); // rB contains SRR1 here + LDR(IndexType::Unsigned, WA, PPC_REG, PPCSTATE_OFF_SPR(SPR_SRR1)); // rB contains SRR1 here ANDI2R(WA, WA, mask & clearMSR13, WB); // rB contains masked SRR1 here ORR(WA, WA, WC); // rB = Masked MSR OR masked SRR1 STR(IndexType::Unsigned, WA, PPC_REG, PPCSTATE_OFF(msr)); // STR rB in to rA - LDR(IndexType::Unsigned, WA, PPC_REG, PPCSTATE_OFF(spr[SPR_SRR0])); + LDR(IndexType::Unsigned, WA, PPC_REG, PPCSTATE_OFF_SPR(SPR_SRR0)); gpr.Unlock(WB, WC); WriteExceptionExit(WA); @@ -80,7 +80,7 @@ void JitArm64::bx(UGeckoInstruction inst) { ARM64Reg WA = gpr.GetReg(); MOVI2R(WA, js.compilerPC + 4); - STR(IndexType::Unsigned, WA, PPC_REG, PPCSTATE_OFF(spr[SPR_LR])); + STR(IndexType::Unsigned, WA, PPC_REG, PPCSTATE_OFF_SPR(SPR_LR)); gpr.Unlock(WA); } @@ -125,9 +125,9 @@ void JitArm64::bcx(UGeckoInstruction inst) FixupBranch pCTRDontBranch; if ((inst.BO & BO_DONT_DECREMENT_FLAG) == 0) // Decrement and test CTR { - LDR(IndexType::Unsigned, WA, PPC_REG, PPCSTATE_OFF(spr[SPR_CTR])); + LDR(IndexType::Unsigned, WA, PPC_REG, PPCSTATE_OFF_SPR(SPR_CTR)); SUBS(WA, WA, 1); - STR(IndexType::Unsigned, WA, PPC_REG, PPCSTATE_OFF(spr[SPR_CTR])); + STR(IndexType::Unsigned, WA, PPC_REG, PPCSTATE_OFF_SPR(SPR_CTR)); if (inst.BO & BO_BRANCH_IF_CTR_0) pCTRDontBranch = B(CC_NEQ); @@ -150,7 +150,7 @@ void JitArm64::bcx(UGeckoInstruction inst) if (inst.LK) { MOVI2R(WA, js.compilerPC + 4); - STR(IndexType::Unsigned, WA, PPC_REG, PPCSTATE_OFF(spr[SPR_LR])); + STR(IndexType::Unsigned, WA, PPC_REG, PPCSTATE_OFF_SPR(SPR_LR)); } gpr.Unlock(WA); @@ -213,13 +213,13 @@ void JitArm64::bcctrx(UGeckoInstruction inst) { ARM64Reg WB = gpr.GetReg(); MOVI2R(WB, js.compilerPC + 4); - STR(IndexType::Unsigned, WB, PPC_REG, PPCSTATE_OFF(spr[SPR_LR])); + STR(IndexType::Unsigned, WB, PPC_REG, PPCSTATE_OFF_SPR(SPR_LR)); gpr.Unlock(WB); } ARM64Reg WA = gpr.GetReg(); - LDR(IndexType::Unsigned, WA, PPC_REG, PPCSTATE_OFF(spr[SPR_CTR])); + LDR(IndexType::Unsigned, WA, PPC_REG, PPCSTATE_OFF_SPR(SPR_CTR)); AND(WA, WA, 30, 29); // Wipe the bottom 2 bits. WriteExit(WA, inst.LK_3, js.compilerPC + 4); @@ -241,9 +241,9 @@ void JitArm64::bclrx(UGeckoInstruction inst) FixupBranch pCTRDontBranch; if ((inst.BO & BO_DONT_DECREMENT_FLAG) == 0) // Decrement and test CTR { - LDR(IndexType::Unsigned, WA, PPC_REG, PPCSTATE_OFF(spr[SPR_CTR])); + LDR(IndexType::Unsigned, WA, PPC_REG, PPCSTATE_OFF_SPR(SPR_CTR)); SUBS(WA, WA, 1); - STR(IndexType::Unsigned, WA, PPC_REG, PPCSTATE_OFF(spr[SPR_CTR])); + STR(IndexType::Unsigned, WA, PPC_REG, PPCSTATE_OFF_SPR(SPR_CTR)); if (inst.BO & BO_BRANCH_IF_CTR_0) pCTRDontBranch = B(CC_NEQ); @@ -265,13 +265,13 @@ void JitArm64::bclrx(UGeckoInstruction inst) SetJumpTarget(far_addr); } - LDR(IndexType::Unsigned, WA, PPC_REG, PPCSTATE_OFF(spr[SPR_LR])); + LDR(IndexType::Unsigned, WA, PPC_REG, PPCSTATE_OFF_SPR(SPR_LR)); AND(WA, WA, 30, 29); // Wipe the bottom 2 bits. if (inst.LK) { MOVI2R(WB, js.compilerPC + 4); - STR(IndexType::Unsigned, WB, PPC_REG, PPCSTATE_OFF(spr[SPR_LR])); + STR(IndexType::Unsigned, WB, PPC_REG, PPCSTATE_OFF_SPR(SPR_LR)); gpr.Unlock(WB); } diff --git a/Source/Core/Core/PowerPC/JitArm64/JitArm64_LoadStorePaired.cpp b/Source/Core/Core/PowerPC/JitArm64/JitArm64_LoadStorePaired.cpp index 27fe19a3e9..366298a2bd 100644 --- a/Source/Core/Core/PowerPC/JitArm64/JitArm64_LoadStorePaired.cpp +++ b/Source/Core/Core/PowerPC/JitArm64/JitArm64_LoadStorePaired.cpp @@ -77,7 +77,7 @@ void JitArm64::psq_l(UGeckoInstruction inst) } else { - LDR(IndexType::Unsigned, scale_reg, PPC_REG, PPCSTATE_OFF(spr[SPR_GQR0 + inst.I])); + LDR(IndexType::Unsigned, scale_reg, PPC_REG, PPCSTATE_OFF_SPR(SPR_GQR0 + inst.I)); UBFM(type_reg, scale_reg, 16, 18); // Type UBFM(scale_reg, scale_reg, 24, 29); // Scale @@ -179,7 +179,7 @@ void JitArm64::psq_st(UGeckoInstruction inst) m_float_emit.FCVTN(32, D0, VS); } - LDR(IndexType::Unsigned, scale_reg, PPC_REG, PPCSTATE_OFF(spr[SPR_GQR0 + inst.I])); + LDR(IndexType::Unsigned, scale_reg, PPC_REG, PPCSTATE_OFF_SPR(SPR_GQR0 + inst.I)); UBFM(type_reg, scale_reg, 0, 2); // Type UBFM(scale_reg, scale_reg, 8, 13); // Scale diff --git a/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.cpp b/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.cpp index 2c4ca41928..7259f54f1e 100644 --- a/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.cpp +++ b/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.cpp @@ -147,13 +147,13 @@ const OpArg& Arm64GPRCache::GetGuestGPROpArg(size_t preg) const Arm64GPRCache::GuestRegInfo Arm64GPRCache::GetGuestGPR(size_t preg) { ASSERT(preg < GUEST_GPR_COUNT); - return {32, PPCSTATE_OFF(gpr[preg]), m_guest_registers[GUEST_GPR_OFFSET + preg]}; + return {32, PPCSTATE_OFF_GPR(preg), m_guest_registers[GUEST_GPR_OFFSET + preg]}; } Arm64GPRCache::GuestRegInfo Arm64GPRCache::GetGuestCR(size_t preg) { ASSERT(preg < GUEST_CR_COUNT); - return {64, PPCSTATE_OFF(cr.fields[preg]), m_guest_registers[GUEST_CR_OFFSET + preg]}; + return {64, PPCSTATE_OFF_CR(preg), m_guest_registers[GUEST_CR_OFFSET + preg]}; } Arm64GPRCache::GuestRegInfo Arm64GPRCache::GetGuestByIndex(size_t index) @@ -457,7 +457,7 @@ ARM64Reg Arm64FPRCache::R(size_t preg, RegType type) // Load the high 64bits from the file and insert them in to the high 64bits of the host // register const ARM64Reg tmp_reg = GetReg(); - m_float_emit->LDR(64, IndexType::Unsigned, tmp_reg, PPC_REG, u32(PPCSTATE_OFF(ps[preg].ps1))); + m_float_emit->LDR(64, IndexType::Unsigned, tmp_reg, PPC_REG, u32(PPCSTATE_OFF_PS1(preg))); m_float_emit->INS(64, host_reg, 1, tmp_reg, 0); UnlockRegister(tmp_reg); @@ -511,7 +511,7 @@ ARM64Reg Arm64FPRCache::R(size_t preg, RegType type) } reg.SetDirty(false); m_float_emit->LDR(load_size, IndexType::Unsigned, host_reg, PPC_REG, - u32(PPCSTATE_OFF(ps[preg].ps0))); + u32(PPCSTATE_OFF_PS0(preg))); return host_reg; } default: @@ -559,8 +559,7 @@ ARM64Reg Arm64FPRCache::RW(size_t preg, RegType type) // We are doing a full 128bit store because it takes 2 cycles on a Cortex-A57 to do a 128bit // store. // It would take longer to do an insert to a temporary and a 64bit store than to just do this. - m_float_emit->STR(128, IndexType::Unsigned, flush_reg, PPC_REG, - u32(PPCSTATE_OFF(ps[preg].ps0))); + m_float_emit->STR(128, IndexType::Unsigned, flush_reg, PPC_REG, u32(PPCSTATE_OFF_PS0(preg))); break; case RegType::DuplicatedSingle: flush_reg = GetReg(); @@ -568,8 +567,7 @@ ARM64Reg Arm64FPRCache::RW(size_t preg, RegType type) [[fallthrough]]; case RegType::Duplicated: // Store PSR1 (which is equal to PSR0) in memory. - m_float_emit->STR(64, IndexType::Unsigned, flush_reg, PPC_REG, - u32(PPCSTATE_OFF(ps[preg].ps1))); + m_float_emit->STR(64, IndexType::Unsigned, flush_reg, PPC_REG, u32(PPCSTATE_OFF_PS1(preg))); break; default: // All other types doesn't store anything in PSR1. @@ -697,7 +695,7 @@ void Arm64FPRCache::FlushRegister(size_t preg, bool maintain_state) if (dirty) { m_float_emit->STR(store_size, IndexType::Unsigned, host_reg, PPC_REG, - u32(PPCSTATE_OFF(ps[preg].ps0))); + u32(PPCSTATE_OFF_PS0(preg))); } if (!maintain_state) @@ -710,17 +708,15 @@ void Arm64FPRCache::FlushRegister(size_t preg, bool maintain_state) { if (dirty) { - if (PPCSTATE_OFF(ps[preg].ps0) <= 504) + if (PPCSTATE_OFF_PS0(preg) <= 504) { m_float_emit->STP(64, IndexType::Signed, host_reg, host_reg, PPC_REG, - PPCSTATE_OFF(ps[preg].ps0)); + PPCSTATE_OFF_PS0(preg)); } else { - m_float_emit->STR(64, IndexType::Unsigned, host_reg, PPC_REG, - u32(PPCSTATE_OFF(ps[preg].ps0))); - m_float_emit->STR(64, IndexType::Unsigned, host_reg, PPC_REG, - u32(PPCSTATE_OFF(ps[preg].ps1))); + m_float_emit->STR(64, IndexType::Unsigned, host_reg, PPC_REG, u32(PPCSTATE_OFF_PS0(preg))); + m_float_emit->STR(64, IndexType::Unsigned, host_reg, PPC_REG, u32(PPCSTATE_OFF_PS1(preg))); } } diff --git a/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.h b/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.h index 55c86bd4ab..9000b0d28a 100644 --- a/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.h +++ b/Source/Core/Core/PowerPC/JitArm64/JitArm64_RegCache.h @@ -6,6 +6,7 @@ #include #include +#include #include #include "Common/Arm64Emitter.h" @@ -25,10 +26,21 @@ constexpr Arm64Gen::ARM64Reg DISPATCHER_PC = Arm64Gen::W26; #define PPCSTATE_OFF(elem) (offsetof(PowerPC::PowerPCState, elem)) +#define PPCSTATE_OFF_ARRAY(elem, i) \ + (offsetof(PowerPC::PowerPCState, elem[0]) + sizeof(PowerPC::PowerPCState::elem[0]) * (i)) + +#define PPCSTATE_OFF_GPR(i) PPCSTATE_OFF_ARRAY(gpr, i) +#define PPCSTATE_OFF_CR(i) PPCSTATE_OFF_ARRAY(cr.fields, i) +#define PPCSTATE_OFF_SR(i) PPCSTATE_OFF_ARRAY(sr, i) +#define PPCSTATE_OFF_SPR(i) PPCSTATE_OFF_ARRAY(spr, i) + +static_assert(std::is_same_v); +#define PPCSTATE_OFF_PS0(i) (PPCSTATE_OFF_ARRAY(ps, i) + offsetof(PowerPC::PairedSingle, ps0)) +#define PPCSTATE_OFF_PS1(i) (PPCSTATE_OFF_ARRAY(ps, i) + offsetof(PowerPC::PairedSingle, ps1)) + // Some asserts to make sure we will be able to load everything -static_assert(PPCSTATE_OFF(spr[1023]) <= 16380, "LDR(32bit) can't reach the last SPR"); -static_assert((PPCSTATE_OFF(ps[0].ps0) % 8) == 0, - "LDR(64bit VFP) requires FPRs to be 8 byte aligned"); +static_assert(PPCSTATE_OFF_SPR(1023) <= 16380, "LDR(32bit) can't reach the last SPR"); +static_assert((PPCSTATE_OFF_PS0(0) % 8) == 0, "LDR(64bit VFP) requires FPRs to be 8 byte aligned"); static_assert(PPCSTATE_OFF(xer_ca) < 4096, "STRB can't store xer_ca!"); static_assert(PPCSTATE_OFF(xer_so_ov) < 4096, "STRB can't store xer_so_ov!"); diff --git a/Source/Core/Core/PowerPC/JitArm64/JitArm64_SystemRegisters.cpp b/Source/Core/Core/PowerPC/JitArm64/JitArm64_SystemRegisters.cpp index ade2d650e4..3eacca7e72 100644 --- a/Source/Core/Core/PowerPC/JitArm64/JitArm64_SystemRegisters.cpp +++ b/Source/Core/Core/PowerPC/JitArm64/JitArm64_SystemRegisters.cpp @@ -111,7 +111,7 @@ void JitArm64::mfsr(UGeckoInstruction inst) JITDISABLE(bJITSystemRegistersOff); gpr.BindToRegister(inst.RD, false); - LDR(IndexType::Unsigned, gpr.R(inst.RD), PPC_REG, PPCSTATE_OFF(sr[inst.SR])); + LDR(IndexType::Unsigned, gpr.R(inst.RD), PPC_REG, PPCSTATE_OFF_SR(inst.SR)); } void JitArm64::mtsr(UGeckoInstruction inst) @@ -120,7 +120,7 @@ void JitArm64::mtsr(UGeckoInstruction inst) JITDISABLE(bJITSystemRegistersOff); gpr.BindToRegister(inst.RS, true); - STR(IndexType::Unsigned, gpr.R(inst.RS), PPC_REG, PPCSTATE_OFF(sr[inst.SR])); + STR(IndexType::Unsigned, gpr.R(inst.RS), PPC_REG, PPCSTATE_OFF_SR(inst.SR)); } void JitArm64::mfsrin(UGeckoInstruction inst) @@ -137,7 +137,7 @@ void JitArm64::mfsrin(UGeckoInstruction inst) UBFM(index, RB, 28, 31); ADD(index64, PPC_REG, index64, ArithOption(index64, ShiftType::LSL, 2)); - LDR(IndexType::Unsigned, gpr.R(d), index64, PPCSTATE_OFF(sr[0])); + LDR(IndexType::Unsigned, gpr.R(d), index64, PPCSTATE_OFF_SR(0)); gpr.Unlock(index); } @@ -156,7 +156,7 @@ void JitArm64::mtsrin(UGeckoInstruction inst) UBFM(index, RB, 28, 31); ADD(index64, PPC_REG, index64, ArithOption(index64, ShiftType::LSL, 2)); - STR(IndexType::Unsigned, gpr.R(d), index64, PPCSTATE_OFF(sr[0])); + STR(IndexType::Unsigned, gpr.R(d), index64, PPCSTATE_OFF_SR(0)); gpr.Unlock(index); } @@ -283,7 +283,7 @@ void JitArm64::mfspr(UGeckoInstruction inst) UMULH(Xresult, Xresult, XB); ADD(Xresult, XA, Xresult, ArithOption(Xresult, ShiftType::LSR, 3)); - STR(IndexType::Unsigned, Xresult, PPC_REG, PPCSTATE_OFF(spr[SPR_TL])); + STR(IndexType::Unsigned, Xresult, PPC_REG, PPCSTATE_OFF_SPR(SPR_TL)); if (CanMergeNextInstructions(1)) { @@ -344,7 +344,7 @@ void JitArm64::mfspr(UGeckoInstruction inst) default: gpr.BindToRegister(d, false); ARM64Reg RD = gpr.R(d); - LDR(IndexType::Unsigned, RD, PPC_REG, PPCSTATE_OFF(spr) + iIndex * 4); + LDR(IndexType::Unsigned, RD, PPC_REG, PPCSTATE_OFF_SPR(iIndex)); break; } } @@ -408,7 +408,7 @@ void JitArm64::mtspr(UGeckoInstruction inst) // OK, this is easy. ARM64Reg RD = gpr.R(inst.RD); - STR(IndexType::Unsigned, RD, PPC_REG, PPCSTATE_OFF(spr) + iIndex * 4); + STR(IndexType::Unsigned, RD, PPC_REG, PPCSTATE_OFF_SPR(iIndex)); } void JitArm64::crXXX(UGeckoInstruction inst)