admin/ci-libretro-fceumm
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

core: strict-aliasing fixes, bounded string ops, drop FCEUDEF_DEBUGGER

Browse Source 
Three follow-up cleanups on top of the determinism / typedef / LE
work in f2a5be3.

================================================================
Pass 1: fix strict-aliasing UB in PPU sprite buffer and FCEU_dwmemset
================================================================

Three sites were doing type-punning through uint32_t* casts that
GCC flags with -Wstrict-aliasing=2:

1. ppu.c FetchSpriteData (two near-identical sites). After populating
   a local SPRB struct (4 bytes: ca[2], atr, x), the prior code did
   '*(uint32_t*)&SPRBUF[ns << 2] = *(uint32_t*)&dst' to store the
   struct as one 32-bit word. This violates strict-aliasing (uint8_t
   buffer accessed as uint32_t, struct accessed as uint32_t) and
   silently assumes 4-byte alignment of SPRBUF + (ns << 2). Use
   memcpy instead - GCC and Clang lower it to the same single
   32-bit store.

2. fceu-memory.h FCEU_dwmemset macro. Same pattern every iteration:
   '*(uint32_t*)& (d)[_x] = c'. Replaced with memcpy(... &v, 4) in
   the macro body. Same generated code, now alias- and align-clean.
   Added #include <string.h> to fceu-memory.h itself (4 callers
   relied on transitive includes for memcpy).

================================================================
Pass 2: bounded string operations
================================================================

Sweep across the core and libretro driver to replace every unbounded
string function with its size-aware counterpart:

  strcpy  -> strlcpy  (with explicit destination size)
  strncpy -> strlcpy  (silent truncation -> guaranteed NUL termination
                       and known-size semantics; behaviourally identical
                       at every call site here because callers either
                       pre-zeroed the buffer or treated truncation as
                       a guaranteed terminator)
  strcat  -> strlcpy at offset (track 'pos' across appends to avoid
                       rescanning the buffer with strlen and to keep
                       the bound explicit at every step)
  sprintf -> snprintf (with sizeof(buf) as the bound)

The compat/strl.h header from libretro-common is on the include path
for every translation unit; the libretro driver files were already
using strlcpy via stdstring.h transitive include. Core files (ines.c,
unif.c, cheat.c, nsf.c, state.c, general.c) gain explicit
'#include <compat/strl.h>'.

Per-file changes:

  general.c
    - FCEUI_SetBaseDirectory: strncpy + manual NUL -> strlcpy
    - FCEU_MakeFName: malloc(strlen+1) + strcpy -> sized strlcpy,
      malloc NULL-check added (was unchecked).

  ines.c
    - 'gigastr' iNES-header-warning building.
      Pre-existing bug fixed: every sprintf(gigastr + gigastr_len, ...)
      wrote at the SAME offset captured once at the top of the block,
      so when multiple 'tofix' bits were set, only the LAST fragment
      survived (each sprintf clobbered the previous). Replaced with a
      running 'pos' offset across snprintf and strlcpy-at-offset calls.
    - 6 sprintf -> snprintf, 3 strcat -> strlcpy at offset, 1 strcpy
      -> strlcpy.

  unif.c
    - GameInfo->name allocation: strcpy -> strlcpy with explicit size.

  cheat.c
    - FCEUI_AddCheat: malloc + strcpy -> sized strlcpy.
    - FCEUI_SetCheat: realloc + strcpy -> sized strlcpy.

  nsf.c
    - FCEUI_NSFGetInfo: 3x strncpy -> strlcpy.
    - Visualiser snbuf sprintf -> snprintf (the 16-byte buffer was
      technically overflowable for very large song counts).

  state.c
    - AddExState description copy: strncpy -> strlcpy. Preserves
      4-byte SFORMAT-tag semantics.

  fds.c
    - Disk-tag formatting: sprintf "DDT%d" -> snprintf.

  boards/__serial.c
    - 2 sprintf -> snprintf (Windows-only SerialOpen path).

  drivers/libretro/libretro.c
    - retro_cheat_set: sprintf "N/A" + strcpy literal -> strlcpy.

  drivers/libretro/libretro_dipswitch.c
    - VS-DIP key building: sprintf -> snprintf.
    - core_key calloc + strcpy -> sized strlcpy. calloc NULL-check
      added (was unchecked - dereferencing NULL on OOM).

  drivers/libretro/libretro_core_options.h
    - values_buf assembly: single strcpy + N strcat replaced with
      strlcpy + strlcpy-at-offset using a running 'pos'. Each step
      bounded by remaining buffer space.

================================================================
Pass 3: remove FCEUDEF_DEBUGGER and the unused debugger scaffolding
================================================================

The FCEUDEF_DEBUGGER macro was never defined for libretro builds, so
every block guarded by it was dead code. Removing the macro takes
out:

  - src/debug.c (FCEUI_DumpMem, FCEUI_DumpVid, FCEUI_LoadMem,
    FCEUI_Disassemble, FCEUI_MemDump, FCEUI_MemSafePeek,
    FCEUI_MemPoke, breakpoint set/get/list, FCEUI_SetCPUCallback).
    Not in Makefile.common SOURCES_C, never compiled. None of the
    declared functions had any caller in any compiled .c file.

  - src/debug.h (declarations for the above).

  - x6502.c X6502_RunDebug. The dual-implementation pattern with a
    function pointer that switched between RunNormal and RunDebug
    is now a single direct X6502_Run.

  - x6502.c X6502_Debug, FCEUI_NMI, FCEUI_IRQ, FCEUI_GetIVectors.
    Set/get debugger hooks. No callers.

  - x6502.c RdMemHook, WrMemHook, XSave, debugmode. Hook scaffolding
    used only by RunDebug.

  - x6502struct.h X6502 fields preexec, CPUHook, ReadHook, WriteHook.
    Set only inside RunDebug, never read elsewhere.

  - fceuindbg variable. Set to 1 only in FCEUI_GetIVectors and
    X6502_RunDebug (both removed); was always 0 elsewhere, so every
    'if (!fceuindbg)' check was a no-op. Removed both the variable
    (defined in ppu.c, declared in fceu.h) and every check site
    across sound.c, input.c, fds.c, nsf.c, ppu.c, mmc5.c, n106.c,
    BMW8544.c, and the input drivers (arkanoid, mahjong, mouse,
    pec586kb, powerpad, zapper).

  - All FCEUDEF_DEBUGGER conditional declarations from driver.h.

The 'if (!fceuindbg)' checks gated side-effects (joypad-bit-counter
increments, PPU register reads, etc.) so a future debugger could peek
at memory without advancing the emulator state. With the debugger
gone, those side-effects are now unconditional - which is what they
should have been anyway in a libretro build.

If a future developer needs a debugger they should resurrect this
out of git history into a separate debugger-frontend project rather
than re-introducing a build-time toggle that nothing in the libretro
build can ever exercise.

================================================================
Build status
================================================================

Build clean on `make platform=unix` with zero errors and zero
warnings. Output binary 184 bytes smaller than upstream f2a5be3
(4,388,840 -> 4,388,656). 31 files changed, 127 insertions, 863
deletions.
This commit is contained in:
U-DESKTOP-SPFP6AQ\twistedtechre
2026-05-04 03:23:56 +02:00
parent f2a5be381d
commit 004c147d32
31 changed files with 130 additions and 863 deletions
Download Patch File Download Diff File Expand all files Collapse all files

181
src/x6502.c
View File

@@ -29,10 +29,6 @@
X6502 X;
uint8_t encryptOpcodes =0;
#ifdef FCEUDEF_DEBUGGER
void (*X6502_Run)(int32_t cycles);
#endif
uint32_t timestamp;
uint32_t sound_timestamp;
void FP_FASTAPASS(1) (*MapIRQHook)(int a);
@@ -66,24 +62,6 @@ static INLINE void WrMemNorm(uint32_t A, uint8_t V) {
BWrite[A](A, V);
}
#ifdef FCEUDEF_DEBUGGER
X6502 XSave; /* This is getting ugly. */
static INLINE uint8_t RdMemHook(uint32_t A) {
if (X.ReadHook)
return(_DB = X.ReadHook(&X, A));
else
return(_DB = ARead[A](A));
}
static INLINE void WrMemHook(uint32_t A, uint8_t V) {
if (X.WriteHook)
X.WriteHook(&X, A, V);
else
BWrite[A](A, V);
}
#endif
static INLINE uint8_t RdRAMFast(uint32_t A) {
return(_DB = RAM[A]);
}
@@ -370,30 +348,6 @@ void TriggerNMI2(void) {
_IRQlow |= FCEU_IQNMI2;
}
#ifdef FCEUDEF_DEBUGGER
/* Called from debugger. */
void FCEUI_NMI(void) {
_IRQlow |= FCEU_IQNMI;
}
void FCEUI_IRQ(void) {
_IRQlow |= FCEU_IQTEMP;
}
void FCEUI_GetIVectors(uint16_t *reset, uint16_t *irq, uint16_t *nmi) {
fceuindbg = 1;
*reset = RdMemNorm(0xFFFC);
*reset |= RdMemNorm(0xFFFD) << 8;
*nmi = RdMemNorm(0xFFFA);
*nmi |= RdMemNorm(0xFFFB) << 8;
*irq = RdMemNorm(0xFFFE);
*irq |= RdMemNorm(0xFFFF) << 8;
fceuindbg = 0;
}
static int debugmode;
#endif
void X6502_Reset(void) {
_IRQlow = FCEU_IQRESET;
}
@@ -409,9 +363,6 @@ void X6502_Init(void) {
ZNTable[x] = N_FLAG;
else
ZNTable[x] = 0;
#ifdef FCEUDEF_DEBUGGER
X6502_Debug(0, 0, 0);
#endif
}
void X6502_Power(void) {
@@ -421,124 +372,7 @@ void X6502_Power(void) {
X6502_Reset();
}
#ifdef FCEUDEF_DEBUGGER
static void X6502_RunDebug(int32_t cycles) {
#define RdRAM RdMemHook
#define WrRAM WrMemHook
#define RdMem RdMemHook
#define WrMem WrMemHook
if (PAL)
cycles *= 15; /* 15*4=60 */
else
cycles *= 16; /* 16*4=64 */
_count += cycles;
while (_count > 0) {
int32_t temp;
uint8_t b1;
if (_IRQlow) {
if (_IRQlow & FCEU_IQRESET) {
_PC = RdMem(0xFFFC);
_PC |= RdMem(0xFFFD) << 8;
_jammed = 0;
_PI = _P = I_FLAG;
_IRQlow &= ~FCEU_IQRESET;
} else if (_IRQlow & FCEU_IQNMI2) {
_IRQlow &= ~FCEU_IQNMI2;
_IRQlow |= FCEU_IQNMI;
} else if (_IRQlow & FCEU_IQNMI) {
if (!_jammed) {
ADDCYC(7);
PUSH(_PC >> 8);
PUSH(_PC);
PUSH((_P & ~B_FLAG) | (U_FLAG));
_P |= I_FLAG;
_PC = RdMem(0xFFFA);
_PC |= RdMem(0xFFFB) << 8;
_IRQlow &= ~FCEU_IQNMI;
}
} else {
if (!(_PI & I_FLAG) && !_jammed) {
ADDCYC(7);
PUSH(_PC >> 8);
PUSH(_PC);
PUSH((_P & ~B_FLAG) | (U_FLAG));
_P |= I_FLAG;
_PC = RdMem(0xFFFE);
_PC |= RdMem(0xFFFF) << 8;
}
}
_IRQlow &= ~(FCEU_IQTEMP);
if (_count <= 0) {
_PI = _P;
return;
} /* Should increase accuracy without a
* major speed hit.
*/
}
if (X.CPUHook) X.CPUHook(&X);
/* Ok, now the real fun starts.
* Do the pre-exec voodoo.
*/
if (X.ReadHook || X.WriteHook) {
uint32_t tsave = timestamp;
XSave = X;
fceuindbg = 1;
X.preexec = 1;
b1 = RdMem(_PC);
_PC++;
if (encryptOpcodes ==12) b1 =b1 &0x39 | b1 >>1 &0x42 | b1 <<1 &0x84;
if (encryptOpcodes ==14) b1 =b1 &0x3F | b1 >>1 &0x40 | b1 <<1 &0x80;
switch (b1) {
#include "ops.h"
}
timestamp = tsave;
/* In case an NMI/IRQ/RESET was triggered by the debugger.
* Should we also copy over the other hook variables?
*/
XSave.IRQlow = X.IRQlow;
XSave.ReadHook = X.ReadHook;
XSave.WriteHook = X.WriteHook;
XSave.CPUHook = X.CPUHook;
X = XSave;
fceuindbg = 0;
}
_PI = _P;
b1 = RdMem(_PC);
ADDCYC(CycTable[b1]);
temp = _tcount;
_tcount = 0;
if (MapIRQHook) MapIRQHook(temp);
if (!overclocked)
FCEU_SoundCPUHook(temp);
_PC++;
if (encryptOpcodes ==12) b1 =b1 &0x39 | b1 >>1 &0x42 | b1 <<1 &0x84;
if (encryptOpcodes ==14) b1 =b1 &0x3F | b1 >>1 &0x40 | b1 <<1 &0x80;
switch (b1) {
#include "ops.h"
}
}
#undef RdRAM
#undef WrRAM
#undef RdMem
#undef WrMem
}
static void X6502_RunNormal(int32_t cycles)
#else
void X6502_Run(int32_t cycles)
#endif
{
#define RdRAM RdRAMFast
#define WrRAM WrRAMFast
@@ -635,18 +469,3 @@ void X6502_Run(int32_t cycles)
#undef RdRAM
#undef WrRAM
}
#ifdef FCEUDEF_DEBUGGER
void X6502_Debug(void (*CPUHook)(X6502 *), uint8_t (*ReadHook)(X6502 *, uint32_t), void (*WriteHook)(X6502 *, uint32_t, uint8_t)) {
debugmode = (ReadHook || WriteHook || CPUHook) ? 1 : 0;
X.ReadHook = ReadHook;
X.WriteHook = WriteHook;
X.CPUHook = CPUHook;
if (!debugmode)
X6502_Run = X6502_RunNormal;
else
X6502_Run = X6502_RunDebug;
}
#endif