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__.
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
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.
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.
System.cpp stamped BootState::Complete unconditionally after InitGPU(), overwriting
the Failed that InitGPU sets when GPU_Init() fails - after it has already run
CPU_Shutdown(). PSP_InitUpdate then took the success path on a core that no longer
existed, down to a null Memory::base, and the first guest access dereferenced it.
InitGPU now reports failure and both callers honor it. (The libretro path had the
same problem from the other direction: it calls InitGPU after the Failed check.)
HandleAssert called g_assertCancelCallback directly on the IDCANCEL path, without
the null check its own BreakIntoPSPDebugger() helper does - and EmuScreen clears the
callback when a game is unloaded. So any assert after returning to the menu turned
"Cancel: skip and break into PPSSPP debugger" into a null jump, from the one button
whose entire purpose is surviving the assert.
__CheatDoState registered the cheat event type when the savestate had no CwCheat
section, but never scheduled it. CoreTiming::DoState has already swapped in the
state's event queue by then, which doesn't contain one either - so loading an old
savestate silently killed cheats, and the enable/disable polling with them, for the
rest of the session.
Achievements::ChangeUMD set g_isIdentifying and returned without clearing it when
hashing failed, leaving IsBlockingExecution() true forever - EmuScreen stops running
the CPU and the game is frozen until restart. Reachable from a disc swap on any ISO
whose PARAM.SFO or EBOOT.BIN can't be read.
x64Analyzer routed opcode 0x88 into the write path but had no case for it, so it hit
the default, logged from inside the crash handler, and failed. 0x88 is exactly what
the x64 JIT emits for a guest sb, so MemFault could never skip or ignore a bad byte
store the way it can a word one. Handle the 8-bit forms, and drop the 0x8a/0x8b cases
in the read path that the same 0xF0 mask made unreachable. Covered by a new
CheckAnalyze case, which fails without this change.
314 pspautotests pass, all unit tests pass.
The three framebuffer upload paths took Memory::GetPointerUnchecked() on a
GE-supplied source address and then read height rows of it, without ever checking
that span was mapped. Only the destination was validated (and DoBlockTransfer's own
memcpy is carefully guarded, so the intent was clearly there). A copy whose source
starts near the end of RAM walks straight off the end of the view. Clamp the row
count to what's actually mapped, and warn when we do.
GhidraClient dereferenced getArray()->value for both "symbols" and "types" without a
null check, and the getTag() test underneath could never catch it - getArray() has
already filtered by tag, so it returns either a JSON_ARRAY node or nullptr. Any
HTTP 200 that parses as JSON but isn't the shape we expect - {}, a bare array, an
incompatible ghidra-rest-api, or the host/port pointed at some other JSON service -
crashed the worker thread. FetchTypes() runs first, so that's the one you'd hit.
RiscV and LoongArch CPU detection divided TotalLogicalCount() by ProcessorCount()
before checking it. ProcessorCount() returns 0 whenever /proc/cpuinfo can't be read
or doesn't parse, which is SIGFPE during static init of the cpu_info global - before
anything could handle it. The existing <= 0 guard sat after the division.
314 pspautotests pass; frametests show the same 3 pre-existing failures as master.
NewThreadExecutor::Run pushed a std::thread per connection and only ever joined them
in the destructor, so a server leaked a joinable thread object for every connection
it had ever served. Measured with 60 connect/disconnect cycles against the debugger:
handle count +60 before, +1 after. Each worker now flags itself done as its last act,
and Run() reaps the finished ones first. Only the accept thread calls Run(), so the
flag is the only thing that needs to be atomic.
Note this doesn't bound how many connections can be in flight at once - it just stops
the finished ones from piling up.
Separately, a received close code was echoed straight back. RFC 6455 7.4.1 reserves
1004, 1005, 1006 and 1015 for describing how a connection ended locally, so they must
never go on the wire - echoing one back would be our protocol violation rather than
the client's. Send PROTOCOL_ERROR when they give us something we can't repeat.
RequestHeader::GetParamValue indexed parts[1] without checking the size. A query
parameter with no '=' at all ("?foo") makes SplitString return a single element, so
both the DEBUG_LOG and the assignment read off the end of the vector. Nothing calls
GetParamValue today, so this is latent rather than live, but it's driven straight
off the request line.
The 64-bit frame length was assembled with header[n] << 24 on uint8_t values, which
promote to int - a byte >= 0x80 in the top position shifts into the sign bit and then
sign-extends when widened to uint64_t. The resulting size was always rejected, just
by the wrong check and via signed overflow to get there. Cast first.
OutputSink::Block() had the same shape as the InputSink one this branch already
fixed: a broken socket is reported ready immediately and forever, so waiting on it
is a spin. Bail if the sink already knows it's broken.
GetStringErrorMsg had the strerror_r result test backwards. The XSI variant returns
0 on success, so every successful lookup returned "Unknown error"; and under glibc
with _GNU_SOURCE the GNU variant is selected instead, which returns the message by
pointer and typically leaves the buffer untouched, so it returned an empty string.
Either way GetLastErrorMsg() was useless on Linux, Android and macOS. Pick the right
handling by overload resolution rather than guessing which signature we got.
KeyMap's "no gamepad button mapped to cancel" fallback pushed into confirmKeys
instead of cancelKeys - and pushed the confirm button. So unmapping cancel left no
gamepad way out of menus, and duplicated an entry in the confirm list.
ControlMapper::AddListener mutated listeners_ without taking mutex_, while
RemoveListener takes it and the input thread iterates the vector under it. Opening a
screen while an axis is moving could reallocate it mid-iteration. The comment about
piggybacking on a screenmanager mutex was stale - there isn't one.
Config's two std::stof calls on PostShaderSetting values ran on user-editable ini
text with no try/catch, so a malformed entry called std::terminate during startup
config load. Use the same checked sscanf that LoadGameConfig already uses.
(CmdLine.cpp and Compatibility.cpp have the same pattern; not touched here.)
The screenshot downscale path leaked its final buffer on every downscaled shot,
which savestate thumbnails hit on every save at 3x and above.
HandleUploadPost is registered unconditionally, so closing the Upload screen left an
unauthenticated file-write endpoint live for as long as anything else kept the server
up. Check the flag in the handler.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DCPmm7FoQUoqrbMdhfqhQ2
Follow-up to the previous commit, from Nemoumbra's questions - which found a worse
spin than the one that fix addressed.
InputSink couldn't tell "nothing right now" from "peer is gone": Fill() treats
recv() == 0 as no data and only sets hasError_ on a real error. Block() then waits
with WaitUntilReady(), which reports a closed socket as ready immediately and
forever, so TakeExact() looped on it without ever returning. A client that
disconnects with half a frame buffered - easy to do while blasting messages - put
the server in an infinite loop inside TakeExact, never even returning to Process().
Measured 7.95 CPU-seconds over 8 seconds; 0.08 after.
So: track EOF explicitly (sticky atEnd_, exposed as AtEnd()), and have Block() give
up when nothing more can arrive.
That information was being thrown away in three more places:
* Process() only tried to fill when the sink was already empty, so a disconnect went
unnoticed for as long as there were leftovers - and if those leftovers were a
partial frame, the read above never completed. Always fill, and close once the
peer is gone and we've consumed what it sent.
* ReadPending() uses TakeAtMost(), which returns 0 both for "nothing right now" and
"nothing ever again", and then reported success having consumed nothing. Ask the
sink which it was.
* Both TakeExact() call sites answered a failed read with POLICY_VIOLATION, blaming
the client for a protocol error when it had simply disconnected. Check the sink
and report ABNORMAL when that's what happened.
Also stop queueing data once our own close frame is queued. RFC 6455 5.5.1 forbids
data frames after a close, and beyond the protocol, anything appended afterwards
keeps the buffers non-empty and starves the "everything is flushed" check that ends
the connection. Observed the server pumping 167MB of log broadcasts after being
asked to close.
The repeated close-and-discard is now one helper.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DCPmm7FoQUoqrbMdhfqhQ2
Reported by Nemoumbra: the debugger server could get stuck in a tight select()
loop after a lot of traffic, burning a core.
Once OutputSink hits a real send() error it latches hasError_, after which Flush()
returns immediately without consuming anything, so out_->Empty() is false forever.
Process() waited for that to empty before finishing the close, kept the fd in the
write set, and select() reports an errored socket as ready every time - so it
returned true on every lap without ever making progress, and WebSocketDebuggerLoop
span. This needs sentClose_ to be set for it to be unrecoverable, since otherwise
the read side notices the disconnect and closes; a client that sends CLOSE (or
trips a protocol error) while output is backed up gets exactly that. Reproduced
with a client that queues ~120MB of responses, sends CLOSE, then resets the
connection without reading: 6.02 CPU-seconds over 6 seconds before, 0.06 after.
Treat an output error as fatal to the connection instead.
Also, select() returning -1 always returned true, so any error that doesn't fix
itself (a bad fd rather than EINTR) was a second busy-loop with no wait at all.
EINTR retries, everything else closes.
Finally, SendFlush() erased the drained bytes off the front of outBuf_ every lap.
With a backlog that's a memmove of the whole buffer per lap, i.e. quadratic in the
backlog, which burns CPU on its own while draining a slow client. Track a consumed
offset and only compact once the dead prefix is worth reclaiming.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DCPmm7FoQUoqrbMdhfqhQ2
Found by a review pass over Common/. Three of these affect code the JITs
actually emit today:
* ARM64 TryMOVI(8) returned true unconditionally ("can always do 8"), but MOVI
with an 8-bit element replicates imm8 into every byte, so it can only encode a
byte-uniform value. TryAnyMOVI always tries size 8 first, so it succeeded for
every constant. MOVI2FDUP(FLT_MAX) - VertexDecoderArm64's Jit_PosFloat - came
out as "movi v0.16b, #0xff", a quiet NaN, and since FMINNM/FMAXNM return the
other operand for a quiet NaN, the infinity clamp silently did nothing.
TryAnyMOVI's replication loop was also shifting by every bit position instead
of by multiples of the element size, and it now only tries an element size the
value actually repeats at. Regression test added.
* RISC-V SW()'s stack-pointer compression path called C_LWSP instead of C_SWSP,
turning a store into a load that clobbers rs2 whenever autocompress is on
(which RiscVJit and VertexDecoderRiscV both enable).
* LoongArch64 EncodeDFj passed the raw register enum instead of DecodeReg(fj),
so bit 10 was always set and MOVFR2GR_S emitted movfr2gr.d - live in the
LoongArch JIT's mfc1 and its FPU/vector compilers.
The rest have no callers today, but are wrong as written:
* ARM64: MOVI/MVNI computed the MSL cmode one too high (MSL #8 is 1100, not
1101); TryMOVI's MVNI-with-MSL branch passed the value instead of its
complement; TBZ/TBNZ put the register size in bit 31 where b5 belongs and
didn't mask the bit index to 5 bits; the LDR/LDRSW/PRFM literal form checked
the wrong mask for imm19 and wrote it unmasked; FCVTZS/FCVTZU's GPR-
destination branch skipped DecodeReg and derived the type field from the GPR
rather than from the float source.
* LoongArch64: LDPTR_D/STPTR_W/STPTR_D all passed Opcode32::LDPTR_W;
AMCAS_DB_D duplicated AMSWAP_DB_D's opcode; EncodeJK shifted rk by 5 instead
of 10; BYTEPICK_D masked its shift to 2 bits instead of 3.
* x64: VGATHERDPD/VGATHERQPS/VGATHERQPD used the wrong opcode/W combinations
(only VGATHERDPS was right).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DCPmm7FoQUoqrbMdhfqhQ2
VulkanGraphicsContext::InitSurface() threw away VulkanContext::InitSurface()'s
VkResult and carried on, so a failed surface init surfaced as
_dbg_assert_(GetAvailablePresentModes().size() > 0) in the VKContext
constructor rather than as a graphics error with the usual backend fallback.
The vkCreate*SurfaceKHR failure path in ReinitSurface() didn't log anything
either, so the assert was the only trace of it.
Now ReinitSurface() logs and sets init_error_ for all three ways it can bail
(surface creation, ChooseQueue, present mode enumeration), InitSurface()
checks the result, and MainThreadFunc() passes the message back out instead of
writing it to a local it then drops - Windows/main.cpp was reporting
"Failed to initialize main thread function." to the user.
Also deletes the Application on that failure path, which was leaked.
The old one was reverse engineered from a handful of symbols and got the
shape of the format wrong - it required a digit right after the kind
character, which most real symbols don't have. Measured against a PSP
executable that shipped with its symbol table intact, it decoded 238 of
4662 mangled symbols, most of those incorrectly.
Worked out properly from that binary, the format turns out to be:
__0 <kind> <name...> <params> [_ <return type>] [<qualifier>]
where the kind character (member function, free function, operator, data)
is the only thing that says how many name components follow, since nothing
separates the last one from the first parameter. Lengths are letters
(A = 0, a = 26); "5" marks an enclosing namespace; "7...._" is a template
argument list, with "4" plus a compact integer for a non-type argument and
"9<index>A" for a back-reference to one; "T<index>" and "N<count><index>"
repeat an earlier parameter; a trailing "K" is const and a trailing "T" is
a static member function. Also handles __TID_/__T_ (the two halves of a
class's RTTI) and __sti__ (a translation unit's static initializers).
That decodes 4661 of the 4662. The one holdout is an STL symbol whose
template argument is a reference to a member of another template.
Declarator wrapping is shared with the CodeWarrior demangler now, so
pointers to arrays come out as "short (**)[64]" in both.
docs/SNSystemsMangling.md describes the format, marking what's inferred
rather than attested.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SF5eS5QDNexLksRDeDZvwY
Checked against two PSP binaries that shipped with intact symbol tables,
which turned up several constructs the format's usual description doesn't
mention:
- Template arguments are written literally inside the length-prefixed name
("39CList<Q38hlScreen5Brwsr13CContentsUnit>"), not with a "__PT" prefix,
and they nest. Function templates put theirs in the base name instead,
followed by the return type.
- A family of "@"-decorated symbols for things with no C++ name: thunks
("@12@__dt__3SonFv"), string literals, function-local statics and their
guard variables. Plus __vt__/__RTTI__/__sinit_, printed in the same style
as the Itanium special names.
- Types are now built as a split declarator, so a pointer to a function
comes out as "int (*)(int)" rather than "int (int) *".
Also stop the lenient pass from turning plain C names with a "__" in them
into nonsense - "I3dClut__FlushCache" became "I3dClut(long, ...)". It now
requires a class qualifier, which costs nothing: over ~10000 symbols the
lenient pass rescued none and only produced those false positives.
Symbol map names go from 128 to 256 characters, since a demangled name
keeps its parameters and templates make short work of 128.
docs/CodeWarriorMangling.md describes the format, marking the parts that
are inferred from cfront rather than attested in a real binary.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SF5eS5QDNexLksRDeDZvwY
Older PSP binaries weren't built with GCC, so the Itanium demangler doesn't
help with them. Add two more, tried in turn by DemangleSymbolName():
- Metrowerks CodeWarrior, a descendant of the AT&T cfront scheme
("getDistance__6KzUtilFP7st_unitP7st_unit"). Handles Q<n> qualified names,
the cfront type codes including T/N back-references, cv-qualifiers, and the
operator/ctor/dtor name codes.
- SN Systems SNC/ProDG ("__0f5DstdIbad_castEwhatvK"), which encodes name
component lengths as letters. Reverse engineered from a small sample, so
the parts that are guesses are marked as such - they don't affect the name.
Both are rougher than the Itanium one: they aim for a correctly qualified name
plus a plausible parameter list, and print "..." for a parameter they can't
decode rather than throwing the name away. Results come back as a
DemangledSymbol with the name, parameters, return type and qualifiers kept
separate, in case a caller wants more than the printed string.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EFV5DUTc9ZYAKgsCMZGwX8
GLRenderManager is documented as "emu thread records, render thread executes",
but GL has to record device object creation as init steps rather than just doing
it, and InitGPU() runs on the ExecLoader thread - GPU_GLES's constructor builds
DrawEngineGLES, whose InitDeviceObjects() reaches initSteps_ through
CreatePushBuffer and CreateInputLayout. The emu thread is still drawing the
loading screen into the same FastVec until the loader thread is joined, so two
concurrent push_uninitialized() can both reallocate, and one writes its step into
a freed buffer - losing a shader or buffer creation, or scribbling an owned
pointer into freed memory.
frameData_[].activePushBuffers is genuinely three-threaded too: inserted into by
whoever creates a push buffer, erased on the render thread via GLDeleter, and
walked on the render thread each frame.
A mutex each, uncontended in practice. Note this makes the existing access safe
rather than fixing the layering - Vulkan avoids the problem by creating objects
directly and deferring the rest to FinishInitOnMainThread, which GPU_GLES has
never had. Moving GL's device object creation there would be the better fix.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vd8ntC2brCUtCrDJMqLbs8
The loop queue-deleted the VkPipeline and nulled the slot without deleting the
Promise the array owns - DestroyVariantsInstant right below it shows the intended
ownership. That's one leaked Promise per destroyed variant per cached pipeline,
on every MSAA or resolution change.
It also wrote pipeline[] without taking mutex_, which the header documents as
protecting that array and which the render thread holds while reading and
replacing the same slots in PerformRenderPass. The two have to be fixed together:
the missing delete was the only thing keeping this a leak rather than a
use-after-free.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vd8ntC2brCUtCrDJMqLbs8
GetRenderPass() looks up and inserts into renderPasses_ from the main thread
(EndCurRenderStep, CreateGraphicsPipeline) and from the render thread
(PerformBindFramebufferAsRenderTarget), unsynchronized. The render thread really
does insert rather than only hit: PreprocessSteps rewrites the load actions to
CLEAR when it merges a clear-only pass into a later one, after the main thread
already looked up the pre-merge key. DenseHashMap::Insert can Grow(), which
reallocates the buckets out from under a concurrent Get().
VKRRenderPass::Get() has the same problem one level down - it creates the passes
lazily and is called from both threads on the same object, so two threads hitting
an empty slot each create a pass and one gets overwritten and leaked, while the
sample-count branch can queue a pass for deletion that the other thread is about
to hand to vkCreateGraphicsPipelines.
A mutex each. Handing the VKRRenderPass pointer out from under the map lock is
fine, entries are only ever erased all at once in DestroyDeviceObjects.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vd8ntC2brCUtCrDJMqLbs8
This is a leftover from the old "native" library. Its only users are the Win32 GE
debugger's preview windows, which call glsl_create_source/destroy/bind/unbind and
read four locations off the struct.
Everything else was dead: glsl_create was declared but never defined anywhere,
which made the entire file-loading and auto-reload half of glsl_recompile
unreachable (glsl_create_source always passes empty filenames), along with the
mtime fields, AutoCharArrayBuf and the VFS/stat includes. glsl_attrib_loc,
glsl_uniform_loc and glsl_get_program had no callers, and the active_programs set
was written and never read. The unused convenience locations cost a
glGetUniformLocation round trip each at link time.
The bug: the vertex shader was leaked when its own compile failed - the fragment
path right below it already deleted it correctly. Failed links leaked the program
object too.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vd8ntC2brCUtCrDJMqLbs8
Henrik Rydgård