Skip to content

Commit 0bb2bcd

Browse files
committed
audit: reconcile modules/numerics and record all four dispositions (#2167)
SR-AUD-278 confirmed -> remediated. SR-AUD-042, SR-AUD-276 and SR-AUD-277 confirmed -> confirmed (design-complete): each has its compatible subpart complete and exactly one recorded gate -- two approvals (a measured 8 -> 16 object-layout growth, and a public return-type change) and one missing evidence, not permission. Every premise correction is appended to the owning per-file report rather than rewritten over the original text: the generic-math surface is 44 members not 3; the vector normalization guard also swallows NaN and fires on squared-length underflow; Plane::Normalize propagates NaN where Vector3 swallows it, so the finding groups two contradictory behaviours; and SR-AUD-277's parenthesis target rests on a doc example that the same report's other citation contradicts. Audit totals 161 remediated / 203 confirmed (150 plain + 53 design-complete) of 364. Numbering stays frozen at 364; no SR-AUD identifier created. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014joExjgZv4aWFpzADeQGZm
1 parent 959aed1 commit 0bb2bcd

6 files changed

Lines changed: 246 additions & 4 deletions

File tree

audit/AUDIT_FINDINGS_INDEX.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ implementation ticket.
5353
| [SR-AUD-039](modules/core/src/System/Math.cpp.audit.md) | medium | confirmed | `Math.cpp` | `Math::Log(double, base)` omits .NET's base-one/zero/positive-infinity special cases and returns infinity or signed zero instead of NaN. |
5454
| [SR-AUD-040](modules/core/include/System/MathF.hpp.audit.md) | medium | confirmed | `MathF.hpp` | MathF ties-to-even rounding depends on C++'s mutable process-wide floating-point mode and returns upward-rounded values under `FE_UPWARD`. |
5555
| [SR-AUD-041](modules/core/include/System/BitConverter.hpp.audit.md) | high | remediated | `BitConverter.hpp` | All typed vector decoders omitted index/remaining-width validation; negative and short `ToInt32` inputs reached ASan-confirmed out-of-bounds reads. Ticket #1851 (CCF-005) routes all 14 decoders through a shared `validateDecodeRange` throwing `ArgumentOutOfRangeException("startIndex")` on a negative/over-large index and `ArgumentException(value)` on insufficient width, before any read; `ToBoolean` throws only `ArgumentOutOfRangeException` (width-1 quirk). ASan clean post-fix; +46 tests (`docs/ConversionBoundaryFamilyPlan.md`). |
56-
| [SR-AUD-042](modules/numerics/include/System/Numerics/TotalOrderIeee754Comparer.hpp.audit.md#sr-aud-042--medium--total-order-comparer-omits-the-equality-comparer-contract) | medium | confirmed | `TotalOrderIeee754Comparer.hpp` | The comparer implements ordering only, omitting .NET's equality-comparer contract required to preserve total-order equality in local hash-based APIs. |
56+
| [SR-AUD-042](modules/numerics/include/System/Numerics/TotalOrderIeee754Comparer.hpp.audit.md#sr-aud-042--medium--total-order-comparer-omits-the-equality-comparer-contract) | medium | confirmed (design-complete) | `TotalOrderIeee754Comparer.hpp` | The comparer implements ordering only, omitting .NET's equality-comparer contract required to preserve total-order equality in local hash-based APIs. Split by review #2167: **ticket #2169 (done)** adds `Equals`/`GetHashCode` to all three specializations with exactly `IEqualityComparer<T>`'s signatures — bit-pattern equality agreeing with `Compare == 0` on every vector, `-0`≠`+0`, distinct NaN payloads distinguished, the binary64 hash folded rather than truncated — and is **layout-neutral (8/8, static_asserted)**. The remaining *polymorphic binding* half is **ticket #2170, needs_user**: adding the base is a measured public object-layout change **8 → 16 bytes** with a second vptr (`build-probe/2167_probe2_layout.log`), the class this repository has gated since #1788/#1789 (`docs/SystemNumericsNamespaceReviewPlan.md` §6.1, §12). |
5757
| [SR-AUD-043](modules/core/include/System/HashCode.hpp.audit.md#sr-aud-043--high--addbytes-converts-a-negative-public-span-length-into-an-unbounded-raw-read) | high | remediated | `Span.hpp`, `Memory.hpp`, `ReadOnlyMemory.hpp`, `ArraySegment.hpp`, `HashCode.hpp` | Span/memory-view constructors and ArraySegment's vector constructor accept negative length metadata after narrowing; HashCode casts it to `size_t`, and ASan confirms an out-of-bounds raw read. **043a — ticket #1852 (CCF-005):** `Span`/`ReadOnlySpan` pointer ctors reject a negative length and the four vector ctors reject a `size()>INT32_MAX` source (shared `detail::checkedSpanLength`), keeping `intcs length_` (no layout change). This closes the *reachable* exploit — a negative-length span can no longer be constructed, so it can never reach `HashCode::AddBytes`; ASan clean post-fix. **043b — ticket #1854 (2026-07-31, approved per `docs/RemainingApprovalDecisions.md` §A.10):** the three `ReadOnlyMemory` ctors and `HashCode::AddBytes(ReadOnlySpan)` dropped `noexcept` (and the pointer ctor its `constexpr`) and now throw `ArgumentOutOfRangeException`. Measured correction: the `ReadOnlyMemory(const T*, intcs)` half was directly **reachable** and stored `-1`/`INTCS_MIN` verbatim, not defence in depth; the `ArraySegment` and `AddBytes` halves are unreachable and their tests pin the exception specification, not a throw. No signature/layout/symbol change. |
5858
| [SR-AUD-044](modules/core/include/System/Span.hpp.audit.md#sr-aud-044--high--copyto-and-trycopyto-corrupt-overlapping-nontrivial-spans) | high | confirmed | `Span.hpp`, `Memory.hpp`, `ReadOnlyMemory.hpp`, `MemoryExtensions.hpp`, `Array.hpp`, `ArraySegment.hpp` | Forward copies lose source data when overlapping Span/ReadOnlySpan, Memory, ReadOnlyMemory, static MemoryExtensions, Array, and ArraySegment nontrivial values, violating .NET's overlap-safe contract. |
5959
| [SR-AUD-045](modules/core/include/System/SpanSplitEnumerator.hpp.audit.md#sr-aud-045--high--an-empty-sequence-separator-produces-an-infinite-stream-of-empty-spans) | high | confirmed | `SpanSplitEnumerator.hpp` | Empty exact-sequence splitting never advances, making range iteration infinite instead of yielding the source once. |
@@ -287,9 +287,9 @@ implementation ticket.
287287
| [SR-AUD-273](modules/diagnostics/src/System/Diagnostics/Process.cpp.audit.md#sr-aud-273--medium--killtrue-is-only-a-process-group-kill-not-a-full-process-tree-kill) | medium | confirmed (design-complete) | `Process.cpp`, `Process.hpp` | `Kill(true)` signals one process group, so a `setsid` descendant survives despite the declared full-process-tree contract. **Design complete, blocked (#2031, cause D-E, 2026-08-03).** Reproduced: after `Kill(true)` the `setsid` grandchild was **ALIVE** and had to be killed by the probe itself. Gated because the repair starts **killing processes it does not kill today** — which is what the contract promises, and is still a change in blast radius — and because the `/proc` descendant walk is Linux-specific, so other POSIX platforms need a documented fallback to today's `killpg`. Approval sentence plan §14.3. |
288288
| [SR-AUD-274](modules/diagnostics/src/System/Diagnostics/Process.cpp.audit.md#sr-aud-274--high--fork-child-calls-non-async-signal-safe-code-in-multithreaded-parents) | high | remediated | `Process.cpp`, `ProcessStartInfo.hpp` | After fork in a multithreaded parent, the child calls non-async-signal-safe `setenv` and `execvp`, risking an unrecoverable pre-exec deadlock. **Reviewed by #2023 (cause D-F); repair ticket #2026, compatible, `todo`.** **Post-audit observation (plan §16 row 4):** the parent is multithreaded exactly when a previous **redirected** `Start` left **this class's own** pipe-reader threads running, so the hazard is reachable without the caller ever creating a thread. The repair builds the `envp` array in the parent before forking and `execve`s it, so the child performs no allocating call; evidence is POSIX `fork(2)`'s async-signal-safety requirement, not .NET. **REMEDIATED (#2026, 2026-08-04).** The environment is marshalled in the **parent**: the process's own `environ` is copied, each configured variable replaces a same-named entry or is appended, and the resulting array is handed to `execvpe` under `__GLIBC__` (elsewhere the global `environ` pointer is replaced -- a single store, async-signal-safe -- before `execvp`). The child's remaining pre-exec calls are `close`/`setpgid`/`dup2`/`chdir`/`_exit`/`write` and the `exec` itself; **no allocating call remains**. Name validation still runs in the parent before the fork, so no entry can be empty or contain `=`. No observable change on any path: inheritance, override-replaces-once, an `=` inside a *value*, an empty value, 100 variables, and synchronous `chdir`/`exec` failure reporting are all pinned by +13 tests. ASan and LSan clean with `Process.cpp` compiled from source. No signature, layout, vtable or `noexcept` change. **A separate defect was found while pinning this and filed rather than absorbed:** `WaitForExit(milliseconds)` blocks past its own deadline when a grandchild holds the redirected pipe (measured 29,951 ms against a 5,000 ms bound) -- inactive ticket **#2032**, blocked on #2029 because every repair decides the reader-thread policy that #2029 gates. No `SR-AUD-*` identifier was issued; numbering stays frozen at 364. |
289289
| [SR-AUD-275](modules/diagnostics/include/System/Diagnostics/Debug.hpp.audit.md#sr-aud-275--high--global-debug-provider-and-diagnostics-state-are-unsynchronised) | high | remediated | `Debug.hpp`, `Trace.hpp`, diagnostics tests | TSan confirms a `Debug::Write`/`SetProvider` race on the global shared_ptr; Diagnostics global state has no synchronization policy. **Reviewed by #2023 (cause D-G); repair ticket #2027, compatible, `todo`.** Measured shape: `Debug::providerStorage()` is a function-local `static std::shared_ptr<DebugProvider>` written by `SetProvider` and read by every `Write`, with no atomic and no lock; `Debug::indentSizeStorage()` and `Trace::indentSizeStorage()` are plain process-global scalars, while `indentLevel` is correctly `thread_local` in both. The audit's TSan evidence was **not** re-run by #2023; #2027 must show the race present before and absent after. `Debug` and `Trace` have **no data members**, so the repair changes no layout or signature. **This is the first namespace in the programme where TSan applies to the compatible half.** **REMEDIATED (#2027, 2026-08-04).** TSan was re-run as required, against the header itself -- `Debug` and `Trace` are header-only, so the production bodies are **necessarily** instrumented and no stale archive is possible. **The before-state was worse than filed:** 20 data races **and 12 `heap-use-after-free`**, the latter inside `Debug::Write`, because a reader could dereference a provider a concurrent `SetProvider` had already destroyed (`build-probe/2027_probe1_tsan_before.log`). Repair: every provider read goes through a new private `currentProvider()` that takes a `std::mutex` and returns an **owning** `shared_ptr` snapshot, so the object cannot die under the caller; the lock is released before the provider is invoked, so a provider that calls back into `Debug` cannot deadlock; `SetProvider` returns the outgoing provider by value for the same reason; and both indent-**size** globals become `std::atomic` (indent-**level** was already correctly `thread_local` and is unchanged). `setIndentSizeProperty` now reports the **clamped local** to `OnIndentSizeChanged` instead of re-reading the global, which a concurrent setter could have changed in between. After: **0 warnings of any kind, clean exit** (`build-probe/2027_probe1_tsan_after.log`). Single-threaded behaviour is unchanged and +11 tests pin it, including the ownership guarantee via `std::weak_ptr`. `Debug` and `Trace` have no data members, so no layout or signature changed; both are public headers, so consumers recompile. |
290-
| [SR-AUD-276](modules/numerics/include/System/Numerics/Vector3.hpp.audit.md#sr-aud-276--medium--degenerate-vector-and-plane-normalization-returns-finite-zero-rather-than-net-nans) | medium | confirmed | `Vector2.hpp`, `Vector3.hpp`, `Vector4.hpp`, `Plane.hpp`, `Matrix4x4.hpp`, `VectorMatrixTests.cpp` | Zero/degenerate vector and plane normalization returns finite zero rather than .NET NaNs, changing direct normalization plus dependent plane and camera geometry. |
291-
| [SR-AUD-277](modules/numerics/include/System/Numerics/Complex.hpp.audit.md#sr-aud-277--medium--complex-abs-signature-and-default-text-diverge-from-net) | medium | confirmed | `Complex.hpp`, `ComplexTests.cpp` | `Complex::Abs` returns a Complex rather than the .NET double and the default textual form is fixed-six-decimal angle-bracket output instead of .NET's formatted parenthesized pair. |
292-
| [SR-AUD-278](modules/numerics/include/System/Numerics/GenericMathInterfaces.hpp.audit.md#sr-aud-278--medium--generic-math-static-interface-members-are-unresolved-link-time-stubs) | medium | confirmed | `GenericMathInterfaces.hpp`, `Task42Tests.cpp` | Public generic-math static members are declarations with no definitions, so consumer code compiles but fails with undefined references at final linkage. |
290+
| [SR-AUD-276](modules/numerics/include/System/Numerics/Vector3.hpp.audit.md#sr-aud-276--medium--degenerate-vector-and-plane-normalization-returns-finite-zero-rather-than-net-nans) | medium | confirmed (design-complete) | `Vector2.hpp`, `Vector3.hpp`, `Vector4.hpp`, `Plane.hpp`, `Matrix4x4.hpp`, `VectorMatrixTests.cpp` | Zero/degenerate vector and plane normalization returns finite zero rather than .NET NaNs. **Measured by review #2167, the finding is wider and far less uniform than recorded:** the vector guard is `Length() > 0`, so it also fires for **any NaN component** (`NaN > 0` is false — the NaN is swallowed, not propagated) and for **any vector whose squared length underflows to zero**; `{-0,-0,-0}` returns `-0`, not "finite zero"; and **`Plane::Normalize` does not share the behaviour at all** — its `len < 1e-10f` guard *propagates* NaN and returns `{1e-11,0,0}` unnormalized while normalizing `{1e-9,0,0}` with `D` scaled to `1e+09`. The module holds **three** thresholds for one question. **Ticket #2173 (done)** states the real contract in all four doc-comments and pins every measured row with 16 `PIN_` tests including both dependents; the semantic change is **ticket #2175, blocked on evidence** — the finding carries no managed probe (§6.2). |
291+
| [SR-AUD-277](modules/numerics/include/System/Numerics/Complex.hpp.audit.md#sr-aud-277--medium--complex-abs-signature-and-default-text-diverge-from-net) | medium | confirmed (design-complete) | `Complex.hpp`, `ComplexTests.cpp` | `Complex::Abs` returns a Complex rather than the .NET double, and the default textual form diverged. Split by review #2167. **Text half — ticket #2171 (done):** measured, the rendering was worse than "fixed six decimals" — `Complex(1e-9, 0)` printed `<0.000000; 0.000000>`, destroying the value, `1e300` produced a 619-character string, and infinities/NaN used the C library's `inf`/`nan`. Both components now render through `System::Double::ToString`, this port's own .NET-compatible renderer. **Premise corrected:** the audit's "parenthesized pair" target rests on a *constructor documentation example*, while the same report links the current .NET source, which gives `<a; b>` — the two citations disagree, so the **bracket/separator skeleton is deliberately unchanged and pinned**, owned by **#2174**. **Signature half — ticket #2172, needs_user:** changing `Abs`'s public return type to `double` is source-breaking with no conversion path, the class gated as approval D-B in `docs/ConsolidatedApprovalPackage.md` (§4.5, §4.6, §6.3). |
292+
| [SR-AUD-278](modules/numerics/include/System/Numerics/GenericMathInterfaces.hpp.audit.md#sr-aud-278--medium--generic-math-static-interface-members-are-unresolved-link-time-stubs) | medium | remediated | `GenericMathInterfaces.hpp`, `Task42Tests.cpp` | Public generic-math static members were declarations with no definitions, so consumer code compiled and then failed with undefined references at final linkage. Measured, the surface is **44 members across 9 templates**, not the 3 the audit's probe named. Ticket #2168 makes every one `= delete`, moving the failure from the linker to the call site, and names the C++ replacement (`<limits>`, `<numbers>`, `<cmath>`, `TSelf{}`/`TSelf{1}`) on each interface. Defining them was rejected: generic-math *conformance* is already a settled documented reduction (`Half.hpp:43-45`, `DivisionRounding.hpp`, the header preamble), and inventing 44 numeric semantics with `/rv` absent is what this programme forbids. **No program that builds today is affected** — a call that exists today already fails to link. Compile-time proof in `test/consumer/numerics_generic_math_negative.cpp` (12 sites); +6 runtime tests (`docs/SystemNumericsNamespaceReviewPlan.md` §4.7, §5 NM-A). |
293293
| [SR-AUD-279](modules/globalization/include/System/Globalization/StringInfo.hpp.audit.md#sr-aud-279--medium--stringinfo-confuses-utf-8-bytescode-points-with-managed-text-elements) | medium | confirmed | `StringInfo.hpp`, `TextElementEnumerator.hpp`, Globalization tests | UTF-8 code points/byte indexes replace .NET grapheme text elements; `SubstringByTextElements` can return an invalid single-byte UTF-8 tail. |
294294
| [SR-AUD-280](modules/globalization/include/System/Globalization/CultureInfo.hpp.audit.md#sr-aud-280--high--currentculture-and-currentuiculture-are-racy-process-global-state-instead-of-per-threadtask-values) | high | confirmed | `CultureInfo.hpp`, Globalization tests | TSan confirms concurrent writes/reads of mutable process-global current culture; workers also observe each other's culture rather than retaining a per-thread/task value. |
295295
| [SR-AUD-281](modules/globalization/include/System/Globalization/Calendar.hpp.audit.md#sr-aud-281--medium--calendar-is-publicly-constructible-with-a-fabricated-gregorian-fallback) | medium | confirmed | `Calendar.hpp`, `CalendarTests.cpp` | C++ exposes an ordinary constructible Calendar with Gregorian behavior although current .NET Calendar is abstract; 82 direct tests lock this invalid shape in. |

audit/modules/numerics/include/System/Numerics/Complex.hpp.audit.md

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,40 @@ and [official constructor example](https://learn.microsoft.com/en-us/dotnet/api/
4444
## Final assessment
4545

4646
SR-AUD-277 applies. No implementation was changed during this audit.
47+
48+
---
49+
50+
## Post-audit correction and partial remediation — tickets #2171 / #2172 / #2174 (2026-08-10)
51+
52+
*Appended by review #2167. The original report above is retained verbatim.*
53+
54+
**The text half is worse than recorded, and its target is contested by this report's own two
55+
citations.** Measured (`build-probe/2167_probe1_numerics_before.log`), `std::to_string`'s fixed six
56+
fractional digits meant `Complex(1e-9, 0)` printed `<0.000000; 0.000000>` — the value destroyed,
57+
not merely coarsened — and `Complex(1e300, -1e300)` printed a **619-character** string of two
58+
309-digit expansions. Infinity and NaN came out as the C library's `inf`/`-inf`/`nan`, not .NET's
59+
`Infinity`/`-Infinity`/`NaN`. This was the only site in the port rendering a `double` with
60+
`std::to_string`.
61+
62+
**Premise corrected.** The report calls the port's `<a; b>` skeleton a divergence and cites .NET's
63+
**constructor documentation example** (`(26.1, 18.06)`) as the target; the same report also links
64+
the **current .NET `Complex` source**, which gives `<a; b>`. Two citations in one report disagree,
65+
`/rv` is absent, and no managed probe measured this member — so the bracket and separator
66+
characters are **deliberately unchanged and pinned**, owned by **#2174**.
67+
68+
**Repair (ticket #2171, done).** Both components render through `System::Double::ToString`, this
69+
port's own settled renderer for a .NET `double`: shortest round-trippable form and the .NET special
70+
-value spellings. The target is a decision this repository already made, which is what makes this
71+
half decidable with `/rv` absent, and the change is strictly closer to .NET under **either**
72+
reading of the bracket question. `ComplexTests.ToStringFormat` asserted `s.find("1.") != npos`
73+
it **pinned the defect** — and was replaced, not deleted.
74+
75+
**Signature half (ticket #2172, needs_user).** `decltype(Complex::Abs(z))` is `Complex`,
76+
`decltype(Complex::AbsD(z))` is `double`. Changing `Abs`'s return type is source-breaking with **no
77+
implicit conversion either way** (`Complex(double, double)` has no defaulted second parameter), so
78+
an affected caller gets a hard compile error rather than a silent change. **Zero in-repository
79+
callers use `Complex::Abs`**; all three call sites use `AbsD`. A public return-type change is the
80+
class gated as approval **D-B** (#2030) in `docs/ConsolidatedApprovalPackage.md`.
81+
82+
**Status:** `confirmed`**`confirmed (design-complete)`**.
83+
See `docs/SystemNumericsNamespaceReviewPlan.md` §4.5, §4.6, §6.3, §12.

0 commit comments

Comments
 (0)