mirror of
https://github.com/libretro/fuse-libretro.git
synced 2026-11-07 10:36:03 -10:00
master
393
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Craig
|
e997e2bc32 | libretro: fix prototype for getopt with newer GCC on psp (#184) | ||
|
Craig
|
958105a90a | libretro: fix compilation new vita sdk (#183) | ||
|
LibretroAdmin
|
7d9826bdc1 |
Prefix Fuse's memory pool API with fuse_ to keep static links clean
Fuse's own pooled allocator exports mempool_free, which is also the name libretro-common gives one of its allocator entry points. On the static-linking targets (wiiu, wii, ngc, vita, ctr, psp1, ps3, libnx, xenon) the core ships as an archive and RetroArch resolves it against a build that already carries libretro-common/memory/mempool.o, so both definitions land in the same link and ld refuses it. debugger.c pulls mempool.o out of the archive for mempool_register_pool, which is what drags the clashing definition in. Give the whole Fuse API a fuse_ prefix - fuse_mempool_malloc, fuse_mempool_free, FUSE_MEMPOOL_UNTRACKED and the rest - so the two allocators can coexist in one image. The flex and bison inputs are updated alongside their generated output so a regeneration keeps the new names. Verified by archiving the core's mempool.o against an object built from libretro-common/memory/mempool.c: the link fails on master and succeeds here. platform=unix builds clean, and rebuilds clean under ASan+UBSan+LSan and under TSan. |
||
|
libretroadmin
|
2a5f1d43fe |
libspectrum: don't prefetch past the end of the generalised data stream
get_generalised_data_bit() consumed a bit and then, if that exhausted the
current byte, immediately loaded the next one:
if( ++state->bits_through_byte == 8 ) {
state->bits_through_byte = 0;
state->bytes_through_stream++;
state->current_byte = block->data[ state->bytes_through_stream ];
}
The load is unconditional on the byte boundary, including the boundary at the
very end of the stream, so the final bit of the last byte fetches
data[ data_count ]. That is one byte past the allocation whenever the bit
stream is a whole number of bytes long, i.e. whenever
bits_per_data_symbol * TOTD is a multiple of 8.
This is not a malformed-input case. It fires on well-formed tapes, and a
byte-aligned data stream is the common case rather than the exception: a
block with a two-symbol data alphabet and any multiple of eight data symbols
hits it, as does any two- or four-bit alphabet with a multiple of four or two
symbols respectively. ASan reports a 1-byte heap-buffer-overflow read in
get_generalised_data_bit() on a 36-byte TZX with TOTD=8, NPD=1, ASD=2 and a
single data byte.
Move the fetch to the top of the function so a byte is only loaded when a bit
is actually wanted from it. The final byte's last bit no longer pulls in a
successor that does not exist, and bytes_through_stream stops one short of
data_count instead of reaching it.
The decoded bit sequence is unchanged: bits_through_byte simply holds 8
between the last bit of one byte and the first bit of the next, rather than
being reset early. Verified by diffing full ( tstates, flags ) edge traces
before and after across 1-, 2- and 3-bit symbol widths, byte-aligned and
non-byte-aligned streams, power-of-two and non-power-of-two alphabets, and
blocks with and without a pilot run: identical in every case.
|
||
|
libretroadmin
|
6e96da12b0 |
libspectrum: bound generalised data symbol indices
Neither of the two symbol lookups in generalised_data_edge() checks the
index against the size of the alphabet it is indexing:
case LIBSPECTRUM_TAPE_STATE_PILOT:
current_symbol = block->pilot_symbols[ state->run ];
symbol = &( table->symbols[ current_symbol ] );
case LIBSPECTRUM_TAPE_STATE_DATA1:
symbol = &( table->symbols[ state->current_symbol ] );
table->symbols holds symbols_in_table entries. Both indices can exceed it:
1. pilot_symbols[] is copied verbatim out of the file's PRLE array, so any
byte 0..255 is possible regardless of ASP. A TOTP=1 NPP=1 ASP=2 block
whose single PRLE entry names symbol 5 indexes six entries into a
two-entry table.
2. state->current_symbol is decoded from the data bit stream using
ceil( log2( ASD ) ) bits, which addresses 2^ceil(log2(ASD)) symbols. That
only equals ASD when ASD is a power of two: with ASD=3 the decoder can
produce 3, one past the end of the table.
In both cases set_tstates_and_flags() then loads symbol->lengths from the
overread libspectrum_tape_generalised_data_symbol and dereferences it as a
libspectrum_word * to fetch a pulse length, so a heap word chosen by the file
is used as a pointer. This is the same shape as the crash in libretro/fuse-
libretro#67 (an access violation at 0x8, the offset of lengths in that struct
on LP64), which commit
|
||
|
libretroadmin
|
14d84de1d9 |
libspectrum: reject TZX 0x19 symbol alphabets with zero pulses per symbol
The NPP/NPD field of a Generalised Data Block gives the maximum number of
pulses per symbol in that alphabet. libspectrum_tape_block_read_symbol_table()
accepted zero and built the table anyway, allocating
symbol->lengths = libspectrum_new( libspectrum_word, table->max_pulses );
i.e. a zero-element array for every symbol in the alphabet.
generalised_data_edge() then plays those symbols, and set_tstates_and_flags()
dereferences symbol->lengths[ edges_through_symbol ] *before* the caller gets
to compare edges_through_symbol against max_pulses. With max_pulses == 0 the
terminating comparison
if( state->edges_through_symbol == table->max_pulses ||
symbol->lengths[ state->edges_through_symbol ] == 0 )
is false on the first edge (1 != 0), so the index keeps climbing and the
symbol is only abandoned when edges_through_symbol, a libspectrum_byte, wraps
back to 0 after 256 increments. That is 256 overreads of a zero-length
allocation per symbol played, and the pulse lengths harvested from adjacent
heap are fed to the tape timing.
Reproducer: 0x19 block with TOTP=1, NPP=0, ASP=0 (so symbols_in_table is
forced to 256) and a 256-byte pilot symdef. ASan reports a 2-byte
heap-buffer-overflow in set_tstates_and_flags() on the first edge.
A symbol with no pulses cannot be played at all, so reject the alphabet at
parse time when the table is in use. The check is inside the existing
`if( table->symbols_in_block )` arm, so an unused (TOTP/TOTD == 0) table with
NPP/NPD == 0 is still accepted, as before.
|
||
|
libretroadmin
|
d5d8ec017a |
libspectrum: compute the TZX 0x19 data stream size in 64 bits
data_count = ( ( bits_per_symbol * symbol_count ) + 7 ) / 8; bits_per_symbol is an int in 0..8 and symbol_count is the block's TOTD, a 32-bit dword read straight out of the file, so the product is evaluated in 32-bit unsigned arithmetic and wraps. With TOTD == 0x80000000 and a 4-symbol data alphabet (2 bits per symbol) the product is exactly 2^32, data_count comes out as 0, and the block is accepted with a zero-length data buffer while data_table.symbols_in_block still says 0x80000000 symbols are to be played. Playback then walks get_generalised_data_bit() across block->data with an ever-increasing bytes_through_stream and no terminating condition until the symbol counter is exhausted, i.e. an unbounded heap overread driven entirely by a 41-byte file. ASan reports the overflow one byte past the data allocation in get_generalised_data_bit(); without ASan this reads gigabytes of adjacent heap and dies wherever the mapping ends. TOTD is also not otherwise tied to the block length, so nothing further along constrains it: the old `end - (*ptr) < data_size` test compares a signed ptrdiff_t against a size_t and passes trivially when data_size is 0. Compute the bit count as a libspectrum_qword and require the resulting byte count to fit in the block's remaining length before allocating or copying. Checking against `length` rather than `end - *ptr` also drops the mixed signed/unsigned comparison and keeps the data stream inside the declared block rather than merely inside the file. |
||
|
libretroadmin
|
20a247c382 |
libspectrum: fix inverted symbol-table length accounting in TZX 0x19
tzx_read_generalised_data() tracks how much of the block body is still
unread in `length`, and after each symbol table it does:
ptr2 = *ptr;
... read_symbol_table( table, ptr, length ) ...
length -= ptr2 - *ptr;
*ptr has advanced past ptr2 by then, so `ptr2 - *ptr` is negative and the
subtraction *adds* the bytes just consumed back onto the remaining budget.
Every length check downstream of the pilot table is therefore inflated by
twice the size of the tables already parsed:
- the `length < 3 * symbol_count` PRLE check, and
- the data table's own `length < ( 2 * max_pulses + 1 ) * symbols_in_table`
check inside libspectrum_tape_block_read_symbol_table().
A block that declares a data symbol table larger than the bytes actually
present passes the inflated check and read_symbol_table() then walks off
the end of the block. When the 0x19 block is the last one in the file
that is a read past the end of the caller's input buffer.
Reproducer (37-byte TZX, ASan heap-buffer-overflow of 1 byte immediately
after the input allocation, in read_symbol_table via tzx_read_generalised_data):
TOTP=1 NPP=1 ASP=1, TOTD=1 NPD=1 ASD=2, block length 22, but only 2 of
the 6 declared data-symdef bytes supplied. Correct accounting leaves
length == 2 and the block is rejected as corrupt; the inverted accounting
leaves length == 8 and it is accepted.
Fix the sign in both occurrences. No underflow is possible afterwards:
read_symbol_table() advances by exactly
( 2 * max_pulses + 1 ) * symbols_in_table, which it has already verified
is <= length, and advances by zero for an empty table.
|
||
|
libretroadmin
|
94beb76e33 |
libretro: detect 128K content loaded on a 48K model
The Model option defaults to Spectrum 48K, matching Fuse's own default
in settings.c. A 128K-only release loaded on a 48K machine then does
what it would do on real hardware: it loads, and dies on a garbled
screen with no way out. Standalone Fuse remembers the machine the user
last picked in its config file, so the same tape "just works" there and
the core looks broken by comparison.
That is fuse-libretro#157, reported against the 128K release of Army
Moves: verified as not a core defect - the tape loads and plays on
128K/+2/+3, on current master and on
|
||
|
libretroadmin
|
2fbeeaeac5 |
libretro: clear the forced-machine state on every content load
forced_machine_at_init/forced_machine_idx are set by the TS2068 .dck
special case in retro_load_game() and never cleared again. A frontend
may load new content without an intervening retro_deinit() - the tape
buffers and Fuse teardown a few lines below already account for that -
so both flags survive into the next load.
Two things then go wrong for the rest of the session:
- update_variables() overrides the fuse_machine option with
forced_machine_idx, pinning the machine to TS2068;
- the identify code takes the 'forced_machine_idx == 10' branch and
declares the content to be a .dck cartridge whatever it is.
Loading a .dck and then a .tzx therefore ran the tape through
utils_open_file() as "*.dck" on a TS2068, with the frontend still
showing whatever the user had chosen under Model.
Before, second load with Model = Spectrum 128K:
[core] Opened "tc2068-0.rom" from memory
[core] Opened "*.dck" from memory
av2: 640x480 fps=60.000000
After:
[core] Opened "*.tzx" from memory
av2: 320x240 fps=50.000000
|
||
|
libretroadmin
|
59d116d0d9 |
libretro: don't dereference a NULL info->path in the .dck check
retro_get_system_info() sets need_fullpath = false, so a frontend is
free to hand retro_load_game() a content buffer with no path at all -
the identify path further down says exactly that and copes with it:
// info->path may legitimately be NULL with need_fullpath=false;
The TS2068 .dck special case runs before any of that and passes
info->path straight to strrchr(), which segfaults on NULL before the
core has emulated a single frame.
Reproduced by calling retro_load_game() with info->path = NULL:
AddressSanitizer: SEGV on unknown address 0x000000000000
#1 strrchr
#2 retro_load_game src/libretro.c:1891
|
||
|
LibretroAdmin
|
7a3b7befdb |
Build with -ffunction-sections: 1796176 -> 1592424 bytes, 11.3% off
This core links whole vendored trees for the sake of a few entry points. libspectrum calls BZ2_bzBuffToBuffDecompress and BZ2_bzDecompress*, and nothing else in bzip2: its compression half and the FILE* API are dead. Of libretro-common only the VFS, path and string helpers are used; filestream_vscanf, filestream_copy and fill_pathname_abbreviated_or_relative are not. Without -ffunction-sections the linker's unit is the whole object file, so one live symbol keeps every dead one beside it. Trimming those functions out of the vendored sources would work and is the wrong fix. Divergence between this tree's deps/libretro-common and upstream is exactly what broke the six build-static jobs three commits ago, and hand-editing bzip2's internals to split decompress from compress buys the same problem in a second dependency. Give the linker per-symbol granularity and let it decide instead of the source. Dropped from the shared build, verified with nm: BZ2_hbMakeCodeLengths, mainSort, fallbackSort, bzopen_or_bzdopen, filestream_vscanf, filestream_copy, fill_pathname_abbreviated_or_relative. The export set is unchanged - the same 47 retro_* symbols before and after, diffed symbol for symbol. Static-linking platforms get the compile flags but not --gc-sections: they produce an archive and it is the frontend's link that resolves it, so the sections are there for RetroArch's linker to use. The findstring guard on SHARED keeps the link flag off emscripten, which sets no SHARED, and theos_ios, which links through its own rules. |
||
|
LibretroAdmin
|
04253082be |
debugger: set YYINITDEPTH to 64, 4032 -> 1584 bytes in yyparse
The generated parser's state and value stacks are automatic arrays of YYINITDEPTH entries, so that constant is yyparse()'s frame: yyssa[200] plus yyvsa[200] of a 16-byte YYSTYPE is 3600 bytes of the 4032 that -fstack-usage reports at bison's default. That frame is not confined to a debugger UI this core does not have. debugger_check() is called from readbyte()/readport_internal()/writebyte() and so from z80_do_opcodes(), and a breakpoint carrying commands reaches debugger_command_evaluate() -> yyparse() from there. fuse_init() also evaluates settings_current.debugger_command directly. Both put 4 KB on the stack of whichever thread the frontend runs the core on. Measured rather than guessed: instrumenting the push site in yyparse() and running the debugger's command set gives a peak stack depth of 8, the deepest being break 0x8000 if ((hl + de) * 2 - 1) / 3 > 100 Depth then rises by exactly one per level of parenthesis nesting in a condition expression, confirmed linear over a 1..512 sweep. 64 leaves room for 58 levels, seven times the deepest command the grammar accepts in practice. Overflowing YYINITDEPTH is not an error and not an alloca: YYSTACK_USE_ALLOCA is unset in this generated parser, so YYSTACK_ALLOC is malloc and the stacks relocate to the heap and back, bounded by YYMAXDEPTH (10000). Verified under ASan and UBSan across 1 to 1024 levels of nesting: every parse completes, no diagnostic. Set in commandy.y as well as the generated commandy.c. The .y prologue is emitted around line 143 of the output, well above the skeleton's #ifndef YYINITDEPTH guard at line 984, so a regeneration keeps it. |
||
|
LibretroAdmin
|
631cb2cf0b |
settings: take the config path off read_config_file's frame
read_config_file() reserved 4224 bytes, measured with -fstack-usage, for a char path[PATH_MAX] it does not touch on any frontend but PS3. compat_get_config_path() returns the empty string everywhere else, so the function returns at if( !*cfgdir ) return 0; two lines before path[] is first used. It is reached from settings_init() under fuse_init(), i.e. on the content-loading path, where the frame sits below retro_load_game()'s own 4256 bytes. Scoping the array to the branch that uses it does not help - GCC lays an automatic array out in the frame regardless of the block it is declared in, and -fstack-usage still reported 4224 after that change. Allocating it instead, after the early return, moves it off the stack entirely: on every non-PS3 build nothing is allocated at all, and on PS3 the cost is one malloc/free per load of a path buffer that was always going to be written once and read twice. read_config_file() no longer appears in the .su output; it is inlined into fuse_settings_init(), whose frame is 160 bytes. Deepest chain from retro_load_game() drops from 13416 to 9088 bytes. settings_write_config() has the same shape but is left alone: it is on the retro_deinit() side rather than the loading path, and its body threads a compat_fd through a goto error chain that a scope change would have to step around for no gain. Both settings.pl and the generated settings.c are patched, so regenerating does not lose it. |
||
|
LibretroAdmin
|
956c54fce1 |
Join the system path in place: 8288 -> 4192 bytes of stack
compat_file_open() carried two MAX_PATH_LEN buffers, system[] and a base[] scratch that existed only to hold the intermediate "<system dir>/fuse" before the leaf name was appended. -fstack-usage put the frame at 8288 bytes. Every other function in the file is 32 bytes or less, and this one sits on the ROM-loading path, where the frontend's core thread stack is not generous on 3DS or PSP. fill_pathname_join() guards its leading copy with `if (s != dir)`, so passing the destination as its own directory argument is the in-place case the function is written for: it skips the copy, finds the string already in the buffer, and appends to it. That is exactly what the scratch buffer was emulating, so both joins can land in system[] and base[] goes away. This is only available since fill_pathname_join_special stopped being used here - the specialized variant had no such guard and required distinct buffers. Truncation is unchanged. The two buffers were the same size, so the first join clipped at the same byte whether it wrote into base[] or into system[], and the second join starts from the same string either way. A/B oracle over 690 (system dir, leaf) combinations - empty, bare and trailing-separator, absolute and relative, mixed / and \, sd:/ and ux0: device prefixes, UTF-8, plus system directories padded to just under and just over the 4096-byte clip - shows byte-identical output buffers, under both the POSIX and _WIN32 compiles of find_last_slash() and PATH_DEFAULT_SLASH_C(), clean under ASan and UBSan. -fstack-usage after: compat_file_open 4192 bytes. |
||
|
libretroadmin
|
f053a87798 | Use fill_pathname_join | ||
|
LibretroAdmin
|
2f5d09291b |
Fix static-link builds: drop the vendored fill_pathname_join_special
The six build-static jobs (ctr, ngc, wii, wiiu, psp, vita) all fail the same way: 38 "multiple definition" errors, every one of them between libretro_<platform>.a(file_path.o) and RetroArch's own libretro-common/file/file_path.o. Root cause is not the 38 symbols. In a static build RetroArch's libretro-common objects are on the link line and the core is an archive, so an archive member is only extracted when it satisfies a symbol that is still undefined. Every symbol the core's file_path.o defines is already defined by RetroArch's, with exactly one exception: fill_pathname_join_special RetroArch turned that identifier into a macro alias of fill_pathname_join(); this vendored copy still had it as a real function. src/compat/file.c and deps/libretro-common/vfs/ vfs_implementation.c call it, RetroArch no longer defines it, so ld pulls file_path.o out of the archive to resolve it and the other 38 symbols in that member collide on the way in. One divergent symbol drags in a whole translation unit. Sync the vendored copy with upstream: the header gets the macro alias, the definition goes. The core's file_path.o now exports a strict subset of RetroArch's, nothing forces its extraction, and the callers resolve to fill_pathname_join() in both build modes. The two implementations differed only in fill_pathname_join()'s `if (s != dir)` guard, which skips the leading copy when the output aliases the directory. Every call site here passes distinct buffers, so the guard is never taken and the substitution is a no-op that also makes the aliasing case safe. A/B oracle over 2640 (dir, path, len) combinations - empty, relative, absolute, trailing- and leading-slash, mixed separators, truncating lengths - shows byte-identical output buffers and identical return values. Verified by rebuilding the archive against upstream RetroArch's libretro-common: before, 38 multiple-definition errors matching the CI log symbol for symbol; after, a clean link. The shared build is unaffected. |
||
|
libretroadmin
|
8cce09e6af |
Fix upgrade builds broken by the UI removal: stale config.h kept USE_WIDGET
|
||
|
libretroadmin
|
c6b73c1a6c |
Remove the dead standalone-Fuse UI: 277 files, 39% off the binary
With __LIBRETRO__ gone this is the only build, and the only build has
no way into any of the standalone frontend machinery it was still
compiling and shipping. The proof is one flag: display_ui_initialised
is set to 1 exclusively by the fb, gtk, sdl, svga, wii, win32 and xlib
display drivers, none of which are in the build, so it is permanently
0 here and widget_do() - the sole entry into the widget UI - returns
on its first line. Every menu, dialog, and the widget error popup were
compiled, linked, and unreachable. Likewise the movie recorder and the
profiler could only ever be started from those menus, the scaler was
selected once at startup and then never used (scalers16.o/scalers32.o
linked with zero references to any symbol in them), and the seven
other frontends' source trees were never compiled at all.
Deleted: fuse/ui/{fb,gtk,sdl,svga,wii,win32,xlib,null,widget,scaler},
fuse/menu.c, fuse/movie.[ch], fuse/profile.[ch], fuse/screenshot.[ch],
src/fuse/scalers16.c, src/fuse/scalers32.c, src/fuse/error.c (it
compiled the widget error dialog under another name), the platform
sound drivers in fuse/sound/ (blipbuffer stays - it is the mixer),
the platform compat trees in fuse/compat/ (getopt.h stays - the
generated settings code needs struct option on platforms without
getopt.h), fuse/timer/sdl.c, fuse/z80/coretest.c, libspectrum/test/,
and USE_WIDGET and the never-definable SOUND_FIFO branch with them.
What real code pulled out of the deleted cluster survives, relocated:
- The option_enumerate_*() string-to-enum mappings that sound.c, the
disk peripherals, the +3 and uimedia.c consume move verbatim from
the generated widget options.c into src/fuse/options_enumerate.c -
they were the only lines of that file the core ever executed.
- The rest of the ui_* surface is stubbed in src/compat/ui.c with
the answer each caller already effectively received: file dialogs
return NULL (cancel, handled quietly by tape/disk/mdr save), eject
with unsaved changes returns DONTSAVE, ui_confirm_joystick makes
the identical decision the widget version resolved to with
joy_prompt off (this core's only configuration), rollback returns
"no point chosen".
- ui_error_specific() now only logs. It previously also called into
the widget dialog, which drew nothing for the reason above.
The movie and profile frame hooks are deleted from display.c,
machine.c, rzx.c, sound.c, spectrum.c and the z80 opcode loop
(z80_ops.c and z80_checks.h stay consistent - both derive from the
same SETUP_CHECK list), fuse.c loses movie_init/movie_stop, the
start_scaler block and the screenshot startup registration, and
utils_read_screen() goes with screenshot.h since nothing called it.
ui_widget_level, pinned at -1 forever, is gone along with its checks
in input.c.
The built core shrinks from 2952496 to 1796304 bytes, 39%.
No functional change: the full verification matrix from issues #176
and #177 - Kempston Mouse hotplug and Fuller joystick wiring with and
without fastload, option flips mid-autoload, all five border modes
measured from the framebuffer, runtime border cycling, the keyboard
overlay, machine boots including Timex and NTSC, and a savestate round
trip - produces identical reset counts, port reads, framebuffer hashes
and audio sample counts to the build before this change, and the
corrupt-content error path hashes identically too, confirming the
widget error dialog never contributed a pixel. ASan and UBSan clean
across all of it. accessor.pl still regenerates snap_accessors.c
byte-identically.
|
||
|
libretroadmin
|
5e3762d4e1 |
Remove __LIBRETRO__: this core only ever builds for libretro
Every one of the 46 __LIBRETRO__ conditionals in the tree guarded a
choice between "the libretro build" and "a build that cannot exist":
this repository has no other frontend, no configure script, and no way
to compile without the define. Keep the libretro side of each one,
delete the other, and stop defining the symbol in Makefile.libretro,
the 22 build/Makefile.* platform files, etc/mkmake.lua and
build/jni/Android.mk.
Everything was stripped mechanically with unifdef -D__LIBRETRO__; the
only hand edits are in fuse/peripherals/joystick.c, where the comment
above the now-unconditional joy_kempston enable described the deleted
condition, and libspectrum/snap_accessors.txt, where the %ifdef around
the TurboSound fields is gone so accessor.pl regenerates exactly the
stripped snap_accessors.c (verified: the regenerated file is
byte-identical).
What falls out:
- machine.h/spectrum.h/periph.c/unittests.c: one
spectrum_unattached_port signature (taking the port, for floating
bus emulation) instead of two.
- ay.c/ay.h/machine.h/szx.c/snap_accessors.*/z80.c: the TurboSound
second AY and its AY2 snapshot chunk are simply part of the core.
- joystick.c/settings.c/widget.c: the libretro joystick-mapping
behaviour (frontend port wiring is authoritative, joy_kempston
always enabled on snapshot load, no joystick_N_output defaults) is
now the only behaviour, unconditionally.
- snapshot.c/snapshot.h: the peripheral-override hook from
|
||
|
libretroadmin
|
2b5c8ce8ac |
Make the border size modes proportionate
The fuse_size_border ladder was 320x288, 288x216, 272x204, 264x198, 256x192 - border thickness a side of 32/48, 16/12, 8/6, 4/3, 0/0 pixels (horizontal/vertical, doubled for Timex). Issue #177's three complaints are three properties of that table: - "full doesn't look right": full showed all 48 lines of vertical border Fuse renders, which is the whole PAL blanking-area border, not what a TV displayed - standalone Fuse crops it to 24 (DISPLAY_BORDER_HEIGHT there). The result was a 10:9 frame whose vertical border was half as thick again as the horizontal. - "medium is a significant jump down": the vertical border fell from 48 to 12 lines in one step. - "the last three are too close": 16/8/4 halves each time, so the small end of the ladder bunches together. Replace it with borders of 4k pixels a side horizontally and 3k vertically for k = 8, 6, 4, 2, 0: full 32/24 320x240 (matches standalone Fuse) medium 24/18 304x228 small 16/12 288x216 (the old medium window, bit-identical) minimum 8/6 272x204 (the old small window, bit-identical) none 0/0 256x192 Every step is the same size, and because the paper is 256x192 = 4:3, (256+8k) x (192+6k) is exactly 4:3 for every k - each mode is the same shape, just more or less border. The full-mode special case for 60Hz machines collapses: they only draw ~24 border lines, which is why full was already clamped to a 240-line window for them, and 240 lines is now what full means everywhere; every smaller mode fits inside it. The five sizes also now fit in one table, so the two switch blocks that update_variables() kept in sync by hand (machine change and option change) become one apply_border_size(). The keyboard overlay improves for free: it is a 320x240 image drawn 24 lines into the canvas, which is exactly the new full window, so in full mode it now fills the frame edge to edge instead of floating between two dead bands of border. Verified against the built core with a headless libretro frontend, on a .z80 that paints the border red and the paper green so the thickness on every side is measurable from the framebuffer: full 320x240 border L/R/T/B = 32/32/24/24 medium 304x228 border L/R/T/B = 24/24/18/18 small 288x216 border L/R/T/B = 16/16/12/12 minimum 272x204 border L/R/T/B = 8/8/6/6 none 256x192 border L/R/T/B = 0/0/0/0 with the new small and minimum windows hashing bit-identically to the old medium and small - the same centred crops, renamed one rung down. Cycling the option through all five modes and back at runtime lands on the same hashes as loading with each mode set. Timex machines report the doubled geometry for every mode, 60Hz machines the same sizes as 50Hz. All twelve machines that boot without external ROMs do so deterministically, a savestate round trip is unaffected, and the whole issue #176 verification matrix (Kempston Mouse hotplug, Fuller joystick, option flips mid-autoload) reproduces the same reset counts, port reads and audio sample counts. ASan and UBSan clean over content load, every border mode, runtime mode cycling, the keyboard overlay and Timex boots. |
||
|
libretroadmin
|
52782d4e2c |
Don't reset the machine when the Kempston Mouse is wired to a port
Selecting the Kempston Mouse hard resets the Spectrum, and if a tape is
autoloading with Tape Fast Load on it locks the frontend up outright
(#176). Both regressed in
|
||
|
libretroadmin
|
ad7e52fb53 |
Let the core options, not the snapshot, decide which peripherals are fitted
Loading snapshot content or restoring a savestate took the optional
peripheral set out of the file and applied it over the top of the core
options. Three ways that goes wrong, all reproduced:
- an SZX carrying the USPE chunk turns the Currah uSpeech on with
fuse_uspeech disabled, and speaks
- a .z80, which has nowhere to record uSpeech at all, turns it off with
fuse_uspeech enabled, so a user who has both ROMs in place and the
option on gets silence the moment they load a game
- with fuse_fuller_box or fuse_melodik enabled, loading any snapshot
hard resets the machine and discards the snapshot that was just
loaded, landing the user back at the BASIC boot screen
The last one is the reason this cannot be fixed where it was being
attempted. snapshot_copy_from() runs periph_disable_optional(), then
module_snapshot_enabled() to read the flags out of the file, then
machine_reset(0), which is what turns settings_current into an actual
peripheral table, and only then module_snapshot_from() to restore the
registers and RAM. sync_periph_from_ports_and_options() was correcting
the settings after all of that had finished, and arming
periph_needs_update so retro_run() would pick the change up. But
periph_posthook() calls machine_reset(1) whenever a peripheral declaring
hard_reset changed, and the Fuller Box, Melodik and uSpeech all declare
it - so the correction was itself destroying the state.
Override the flags in the one place where it costs nothing instead:
between module_snapshot_enabled() and the reset. The peripheral table is
then built correctly the first time, the register and RAM restore that
follows is untouched, and no second reset is needed. Assignment only, so
deliberately no periph_needs_update.
sync_periph_from_ports_and_options() stays as the path for options and
port wiring changed while already running; the port/option combining
rule the two now share is factored into resolve_port_peripherals() so
they cannot drift apart.
Verified against the built core with a headless libretro frontend, using
an SZX generated through libspectrum carrying the USPE chunk and a .z80
without it, both holding a Z80 program that pages uSpeech in and writes
allophones, so "is the peripheral actually fitted" is answerable from
the audio:
SZX with chunk, uspeech off before: RMS 411.6 after: silent
z80 no chunk, uspeech on before: silent after: RMS 411.3
SZX with chunk, uspeech on before: RMS 411.6 after: RMS 411.6
SZX, fuller_box on before: silent, reset to BASIC
after: RMS 411.6, snapshot intact
SZX, melodik on before: silent, reset to BASIC
after: RMS 411.6, snapshot intact
No-content boot framebuffers are unchanged with the option both on and
off, the twelve machines the core can start without external ROMs are
unchanged, and a savestate round trip still comes back with uSpeech
fitted. ASan and UBSan clean over content load, savestate restore and
both option states.
Note for anyone editing fuse/snapshot.c: src/fuse/snapshot.c includes it
as a source file, and this build system has no header dependency
tracking, so make does not rebuild the object when only the included
file changes. Touch src/fuse/snapshot.c, or the build silently keeps the
old code - the same trap settings.h has.
|
||
|
libretroadmin
|
010d112156 |
Currah uSpeech: add the paging and SP0256 hooks the backport left out
Fixes libretro/fuse-libretro#175. With fuse_uspeech enabled and both ROMs
in place, nothing happened: no 'Speech System (c) CURRAH 1983' banner, no
speech.
The peripheral itself was fine. The backport in
|
||
|
Lib
|
000a8ae51a |
Add a Currah uSpeech core option
Exposes the peripheral added in the previous commit as fuse_uspeech, defaulting to disabled, and applies it from sync_periph_from_ports_and_options() alongside the Fuller Box, Melodik and Issue 2. The setting is assigned unconditionally, the way Issue 2 already is. uspeech_enabled_snapshot() copies libspectrum_snap_uspeech_active() straight into settings_current, so restoring a state captured while uSpeech was on would otherwise leave the setting reading enabled even with the option since turned off. Arming the deferred peripheral update is a separate question, and this one does not follow the Fuller Box. uspeech_reset() clears settings_current.uspeech by itself whenever either of the two ROMs it needs is absent - the option is on, the hardware cannot come up, so it stays down. Comparing the option against settings_current, as the other peripherals do, reads that refusal as a difference and pushes the option straight back in, forcing a fresh hard reset every time and leaving option and setting permanently disagreeing. Compare against the value last applied instead, so the refusal stands until the user actually changes the option. The SZX chunk stays conditional on uspeech_active, matching upstream libspectrum, rather than being written unconditionally the way write_amxm_chunk() and write_ay2_chunk() are. Those exist because a snapshot whose size depends on a live-toggleable option breaks a frontend's frozen rewind buffer, but retro_serialize() pads to a fixed size, so a conditional chunk changes the size by nothing - measured at 131072 bytes with the option both on and off. Writing it unconditionally would also mean uspeech_enabled_snapshot() saw an active uSpeech in every snapshot, and would hand uspeech_from_snapshot() a paged flag it might act on via uspeech_toggle(), which has no uspeech_available guard and would page in a bank that was never loaded. Verified with both ROMs absent, which is the state every user starts in: the core survives with the option enabled, Fuse clears the setting, the peripheral never activates, ROMCS stays low, savestate size is unchanged, states round-trip, and a state captured with uSpeech on does not re-enable it when the option is off. Clean under ASan and UBSan with -fno-sanitize-recover=all, with the peripheral, volume, SZX and path harnesses re-run for regressions. Still unverified: the speech itself. See the previous commit. |
||
|
Lib
|
6a8cdd2088 |
Backport the Currah uSpeech peripheral from Fuse 1.8.0
Adds peripherals/sound/sp0256.c and peripherals/sound/uspeech.c, the SP0256-AL2
allophone synthesiser and the Currah uSpeech interface that drives it. Fuse
gained both in 1.7.0; the tree vendored here is 1.6.0.
Taken as a backport rather than as part of a version bump. Measured first: the
vendored fuse/ tree carries local changes to 45 source files and libspectrum/
to 34, and 51 of those are also touched by upstream between 1.6.0 and 1.7.0,
which alongside the peripherals/ directory restructure makes a bump a
several-thousand-line merge. The two files themselves turned out to need
nothing: compiled against this tree, sp0256.c produces no errors at all and
uspeech.c only five, every one a declaration that has to be added here anyway.
The plumbing those five need:
- PERIPH_TYPE_USPEECH in periph.h, offered as PERIPH_PRESENT_OPTIONAL from
machines_periph_48(); uSpeech is a 48K peripheral
- STARTUP_MANAGER_MODULE_USPEECH, and uspeech_register_startup() from fuse.c
- uspeech, rom_uspeech, rom_sp0256 and volume_uspeech in settings.dat
- sound_sp0256_write() and its pair of Blip_Synths in sound.c, following the
existing specdrum and covox pattern, plus sp0256_do_frame() at the top of
sound_frame() so speech generated during a frame lands in that frame
Two changes to the imported files. Upstream's uspeech.c includes "memory.h",
which does not exist in Fuse - the header is memory_pages.h, and the include
silently picks up glibc's memory.h, with the real declarations arriving
transitively through machine.h. Point it at the right header. Doing so removed
the string.h that was arriving through the same accident, so include that
explicitly too.
Both ROMs uSpeech needs are copyrighted and cannot ship with the core, so they
are looked up by bare name in the frontend's system directory as
'uspeech.rom' and 'sp0256-al2.bin'. When either is missing, uspeech_reset()
clears settings_current.uspeech and deactivates the peripheral, which is the
intended degradation.
Note for anyone building on this: regenerating settings.h changes the layout
of settings_info, and this build system has no header dependency tracking - no
-MMD, no depfiles - so objects compiled before the change keep reading fields
at their old offsets. The result is a crash in free() inside
update_variables() that looks nothing like a layout problem. Run make clean
after touching settings.dat, periph.h or any other widely included header.
Not verified: whether it speaks correctly. The peripheral registers, decodes
its ports, pages on /ROMCS and serialises, and all of that is covered. Not one
allophone has been checked against a reference, because that needs the two
copyrighted ROMs. Upstream's regression cases are the Currah demo tape,
Booty's hidden game, and Colour Clash.
|
||
|
Lib
|
215ac98a57 |
compat: search the frontend's system directory for auxiliary files
Every asset this core needs is baked into the binary as a generated header (see mem_entries[] in compat/file.c), so compat_get_next_path() offered a single empty search path and the core never asked the frontend where its system directory was. That stops working for files which cannot be shipped: Currah uSpeech needs both the interface ROM and the SP0256-AL2 allophone ROM, and both are copyrighted, so they have to come from the user. Ask for the system directory in retro_set_environment() - the frontend is required to answer before retro_init(), and Fuse resolves ROM paths during fuse_init(), so retro_init() would already be too late - and offer it from compat_get_next_path() as a second search location. It is deliberately second. The empty path stays first, so a baked-in ROM still beats a same-named file in the system directory and no existing content changes behaviour. A frontend that declines the environment call, or answers with NULL or an empty string, leaves the search exactly as it was. Also give compat_is_absolute_path() a real implementation. It used to answer "never", which made utils_find_auxiliary_file() and utils_find_file_path() treat an absolute name as relative and glue a search directory in front of it. Nothing exercised that while the only search path was the empty one and every asset was baked in, but with a real directory in the list it would have produced paths like "<sysdir>/C:\roms\foo.rom". Delegate to libretro-common's path_is_absolute(), which handles drive letters and UNC prefixes as well as a leading separator. Verified through the real resolver rather than by unit-testing the switch statement: a file present only in the system directory resolves and reads back byte-exact; 48.rom still returns the real 16384-byte baked-in ROM rather than a same-named decoy planted in the system directory; an absent file fails cleanly instead of resolving to something wrong; an absolute path passes straight through and reads byte-exact; and with the frontend refusing to supply a system directory, the system-directory-only file is not found while baked-in ROMs still resolve. Clean under ASan and UBSan with -fno-sanitize-recover=all, with the peripheral and volume harnesses re-run for regressions. |
||
|
Lib
|
3027d0921c |
libspectrum: read and write the SZX uSpeech chunk
Groundwork for Currah uSpeech emulation. Fuse gained uSpeech in 1.7.0 and
libspectrum gained the snapshot support for it in 1.6.0; the vendored trees
here are 1.6.0 and 1.5.0 respectively, so neither side has it. Backport the
libspectrum half.
szx.c already had ZXSTBID_USPEECH ("USPE") defined and wired into the chunk
table, but pointed at skip_chunk, so the chunk was parsed and discarded.
Point it at a real reader and add the matching writer, both taken from
libspectrum 1.6.0. The payload is a single byte holding the paged flag;
presence of the chunk is itself the active flag.
The two accessors this needs, uspeech_active and uspeech_paged, are added to
snap_accessors.txt. Getting them into the generated snap_accessors.c meant
fixing that file's own drift first: the TurboSound accessors had been wrapped
in "#ifdef __LIBRETRO__" by hand in the output, which accessor.pl could not
express and would therefore have dropped on the next regeneration. Add an
%ifdef/%endif directive to accessor.pl, mark the TurboSound block with it in
snap_accessors.txt, and regenerate, so the file is reproducible again:
perl accessor.pl snap_accessors.txt > snap_accessors.c
Nothing sets uspeech_active yet - the peripheral itself does not exist in the
vendored Fuse tree - so no snapshot written by this core changes by a single
byte until it does. That matters here: retro_serialize_size() is sampled once
by frontends to size the rewind ring buffer and never resampled, so when the
peripheral does land, this chunk must be written under a session-constant
condition rather than gated on uspeech_active, the way write_amxm_chunk() and
write_ay2_chunk() already are for exactly this reason.
Verified by round-tripping real SZX data rather than by checking the accessors
against themselves: the chunk id appears in the written buffer, its length
field reads 1, its payload byte matches the paged flag for both values, both
fields survive a read back, an inactive snapshot emits no chunk at all, and an
SZX carrying no USPE chunk still loads with uspeech reported inactive. Clean
under ASan and UBSan with -fno-sanitize-recover=all.
|
||
|
Lib
|
60b60fb887 |
settings: fold the local settings.c edits back into settings.pl
fuse/settings.c is generated from settings.dat by settings.pl, but the checked-
in copy had drifted from its generator: three changes had been applied to the
output by hand and never pushed back into the script.
- CONFIG_FILE_NAME picks "fuse.cfg" on __PS3__ as well as WIN32
- a getopt_long() stub, since the core is configured entirely through core
options and never parses a command line
- settings_init() skips settings_command_line() when argc <= 1
Regenerating for any other reason - such as adding a setting to settings.dat -
would therefore have silently reverted all three. The #line directives had also
gone stale, pointing at line numbers from an older revision of the script.
Move the three edits into settings.pl, with comments explaining why each is
there, and regenerate. Output is now identical to the checked-in file bar those
comments and corrected #line numbers, so settings.c is reproducible again:
perl -I perl settings.pl settings.dat > settings.c
No functional change.
|
||
|
Lib
|
91911e6c75 |
Add an Issue 2 Keyboard core option
settings_current.issue2 already exists (fuse/settings.h:89) and is consumed live by ula_write(), which latches a different default value for the unused keyboard-port bits on an Issue 2 machine - 0xff when either bit 3 or bit 4 of the last 0xfe write was set, rather than bit 4 alone (fuse/peripherals/ula.c:217-223). The core never exposed it, so titles that only respond to the keyboard on an Issue 2 Spectrum, Rasputin among them, were unplayable. Expose it as fuse_issue2, defaulting to disabled so the emulated machine stays an Issue 3 unless asked otherwise. No port registration is involved, so unlike the Fuller Box this needs no peripheral update - but ula_from_snapshot() does re-derive it from a loaded snapshot, so it is applied from sync_periph_from_ports_and_options() along with the rest. Verified against the ULA rather than the setting: after writing 0x08 to port 0xfe (bit 3 set, bit 4 clear, the case that separates the two revisions), bit 6 of a subsequent read comes back clear as an Issue 3 and set as an Issue 2. |
||
|
Lib
|
3c725a7c61 |
Make the Fuller Box optional and add Melodik, both defaulting off
src/fuse/settings.c hardcoded settings_current.fuller = 1 in the replacement settings_init(), overriding Fuse's own default of 0 (fuse/settings.c:126). The Fuller Box is one periph_t covering an AY-3-8912 at ports 0x3f/0x5f and a joystick at 0x7f (fuse/peripherals/fuller.c:51-55), so that left the AY permanently decoded on every machine, including plain 48K. Titles that poke those ports on the off chance - Vixen, Andy Capp and others - therefore played AY music that no unexpanded Spectrum could have produced. Drive it from a fuse_fuller_box core option instead, defaulting to disabled to match real hardware, and add fuse_melodik alongside it: Melodik is the other way a 48K gets an AY, decoding the 128K ports (0xfffd/0xbffd), and it is the same one-line plumbing job. Three things fall out of this: - The Fuller joystick is part of the same periph_t, so leaving the box off would silently break RETRO_DEVICE_FULLER_JOYSTICK. Generalise sync_kempston_mouse_from_ports() into sync_periph_from_ports_and_options() and OR the option with "any port is wired to a Fuller joystick", the mirror image of what that function already does for the Kempston Mouse. - Both peripherals declare hard_reset, so the deferred update at the top of retro_run() becomes periph_posthook() rather than periph_update(). periph_posthook() only resets when a peripheral that needs it actually changed, so the Kempston Mouse path through there is unaffected. - fuller_enabled_snapshot() and melodik_enabled_snapshot() re-derive both settings from a loaded snapshot, exactly as kempmouse_snapshot_enabled() does. The existing re-sync call sites in retro_load_game(), retro_unserialize() and the snapshot content path now cover all three. Note that this is a user-visible change for anyone who has been getting AY music out of 48K content: they were getting it by accident, but they will notice it stop. Verified against the real port decoding with Fuse's own readport()/writeport() on a 48K machine rather than by inspecting settings_current: 0x3f reads back 0xff by default and the written AY register once enabled, 0xfffd likewise for Melodik, wiring and unwiring a Fuller joystick toggles the box, and a state captured with the box on does not re-enable it on restore. |
||
|
Lib
|
e9326116ab |
Add AY Volume and Beeper Volume core options
Fuse already carries settings_current.volume_ay and .volume_beeper and applies
both in sound_init() via blip_synth_set_volume() (sound.c:158, 231-239), but
the core never exposed them, so both stayed pinned at 100. Games with an
intrusive AY soundtrack could not be turned down without losing the beeper
effects, and vice versa.
Expose both as 0-100 options in ten-point steps, sharing one value list
between the v2 definitions and the legacy retro_variable strings so the two
cannot drift. Reuse the sound reinitialisation added in the previous commit so
a change takes effect immediately rather than at the next content load.
Read the selected level from the string coreopt() hands back rather than from
its return value. The index is not trustworthy here: coreopt() walks the value
list with strstr() and its acceptance test at coreopt.c:44 is (A || B) where it
should be (A && B), so "10" and "0" both match inside "100" and report index 0.
That bug is pre-existing and also latent in fuse_emulation_speed ("50" inside
"150"), which reads the string for the same reason; it wants fixing separately.
Measured on a 128K machine with a full-amplitude square wave parked on AY
channel A: peak output 5490 at volume 100, 1025 at volume 20, 5456 on the way
back up to 100.
|
||
|
Lib
|
bb1589b73a |
Apply speaker type and AY stereo separation changes without a content reload
settings_current.speaker_type and settings_current.stereo_ay are only ever read by sound_init(), which uses them to pick the Blip_Buffer bass frequency, the synth treble EQ and the number of AY output buffers (sound.c:161-162, 300-332). update_variables() has been writing both fields on every options change since they were added, but nothing tore sound down and rebuilt it, so changing either in the frontend did nothing at all until the next content load. Track the previously selected value for both and, when it changes, arm a flag serviced at the top of retro_run(). Doing it there rather than inline in update_variables() keeps the reinitialisation on a frame boundary and avoids the settings_init() path, which runs update_variables(1) from inside fuse_init() before there is any sound to reinitialise. sound_unpause() declines while a fastload is in progress, so leave the flag armed in that case; the new settings then get picked up either on a later frame or by timer_stop_fastloading(), whichever comes first. Also normalise the two coreopt() results with the "option += option < 0" idiom used everywhere else in update_variables(), so an unset option compares equal to its default rather than looking like a change on the next pass. |
||
|
libretroadmin
|
a183c374f5 |
Fix savestates on machines the content selects, and stop copying unused RAM
Three things, found by adding a savestate round-trip check to the test harness: serialize, run on, restore, re-serialize, and require the two snapshots to match. retro_serialize_size() keyed its fixed-size table off `machine`, which only ever tracks the fuse_machine core option. Content routinely selects something else - a 128K or +3 snapshot switches machine on load, utils_open_file() promotes to TC2068 for a Timex cartridge, a +3 disk pulls in the +3 - and `machine` is never updated when that happens, while machine_current is. Loading libspectrum's own plus3.z80 with the option left at 48K reported 2*64K for a snapshot that needs 2*128K, so retro_serialize() rejected every call with "Snapshot size is larger than fixed size" and savestates and rewind silently did nothing. Key the table off machine_current->machine instead. plus3.z80 now reports 262144 and round-trips. memory_to_snapshot() copied all 64 RAM pages under "if( RAM[i] != NULL )", but RAM is a two-dimensional array, so RAM[i] is the address of a row and is never NULL - the guard was dead and every snapshot allocated and memcpy'd a megabyte regardless of the machine. Both the szx and .z80 page writers pick their pages from the machine capabilities and skip anything the snapshot does not carry, so the surplus was built and thrown away. On the libretro side that is paid once per frame while rewinding. Copy only the pages the machine has, derived the same way the writers derive theirs. Measured over 2000 calls: 580 -> 43 us for the +3, 480 -> 21 us for a 48K tape, with the snapshots byte-identical. data_add() memcpy'd from `data` whenever `buffer` was NULL, and trackgen() always passes data as NULL. Every call site supplies exactly one of the two, so this never fired, but nothing enforced it - which is what GCC reports when it inlines the trackgen() path and cannot prove buffer is non-NULL. Treat "no source at all" as no data available and let the existing autofill handle it. That leaves two warnings in a clean build, both unused-symbol noise. Verified over the full 106-input corpus under ASan, UBSan and LeakSanitizer with every input still loading; a corpus-wide round-trip sweep where every input now serializes, restores, and re-serializes byte-identically, and running the same frames from a restored state lands in the same state as running them from the original; and a two-instance run. One case is still outside the fixed-size path: a snapshot carrying ZXATASP or ZXCF RAM needs far more than the machine alone implies - ZXAT.szx builds to 574062 bytes - and the fixed table only knows about machines. It is correct under the default auto-sized savestates and was equally broken before this change; sizing the fixed path to cover peripheral RAM is a trade against rewind memory for everyone using it, so it is left alone here. |
||
|
libretroadmin
|
17e17754e9 |
compat: match the content wildcard by name, not by leading '*'
The core stands the loaded content in for a real file by handing utils_open_file() a synthetic name of the form "*.ext", and find_entry() recognised it by testing path[0] alone. Any path beginning with '*' therefore resolved to the content buffer. fuse looks for a sidecar poke file by swapping the caller's extension for ".pok", so utils_open_file() probes "*.pok" on every snapshot and tape load, and that probe matched: Checking if "*.pok" exists Opened "*.pok" from memory compat_file_exists() reported a poke file that does not exist, and pokemem_find_pokfile() recorded it. Nothing in this frontend calls pokemem_autoload_pokfile(), so it is never parsed today and nothing leaks - but had it been, the content would have been handed to the .pok parser as its own poke file. Record the synthetic name in tape_wildcard when it is built, clear it when the content is released, and compare against it exactly. The two places that construct it - the initial load and retro_reset() - both set it, so a reset re-establishes it. Paths that do not begin with '*' are untouched, which leaves sidecar lookups against the real filenames an M3U supplies working as before, along with the baked-in ROM and tape assets. After this, the four probes fuse makes - "*.pok", "*.POK", "POKES/*.pok" and "POKES/*.POK" - all correctly report that no poke file exists, while "*.tap", "*.z80" and friends still open from memory, including across retro_reset(). Verified over the full 106-input corpus under ASan, UBSan and LeakSanitizer with every input still loading, the M3U disk-swap path, and a two-instance run, since tape_wildcard is new global state. |
||
|
libretroadmin
|
ca1ad5d8b8 |
pokemem: fix .pok parser overreads, and drop scanf from the core
The .pok reader is reached from utils_open_file(), so it sits on the same
path as content loading, and it parses the file with the buffer utils_read_file()
allocated - which is exactly the file length, with no terminator of its own.
Two places relied on one being there.
pokemem_read_trainer() trimmed trailing whitespace starting from the
character cpos points at. For a trainer line that runs to the end of the
file, cpos is one past the buffer, so isspace() read it:
ERROR: AddressSanitizer: heap-buffer-overflow
READ of size 1
#0 pokemem_read_trainer fuse/pokefinder/pokemem.c:248
#1 pokemem_read_from_buffer #2 pokemem_read_from_file
Start the trim from the last character that is actually part of the line
instead. A line ending in a terminator behaves as before; one ending at EOF
no longer reads past it.
pokemem_read_poke() called sscanf() straight off the same unterminated
buffer. A trailing poke line therefore had nothing to stop the scan at the
end of the file. This one is invisible to ASan - the scan happens inside
libc, which is not instrumented - but it is a read past the allocation all
the same.
Rather than copy each line into a scratch buffer to give sscanf something to
stop on, parse the fields directly with an explicit end pointer. That also
takes the last scanf call sites in the core with it:
fuse/fuse.c creator_init(), VERSION as "%u.%u.%u.%u"
libspectrum.c libspectrum_check_version(), twice
szx.c read_crtr_chunk(), the creator string
The two dotted-version forms share a new libspectrum_parse_dotted_version();
fuse/fuse.c keeps a local copy rather than widen the public header for one
caller. None of these sites is hot - they all run once per load or once per
init - so this is not a throughput change. What it buys is code size and
locale independence: sscanf drags the formatted-input machinery into
statically linked builds, which is most of the libretro targets, measured
here at ~130K on a glibc static link. The core no longer references the
scanf family at all.
Behaviour is unchanged. A differential harness compares the old sscanf forms
against the replacements over the accept/reject outcome that
pokemem_poke_add() actually applies: 20 poke-line cases and 16 version
strings in both the three- and four-component forms, no differences. The one
raw-field divergence, a negative address, is rejected by both paths - by the
range check with sscanf, by the field count without it. 400 random .pok
files through the parser find nothing, and the full corpus of 106 tape,
snapshot, disk, zip and m3u inputs stays clean under ASan, UBSan and
LeakSanitizer.
|
||
|
libretroadmin
|
4dc35318e3 |
Fix .sp heap overflow and RZX playback leaks
Second pass over the core, driving every libspectrum reader directly with the type forced rather than only the one identification picks. That exposed the .sp reader, which nothing in the test corpus had ever reached. libspectrum/sp.c: three bugs in libspectrum_sp_read(). The memory dump is copied into a 48K buffer covering 0x4000..0xffff, but the copy indexed that buffer with the dump's raw Z80 start address instead of an offset from the start of RAM. A dump of the full 48K from 0x4000 - which is what an ordinary .sp contains - therefore wrote 0xc000 bytes at offset 0x4000 of a 0xc000 buffer, running 16K past the end. The same mistake also placed every .sp's contents 16K higher in memory than they belong, so the format cannot have been loading correctly either. The dump was not required to be present in the file. Only the header length was checked, so a 38-byte .sp declaring a 32K dump copied 32K of whatever followed the file in memory into the emulated RAM, where it is visible through the memory browser and gets written back out into snapshots. SP_HEADER_LENGTH was 37, but the header is 38 bytes: signature, memory length and start address (6), thirty bytes of registers, then the flags word. A 37-byte file read the flags word one byte past the end. Also reject a start address below 0x4000. Nothing checked it, and the rebased index needs it to avoid wrapping. fuse/rzx.c: rzx_start_playback_from_buffer() returned without releasing the libspectrum_rzx it had just allocated when the recording failed to parse - the one failure path in that function that did not. rzx_start_playback() had the same omission in three places: an unreadable file, a recording that would not parse, and a missing external snapshot. The rzx global is left dangling by the existing free paths too, but rzx_playback is only set after a successful start, so nothing reads it in those cases. Verified with ASan+UBSan+LeakSanitizer over 106 inputs covering tape, snapshot, disk, zip, m3u and newly built rzx, sna, snp, sp and zxs seeds; mutational fuzzing of the parsers with every reader type forced; fuzzing of the new formats through retro_load_game(); and a two-instance run. A functional check confirms .sp contents now land at the addresses they name across all three 16K pages. The release build is unchanged at its five pre-existing warnings. |
||
|
libretroadmin
|
2455c615ee |
Fix memory corruption, out-of-bounds access and leaks across the core
Omnibus of eleven memory-safety fixes found by auditing the core under ASan/UBSan/LeakSanitizer with a libretro harness driving the full content lifecycle, plus mutational fuzzing of the libspectrum parsers and of the disk/zip/m3u paths through retro_load_game(). Every item below has a reproducer; all of them are reachable from ordinary use or from loading a file with one of the extensions we advertise. libretro.c: spectrum_key_from_option() recursed into itself instead of indexing spectrum_keys_map[], so it never terminated. update_variables() calls it fifteen times and settings_init() calls update_variables(1) during startup, making this unconditional. It escapes casual testing because coreopt() returns -1 when the frontend does not answer GET_VARIABLE, which takes the early bounds-check return; it only triggers once a frontend supplies a configured value. At -O3 the self-call is a tail call and becomes an infinite loop, so release builds hang where debug builds overflow the stack. libretro.c: retro_cheat_set() took the bank and address straight from the cheat text via strtol() and neither was bounded. RAM is [SPECTRUM_RAM_PAGES][0x4000], so an out-of-range bank read and wrote a byte at bank * 0x4000 + (address & 0x3fff) past the start of the array - a controlled write into whatever global follows it (bank 65 lands in tape.c's rec_state). The address was also left unmasked on the read side while the write masked it. Bank 8 keeps its "ignore bank" meaning and its full 16-bit Z80 address; only the direct RAM page path is masked to 0x3fff. retro_cheat_reset() gets the same guard, since poke_t::bank is a byte and can still hold 65..255. kempmouse.c: kempmouse_ports[] had no zero-mask terminator, and periph_activate_type() walks the table until it finds one. Enabling the Kempston mouse therefore ran off the end of the table into the next global and registered those entries as ports, function pointers included. All 44 port tables in the tree were checked; this was the only one affected. szx.c, zxcf.c, zxatasp.c: the ZXCF and ZXATASP chunk readers stored the page count as a raw byte from the file, but it indexes zxcf_ram[64] / zxatasp_ram[32], and *_from_snapshot() memcpy()s a page through every pointer up to that count into ZXCFMEM[64] / ZXATASPMEM[32] - out of bounds on both source and destination. The chunk readers already range-check the page index; only the count was unchecked. Rejected at the read boundary and clamped in both consumers. z80.c: read_header() took no buffer length and indexed the header unconditionally. Beyond short files, a spec-complete 55-byte v2 snapshot overran by 28 bytes, because extra_header[51] (MGT type) and extra_header[27] (+D paged) are v3-only fields but were read for every version against a 23-byte v2 extended header. The length is now threaded through and validated at 30, 32 and 32 + extra_length, and the two v3-only fields are guarded. Separately, read_v2_block() read three header bytes when read_blocks() only guarantees buffer < end. csw.c: the v2 header extension length was subtracted in the bounds check and in the length decrement, but added to the pointer. The buffer advanced 29 + ext bytes while length only dropped by 29 - ext, so 2 * ext bytes of heap past the end of the file were described as tape data and copied into the block. Also cast the sample rate bytes before shifting. zlib.c: three bugs in the gzip header. skip_null_terminated_string() tested **ptr before *length, so a string running exactly to the end of the buffer was read one byte past it; the extra field was validated but never skipped, so a following FNAME or comment was parsed out of the middle of it; and the header CRC branch decremented the pointer a second time instead of the length, leaving the CRC bytes in front of the deflate stream. zip.c: libspectrum_zip_read() returned on a CRC mismatch without freeing the decompressed member. The caller cannot know a buffer was produced on a failure path, so the whole allocation leaked. warajevo_read.c: get_next_block() freed the tape on a block error, but the tape belongs to the caller - every other reader in libspectrum just returns the error. This was a double free for any caller that frees it itself, and in fuse it is worse: tape.c holds the tape in a file-scope global allocated once during init, so a malformed Warajevo tape left that pointer dangling for the rest of the session. Its bounds check also only demanded 8 bytes while the function reads to offset + 9 and read_rom_block() / read_raw_data() go on to index as far as offset + 16 before checking anything themselves; both now validate their own reach. warajevo_read.c, tape.c, mmc.c, display.c: libspectrum_byte promotes to int, so shifting a byte of 0x80 or above left by 24 overflows the sign bit. Cast before shifting when assembling dwords from bytes. The display.c instance is not limited to malformed input - data4 there is a screen byte on the Pentagon 16-colour path, so it fires during ordinary emulation. Verified with ASan+UBSan+LSan over 73 tape, snapshot, disk, zip, m3u and crafted inputs; three fuzzer seeds of 45k iterations each over the libspectrum parsers; mutational fuzzing of the disk and zip/m3u paths; a TSan pass; and a two-instance test modelling runahead's second core instance, including cross-instance state transfer. All clean, no leaks. The release build is unchanged at its five pre-existing warnings. |
||
|
LibretroAdmin
|
08ffef072d |
libspectrum: avoid log(0) when a TZX 0x19 data alphabet is empty
bits_per_symbol for a Generalised Data Block (TZX ID 0x19) was computed as ceil( log( symbols_in_table ) / M_LN2 ) without guarding symbols_in_table == 0, so an empty data alphabet evaluated log(0) (undefined) and cast the resulting -inf to int. Compute 0 bits directly in that case; an empty alphabet carries no data symbols, so downstream data_count is 0 and the existing bounds check is unaffected. The one-symbol case still yields 0 bits, matching the TZX specification. |
||
|
LibretroAdmin
|
24fe44eace |
libspectrum: fix NULL/short symbol-table crashes in TZX 0x19 blocks
A malformed Generalised Data Block (TZX ID 0x19) can drive the tape state
machine into DATA1 with data_table.symbols == NULL, after which
set_tstates_and_flags() dereferences symbol->lengths and faults on the
near-NULL address 0x8 (the offset of the lengths pointer in
libspectrum_tape_generalised_data_symbol on LP64). Two independent paths
reach this:
1. Empty data table (TOTD == 0): read_symbol_table() allocates nothing,
but generalised_data_edge()'s PILOT->DATA1 transition is taken
unconditionally once the pilot run is exhausted. The subsequent
get_generalised_data_symbol() then also walks block->data with a
bits_per_symbol derived from log(0), overrunning the buffer.
2. Truncated data table (TOTD > 0 but body too short):
tzx_read_generalised_data() ignored the data-table
read_symbol_table() return, so the block was appended with
symbols_in_block > 0 and symbols == NULL. Playback then indexes
table->symbols[current_symbol] and faults at 0x8.
Fixes:
- Check the data-table read_symbol_table() return, mirroring the pilot
table, so a corrupt table rejects the file at parse time (path 2).
- In the PILOT->DATA1 transition, fall through to PAUSE when the data
table / data buffer is absent instead of entering DATA1 (path 1).
- Guard the DATA1 edge itself against a NULL data symbol table as a
belt-and-suspenders backstop.
Reproduced and verified fixed under ASan/UBSan with two crafted fixtures
(the exact 0x8 SEGV for path 2, a heap-overflow for path 1); both are
clean after the fix.
|
||
|
libretroadmin
|
ecfee0006d |
Close disks left in drives when the drive list is torn down
ui_media_drive_end() freed the list of registered drives and left whatever
was in them alone. disk_close() is called from exactly one place,
ui_media_drive_do_eject(), so a disk still inserted at shutdown had its
image buffer abandoned.
That is harmless in a process about to exit, which is why upstream has
never needed it, but a libretro core tears the machine down and rebuilds
it on every content load. Eighteen cycles rotating through six +3 images
leaked 12MB:
Direct leak of ... allocated from:
disk_open fuse/peripherals/disk/disk.c
specplus3_disk_insert fuse/machines/specplus3.c
ui_media_drive_insert fuse/uimedia.c
Close each drive's disk before dropping the list. The same eighteen cycles
are clean afterwards.
This went unnoticed until now because nothing in the test set ever reached
a drive: the M3U fixtures were zero-filled files that failed to open, and
IPF images were classed as generic disk content and never inserted. The
preceding commit fixed the latter, which is what exposed this.
|
||
|
libretroadmin
|
106c8addc6 |
Insert IPF images into the +3 drive
IPF was classed as DISK_GENERIC, matching UDI and FDI. That class does not
select a machine: utils_open_file() hands the image to whichever drive is
already live, and if the current machine has no disk drive and no Beta,
DISCiPLE or +D is active, it silently hands it to nothing. Loading an IPF
on the default 48K therefore decoded the image, reported success and never
inserted it.
[core INF] Opened "48.rom" from memory
[core INF] Opened "*.ipf" from memory
== retro_load_game: OK ==
DISK_PLUS3 is the class that switches machine first:
if( !( machine_current->capabilities &
LIBSPECTRUM_MACHINE_CAPABILITY_PLUS3_DISK ) ) {
error = machine_select( LIBSPECTRUM_MACHINE_PLUS3 );
and it is the right one here. Every Spectrum IPF is a +3 disk - the +3 is
the only Spectrum with a drive that writes this format - and the images
say so themselves, declaring platform 5 in their INFO chunk. All seven to
hand now bring up a +3 and load its ROMs.
The 80-track probe needed adjusting to suit. It read byte 0x30 of any
image that arrived as +3 disk content, which is the track count in a .dsk
header and arbitrary bytes in anything else, so an IPF could have flipped
the drive to double-sided 80 track on the strength of a byte that means
nothing. It now reads the .dsk header only for .dsk, takes the highest
cylinder from the INFO chunk for IPF, and bounds-checks both - the old
read had no length guard at all, so a +3-class file shorter than 49 bytes
read past the end of the buffer.
|
||
|
libretroadmin
|
c710ea4ce3 |
Mark the C2 index-mark sync in IPF images
The IPF decoder recognised 0x4489 - the A1 ID/data-mark sync with its
clock bit suppressed - and recorded the missing clock in the clock
bitmap, but not 0x5224, which is the C2 index-mark sync and has its clock
suppressed in the same way. The C2s were being written as ordinary data
bytes.
Nothing reads them today: sector lookup keys off A1 followed by 0xFE, and
fuse's own track builder writes an A1 sync ahead of the 0xFC index mark
rather than the C2 a real controller lays down, so no code path looks for
one. It is still the wrong representation, and cheap to get right.
Six +3 images that exercise it, four of which carry index marks:
Badlands 40 tracks 360 sectors 0 IAM 0 deleted DAM
Beverly Hills Cop 40 tracks 360 sectors 40 IAM 4 deleted DAM
Bloodwych 40 tracks 360 sectors 40 IAM 6 deleted DAM
Boggit, The 40 tracks 360 sectors 40 IAM 0 deleted DAM
Book of the Dead 40 tracks 360 sectors 40 IAM 0 deleted DAM
Buffalo Bill's 40 tracks 360 sectors 0 IAM 0 deleted DAM
Every track decodes to exactly the length its IMGE chunk declares and
every ID field matches the cylinder and head recorded there, so the
layout is being reproduced rather than approximated. The deleted data
address marks in two of them are the copy protection, and they survive
the round trip.
ASan, UBSan, LeakSan and TSan clean on all six loaded singly, on a
three-image M3U, on eighteen reload cycles rotating through them, and on
78 fuzz cases built by truncating each image at five lengths and
corrupting each at eight random byte positions.
|
||
|
libretroadmin
|
888a4cf2cb |
Add support for IPF disk images
IPF is the Software Preservation Society's format, and the only one the five ZX Spectrum releases I have to hand are distributed in. Nothing in the libretro core, upstream Fuse, libspectrum or the FuseX fork could read it: libspectrum has no signature for the format, so identify returned UNKNOWN and the core's TRD fallback claimed the file. The previous commit made that fail cleanly rather than booting to BASIC and reporting success; this one loads it. The format stores a track as a list of blocks, each a data description followed by a gap. The data description is a stream of elements holding either raw MFM cells - used for the address marks, whose clock pattern is deliberately illegal - or already-decoded bytes that a reader is expected to MFM-encode. That maps onto the byte-plus-clock-mark representation this file already uses for UDI, so the decoder needs no cell-level intermediate: a raw 0x4489 becomes 0xA1 with its clock bit set, everything else becomes itself, and weak sections set the weak bitmap. Gaps carry no recoverable information, so only their length and fill byte are taken from the gap description. Derived from the format description in MAME's src/lib/formats/ipf_dsk.cpp by Olivier Galibert, BSD-3-Clause, which is compatible with the GPL this file is under. No SPS library is needed at build or run time. libspectrum gains the CAPS signature and an id, appended to the enum so existing values keep their numbering. The class is DISK_GENERIC, matching UDI and FDI, so the +3, +D and Beta paths can all take one. Decoding Badlands' first track gives 6456 bytes - exactly the size_cells the image declares - carrying nine 512-byte sectors with the IDAMs a reader recovers as cyl 0, head 0, sectors 1 to 9: a standard +3 layout. All five images load, singly and through an M3U, under every machine model. ASan, UBSan, LeakSan and TSan are clean on those, on twelve reload cycles interleaving IPF with snapshot and M3U content, and on 32 fuzz cases built by truncating and corrupting a real image at 7 lengths and 25 random byte positions. Snapshot, tape and existing disk content is unaffected; the win64 target builds with no new warnings. Read-only: disk_write() has no IPF case, so these images cannot be saved back, which is the right default for preservation dumps. Cell-level timing and the exact gap bytes are not reproduced either, so protections that measure them rather than reading sectors will still fail. |
||
|
libretroadmin
|
02ecda17be |
Reject IPF disk images instead of loading them as raw TRD
identify_file() falls back to LIBSPECTRUM_ID_DISK_TRD for anything
libspectrum cannot name, so that anonymous TRD images still work.
libspectrum has no signature for IPF, so every IPF image took that
fallback: the core reported the load as successful, switched to a
Scorpion 256, failed to find 256s-0.rom and dropped the user at a 48K
BASIC prompt with nothing to say about why.
[core INF] Opened "*.trd" from memory
[core ERR] Could not find file "./fuse/256s-0.rom" on the file system
fuse: error: couldn't find ROM '256s-0.rom'
== retro_load_game: OK ==
Reporting success for content that did not load is worse than refusing
it. Check for the CAPS chunk id the format opens with and fail the load
with a message naming the format.
M3U entries get the same treatment, by magic rather than by extension so
a renamed file is still caught. utils_open_file() would reject an IPF
there too, but only after it had been counted as a disk and registered
with the frontend's disk control, leaving a playlist slot that can never
be inserted; a mixed playlist now keeps its loadable entries and drops
the rest.
This does not add IPF support - there is none to add. Upstream Fuse,
libspectrum and the FuseX fork all lack it, and the format has no free
reference implementation to vendor. Supporting it means either dlopening
the SPS capsimg library at runtime or writing a decoder from MAME's
ipf_dsk.cpp, and both are a larger decision than this.
Verified against five ZX Spectrum IPF images (SPS encoder, 42 cylinders,
both heads, 84 IMGE/DATA chunk pairs each): all five now fail cleanly,
an all-IPF playlist yields no disks, and a mixed playlist keeps the two
loadable entries. Snapshot, tape and M3U content is unaffected, as are
twelve reload cycles interleaving a rejected IPF with content that
loads. ASan, UBSan, LeakSan and TSan clean throughout; win64 builds with
no new warnings.
|
||
|
libretroadmin
|
5d792f6851 |
Fail the serialize when the snapshot could not be built
snapshot_update() returns non-zero when the SZX write fails or the buffer cannot be allocated, and on the allocation path it frees snapshot_buffer and zeroes snapshot_size on the way out. Both callers threw the result away. retro_serialize() then memcpy'd zero bytes, filled the caller's entire buffer with 0xFF padding and returned true - a savestate that reports success and contains nothing at all. retro_serialize_size() returned the zeroed size, which a frontend reads as "this core cannot serialize". Check the return value in both and report the failure instead. |
||
|
libretroadmin
|
7b6f3fa5b4 |
Track the snapshot buffer's capacity separately from its length
snapshot_update() used snapshot_size for two different things: the number
of bytes allocated in snapshot_buffer, and the length of the snapshot
currently held there. The growth test is
if (snapshot_size < length) { ...realloc...; snapshot_size = length; }
memcpy(snapshot_buffer, buffer, length);
so an SZX that comes out smaller than the previous one skips the branch
entirely and leaves snapshot_size at the older, larger value while only
length bytes are refreshed. snapshot_size is what retro_serialize() copies:
memcpy(data, snapshot_buffer, snapshot_size);
memset(data + snapshot_size, 0xFF, size - snapshot_size);
so every state written after a shrink carries the tail of a previous,
larger snapshot. That is exactly what the 0xFF padding on the next line
exists to prevent - its comment notes that frontends keep passing the size
from the first retro_serialize_size() call "even after the SZX shrinks",
and then the padding starts too late to cover the stale bytes.
retro_serialize_size() also becomes a high water mark that never comes back
down.
Keep the allocated size in its own variable and always record the real
length. The realloc is additionally forced when snapshot_buffer is NULL,
since retro_unload_game() and retro_load_game() clear the pointer without
the capacity being visible to them.
Verified as no change in behaviour where the length is stable: snapshot
rounds, replay determinism, the fixed-size savestate path and twelve reload
cycles are all unaffected, ASan/UBSan/LeakSan clean, and the win64 target
builds.
The shrink itself was not reproduced locally. In this build the SZX length
turns out to be constant for a given machine - 49406 bytes on a 48K, 131433
on a 128K - which is uncompressed RAM plus chunk headers, so nothing varied
enough to trigger it with the content available. The defect is plain in the
code regardless, and the comment quoted above indicates it has been seen in
the field.
|
||
|
libretroadmin
|
23c6f71a7a |
Release the scaler name when startup fails early
fuse_init() copies settings_current.start_scaler_mode and holds the copy
until scaler_select_id() consumes it. Two failure paths return in between
without freeing it:
start_scaler = utils_safe_strdup( settings_current.start_scaler_mode );
...
if( run_startup_manager( &argc, &argv ) ) return 1;
error = machine_select_id( settings_current.start_machine );
if( error ) return error;
error = scaler_select_id( start_scaler ); libspectrum_free( start_scaler );
The machine_select_id() one is reachable from the frontend: the Pentagon
and Scorpion models need ROMs installed in system/fuse, and selecting one
without them fails there.
Direct leak of 7 byte(s) in 1 object(s) allocated from:
utils_safe_strdup fuse/utils.c:459
fuse_init fuse/fuse.c:388
Upstream leaks it once and exits. A core retries on the next content load,
so it accumulates for as long as the model stays selected - every attempt,
for as long as the user leaves the option set to a machine they have no
ROMs for.
Found by rotating the model core option across all sixteen machines over
repeated load cycles. With this, thirty-six cycles covering every model
that has built-in ROMs, interleaved across snapshot, tape and M3U content,
complete with no leaks and no sanitiser reports; so does repeatedly
retrying a model whose ROMs are missing.
|
||
|
libretroadmin
|
e44ff7ff04 |
Free the +3 floppy controller on machine shutdown
specplus3_765_init() allocates a upd_fdc and stores it in a file-scope
pointer. specplus3_shutdown() returned 0 and did nothing else, so nothing
ever released it: each machine table build allocated a new one and
abandoned the previous.
Direct leak of 320 byte(s) in 1 object(s) allocated from:
upd_fdc_alloc_fdc fuse/peripherals/disk/upd_fdc.c:387
specplus3_765_init fuse/machines/specplus3.c:151
specplus3_init fuse/machines/specplus3.c:130
machine_add_machine fuse/machine.c:123
Free it in the shutdown hook. Both the +3 and the +3e install that same
hook, so machine_end() calls it once per machine; clearing the pointer
makes the second call a no-op rather than a double free.
With this and the startup-manager registration fix, twenty consecutive
load/unload/load cycles complete with no leaks and no sanitiser reports at
all.
|
||
|
libretroadmin
|
0b77517ce1 |
Register the keyboard and joystick end functions in the right slot
startup_manager_register() takes the init function, then the context
pointer it should be called with, then the end function:
void startup_manager_register(
startup_manager_module module, startup_manager_module *dependencies,
size_t dependency_count, startup_manager_init_fn init_fn,
void *init_context, startup_manager_end_fn end_fn );
keyboard.c and joystick.c passed their end function where the context
belongs and NULL where the end function belongs, so neither was ever added
to the startup manager's end list. Both compile without complaint: the
context is a void * and the end function converts to one silently.
The effect is invisible in upstream Fuse, which runs the end functions once
at exit and then exits, but a libretro core tears down and re-initialises
whenever content is loaded. keyboard_init() builds four hash tables and
keyboard_end() destroys them, so a reload orphaned all four:
Direct leak of 48 byte(s) in 1 object(s) allocated from:
g_hash_table_new libspectrum/myglib/ghash.c:89
keyboard_init fuse/keyboard.c:280
Traced by counting calls: three keyboard_init(), zero keyboard_end(), with
the end list holding 27 entries and neither of these among them.
Swapping the arguments back accounts for eight of the nine allocations that
leaked per load/unload/load cycle. The other 54 registration sites were
checked mechanically; these two were the only ones with the arguments the
wrong way round.
|