Re: Claude AI code audit of GNUstep core stack — 150 fixes, 12 perf optimizations, all available f or upstream
David Chisnall <[email protected]> Mon, 13 Apr 2026 16:28:34 +0100
| Newsgroups | gmane.comp.lib.gnustep.devel |
|---|---|
| Message-ID | <[email protected]> |
--Apple-Mail=_62CB8A5B-1E84-4F76-AAFD-F41E1D12D1CB Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 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: >=20 > Hi, >=20 > Looking at the libobjc2 ones: >=20 > 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. >=20 > 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. >=20 > TS-7 looks like a fix that we need in a few more places, not sure why = it=E2=80=99s only highlighed in the place the code was copied to, not = the place it was copied from. >=20 > 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? >=20 > 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. >=20 > 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=E2=80=99= s actually dead code. This function also should be setting `*outCount =3D= 0` in the early returns. >=20 > PF-6, yes that refactoring would probably be good to do, though note = that we don=E2=80=99t 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=E2=80=99t* a contrived microbenchmark? The quoted = slowdown looks incredibly unlikely unless you have a microbenchmark = doing nothing but hitting weak references from multiple threads. >=20 > 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. >=20 > TS-3 is incorrect. This counter grows monotonically. If a selector = is registered *while* this call is happening, then the result is = undefined. It=E2=80=99s technically UB, in that there is an = unsynchronised read. >=20 > PF-4 was an intentional design choice. Method replacements are = infrequent. The proposed change would make things worse. >=20 > David >=20 >> On 13 Apr 2026, at 04:35, Todd White <[email protected]> = wrote: >>=20 >> Hi GNUstep Team, >>=20 >> 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 =E2=80=94 all seven repositories =E2=80=94 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: >>=20 >> https://github.com/DTW-Thalion/gnustep-audit >>=20 >> I wanted to share what we found and offer to contribute any or all of = the changes back upstream. >>=20 >> ## What we did >>=20 >> We audited the entire stack bottom-up =E2=80=94 runtime through UI = layer =E2=80=94 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. >>=20 >> ## What we found and fixed >>=20 >> Across all seven repos, we identified and fixed 150 findings: >>=20 >> - 22 Critical =E2=80=94 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. >>=20 >> - 46 High =E2=80=94 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. >>=20 >> - 61 Medium =E2=80=94 thread safety gaps in GSLayoutManager, NSView, = and NSApplication event dispatch; missing input validation; and general = robustness issues. >>=20 >> - 14 Low + 10 confirmed bugs =E2=80=94 documentation issues, minor = optimizations, swapped arguments, wrong variables, and inverted = conditions (e.g., TIFF destination init was inverted, making TIFF = writing 100% broken). >>=20 >> We also implemented 12 targeted performance optimizations, including: >>=20 >> - 64-way lock striping for weak references (5=E2=80=938=C3=97 = 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=C2=B2) 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 >>=20 >> Benchmark results on MSYS2/ucrt64 show +29=E2=80=9331% for = retain/release, +12=E2=80=9318% for message dispatch, +46=E2=80=9355% = for array operations, and +25% for NSCache. >>=20 >> ## How the work is organized >>=20 >> 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: >>=20 >> - 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 >>=20 >> All 32 regression tests pass on the patched stack (up from 18/32 on = unpatched). >>=20 >> ## Offer to contribute upstream >>=20 >> We'd be happy to contribute any or all of these changes back into the = main GNUstep repositories =E2=80=94 whether as pull requests, individual = patches, or in whatever form works best for your workflow. Feel free to = help yourself to the repo. >>=20 >> 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. >>=20 >> 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. >>=20 >> 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. >>=20 >> Best regards, >>=20 >> Todd >>=20 >> Todd White >> Managing Director >>=20 >>=20 >> 177 Huntington Avenue, 17th Floor >> Boston, MA 02115 >> Telephone: +1 617 237-2835 Ext. 101 >> EIN: 33-2228263 >>=20 >> Website <https://www.thalion.global/> | Twitter/X = <https://x.com/TTIScience> | YouTube = <https://www.youtube.com/@TTIScience> >>=20 >> = <https://app.candid.org/profile/16308928/the-thalion-initiative-us-inc/?pk= Id=3D39bf89e9-f544-478c-a3fe-e157e345181d&isActive=3Dtrue> >=20 --Apple-Mail=_62CB8A5B-1E84-4F76-AAFD-F41E1D12D1CB Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 <html aria-label=3D"message body"><head><meta http-equiv=3D"content-type" = content=3D"text/html; charset=3Dutf-8"></head><body = style=3D"overflow-wrap: break-word; -webkit-nbsp-mode: space; = line-break: after-white-space;">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.<div><div><br></div><div>David<br = id=3D"lineBreakAtBeginningOfMessage"><div><br><blockquote = type=3D"cite"><div>On 13 Apr 2026, at 16:26, David Chisnall = <[email protected]> wrote:</div><br = class=3D"Apple-interchange-newline"><div><meta http-equiv=3D"content-type"= content=3D"text/html; charset=3Dutf-8"><div style=3D"overflow-wrap: = break-word; -webkit-nbsp-mode: space; line-break: = after-white-space;">Hi,<div><br></div><div>Looking at the libobjc2 = ones:</div><div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><div>TS-7 looks like a fix that we need = in a few more places, not sure why it=E2=80=99s only highlighed in the = place the code was copied to, not the place it was copied = from.</div><div><br></div><div>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?</div><div><br></div><div>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.</div><div><br></div><div>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=E2=80=99s actually dead code. = This function also should be setting `*outCount =3D 0` in the = early returns.</div><div><br></div><div>PF-6, yes that refactoring would = probably be good to do, though note that we don=E2=80=99t 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=E2=80=99t= * a contrived microbenchmark? The quoted slowdown looks incredibly = unlikely unless you have a microbenchmark doing nothing but hitting weak = references from multiple threads.</div><div><br></div><div>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.</div><div><br></div><div>TS-3 is incorrect. This counter = grows monotonically. If a selector is registered *while* this call = is happening, then the result is undefined. It=E2=80=99s = technically UB, in that there is an unsynchronised = read.</div><div><br></div><div>PF-4 was an intentional design choice. = Method replacements are infrequent. The proposed change = would make things = worse.</div><div><br></div><div>David</div><div><div><br><blockquote = type=3D"cite"><div>On 13 Apr 2026, at 04:35, Todd White = <[email protected]> wrote:</div><br = class=3D"Apple-interchange-newline"><div><div dir=3D"ltr"><div><div = class=3D"gmail_default" style=3D"font-family:arial,sans-serif"><span = style=3D"font-family:Arial,Helvetica,sans-serif">Hi GNUstep = Team,</span></div><br><span class=3D"gmail_default" = style=3D"font-family:arial,sans-serif">As an exercise to test out the = latest Claude AI capabilities, w</span>e recently completed a = comprehensive, bottom-up code audit of the GNUstep core stack =E2=80=94 = <span class=3D"gmail_default" = style=3D"font-family:arial,sans-serif">all </span>seven = repositories =E2=80=94 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:<br><br><a = href=3D"https://github.com/DTW-Thalion/gnustep-audit">https://github.com/D= TW-Thalion/gnustep-audit</a><br><br>I wanted to share what we found and = offer to contribute any or all of the changes back upstream.<br><br>## = What we did<br><br>We audited the entire stack bottom-up =E2=80=94 = runtime through UI layer =E2=80=94 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.<br><br>## What we found and fixed<br><br>Across all seven = repos, we identified and fixed 150 findings:<br><br>- 22 Critical =E2=80=94= 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.<br><br>- 46 High = =E2=80=94 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.<br><br>- 61 = Medium =E2=80=94 thread safety gaps in GSLayoutManager, NSView, and = NSApplication event dispatch; missing input validation; and general = robustness issues.<br><br>- 14 Low + 10 confirmed bugs =E2=80=94 = documentation issues, minor optimizations, swapped arguments, wrong = variables, and inverted conditions (e.g., TIFF destination init was = inverted, making TIFF writing 100% broken).<br><br>We also implemented = 12 targeted performance optimizations, including:<br><br>- 64-way lock = striping for weak references (5=E2=80=938=C3=97 concurrent = throughput)<br>- O(1) LRU linked list for NSCache (replacing an O(n) = implementation that also never evicted)<br>- Geometric growth for = CFArray (O(n) vs O(n=C2=B2) sequential appends)<br>- 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<br><br>Benchmark results on = MSYS2/ucrt64 show +29=E2=80=9331% for retain/release, +12=E2=80=9318% = for message dispatch, +46=E2=80=9355% for array operations, and +25% for = NSCache.<br><br>## How the work is organized<br><br>Each of the seven = repos has its own fork under our GitHub org (<a = href=3D"https://github.com/DTW-Thalion">https://github.com/DTW-Thalion</a>= ) with fix commits on master. The gnustep-audit repo itself = contains:<br><br>- Per-phase findings reports (docs/phase1 through = phase6)<br>- A master audit summary (docs/AUDIT-SUMMARY.md)<br>- 51 = regression tests and 13 benchmarks under instrumentation/<br>- A = Makefile-driven test and benchmark harness with baseline/compare = support<br><br>All 32 regression tests pass on the patched stack (up = from 18/32 on unpatched).<br><br>## Offer to contribute = upstream<br><br>We'd be happy to contribute any or all of these changes = back into the main GNUstep repositories =E2=80=94 whether as pull = requests, individual patches, or in whatever form works best for your = workflow. <span class=3D"gmail_default" = style=3D"font-family:arial,sans-serif">Feel free to help yourself to the = repo.</span><br><br>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<span = class=3D"gmail_default" = style=3D"font-family:arial,sans-serif">.</span><br><br>Please feel free = to reach out with any questions. We have a lot of respect for the = GNUstep project <span class=3D"gmail_default" = style=3D"font-family:arial,sans-serif">(and a bit nostalgic for heady = days of NeXTStep/OpenStep) </span>and would like to see this work = benefit the broader community.</div><div><br></div><div><div = class=3D"gmail_default" style=3D"font-family:arial,sans-serif">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.</div><br>Best regards,</div><div><br></div><div><div = class=3D"gmail_default" = style=3D"font-family:arial,sans-serif">Todd</div><br = clear=3D"all"></div><div><div dir=3D"ltr" class=3D"gmail_signature" = data-smartmail=3D"gmail_signature"><div dir=3D"ltr"><b>Todd = White</b><div>Managing Director</div><div><br></div><div></div><div><img = src=3D"https://ci3.googleusercontent.com/mail-sig/AIorK4xw08RqGgDKsHOD8XqE= Yb6-GgGIJIf1cRrXVWqgRb0R08UESMvMdTT1W33NqQlLRUIicyduuegMbbIF" width=3D"96"= height=3D"71"><br></div><div>177 Huntington Avenue, 17th = Floor</div><div>Boston, MA 02115</div><div>Telephone: +1 617 237-2835 = Ext. 101</div><div>EIN: 33-2228263</div><div><br></div><div><a = href=3D"https://www.thalion.global/" target=3D"_blank">Website</a> | = <a href=3D"https://x.com/TTIScience" = target=3D"_blank">Twitter/X</a> | <a = href=3D"https://www.youtube.com/@TTIScience" = target=3D"_blank">YouTube</a></div><div><br></div><div><a = href=3D"https://app.candid.org/profile/16308928/the-thalion-initiative-us-= inc/?pkId=3D39bf89e9-f544-478c-a3fe-e157e345181d&isActive=3Dtrue" = target=3D"_blank"><img = src=3D"https://cdn.candid.org/seals-of-transparency/2026/candid-seal-gold-= 2026.png" width=3D"96" height=3D"96" = alt=3D"https://app.candid.org/profile/16308928/the-thalion-initiative-us-i= nc-33-2228263/?pkId=3D39bf89e9-f544-478c-a3fe-e157e345181d"></a><br></div>= </div></div></div></div> = </div></blockquote></div><br></div></div></div></blockquote></div><br></di= v></div></body></html>= --Apple-Mail=_62CB8A5B-1E84-4F76-AAFD-F41E1D12D1CB--