Skip to content

Commit a24ab5e

Browse files
committed
ppc64-QVM: branch-class correctness fixes + debug instrumentation
Two layers of changes to the PPC64 JIT ==== Correctness ==== The JIT keeps opstack slots as TYPE_CONST/TYPE_LOCAL compile-time metadata when possible — no register write, no memory write — and materialises them lazily. At branch sources, this leaves the branch-taken path without the values that the linear fall-through path would eventually have written, so the target reads stale or zero memory. Fix at four sites: - Conditional cmp branches (OP_EQ/NE/LTI/LEI/GTI/GEI/LTU..GEU): flush_opstack() before the CMP+branch in both the ConstOptimize folded path and the general two-operand path. Cmp operand register(s) are kept masked through the flush so they can't be reused for a const-store materialisation. - Float-cmp branches (OP_EQF/NEF/LTF/LEF/GTF/GEF): same fix at the general cmp path. FPRs kept masked through the flush. - OP_JUMP (unconditional indirect): same fix at both the ConstOptimize folded path and the general indirect path. - ConstOptimize sign-vs-unsigned imm bug: the BAND/BOR/BXOR fix on master (origin/refactoring) used (int16_t)v==v as a shared fit check for both signed CMPWI and unsigned CMPLWI cmps. CMPLWI's immediate is zero-extended, so negative values fold into the wrong unsigned comparison. Split the fit check by signedness: signed cmps keep (int16_t)v==v; unsigned cmps reject any value with upper 16 bits non-zero. ==== Tier-1 emit-macro range asserts ==== PPC_D silently truncates immediates to 16 bits, so a caller passing a wider value gets a wrong instruction with no warning. Wrap the D-form macros in Debug-only range checks (compiled out in release): PPC_IMM_S16 (accepts [INT16_MIN, UINT16_MAX] — relaxed upper bound allows callers like emit_MOVi64 that pre-extract uint16 halves; the CPU's sign-extension makes both views equivalent): PPC_ADDI, PPC_ADDIS, PPC_MULLI, PPC_SUBFIC, PPC_LI, PPC_LIS, PPC_CMPWI PPC_IMM_U16 (accepts [0, UINT16_MAX]): PPC_ANDI, PPC_ORI, PPC_ORIS, PPC_XORI, PPC_XORIS, PPC_CMPLWI Verified by injection: reintroducing the BAND/BOR/BXOR regression now fires Com_Error: JIT: ORI unsigned-16 immediate out of range: 0xfffffff1 at JIT-compile time, instead of silently producing wrong arithmetic. ==== Debug instrumentation ==== Adds FUNC_HIBITSET — an #ifndef NDEBUG diagnostic entry that fires when emit_CheckReg sees a load/store address with any bit set above dataMask. ErrHighBitsSet(qvm_ip, bad_addr) Com_Errors with both values baked into R3/R4 before the bl, so the offending QVM op can be identified from the log alone. Release builds compile this out completely; the JIT emits only the unconditional AND mask. Used to diagnose the 2026-05-18 q3ut4 cgame data-mask UB.
1 parent 05329ea commit a24ab5e

1 file changed

Lines changed: 141 additions & 28 deletions

File tree

code/qcommon/vm_powerpc.c

Lines changed: 141 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -184,8 +184,15 @@ typedef enum
184184
FUNC_OSOF,
185185
FUNC_BADJ,
186186
FUNC_OUTJ,
187+
#ifndef NDEBUG
188+
FUNC_HIBITSET,
189+
#endif
187190
FUNC_ERR_BEGIN = FUNC_PSOF,
191+
#ifndef NDEBUG
192+
FUNC_ERR_END = FUNC_HIBITSET,
193+
#else
188194
FUNC_ERR_END = FUNC_OUTJ,
195+
#endif
189196
OFFSET_T_LAST
190197
} offset_t;
191198

@@ -240,6 +247,46 @@ static void VM_FreeBuffers( void )
240247
( (((unsigned)(op)&0x3F)<<26) | (((unsigned)(rt)&0x1F)<<21) | \
241248
(((unsigned)(ra)&0x1F)<<16) | ((unsigned)(d)&0xFFFF) )
242249

250+
// Debug-only range checks for D-form immediate fields. The encoding
251+
// silently truncates to 16 bits; these helpers turn out-of-range
252+
// immediates into a loud Com_Error rather than a wrong instruction
253+
// at JIT-compile time. Useful as a tier-1 defence against the class
254+
// of bug that the 2026-05-18 CONST_OPTIMIZE BAND/BOR/BXOR fix
255+
// addressed: signed/unsigned 16-bit immediate confusion.
256+
#ifndef NDEBUG
257+
// Signed 16-bit immediate: accept either the signed view
258+
// (INT16_MIN..INT16_MAX) or the unsigned bit-pattern (0..UINT16_MAX),
259+
// since callers sometimes pre-extract a uint16 half (e.g. emit_MOVi64).
260+
// The CPU's sign-extension means both views encode the same 16 bits.
261+
static inline uint32_t _ppc_chk_si16( int32_t v, const char *opname )
262+
{
263+
if ( v < INT16_MIN || v > UINT16_MAX ) {
264+
Com_Error( ERR_DROP,
265+
"JIT: %s signed-16 immediate out of range: 0x%x",
266+
opname, (uint32_t)v );
267+
}
268+
return (uint32_t)v;
269+
}
270+
// Unsigned 16-bit immediate (ANDI/ORI/XORI/CMPLWI etc.): the
271+
// instruction zero-extends to 32 bits, so anything with the upper
272+
// 16 bits non-zero is a sign-vs-unsigned bug in waiting (the 2026-
273+
// 05-18 BAND/BOR/BXOR fix class).
274+
static inline uint32_t _ppc_chk_ui16( int32_t v, const char *opname )
275+
{
276+
if ( ( (uint32_t)v & 0xFFFF0000u ) != 0 ) {
277+
Com_Error( ERR_DROP,
278+
"JIT: %s unsigned-16 immediate out of range: 0x%x",
279+
opname, (uint32_t)v );
280+
}
281+
return (uint32_t)v;
282+
}
283+
#define PPC_IMM_S16(v, op) _ppc_chk_si16((v), (op))
284+
#define PPC_IMM_U16(v, op) _ppc_chk_ui16((v), (op))
285+
#else
286+
#define PPC_IMM_S16(v, op) ((uint32_t)(v))
287+
#define PPC_IMM_U16(v, op) ((uint32_t)(v))
288+
#endif
289+
243290
// ---- DS-form ----
244291
// ld, std, lwa
245292
// The DS field is 14 bits; actual byte displacement = DS << 2
@@ -332,17 +379,17 @@ static void VM_FreeBuffers( void )
332379

333380
// -- Arithmetic (D-form) --
334381
// addi rt, ra, si (li rt, si when ra=0)
335-
#define PPC_ADDI(rt, ra, si) PPC_D(14, rt, ra, si)
382+
#define PPC_ADDI(rt, ra, si) PPC_D(14, rt, ra, PPC_IMM_S16(si, "ADDI"))
336383
// addis rt, ra, si (lis rt, si when ra=0)
337-
#define PPC_ADDIS(rt, ra, si) PPC_D(15, rt, ra, si)
384+
#define PPC_ADDIS(rt, ra, si) PPC_D(15, rt, ra, PPC_IMM_S16(si, "ADDIS"))
338385
// li rt, si (pseudo for addi rt, 0, si)
339386
#define PPC_LI(rt, si) PPC_ADDI(rt, R0, si)
340387
// lis rt, si (pseudo for addis rt, 0, si)
341388
#define PPC_LIS(rt, si) PPC_ADDIS(rt, R0, si)
342389
// subfic rt, ra, si
343-
#define PPC_SUBFIC(rt, ra, si) PPC_D(8, rt, ra, si)
390+
#define PPC_SUBFIC(rt, ra, si) PPC_D(8, rt, ra, PPC_IMM_S16(si, "SUBFIC"))
344391
// mulli rt, ra, si
345-
#define PPC_MULLI(rt, ra, si) PPC_D(7, rt, ra, si)
392+
#define PPC_MULLI(rt, ra, si) PPC_D(7, rt, ra, PPC_IMM_S16(si, "MULLI"))
346393

347394
// -- Arithmetic (XO-form) --
348395
// add rt, ra, rb
@@ -380,15 +427,15 @@ static void VM_FreeBuffers( void )
380427
#define PPC_MR(ra, rs) PPC_OR(ra, rs, rs)
381428

382429
// andi. ra, rs, ui (always sets CR0)
383-
#define PPC_ANDI(ra, rs, ui) PPC_D(28, rs, ra, ui)
430+
#define PPC_ANDI(ra, rs, ui) PPC_D(28, rs, ra, PPC_IMM_U16(ui, "ANDI"))
384431
// ori ra, rs, ui
385-
#define PPC_ORI(ra, rs, ui) PPC_D(24, rs, ra, ui)
432+
#define PPC_ORI(ra, rs, ui) PPC_D(24, rs, ra, PPC_IMM_U16(ui, "ORI"))
386433
// oris ra, rs, ui
387-
#define PPC_ORIS(ra, rs, ui) PPC_D(25, rs, ra, ui)
434+
#define PPC_ORIS(ra, rs, ui) PPC_D(25, rs, ra, PPC_IMM_U16(ui, "ORIS"))
388435
// xori ra, rs, ui
389-
#define PPC_XORI(ra, rs, ui) PPC_D(26, rs, ra, ui)
436+
#define PPC_XORI(ra, rs, ui) PPC_D(26, rs, ra, PPC_IMM_U16(ui, "XORI"))
390437
// xoris ra, rs, ui
391-
#define PPC_XORIS(ra, rs, ui) PPC_D(27, rs, ra, ui)
438+
#define PPC_XORIS(ra, rs, ui) PPC_D(27, rs, ra, PPC_IMM_U16(ui, "XORIS"))
392439
// nop (ori 0, 0, 0)
393440
#define PPC_NOP() PPC_ORI(R0, R0, 0)
394441

@@ -418,9 +465,9 @@ static void VM_FreeBuffers( void )
418465
// -- Compare (D-form and X-form) --
419466
// cmpwi cr, ra, si (signed word compare immediate)
420467
// L=0 for 32-bit compare, cr field in bits 21-23
421-
#define PPC_CMPWI(cr, ra, si) PPC_D(11, ((cr)<<2), ra, si)
468+
#define PPC_CMPWI(cr, ra, si) PPC_D(11, ((cr)<<2), ra, PPC_IMM_S16(si, "CMPWI"))
422469
// cmplwi cr, ra, ui (unsigned word compare immediate)
423-
#define PPC_CMPLWI(cr, ra, ui) PPC_D(10, ((cr)<<2), ra, ui)
470+
#define PPC_CMPLWI(cr, ra, ui) PPC_D(10, ((cr)<<2), ra, PPC_IMM_U16(ui, "CMPLWI"))
424471
// cmpw cr, ra, rb (signed word compare)
425472
#define PPC_CMPW(cr, ra, rb) PPC_X(31, ((cr)<<2), ra, rb, 0, 0)
426473
// cmplw cr, ra, rb (unsigned word compare)
@@ -1048,6 +1095,20 @@ static void __attribute__((__noreturn__)) ErrBadOpStack( void )
10481095
}
10491096

10501097

1098+
#ifndef NDEBUG
1099+
// Diagnostic: fired when emit_CheckReg sees a data-segment address whose
1100+
// 64-bit value exceeds dataMask. The JIT loads the offending QVM ip into
1101+
// R3 and the unmasked address into R4 before calling this stub, so we
1102+
// can identify the buggy op directly from the error message.
1103+
static void __attribute__((__noreturn__)) ErrHighBitsSet( int qvm_ip, uint64_t bad_addr )
1104+
{
1105+
Com_Error( ERR_DROP,
1106+
"JIT: data address overflowed dataMask: ip=%d (0x%x) bad_addr=0x%llx",
1107+
qvm_ip, qvm_ip, (unsigned long long)bad_addr );
1108+
}
1109+
#endif
1110+
1111+
10511112
// =========================================================================
10521113
// Runtime check emission
10531114
// =========================================================================
@@ -1057,8 +1118,22 @@ static void __attribute__((__noreturn__)) ErrBadOpStack( void )
10571118
// interpreter's 32-bit wrap semantics and side-effect-clears any high
10581119
// bits left by a 64-bit ADD overflow (e.g. lwzx-loaded 0xFFFFFFFF +
10591120
// positive constant), which the prior cmplw/bgt check could not detect.
1060-
static void emit_CheckReg( int reg )
1121+
static void emit_CheckReg( vm_t *vm, int reg, int qvm_ip )
10611122
{
1123+
#ifndef NDEBUG
1124+
// Diagnostic trap: 64-bit unsigned compare reg against dataMask.
1125+
// On fail, hand ErrHighBitsSet(qvm_ip, bad_addr) via R3,R4 so the
1126+
// error message names the offending QVM op directly.
1127+
emit( PPC_CMPLD( 0, reg, rDATAMASK ) );
1128+
emit( PPC_BLE( +20 ) ); // skip 4 insns
1129+
emit( PPC_MR( R4, reg ) ); // R4 = bad addr (full 64-bit)
1130+
emit( PPC_LIS( R3, ( qvm_ip >> 16 ) & 0xFFFF ) ); // R3 = ip hi
1131+
emit( PPC_ORI( R3, R3, qvm_ip & 0xFFFF ) ); // R3 |= ip lo
1132+
emitFuncBranch( vm, FUNC_HIBITSET );
1133+
#else
1134+
(void)vm;
1135+
(void)qvm_ip;
1136+
#endif
10621137
emit( PPC_AND( reg, reg, rDATAMASK ) );
10631138
}
10641139

@@ -1522,7 +1597,7 @@ static qboolean ConstOptimize( vm_t* vm, instruction_t* ci, instruction_t* ni )
15221597

15231598

15241599
case OP_JUMP:
1525-
flush_volatile();
1600+
flush_opstack(); // materialise cached opstack before unconditional jump
15261601
emit( PPC_B( vm->instructionPointers[ci->value] - compiledOfs ) );
15271602
ip += 1; // OP_JUMP
15281603
return qtrue;
@@ -1572,9 +1647,28 @@ static qboolean ConstOptimize( vm_t* vm, instruction_t* ci, instruction_t* ni )
15721647
case OP_LEU:
15731648
case OP_GTU:
15741649
case OP_GEU:
1575-
if ( (int16_t)ci->value != ci->value )
1576-
return qfalse;
1650+
// PPC CMPWI takes signed 16-bit (sign-extended); CMPLWI takes
1651+
// UNSIGNED 16-bit (zero-extended). For unsigned comparisons
1652+
// we must reject negative ci->value (= upper 16 bits set in
1653+
// the 32-bit form) to avoid silently comparing against the
1654+
// 16-bit truncated value.
1655+
switch ( ni->op ) {
1656+
case OP_LTU:
1657+
case OP_LEU:
1658+
case OP_GTU:
1659+
case OP_GEU:
1660+
if ( ( (uint32_t)ci->value & 0xFFFF0000u ) != 0 )
1661+
return qfalse;
1662+
break;
1663+
default:
1664+
if ( (int16_t)ci->value != ci->value )
1665+
return qfalse;
1666+
break;
1667+
}
15771668
rx[0] = load_rx_opstack( R3 | RCONST ); dec_opstack(); // r3 = *opstack; opstack -= 4
1669+
// keep rx[0] masked through flush_opstack so a TYPE_CONST
1670+
// materialisation can't clobber the CMP's operand register
1671+
flush_opstack(); // materialise cached opstack before conditional branch
15781672
switch ( ni->op ) {
15791673
case OP_LTU:
15801674
case OP_LEU:
@@ -1765,6 +1859,11 @@ qboolean VM_Compile( vm_t *vm, vmHeader_t *header )
17651859
emitFuncBranch( vm, FUNC_OUTJ );
17661860
emitFuncBranch( vm, FUNC_BADJ );
17671861
}
1862+
#ifndef NDEBUG
1863+
// FUNC_HIBITSET is emitted unconditionally in debug builds so that
1864+
// emit_CheckReg's diagnostic trap can reach it via a short bl.
1865+
emitFuncBranch( vm, FUNC_HIBITSET );
1866+
#endif
17681867

17691868
saveBranchOffsets();
17701869

@@ -1961,7 +2060,7 @@ qboolean VM_Compile( vm_t *vm, vmHeader_t *header )
19612060
case OP_JUMP:
19622061
// indirect jump: target = *opstack
19632062
rx[0] = load_rx_opstack( R3 | RCONST ); dec_opstack(); // r3 = *opstack; opstack -= 4
1964-
flush_volatile();
2063+
flush_opstack(); // materialise cached opstack before unconditional jump
19652064
emit_CheckJump( vm, rx[0], proc_base, proc_len ); // check if r3 is within current proc
19662065
// Load target address from instructionPointers[R3]
19672066
// R11 = R3 << 3
@@ -1984,11 +2083,17 @@ qboolean VM_Compile( vm_t *vm, vmHeader_t *header )
19842083
case OP_LEU:
19852084
case OP_GTU:
19862085
case OP_GEU:
1987-
// pop two, compare, branch
2086+
// pop two, compare, branch. Spill ALL cached opstack slots
2087+
// to memory BEFORE the cmp+branch so the branch-taken path
2088+
// arrives at the target with the same memory state the
2089+
// linear fall-through would have written (the JIT cannot
2090+
// reconcile compile-time TYPE_CONST/TYPE_LOCAL metadata
2091+
// across the in-edges of a join point).
19882092
rx[0] = load_rx_opstack( R4 | RCONST ); dec_opstack(); // r4 = *opstack; opstack -= 4
19892093
rx[1] = load_rx_opstack( R3 | RCONST ); dec_opstack(); // r3 = *opstack; opstack -= 4
1990-
unmask_rx( rx[0] );
1991-
unmask_rx( rx[1] );
2094+
// keep rx[0]/rx[1] masked through flush_opstack so the
2095+
// const-store allocator can't clobber the CMP's operands
2096+
flush_opstack(); // materialise all cached opstack slots
19922097
switch ( ci->op ) {
19932098
case OP_LTU:
19942099
case OP_LEU:
@@ -1999,6 +2104,8 @@ qboolean VM_Compile( vm_t *vm, vmHeader_t *header )
19992104
emit( PPC_CMPW( 0, rx[1], rx[0] ) ); break;
20002105
}
20012106
emit_branchConditionalShort( vm, ci );
2107+
unmask_rx( rx[0] );
2108+
unmask_rx( rx[1] );
20022109
break;
20032110

20042111
// ---- Float comparisons ----
@@ -2008,14 +2115,15 @@ qboolean VM_Compile( vm_t *vm, vmHeader_t *header )
20082115
case OP_LEF:
20092116
case OP_GTF:
20102117
case OP_GEF:
2011-
// pop two floats from opstack, compare, branch
2012-
// Load as 32-bit words into FPRs via memory
2013-
// We use lfs which loads a single-precision float and converts to double in the FPR
2014-
// opstack values are stored as 32-bit IEEE 754 floats
2118+
// pop two floats from opstack, compare, branch. Same
2119+
// cached-state-at-branch reconciliation issue as the
2120+
// int cmps — flush_opstack before the branch. Keep
2121+
// sx[0]/sx[1] masked through the flush so they don't
2122+
// get reused by const-store materialisations.
20152123
sx[1] = load_sx_opstack( F1 | RCONST ); dec_opstack(); // F1 = *opstack; opstack -= 4
20162124
sx[0] = load_sx_opstack( F0 | RCONST ); dec_opstack(); // F0 = *opstack; opstack -= 4
2125+
flush_opstack();
20172126
emit( PPC_FCMPU( 0, sx[0], sx[1] ) );
2018-
// emit_branchConditional( vm, ci, ci->op );
20192127
emit_branchConditionalShort( vm, ci );
20202128
unmask_sx( sx[1] );
20212129
unmask_sx( sx[0] );
@@ -2053,7 +2161,7 @@ qboolean VM_Compile( vm_t *vm, vmHeader_t *header )
20532161
} else {
20542162
// address specified by a register
20552163
rx[0] = load_rx_opstack( R4 ); // R4 = *opStack
2056-
emit_CheckReg( rx[0] ); // R4 &= dataMask
2164+
emit_CheckReg( vm, rx[0], ip ); // R4 &= dataMask
20572165
sx[0] = alloc_sx( F0 );
20582166
emit( PPC_LFSX( sx[0], rx[0], rDATABASE ) ); // F0 = dataBase[R4]
20592167
store_sx_opstack( sx[0] ); // *opStack = F0
@@ -2117,7 +2225,7 @@ qboolean VM_Compile( vm_t *vm, vmHeader_t *header )
21172225
// the address into a writable register (no RCONST) and
21182226
// reuse it as the load destination.
21192227
rx[0] = rx[1] = load_rx_opstack( R3 ); // target = address = *opStack
2120-
emit_CheckReg( rx[1] ); // R3 &= dataMask
2228+
emit_CheckReg( vm, rx[1], ip ); // R3 &= dataMask
21212229
switch ( ci->op ) {
21222230
case OP_LOAD1: emit( PPC_LBZX( rx[0], rx[1], rDATABASE ) ); set_rx_ext( rx[0], Z_EXT8 ); break; // R3 = dataBase[R4] (byte)
21232231
case OP_LOAD2: emit( PPC_LHZX( rx[0], rx[1], rDATABASE ) ); set_rx_ext( rx[0], Z_EXT16 ); break; // R3 = dataBase[R4] (halfword)
@@ -2156,7 +2264,7 @@ qboolean VM_Compile( vm_t *vm, vmHeader_t *header )
21562264
// address specified by register
21572265
rx[0] = load_rx_opstack( R4 ); // R4 = *opStack
21582266
dec_opstack(); // opStack -= 4
2159-
emit_CheckReg( rx[0] ); // R4 &= dataMask
2267+
emit_CheckReg( vm, rx[0], ip ); // R4 &= dataMask
21602268
emit( PPC_STFSX( sx[0], rx[0], rDATABASE ) ); // dataBase[R4] = F0
21612269
unmask_rx( rx[0] );
21622270
wipe_vars(); // unknown/dynamic address, wipe all register mappings
@@ -2192,7 +2300,7 @@ qboolean VM_Compile( vm_t *vm, vmHeader_t *header )
21922300
// address specified by register
21932301
rx[1] = load_rx_opstack( R4 ); // R4 = *opStack
21942302
dec_opstack(); // opStack -= 4
2195-
emit_CheckReg( rx[1] ); // R4 &= dataMask
2303+
emit_CheckReg( vm, rx[1], ip ); // R4 &= dataMask
21962304
switch ( ci->op ) {
21972305
case OP_STORE1: emit( PPC_STBX( rx[0], rx[1], rDATABASE ) ); break; // (byte) dataBase[R4] = R3
21982306
case OP_STORE2: emit( PPC_STHX( rx[0], rx[1], rDATABASE ) ); break; // (short) dataBase[R4] = R3
@@ -2443,6 +2551,11 @@ qboolean VM_Compile( vm_t *vm, vmHeader_t *header )
24432551
funcOffset[ FUNC_PSOF ] = compiledOfs;
24442552
emitFuncEntry( ErrBadProgramStack );
24452553

2554+
#ifndef NDEBUG
2555+
funcOffset[ FUNC_HIBITSET ] = compiledOfs;
2556+
emitFuncEntry( ErrHighBitsSet );
2557+
#endif
2558+
24462559
} // pass
24472560

24482561
if ( vm->codeBase.ptr == NULL ) {

0 commit comments

Comments
 (0)