Please note also, I will not merge any changes to libobjc2 that come from LLMs. LLM-generated code is hard to review, because it comes from a plausible-next-token generator and so is very likely to *look* correct, even if it is correct. According to the US copyright office and case law, it cannot be copyrighted, but it *may* be a derived work of something in the training set and so is far too high legal risk to merge.
David > On 13 Apr 2026, at 16:26, 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> >> >> >> <https://app.candid.org/profile/16308928/the-thalion-initiative-us-inc/?pkId=39bf89e9-f544-478c-a3fe-e157e345181d&isActive=true> >
