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:26:25 +0100
Newsgroups gmane.comp.lib.gnustep.devel
Message-ID <[email protected]>
--Apple-Mail=_6BEC707D-383E-46CB-962E-C0D356431A48
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=utf-8

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=E2=80=99s 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=E2=80=99=
s actually dead code.  This function also should be setting `*outCount =3D=
 0` in the early returns.

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.

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=E2=80=99s 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:
>=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>


--Apple-Mail=_6BEC707D-383E-46CB-962E-C0D356431A48
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;">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. &nbsp;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. &nbsp;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. &nbsp;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. &nbsp;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. &nbsp;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. =
&nbsp;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. =
&nbsp;Have you measured slowdown from this on anything that *isn=E2=80=99t=
* a contrived microbenchmark? &nbsp;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). &nbsp;We =
should move this code over to C++11 atomics at some =
point.</div><div><br></div><div>TS-3 is incorrect. &nbsp;This counter =
grows monotonically. &nbsp;If a selector is registered *while* this call =
is happening, then the result is undefined. &nbsp;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. =
&nbsp;Method replacements are infrequent. &nbsp;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 =
&lt;[email protected]&gt; 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&nbsp;</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.&nbsp;<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)&nbsp;</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>&nbsp;| =
<a href=3D"https://x.com/TTIScience" =
target=3D"_blank">Twitter/X</a>&nbsp;| <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&amp;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></body></html>=

--Apple-Mail=_6BEC707D-383E-46CB-962E-C0D356431A48--