Hi David, All good feedback - and certainly pick and choose what you wish to take advantage of / feel is of value.
I will go back and look at the comments you made on RB-1, RB-2, etc. and reintegrate those questions into the 2nd pass Claude is making through the code and advise. Cheers, Todd On Mon, Apr 13, 2026 at 11:26 AM David Chisnall <[email protected]> wrote: > Hi, > > Looking at the libobjc2 ones: > > The issue RB-1 is kind-of real, but the fix is incorrect (we should free > the object before calling the unexpected exception handler, because it may > not return), though this is almost unreachable code. It can basically > happen only if there is an internal error in the unwind library. > > RB-2 is not correct, selectors being null is undefined behaviour and > cannot happen in compiler-generated code. Adding a null check on one of > the hottest paths in the runtime would be a regression. > > TS-7 looks like a fix that we need in a few more places, not sure why it’s > only highlighed in the place the code was copied to, not the place it was > copied from. > > I think TS-14 is spurious, this should not be called twice, the first > caller nulls out the pointer after the cleanup. The one corner case where > it can be called twice is if cleanup *reallocates* the TLS, in which case > doing the cleanup twice is correct. Do you have a test case that > demonstrates this? > > RB-6 looks like the right fix, simple copy-and-paste bug. Note that this > happens only when memory is exhausted, at which point most Objective-C > programs will start failing. > > RB-7, the null check is in a silly place (after the dereference), but the > API contract here is that the argument must not be null, so it’s actually > dead code. This function also should be setting `*outCount = 0` in the > early returns. > > PF-6, yes that refactoring would probably be good to do, though note that > we don’t hit the weak lock in most cases, only if an object is marked as > having weak refs. Have you measured slowdown from this on anything that > *isn’t* a contrived microbenchmark? The quoted slowdown looks incredibly > unlikely unless you have a microbenchmark doing nothing but hitting weak > references from multiple threads. > > PF-7, this will generate exactly the same code unless we explicitly use a > weaker memory order (both are sequentially consistent by default). We > should move this code over to C++11 atomics at some point. > > TS-3 is incorrect. This counter grows monotonically. If a selector is > registered *while* this call is happening, then the result is undefined. > It’s technically UB, in that there is an unsynchronised read. > > PF-4 was an intentional design choice. Method replacements are > infrequent. The proposed change would make things worse. > > David > > On 13 Apr 2026, at 04:35, Todd White <[email protected]> wrote: > > Hi GNUstep Team, > > As an exercise to test out the latest Claude AI capabilities, we recently > completed a comprehensive, bottom-up code audit of the GNUstep core stack — > all seven repositories — covering libobjc2, libs-base, libs-corebase, > libs-opal, libs-quartzcore, libs-gui, and libs-back. The full results, > documentation, and all fix commits are publicly available at: > > https://github.com/DTW-Thalion/gnustep-audit > > I wanted to share what we found and offer to contribute any or all of the > changes back upstream. > > ## What we did > > We audited the entire stack bottom-up — runtime through UI layer — > examining every file for robustness issues, thread safety gaps, security > vulnerabilities, correctness bugs, and performance bottlenecks. Each > finding was severity-rated, fixed in an atomic commit tagged with a finding > ID, and validated with a dedicated regression test. We also wrote 13 > performance benchmarks with a baseline/compare workflow so improvements can > be measured reproducibly. > > ## What we found and fixed > > Across all seven repos, we identified and fixed 150 findings: > > - 22 Critical — including NSSecureCoding bypass (class whitelist > completely unimplemented), TLS server verification disabled by default, > use-after-free in objc_exception_rethrow, NULL dereferences, data races in > CFRunLoop and CATransaction, zero thread safety across the entire libs-back > backend (189 files, 0 locks), and a swapped sendto() argument in CFSocket > that prevented any data from being sent. > > - 46 High — deadlocks in property spinlocks, race conditions, buffer > overflows (CGContext dash buffer allocated in bytes instead of doubles), > broken APIs, JSON parser with no recursion depth limit (stack overflow > DoS), and integer overflow in binary plist bounds checking. > > - 61 Medium — thread safety gaps in GSLayoutManager, NSView, and > NSApplication event dispatch; missing input validation; and general > robustness issues. > > - 14 Low + 10 confirmed bugs — documentation issues, minor optimizations, > swapped arguments, wrong variables, and inverted conditions (e.g., TIFF > destination init was inverted, making TIFF writing 100% broken). > > We also implemented 12 targeted performance optimizations, including: > > - 64-way lock striping for weak references (5–8× concurrent throughput) > - O(1) LRU linked list for NSCache (replacing an O(n) implementation that > also never evicted) > - Geometric growth for CFArray (O(n) vs O(n²) sequential appends) > - X11 expose event coalescing, live resize throttling at 60fps, dirty > region tracking in NSView, DPSimage conversion caching, and stack buffer > allocation in CFRunLoop to eliminate per-iteration malloc > > Benchmark results on MSYS2/ucrt64 show +29–31% for retain/release, +12–18% > for message dispatch, +46–55% for array operations, and +25% for NSCache. > > ## How the work is organized > > Each of the seven repos has its own fork under our GitHub org ( > https://github.com/DTW-Thalion) with fix commits on master. The > gnustep-audit repo itself contains: > > - Per-phase findings reports (docs/phase1 through phase6) > - A master audit summary (docs/AUDIT-SUMMARY.md) > - 51 regression tests and 13 benchmarks under instrumentation/ > - A Makefile-driven test and benchmark harness with baseline/compare > support > > All 32 regression tests pass on the patched stack (up from 18/32 on > unpatched). > > ## Offer to contribute upstream > > We'd be happy to contribute any or all of these changes back into the main > GNUstep repositories — whether as pull requests, individual patches, or in > whatever form works best for your workflow. Feel free to help yourself to > the repo. > > The security-critical fixes (NSSecureCoding, TLS defaults, JSON depth > limit, binary plist overflow) and the confirmed crash bugs (use-after-free, > NULL derefs, inverted conditions) are probably the highest-priority > candidates for upstream integration. > > Please feel free to reach out with any questions. We have a lot of respect > for the GNUstep project (and a bit nostalgic for heady days of > NeXTStep/OpenStep) and would like to see this work benefit the broader > community. > > It's unclear if the codebase is actively maintained or if many people > still use it, but we hope that this exercise provides some value. > > Best regards, > > Todd > > *Todd White* > Managing Director > > > 177 Huntington Avenue, 17th Floor > Boston, MA 02115 > Telephone: +1 617 237-2835 Ext. 101 > EIN: 33-2228263 > > Website <https://www.thalion.global/> | Twitter/X > <https://x.com/TTIScience> | YouTube <https://www.youtube.com/@TTIScience> > > [image: > https://app.candid.org/profile/16308928/the-thalion-initiative-us-inc-33-2228263/?pkId=39bf89e9-f544-478c-a3fe-e157e345181d] > <https://app.candid.org/profile/16308928/the-thalion-initiative-us-inc/?pkId=39bf89e9-f544-478c-a3fe-e157e345181d&isActive=true> > > >
