The worker reads a queued CURL* out of a pipe eight bytes at a time, tracking
how much it has so far - but it always read into the start of the buffer rather
than at that offset. A short read would have resumed mid-pointer and eventually
handed curl_multi_add_handle a spliced pointer. It never bit because a write
that size to a pipe is atomic, so reads are all-or-nothing, but the code is
written as though it isn't.
The write that queues the request was unchecked. If it ever failed, the request
was never run and never completed, and the caller sits in naettComplete
forever. It reports the failure now, and retries on EINTR.
curl wants an easy handle out of its multi before cleanup; that needs
curl_multi_remove_handle, which meant adding it - and curl_multi_cleanup, for
the init failure paths that leaked the multi handle and the pipe - to the dlopen
table.
workerRunning is written by the worker and read by the request path, so it's an
atomic rather than a plain int. And the read/write callbacks passed the body
callbacks' int return straight back to curl, which takes a size_t: a negative
came through as an enormous count rather than the error it was.
WinHttpQueryHeaders only writes the size it needs when it fails with
ERROR_INSUFFICIENT_BUFFER. Any other failure left bufSize at zero, so we
allocated nothing and unpackHeaders walked wcslen over it looking for the
double-null that terminates the list. Check the size, check the second query,
and allocate zeroed with room for a terminator.
winToUTF8, winFromUTF8 and wcsndup can all return NULL - on a failed conversion
or a failed allocation - and not one caller checked. packHeaders is the one that
mattered: its result goes straight into headers[0].
res->bytesLeft is a size_t counting down from what WinHTTP announced. If a read
ever returned more than that, it wrapped to an enormous count and the loop kept
reading.
createDelegate built its NSURLSessionDataDelegate with objc_allocateClassPair
and then sent it +alloc without ever calling objc_registerClassPair. The runtime
requires registration before a class pair can be used; everything up to then is
still being assembled. Register it, after the methods and the ivar go on.
The response header arrays were VLAs sized from the count the server sent, so a
response with enough headers walked the stack off the end, and one with no
headers at all declared zero-length VLAs, which is undefined by itself. Heap
now, and skipped when there's nothing to read.
The NSURLSession was also kept in the response without retaining it, while the
autorelease pool it came from is drained on the way out of the function. It only
survived because a session keeps itself alive while it has tasks running. Retain
it, release it when the response closes.
Finally, addMethod/addIvar signalled failure with assert alone, which is
compiled out in release - a delegate missing its methods would just never
receive data and the request would hang with nothing logged.
naettFree frees the method and url it strdup'd but never the user agent, which
stringSetter allocates exactly the same way. We set a user agent on every
request, so that leaked on every one of them, on all platforms.
On Linux, headerCallback strndup's each header line and only hands it to the
header list when it finds a colon - the status line and the blank line that ends
the block don't have one, so it leaked those every response, and again per hop
when following redirects.
defaultBodyWriter grew its capacity by doubling an int until the new data fit.
Both the length and the resulting capacity come from the response, so that's
signed overflow on a large one, and a negative capacity then reaches realloc as
a huge size_t. It also assigned the realloc result straight over the old
pointer, so a failed allocation lost the buffer and the memcpy below went
through NULL. Grow in int64_t, cap at INT_MAX, and report failure by returning
short - which is what every caller already checks for.
Centralizing command line parsing replaced the hand-rolled --gamesettings and
--touchscreentest argv checks in NativeInit with a single --start-screen=<name>
option, and made an unrecognized "--" argument a hard parse error. The two
System_RestartApp callers still passed the old flags.
So changing the graphics backend killed PPSSPP for good: the new process starts,
fails to parse --gamesettings, and returns 1 from WinMain before a window ever
exists. The error goes to stderr, which nobody sees in a GUI build, so it just
looks like the app quit instead of restarting.
Restarts that pass no arguments (the memstick screen, and the edit-then-restore
path) were unaffected, since an empty argument string makes ExitAndRestart reuse
the original command line.
Also move the TouchTestScreen push inside the touchscreentest branch - it looks
like a brace that didn't move during the refactor, and it would otherwise push a
touch test screen for every --start-screen value.
--state could load a savestate but nothing could produce one without a
GUI, so savestate bugs couldn't be reproduced or regression-tested from
a script. This saves one partway through the run, once the game is
actually up.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01JvJR8oJNSCimCM9KXVLjfq
Three ways our rename differed from the PSP's:
- A wildcard in either path was passed through to the host, so
renaming "test*.txt" could quietly rename a real file. The PSP
doesn't expand them here, it rejects them outright.
- Renaming onto a file that already exists succeeded, because the host
rename() replaces the destination. The PSP refuses, and renaming a
file onto itself counts as that too.
- Crossing devices returned the right error, but after the same wait
as everything else. The hardware fails that one immediately.
Fixes io/file/rename, moved to tests_good.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01JvJR8oJNSCimCM9KXVLjfq
GetFreeChannel counted down with an unsigned loop variable, so i >= 0
was always true. With every channel already reserved it wrapped past
zero and kept indexing g_audioChans until it walked off the end -
sceAudioChReserve(-1, ...) segfaulted the emulator instead of returning
"no channels available". Reproduces on audio/sceaudio/reserve, which
crashed before printing anything.
Also gives sceVaudioChReserve the parameter checks it never had. It
took any sample count, channel count and frequency; the hardware allows
256, 1024 or 2048 samples, stereo only, and the same sample rates the
SRC channel accepts. Every value in the test now matches - what's left
there is only reschedule markers.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01JvJR8oJNSCimCM9KXVLjfq
Two things utility/systemparam caught:
A negative size passed to sceUtilityGetSystemParamString went through
Memory::IsValidRange, where it became an enormous range and came back
as a generic -1. The PSP just reports that the string doesn't fit, same
as any other size too small to hold it.
sceUtilityGetSystemParamInt returned 0x800ADF4 for an automatic adhoc
channel unconditionally. The FIXME there wondered whether the hardware
only does that once adhocctl is initialized - it does. Before any adhoc
module is up, which is the state nearly every game asks this in, the
hardware returns 0 and writes the channel out.
Fixes utility/systemparam/systemparam, moved to tests_good.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01JvJR8oJNSCimCM9KXVLjfq
Follow-up to #22208, which bounded the index table but left the frame
size itself unbounded - readBuffer and zlibBuffer are sized straight
from it, so a 96-byte header could still ask for a couple of gigabytes.
Harmless enough on 64-bit, where the pages never get touched, but a
32-bit build would just fail the allocation.
Real images use 2KB to 64KB frames, so 16MB leaves plenty of headroom.
All 19 CSOs I have on hand still load.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01JvJR8oJNSCimCM9KXVLjfq
They record whether a reschedule happened while the code under test
ran, so a diff where only the marker differs is a scheduling
difference, not a wrong value - worth knowing before going looking for
a value bug that isn't there.
Also fixes the O/E description, which had them the wrong way round: O
is PPSSPP's output, E is the expected file.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01JvJR8oJNSCimCM9KXVLjfq
31 languages, following each file's existing chat terminology (chatt,
bate-papo, obrolan, sembang, 聊天, الشات, ...). The rest are left to fall
back to English rather than guessed at.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01JvJR8oJNSCimCM9KXVLjfq
Identify_File and friends compare against lowercase extensions, the way
Path::GetFileExtension returns them, but ZipFileLoader passed the name
from the zip through unchanged. So a file stored as e.g. "DUMP.PPDMP"
inside a zip failed to load, while "dump.ppdmp" worked.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01JvJR8oJNSCimCM9KXVLjfq
The log was trimmed to 50 lines, which isn't enough to scroll back
through a conversation - now 250. Chat entries also carry the time they
arrived, shown as a dimmed HH:MM in front of the name, behind a new
"Show timestamps in chat" setting.
The timestamp is kept next to the text rather than baked into it, since
the chat view finds the sender by splitting the line at the first colon.
Fixes#15444
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01JvJR8oJNSCimCM9KXVLjfq
The generic Android pad map and the Retroid map were the only pad
defaults without it, so the right stick did nothing until mapped by
hand. Uses the same axes and directions as the desktop pad default.
Fixes#21591
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01JvJR8oJNSCimCM9KXVLjfq
The last platform that was missing this - it was held back waiting for
the SDL 3.0 migration, which has now happened. Without it, whatever was
held when the pad vanished stays held, which tends to walk the player
off a cliff.
Fixes#20418
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01JvJR8oJNSCimCM9KXVLjfq
These two were lower case for legacy reasons, unlike every other
directory, which actually matters on case sensitive file systems like
the one iOS uses. An existing lower case directory is still used if
there's no upper case one, so nobody has to move their files.
Fixes#20527
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01JvJR8oJNSCimCM9KXVLjfq
Previously the cursor was only hidden when entering fullscreen or
starting a game, and never came back on mouse movement. Now it hides
half a second after the mouse stops moving and reappears when it moves
again, which also covers the touch-controls case that used to be
excluded entirely.
Fixes#20572
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01JvJR8oJNSCimCM9KXVLjfq
Three frames in flight showed no performance benefit over two, so drop
the option and clamp existing configs (and the default) down to 2.
The now-unused "Up to 2" translation keys are left in place.
Fixes#21419
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01JvJR8oJNSCimCM9KXVLjfq
Choices that are just numbers have no translation, so every lookup got
logged as a missing translation - the Graphics settings screen spammed
the log once per frame as long as Frame Skipping was on.
Fixes#21455
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01JvJR8oJNSCimCM9KXVLjfq
* Destroy the ImGui debugger explicitly, not at static destruction time
* Windows: destroy the debugger windows before NativeShutdown()
* Clear the disassembly cache where it goes stale, not in view destructors
(behind a no-op stub for libretro, which doesn't build DisassemblyManager.cpp)
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
ReadFile and the read-path-table ioctl both read a sector into a stack buffer
and memcpy it to the destination without checking whether the read succeeded.
FileBlockDevice::ReadBlock returns false on a short read and leaves the buffer
untouched, so a read past the end of a truncated or crafted image copies 2KB of
uninitialized host stack into guest-visible memory.
Zero the buffer on failure, and bail out of the ioctl if the volume descriptor
can't be read instead of using a garbage path table length. The constructor
already checked that same read.
ReadBlocks writes straight into the caller's buffer, so a partial read there
leaves stale data rather than host memory - left alone deliberately, since
zeroing it would throw away the valid prefix on a truncated image.
The size in an ISO directory record is untrusted, and callers allocate host
buffers from it - GetISOGameID and ReadFileToString did so directly until the
previous commit, and ReadFile clamps reads to the claimed size rather than to
the image.
We've warned about out-of-range extents since c766536914, but deliberately kept
the file, and rounded down so the warning wouldn't fire on borderline images.
Keep that behavior and just clamp the recorded size to the bytes that really
exist. For a well-formed file the extent always fits within its sectors, so this
never triggers; for a truncated one the game keeps booting instead of losing the
file entirely.
Sizes up the work against the existing backends, lays out the shader generation
vs. SPIRV-Cross translation choice, and argues that programmable blending (the
framebufferFetchSupported cap that Vulkan hardcodes to false) is a better reason
to do it than dropping the MoltenVK dependency.
The pipeline debug listing printed the color blend factors in the alpha slot,
which is doubly unhelpful since that branch is only taken when the alpha factors
differ from the defaults.
CompileShaderModuleAsync takes ownership of the tag but only deleted it on the
success path, leaking it whenever GLSLtoSPV failed.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01SGz54K3ZXa2Qzyc3aMEYyY
The failure branch inserted a null pipeline and then fell through to the normal
insert of the same key, which trips DenseHashMap's duplicate-key assert - and
_assert_msg_ is live in release builds, so a logged error became a crash.
Also skip the null entries when deleting cached pipelines.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01SGz54K3ZXa2Qzyc3aMEYyY
They were OR-ed into pipelineFlags just after the CreateGraphicsPipeline call that
consumes them, so the render manager's "don't compile a pipeline that requires
depth for a non-depth renderpass type" check could never fire for game pipelines.
thin3d_vulkan.cpp sets the flag before its call, which is the intended order.
Note this can now legitimately skip some variants when loading the shader cache -
those were invalid combinations that the check was written to reject.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01SGz54K3ZXa2Qzyc3aMEYyY
If reading the shader's constant buffer file failed, we'd skip writing descriptor
binding 4 but still dispatch the compute shader, which declares it - a statically
used but unwritten descriptor. It also re-read the missing file on every single
texture upload. Now we drop the scaling shaders instead, so following textures
take the CPU scaling path.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01SGz54K3ZXa2Qzyc3aMEYyY
The out-of-VRAM retry path cleared plan.replaced but left plan.doReplace set.
GetMipSize() dereferences plan.replaced when doReplace is true, so the fallback
crashed instead of recovering. The common code sets both together.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01SGz54K3ZXa2Qzyc3aMEYyY
The shader creation helpers returned S_FALSE when compilation produced no
bytecode - but S_FALSE is a success code, so the FAILED() checks in the
D3D11VertexShader/D3D11FragmentShader constructors never fired and failed_ was
never set. Return E_FAIL instead, and only hand out the bytecode when the shader
object was actually created.
Failed() had no callers at all, so a failed shader was handed to the draw as
usual: VSSetShader(nullptr), CreateInputLayout on empty bytecode, and the
ignored HRESULT from SetupDecFmtForDraw meant IASetInputLayout(nullptr). The
draw then did nothing, with no log line to explain it. Skip the draw and warn
instead, like the GL backend does.
The cleanup loop was hardcoded to 12 entries while the array holds more and
levels can exceed that with a texture replacement pack - a 8192-pixel
replacement has 14 mip levels, so every level from 12 up leaked on each texture
build. Use ARRAY_SIZE, and shrink the array to 16 since D3D11 caps textures at
16384 pixels anyway.
The out-of-memory bail-out returned before that loop, leaking every level
decoded so far. Since the entry ends up without a texture it gets rebuilt, and
leaks again, every following frame.
Scissor the stencil readback to the region actually being read back - latent,
every caller passes a zero origin today.
Remove a DecodeVerts call that can never do anything: both branches above it
have already advanced decodeVertsCounter_ to numDrawVerts_. Worse than useless,
since in the non-skinning branch the vertices went to the push buffer, so
decoded_ doesn't hold them.
The out-of-memory bail-out added in the previous commit returned before the
status flag was set, leaving a GL_TEXTURE_3D object bound while ApplyTexture
told the shader generator it was a 2D texture. The entry stays cached, so it
would repeat every frame, not just the one that failed to allocate.
TextureCacheGLES passed a hardcoded 0.0f instead of key.aniso, so the
Anisotropic Filtering setting did nothing at all on the OpenGL backend, even
though GPU_USE_ANISOTROPY was advertised and D3D11/Vulkan both honor it. Looks
like it was left behind by the 2017 render manager refactor.
The queue runner now clamps to the device maximum it already queried into
maxAnisotropyLevel_ (until now unused), and only touches the parameter when the
extension is actually supported - the anisotropy branch there has been dead
since every caller passed 0.0f, so this is the first time it runs.
0.0f keeps its meaning of "don't care" for the CLUT/fragment-test/thin3d
callers; the texture cache now passes 1.0f when the setting is off, so turning
it off takes effect on already-uploaded textures instead of only new ones.
TexCache: Never use anisotropic filtering for CLUT8-indexed textures
What gets sampled for those is palette indices, depalettized by the shader
afterwards - averaging indices across an anisotropic footprint produces garbage
colors. Affects all backends, not just the GL one that just started honoring
key.aniso.
TexCache: Clear key.aniso wherever filtering is forced to nearest
It was only cleared in the two places inside the AUTO_MAX_QUALITY branch, so the
TEX_FILTER_AUTO path (pixel-mapped textures, the ugly color test heuristic), the
FORCE_NEAREST setting and the replacement-texture override could all end up
requesting nearest filtering with anisotropy still on.
Doing it in the switch that applies forceFiltering covers every path, so it
can't drift apart again.
GLES: Only record the applied anisotropy, and log skipped draws
The queue runner updated tex->anisotropy even when it skipped the call because
the value was 0.0f ("don't care") - harmless while nothing ever set anisotropy,
but now it would make the tracked state disagree with GL, so a later request for
the value it thinks is set would be wrongly skipped.
Also log when a draw is skipped for a missing vertex shader. The failure is
cached per shader ID, so without it geometry silently disappears for the rest of
the session after the one-shot OSD message.
The cache loader indexed &vec[0] on vectors that can legitimately be empty (a
header-sized file with zero counts passes both sanity checks), and the counts
are signed ints where only the upper bound was checked - a negative count would
reach resize() as a huge size_t.
The 3D texture branch had the out-of-memory assert but not the bail-out the 2D
branch has, so an ignored assert fell straight into memset(nullptr).
ApplyVertexShader can return null - if the requested shader fails to compile it
retries with a software transform ID, and if that fails too it returns (and
caches) null. We then called UseHWTransform() on it.
ApplyFragmentShader can likewise return null, and the hardware path ignored it,
unlike the software path. Without a linked shader nothing binds a program for
this render pass, so the draw would have gone through with whatever program a
previous pass left bound.
53aa2cc596 changed the first argument from "true" to stencilState_.writeMask,
but that slot is "bool enabled" - the writeMask argument stayed hardcoded to
0xFF, so the mask still never reached GL. The clear-mode call just above gets
the slots right.
Reachable because SoftwareTransformCommon refuses the fast clear path when the
stencil write mask is partial, so exactly those clears end up here.
Nothing has generated or used a geometry shader since the GS paths were removed
- GeometryShaderGenerator is gone, and ShaderWriter's BeginGSMain/EndGSMain had
no callers at all. Removes ShaderStage::Geometry and everything hanging off it:
the GS preambles and GSMain helpers in ShaderWriter, the stage mappings in all
three thin3d backends, the D3D11 geometry shader plumbing (curGS_, the pipeline
and module members, gs_4_0 compilation), CreateGeometryShaderD3D11, the unused
PipelineFlags::USES_GEOMETRY_SHADER and PipelineManagerVulkan's
UsesGeometryShader().
Also stop enabling the Vulkan geometryShader device feature, since we no longer
have any use for it.
Kept on purpose: the device feature is still listed in the feature dumps (like
other capabilities we don't use), and the Vulkan shader cache header keeps its
now-always-zero geometry shader count so the on-disk format stays compatible.
Git Bash strips one level of backslash escaping even with a quoted delimiter, so a
'\n' meant to land in the output arrives as '\n' and Python writes a real newline
instead. Records both failure signatures - a C2001 for the first, a silently
non-matching anchor for the second - and when to reach for Edit or a script file.
Delete file that shouldn't be there
CustomButtonMappingScreen indexes customKeyImages[36] and customKeyShapes[11] with
values read straight from the ini, and it's reachable from the main menu - so
neither GamepadEmu nor TouchControlLayoutScreen, which both sanitize first, need
have run. Those two had the same fixup copy-pasted; hoisted it into a Sanitize()
next to the tables and called it from all three.
CmdLine used std::stoi/std::stod and Compatibility used stoi/stof, all of which
throw on junk with nothing catching them. 'PPSSPPHeadless --timeout=abc' aborted
the process, and so did a bad value in a [PostShaderSetting]-style compat section -
at startup, with no diagnostic. Parse with sscanf and report it: CmdLine already
has the pattern for this in its Bool case, and Config.cpp's ini reads were fixed
the same way earlier. A bad compat.ini entry now warns and keeps the default.
corners and verts are carved out of decoded_ at fixed offsets 6*65536 bytes apart,
and NormalizeVertices fills corners with indexUpperBound - indexLowerBound + 1
SimpleVertex. The vertexCount > 1024 guard doesn't bound that: the index values come
from the game, so 1024 indices can span the full 16-bit range and run corners into
verts, making the cull decision from overwritten data. Bail on an index over 1024
and report visible - a bbox test that large isn't worth doing anyway.
A hashrange of 'addr,w,h = 0,0' passed validation (0 isn't bigger than the source),
became desc_.newW/newH, and ReplacedTexture::Prepare divides by them. A post-shader
SSAA level multiplies the render resolution with no upper bound, while the
texture-shader Scale sitting a few lines away is checked against 2..8.
Horizon's CreateView printed 'Fatal error creating the view' and then returned base
anyway, so the caller recorded an unmapped address as a live view. Posix's
ftruncate failure was logged with a '// Should this be a failure?' - it is: the
mmaps afterwards succeed against a short file and the first touch past its end
raises SIGBUS, which is only hooked under __APPLE__.
The ARM64 IR JIT crashed on any load/store to a constant address with the
top bit set and an offset too large for an immediate - the kernel RAM mirror
at 0x88000000, for instance.
PrepareSrc1Address is careful about this: a constant address like 0x89100010
arrives sign-extended as a negative int64_t, and the (imm & 0xC0000000) ==
0x80000000 check turns it back into the positive value it should be. But when
the offset doesn't fit an immediate we fall back to loading it into a register
and using the register-offset addressing mode, which only takes the W half of
that register plus an extend - and we asked for SXTW, undoing the fix and
pointing the access ~2GB below the memory view.
Sign extension is still right when the offset is genuinely negative, which
happens when the base is a pointerified register and the displacement is a
negative one from the MIPS instruction. So extend based on the sign of imm.
Found by the Jit unit test, which stores to 0x89100000 and segfaults in the
JIT_IR phase - only reproducible on an actual ARM64 CPU, which is why it never
showed up on CI. The RISC-V and LoongArch backends get this case right already.
naett has had a complete libcurl backend all along; we just never built it,
so Linux ran with HTTPS_NOT_AVAILABLE. That means no homebrew store over
HTTPS, and RetroAchievements talking to plain http://retroachievements.org.
libcurl is loaded with dlopen rather than linked, the same way we handle the
Vulkan loader, so it stays a soft dependency: we need the curl headers at
build time, but a build made here still starts on a machine without libcurl
installed - it just reports HTTPS as unavailable, exactly like today. Distro
packagers get the behavior they'd expect either way, and certificate
validation comes free from the system CA store.
New net::HTTPSAvailable() answers "did that work", and SDLMain folds it into
SYSPROP_SUPPORTS_HTTPS, which everything downstream already degrades on.
Four fixes to the backend itself, all noted in ext/naett/README-ppsspp.md:
- panic() called exit(1) on a pipe or curl_multi_perform failure. Taking the
emulator down because a download failed isn't acceptable - the backend now
disables itself and requests complete with naettGenericError.
- CURLINFO_RESPONSE_CODE writes a long into res->code, which is an int. Eight
bytes into four, getting away with it only because the next field absorbs
the zeroes.
- curl_easy_setopt is varargs and wants a long for these options; int literals
and int variables are UB on LP64.
- naettPlatformCloseResponse called through a null function pointer when
libcurl was missing. Found by testing that path, which segfaulted.
CI needs libcurl4-openssl-dev (curl-dev on Alpine) or it would quietly keep
building without HTTPS.
naett has been a submodule pinned at v0.3.3; upstream has had no commits since
April 2024, and we want to carry local changes (next up: a libcurl-backed HTTPS
path for Linux). It's ~1500 lines of MIT C, smaller than several things we
already vendor, so bring it in-tree and drop the submodule.
Also drop the generated single-file amalgam (naett.c) that every build system
was compiling, and build src/*.c directly instead - otherwise the file you edit
isn't the file that gets compiled, which is a trap for anyone patching this.
example/ and testrig/ (a whole Android Studio project) are gone with it.
Two changes were needed to make the sources build on their own, both noted in
ext/naett/README-ppsspp.md along with the upstream commit:
- naett_internal.h now includes naett.h, which the amalgam pulled in first.
- naett_linux.c now includes stdio.h/stdlib.h. It calls exit/calloc/realloc/
free/fprintf without ever including either, and only got away with it because
naett_core.c sat above it in the concatenation.
No functional change - Linux still has HTTPS_NOT_AVAILABLE set, so it doesn't
build naett at all yet.
Rename naett to naett-lib
Written as (2 || 4 || 8 || misaligned), so every 2/4/8-byte access got labeled
'(unaligned)' and a genuinely misaligned larger access never reached the struct
branch. Log text only.
A failed FindTransferFramebuffer leaves dstRect zero-initialized, and RASTER_COLOR is
0, so the channel check passes and vfb is read through a null pointer. Only reachable
for a depth-source transfer to an address with no matching framebuffer.
Check dstBuffer first, like every other use of dstRect.vfb below.
The per-game override filename from [games] went straight to LoadFromVFS with no
check, while the [hashes] filenames and ReplacedTexture::Prepare both run theirs
through HasParentDirComponent. For a directory-backed pack DirectoryReader resolves
it against the pack directory, so '../../..' reads anything on disk - and texture
packs are third-party downloads. Check it the same way. (Zip-backed packs weren't
affected.)
Turning replacement off mid-session did 'delete vfs_; vfs_ = nullptr;' without
updating the cached ReplacedTextures that hold the same pointer - LoadIni has a fixup
loop for exactly this when it swaps the VFS, and the disable path needed one too.
Decimate(ALL) right after doesn't help: it only frees their data, it doesn't erase
the entries. A texture still PENDING (or one whose try_lock in Decimate failed) then
used the freed VFS from a worker thread, or from ~ReplacedTexture's ReleaseFile.
Clear the back-pointers, and make the destructor and Prepare() tolerate a null one.
The POSIX handler returned early for any si_code other than SEGV_MAPERR/SEGV_ACCERR,
without passing the signal on. Returning from a fault handler re-executes the
faulting instruction, so anything that keeps faulting - an MTE fault on Android
arm64, a protection-key or shadow-stack fault - became a livelock at 100% CPU rather
than a crash with a usable report. It also swallowed SIGSEGV sent via kill()
(si_code SI_USER), and meant whatever handler was installed before us, such as a
crash reporter, never ran.
The code to chain properly was already there, just inside the 'we couldn't handle
this address' branch further down - factored out and used for both.
This branch is live on the Switch and the BSDs - ppsspp_config.h only maps LINUX to
Android and Linux - and three of its functions didn't do what their callers assume.
time_now_raw() is documented and used as nanoseconds, and from_time_raw() scales it
by 1/nanos, but this built a double of *seconds* and returned it through the uint64_t
return type: the fraction was truncated away and the result was off by a factor of a
billion. Return real monotonic nanoseconds, like every other platform branch.
time_now_unix_utc() just forwarded to time_now_raw(), which is now explicitly a
monotonic clock with no relation to the epoch (and before this, was seconds truncated
to a whole number). Read the wall clock.
Instant took gettimeofday's tv_usec into nsecs_ and stored tv_sec as the start, while
ElapsedNanos() subtracts those from clock_gettime(CLOCK_MONOTONIC) - so it mixed two
different clocks *and* two different units, then applied a microsecond borrow to a
nanosecond difference. Elapsed times were nonsense and could come out negative.
InitMemorySizeForGame read all of disc0:/UMD_DATA.BIN into a vector and then copied
it into a string, with no size limit, from an image we don't control - and the
DISC_ID that gets us here is equally forgeable, it just has to match one of the 16
g_HDRemasters entries. A real UMD_DATA.BIN is a few dozen bytes; anything larger is
a mistake or an attack, so check the size before reading.
The POSIX path used mlock() as though it were a mutex. mlock only pins pages in
RAM - it provides no mutual exclusion at all, so the read-modify-write of the
cross-process instance counter was unsynchronized. Two instances launched at the
same moment could both come away with PPSSPP_ID == 1, at which point both pass
IsFirstInstance() and write ppsspp.ini over each other, and both compute the same
adhoc local IP. Take an advisory lock on the shm fd instead. (The Windows path was
already fine - it uses a named mutex.)
Also, next/total are uint8_t in a segment that outlives the processes using it, so
next climbs across runs and wraps. Landing on 0 is worse than it looks: it isn't a
valid instance id, IsFirstInstance() fails, and config saving is silently disabled
from then on. Skip past it on wrap.
The x86-64 path searched for free memory near the code, and if it found some,
committed to it - if that VirtualAlloc failed, ptr was left null and we returned
null, never reaching the else branch that exists precisely to say "can still run,
thanks to RipAccessible".
Finding a free region isn't the same as being able to reserve it. VirtualAlloc
rounds a non-null lpAddress down to the 64K allocation granularity while
SearchForFreeMem only guarantees page alignment, so the rounded-down base can land
back inside a committed region; a concurrent allocation between the VirtualQuery and
the VirtualAlloc does it too. Callers don't check the result - AllocCodeSpace stores
it unchecked and the emitters write from there - so this turned into a wild write
rather than a clean JIT-unavailable fallback.
Henrik Rydgård
