Skip to content

Commit 0c6e7df

Browse files
committed
ppc64-QVM: debug-only emit-macro range checks + diagnostic data trap
Two debug-only additions (compiled out in release) and one correctness fix in the same vein as the BAND/BOR/BXOR fix in 05329ea. Range checks for D-form immediate fields: PPC_D silently truncates its immediate to 16 bits, so a caller passing a wider value gets a wrong instruction with no warning. Wrap the macros in PPC_IMM_S16 (ADDI/ADDIS/SUBFIC/MULLI/CMPWI; accepts the unsigned 16-bit bit pattern too, since callers like emit_MOVi64 pre-extract uint16 halves) and PPC_IMM_U16 (ANDI/ORI/ORIS/XORI/XORIS/CMPLWI; rejects upper-16-bits non-zero). On out-of-range, fires Com_Error at JIT-compile time naming the opcode. Diagnostic data-segment trap: emit_CheckReg now also takes (vm, qvm_ip) and, in debug builds, emits a CMPLD against rDATAMASK and calls FUNC_HIBITSET / ErrHighBitsSet(qvm_ip, bad_addr) on overflow. Release builds emit only the unconditional AND mask, same as before a8b2080. CONST_OPTIMIZE OP_LTU/LEU/GTU/GEU fit check: the prior shared (int16_t)v == v check let negative ci->value fold into PPC_CMPLWI, whose immediate is zero-extended -- so the JIT silently compared against the 16-bit-truncated value instead of the original. Split the fit check by signedness; same root cause as the BAND/BOR/BXOR case already fixed in 05329ea.
1 parent 05329ea commit 0c6e7df

1 file changed

Lines changed: 118 additions & 19 deletions

File tree

code/qcommon/vm_powerpc.c

Lines changed: 118 additions & 19 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,44 @@ 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 defence against the
254+
// signed/unsigned-16-bit immediate confusion class of bug.
255+
#ifndef NDEBUG
256+
// Signed 16-bit immediate: accept either the signed view
257+
// (INT16_MIN..INT16_MAX) or the unsigned bit-pattern (0..UINT16_MAX),
258+
// since callers sometimes pre-extract a uint16 half (e.g. emit_MOVi64).
259+
// The CPU's sign-extension means both views encode the same 16 bits.
260+
static inline uint32_t _ppc_chk_si16( int32_t v, const char *opname )
261+
{
262+
if ( v < INT16_MIN || v > UINT16_MAX ) {
263+
Com_Error( ERR_DROP,
264+
"JIT: %s signed-16 immediate out of range: 0x%x",
265+
opname, (uint32_t)v );
266+
}
267+
return (uint32_t)v;
268+
}
269+
// Unsigned 16-bit immediate (ANDI/ORI/XORI/CMPLWI etc.): the
270+
// instruction zero-extends to 32 bits, so anything with the upper
271+
// 16 bits non-zero is a sign-vs-unsigned bug in waiting.
272+
static inline uint32_t _ppc_chk_ui16( int32_t v, const char *opname )
273+
{
274+
if ( ( (uint32_t)v & 0xFFFF0000u ) != 0 ) {
275+
Com_Error( ERR_DROP,
276+
"JIT: %s unsigned-16 immediate out of range: 0x%x",
277+
opname, (uint32_t)v );
278+
}
279+
return (uint32_t)v;
280+
}
281+
#define PPC_IMM_S16(v, op) _ppc_chk_si16((v), (op))
282+
#define PPC_IMM_U16(v, op) _ppc_chk_ui16((v), (op))
283+
#else
284+
#define PPC_IMM_S16(v, op) ((uint32_t)(v))
285+
#define PPC_IMM_U16(v, op) ((uint32_t)(v))
286+
#endif
287+
243288
// ---- DS-form ----
244289
// ld, std, lwa
245290
// The DS field is 14 bits; actual byte displacement = DS << 2
@@ -332,17 +377,17 @@ static void VM_FreeBuffers( void )
332377

333378
// -- Arithmetic (D-form) --
334379
// addi rt, ra, si (li rt, si when ra=0)
335-
#define PPC_ADDI(rt, ra, si) PPC_D(14, rt, ra, si)
380+
#define PPC_ADDI(rt, ra, si) PPC_D(14, rt, ra, PPC_IMM_S16(si, "ADDI"))
336381
// addis rt, ra, si (lis rt, si when ra=0)
337-
#define PPC_ADDIS(rt, ra, si) PPC_D(15, rt, ra, si)
382+
#define PPC_ADDIS(rt, ra, si) PPC_D(15, rt, ra, PPC_IMM_S16(si, "ADDIS"))
338383
// li rt, si (pseudo for addi rt, 0, si)
339384
#define PPC_LI(rt, si) PPC_ADDI(rt, R0, si)
340385
// lis rt, si (pseudo for addis rt, 0, si)
341386
#define PPC_LIS(rt, si) PPC_ADDIS(rt, R0, si)
342387
// subfic rt, ra, si
343-
#define PPC_SUBFIC(rt, ra, si) PPC_D(8, rt, ra, si)
388+
#define PPC_SUBFIC(rt, ra, si) PPC_D(8, rt, ra, PPC_IMM_S16(si, "SUBFIC"))
344389
// mulli rt, ra, si
345-
#define PPC_MULLI(rt, ra, si) PPC_D(7, rt, ra, si)
390+
#define PPC_MULLI(rt, ra, si) PPC_D(7, rt, ra, PPC_IMM_S16(si, "MULLI"))
346391

347392
// -- Arithmetic (XO-form) --
348393
// add rt, ra, rb
@@ -380,15 +425,15 @@ static void VM_FreeBuffers( void )
380425
#define PPC_MR(ra, rs) PPC_OR(ra, rs, rs)
381426

382427
// andi. ra, rs, ui (always sets CR0)
383-
#define PPC_ANDI(ra, rs, ui) PPC_D(28, rs, ra, ui)
428+
#define PPC_ANDI(ra, rs, ui) PPC_D(28, rs, ra, PPC_IMM_U16(ui, "ANDI"))
384429
// ori ra, rs, ui
385-
#define PPC_ORI(ra, rs, ui) PPC_D(24, rs, ra, ui)
430+
#define PPC_ORI(ra, rs, ui) PPC_D(24, rs, ra, PPC_IMM_U16(ui, "ORI"))
386431
// oris ra, rs, ui
387-
#define PPC_ORIS(ra, rs, ui) PPC_D(25, rs, ra, ui)
432+
#define PPC_ORIS(ra, rs, ui) PPC_D(25, rs, ra, PPC_IMM_U16(ui, "ORIS"))
388433
// xori ra, rs, ui
389-
#define PPC_XORI(ra, rs, ui) PPC_D(26, rs, ra, ui)
434+
#define PPC_XORI(ra, rs, ui) PPC_D(26, rs, ra, PPC_IMM_U16(ui, "XORI"))
390435
// xoris ra, rs, ui
391-
#define PPC_XORIS(ra, rs, ui) PPC_D(27, rs, ra, ui)
436+
#define PPC_XORIS(ra, rs, ui) PPC_D(27, rs, ra, PPC_IMM_U16(ui, "XORIS"))
392437
// nop (ori 0, 0, 0)
393438
#define PPC_NOP() PPC_ORI(R0, R0, 0)
394439

@@ -418,9 +463,9 @@ static void VM_FreeBuffers( void )
418463
// -- Compare (D-form and X-form) --
419464
// cmpwi cr, ra, si (signed word compare immediate)
420465
// 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)
466+
#define PPC_CMPWI(cr, ra, si) PPC_D(11, ((cr)<<2), ra, PPC_IMM_S16(si, "CMPWI"))
422467
// cmplwi cr, ra, ui (unsigned word compare immediate)
423-
#define PPC_CMPLWI(cr, ra, ui) PPC_D(10, ((cr)<<2), ra, ui)
468+
#define PPC_CMPLWI(cr, ra, ui) PPC_D(10, ((cr)<<2), ra, PPC_IMM_U16(ui, "CMPLWI"))
424469
// cmpw cr, ra, rb (signed word compare)
425470
#define PPC_CMPW(cr, ra, rb) PPC_X(31, ((cr)<<2), ra, rb, 0, 0)
426471
// cmplw cr, ra, rb (unsigned word compare)
@@ -1048,6 +1093,20 @@ static void __attribute__((__noreturn__)) ErrBadOpStack( void )
10481093
}
10491094

10501095

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

@@ -1129,7 +1202,7 @@ static void emit_CheckProc( vm_t *vm, instruction_t *ins )
11291202
uint32_t n = ins->opStack;
11301203
mov_rx_imm32( R11, n );
11311204
emit( PPC_ADD( R11, rOPSTACK, R11 ) );
1132-
emit( PPC_CMPLD( 0, R11, rOPSTACKTOP ) ); //
1205+
emit( PPC_CMPLD( 0, R11, rOPSTACKTOP ) ); //
11331206

11341207
offset = branchOffset[FUNC_PSOF] - compiledOfs;
11351208
if ( (int16_t)offset == offset ) {
@@ -1572,8 +1645,24 @@ static qboolean ConstOptimize( vm_t* vm, instruction_t* ci, instruction_t* ni )
15721645
case OP_LEU:
15731646
case OP_GTU:
15741647
case OP_GEU:
1575-
if ( (int16_t)ci->value != ci->value )
1576-
return qfalse;
1648+
// PPC CMPWI takes signed 16-bit (sign-extended); CMPLWI takes
1649+
// UNSIGNED 16-bit (zero-extended). For unsigned comparisons
1650+
// we must reject negative ci->value (= upper 16 bits set in
1651+
// the 32-bit form) to avoid silently comparing against the
1652+
// 16-bit truncated value.
1653+
switch ( ni->op ) {
1654+
case OP_LTU:
1655+
case OP_LEU:
1656+
case OP_GTU:
1657+
case OP_GEU:
1658+
if ( ( (uint32_t)ci->value & 0xFFFF0000u ) != 0 )
1659+
return qfalse;
1660+
break;
1661+
default:
1662+
if ( (int16_t)ci->value != ci->value )
1663+
return qfalse;
1664+
break;
1665+
}
15771666
rx[0] = load_rx_opstack( R3 | RCONST ); dec_opstack(); // r3 = *opstack; opstack -= 4
15781667
switch ( ni->op ) {
15791668
case OP_LTU:
@@ -1765,6 +1854,11 @@ qboolean VM_Compile( vm_t *vm, vmHeader_t *header )
17651854
emitFuncBranch( vm, FUNC_OUTJ );
17661855
emitFuncBranch( vm, FUNC_BADJ );
17671856
}
1857+
#ifndef NDEBUG
1858+
// FUNC_HIBITSET is emitted unconditionally in debug builds so that
1859+
// emit_CheckReg's diagnostic trap can reach it via a short bl.
1860+
emitFuncBranch( vm, FUNC_HIBITSET );
1861+
#endif
17681862

17691863
saveBranchOffsets();
17701864

@@ -2053,7 +2147,7 @@ qboolean VM_Compile( vm_t *vm, vmHeader_t *header )
20532147
} else {
20542148
// address specified by a register
20552149
rx[0] = load_rx_opstack( R4 ); // R4 = *opStack
2056-
emit_CheckReg( rx[0] ); // R4 &= dataMask
2150+
emit_CheckReg( vm, rx[0], ip ); // R4 &= dataMask
20572151
sx[0] = alloc_sx( F0 );
20582152
emit( PPC_LFSX( sx[0], rx[0], rDATABASE ) ); // F0 = dataBase[R4]
20592153
store_sx_opstack( sx[0] ); // *opStack = F0
@@ -2117,7 +2211,7 @@ qboolean VM_Compile( vm_t *vm, vmHeader_t *header )
21172211
// the address into a writable register (no RCONST) and
21182212
// reuse it as the load destination.
21192213
rx[0] = rx[1] = load_rx_opstack( R3 ); // target = address = *opStack
2120-
emit_CheckReg( rx[1] ); // R3 &= dataMask
2214+
emit_CheckReg( vm, rx[1], ip ); // R3 &= dataMask
21212215
switch ( ci->op ) {
21222216
case OP_LOAD1: emit( PPC_LBZX( rx[0], rx[1], rDATABASE ) ); set_rx_ext( rx[0], Z_EXT8 ); break; // R3 = dataBase[R4] (byte)
21232217
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 +2250,7 @@ qboolean VM_Compile( vm_t *vm, vmHeader_t *header )
21562250
// address specified by register
21572251
rx[0] = load_rx_opstack( R4 ); // R4 = *opStack
21582252
dec_opstack(); // opStack -= 4
2159-
emit_CheckReg( rx[0] ); // R4 &= dataMask
2253+
emit_CheckReg( vm, rx[0], ip ); // R4 &= dataMask
21602254
emit( PPC_STFSX( sx[0], rx[0], rDATABASE ) ); // dataBase[R4] = F0
21612255
unmask_rx( rx[0] );
21622256
wipe_vars(); // unknown/dynamic address, wipe all register mappings
@@ -2192,7 +2286,7 @@ qboolean VM_Compile( vm_t *vm, vmHeader_t *header )
21922286
// address specified by register
21932287
rx[1] = load_rx_opstack( R4 ); // R4 = *opStack
21942288
dec_opstack(); // opStack -= 4
2195-
emit_CheckReg( rx[1] ); // R4 &= dataMask
2289+
emit_CheckReg( vm, rx[1], ip ); // R4 &= dataMask
21962290
switch ( ci->op ) {
21972291
case OP_STORE1: emit( PPC_STBX( rx[0], rx[1], rDATABASE ) ); break; // (byte) dataBase[R4] = R3
21982292
case OP_STORE2: emit( PPC_STHX( rx[0], rx[1], rDATABASE ) ); break; // (short) dataBase[R4] = R3
@@ -2443,6 +2537,11 @@ qboolean VM_Compile( vm_t *vm, vmHeader_t *header )
24432537
funcOffset[ FUNC_PSOF ] = compiledOfs;
24442538
emitFuncEntry( ErrBadProgramStack );
24452539

2540+
#ifndef NDEBUG
2541+
funcOffset[ FUNC_HIBITSET ] = compiledOfs;
2542+
emitFuncEntry( ErrHighBitsSet );
2543+
#endif
2544+
24462545
} // pass
24472546

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

0 commit comments

Comments
 (0)