Skip to content

refcount linter - #1059

Draft
dozreg-toplud wants to merge 62 commits into
developfrom
dozreg/refcount-check
Draft

dozreg-toplud wants to merge 62 commits into
developfrom
dozreg/refcount-check

Conversation

@dozreg-toplud

@dozreg-toplud dozreg-toplud commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

This PR contains the refcount linter, comment annotations for the linter as well as some jet fixes that are also PR'd separately in #1058.

The linter itself is fully vibecoded for now and I can't guarantee that it doesn't produce false negatives (it already did on previous iterations). At some point I'll rewrite the abstract interpreter.

What I would like to understand is whether the annotation grammar is uncontroversial. The annotations are placed either on functions, blocks of code (e.g. if we transfer a noun to some persistent structure) or the entire files (e.g. to supress checks for nock.c)

TODO:

  • Reread/rewrite the abstract interpreter
  • Add checking borrowed reference correctness considering unifying equality

Depends on #1057, #1079, #1080

Prompted by ~dozreg-toplud's refcount linter, which flagged leaks in
_check, plus a hand audit of the surrounding code.  The fixes fall
into four groups.

_check helper:
* take [meta data] as two borrowed nouns instead of consing a cell at
  every call site (the cell leaked, and it silently assumed ownership
  of the borrowed sample legs); drop the leaked u3qa_dec result
* stop punning raw c3_d/c3_w values as nouns: the shape product was
  truncated through a u3_atom and compared against a noun, so any ray
  with 2^31 or more elements failed _check spuriously (and a crafted
  overflowing shape could pass it)
* compute the product with 64-bit overflow detection; an overflowed
  product cannot match the block count of a real atom, mirroring the
  Hoon exactly
* validate the bloq before calling u3r_met, which is UB at or above
  block size 37 (release builds compile the guard out); at such sizes
  any nonzero atom is one block
* bail %exit only where +check itself crashes (cell dims, improper
  shape list, zero data underflowing +dec)

wrapper/Hoon divergences (jet computed where the Nock crashes, or
vice versa, or produced a different noun):
* transpose/diag passed the whole core to _check, reading the gate's
  battery as ray metadata -- those jets unconditionally bailed %exit
  whenever they fired
* the elementwise (+add/+sub/+mul/+div/+mod), comparison
  (+gth/+gte/+lth/+lte), scalar (+add-scalar &c), +abs, and +trace
  wrappers never called _check at all, though the Hoon asserts it
* +ravel has no +check and never reads the kind, so its wrapper now
  punts (not bails) on an inconsistent ray
* +dot only asserts equal shapes outside the %i754 path; the full
  meta equality and consistency checks moved inside the %i754 case
* +mmul never compared the two rays' bloq/kind, and its gemm switch
  silently returned a zero matrix for an out-of-range bloq; it now
  punts on both
* reduction results (+cumsum/+min/+max/+dot) hard-coded shapes ~[1]
  or ~[1 1] or ~[n 1]; +scalar-to-ray gives an all-ones shape of the
  input's rank
* _set_rounding returns c3n on an unrecognized mode and callers punt,
  instead of bailing %fail

kernel bugs (wrong results, previously unobservable because the
wrapper bailed or untested):
* trace computed (dot d d) -- the sum of SQUARES of the diagonal --
  instead of (cumsum (diag a)); identity-matrix tests masked it
* transpose read and wrote with rows/columns swapped, correct only
  for square inputs, and the wrapper did not swap the result shape
* argmin/argmax returned (len - i - 1) instead of the ravel index i
* diag and abs return elements in natural order, in lockstep with
  urbit/urbit#7388, which removes the erroneous flops in +diag and
  +el-wise-op (the old +abs jet already returned natural order, so
  jetted and unjetted ships disagreed about +abs; the diag jet never
  returned at all)
* trace honors the door rounding mode; linspace fences an indirect
  count atom before using it as a raw integer
* mod (ray and scalar) rounded the quotient with a hardcoded
  round-toward-zero; +toi rounds in the door mode (7 mod 2 is -1
  under %n, not 1).  It also returned values where the Hoon crashes:
  a non-finite quotient (zero or NaN divisor) is (need ~).  And
  mod-scalar/div-scalar multiplied by a rounded 1/n instead of
  dividing -- wrong even for exact quotients (21 mod 7 gave 7,
  21/7 gave 2.9999998).  All four now divide directly, round in
  softfloat_roundingMode, and bail %exit on a non-finite quotient
* gth/gte used SoftBLAS's f_gt/f_ge macros, which are (!le)/(!lt)
  and thus TRUE whenever either operand is NaN; IEEE gt/ge are
  false.  Same disease via f_min/f_max in min/max/argmax.  All now
  use lt/le with swapped operands, which also reproduces the Hoon
  reel-fold exactly (NaN in the head is sticky, interior NaN is
  skipped, first-of-ties wins)
* range counted elements with one-shot ceil((b-a)/d), but the Hoon
  iterates x+d in the door mode, testing each sum against b -- for
  d=.0.1 the shapes disagreed outright (10 vs 11).  The kernel now
  replicates the iteration (values are the accumulated sums) and
  the wrapper takes the count from the result's block count; it
  punts on non-finite bounds/step, a zero step, or a stalled
  accumulator, where the Nock loops forever

leaks:
* the _check cell at every call site; u3qa_dec inside _check;
  _get_dims arrays in the diag/dot wrappers and trace kernel (plus
  raw c3_d dimensions used directly as nouns there); argmin/argmax
  scan buffers; linspace/range result-shape overkeeps

Verified on a fresh fake ship against the base lagoon (with the
urbit/urbit#7388 fixes applied), differentially against a de-jetted
copy of the same library: 36/36 probes agree, including transpose of
non-square, trace, diag, reduction shapes at rank 1, argmin/argmax
with NaN at and after the head, mod under %n/%u, mod-scalar and
div-scalar on exact quotients, gth/gte/lth against NaN, range with
d=.0.1 and a negative step, and add/cumsum/mod-scalar/range under
non-default rounding modes; zero-divisor mod crashes on both doors.
A 40x40 %i754 mmul runs ~3s jetted vs ~25s interpreted, confirming
the jets fire.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@dozreg-toplud

Copy link
Copy Markdown
Contributor Author

Notes from core blitz:

  • Need more clear annotation for "custom" case:
    • functions in a "custom" file are all custom unless they have some @Refcount annotations
    • a "custom" function may only be called by custom functions and functions with asserted (i.e. not checked) refcount protocol

I'll describe refcount annotation syntax here:

  • A refcount annotation starts with @Refcount:, followed by a space and a refcount directive.

  • A refcount annotation of a function may appear in the header comment of a function definition, the header comment of a function declaration, or the same line as the function declaration if it spans for just one line:

/*  @Refcount: retain
 *  (header comment)
*/
u3_noun
foo(u3_noun a)
{ ... }

u3_noun bar(u3_noun u3_noun); // @Refcount: transfer (same line for declarations is OK)
  • A refcount annotation of a code block may appear on the same line as the opening brace:
if ( condition ) {  //  @Refcount: assert transfer (this assignement consumes)
  *ptr_u = u3k(u3h(list));
}
  • A refcount annotation of a file may appear in the first 4KB of a file:
//  @Refcount: assert custom file
  • Unless there is ambiguity, @Refcount: X, Y, Z is same as:
//  @Refcount: X
//  @Refcount: Y
//  @Refcount: Z
  • The function refcount directives are applied in order top to bottom, with each directive changing the refcount protocol of the function. If there are conflicting directives then the last one wins, but a warning about conflicting directives is raised.

  • A function must have the same directives across all its declarations and its definition.

  • If a function has no directive, and the file is not custom, then its refcount protocol is governed by the rules layed out in u3.md:

    • By default it transfers its arguments and its product;
    • u3r_*, u3x_*: retain arguments and the product;
    • u3q*, u3w*, u3z_*: retain arguments, transfer the product;
    • static functions or _-named functions in jets/[a-f]/: retain args, transfer product
  • If a function has no directive, and the file is custom, the function follows "custom" transfer protocol. It can be only called by other custom functions and functions with asserted refcount protocol.

  • List of refcount directives for a function:

    • assert: the body of the function is not checked
    • assert custom: marks the function as "custom". No other directives may be present
    • transfers product: the product of the function is owned by the caller
    • transfers `x`, `y`, `z`: the function takes ownership of the listed arguments
    • transfers arguments: the function takes ownership of all arguments
    • transfers: the function takes ownership of all arguments and the product of the function is owned by the caller
    • retains ...: as transfers, mutatis mutandis: the product is an uncounted reference, arguments are borrowed
    • passthrough `x`: identity: the product IS argument x, with unchanged ownership, counts are untouched
    • direct `x`, `y`, `z`: if the function returned, these arguments were direct
    • direct product: the product is a direct atom
    • direct arguments: if the function returned, all arguments were direct
  • List of refcount directives for a code block:

    • assert transfer `x`, `y`, `z`: if these variables were assigned, that assignement consumed them. Useful for assignements to persistent data structures or in defcons patterns;
    • assert retain `x`, `y`, `z`: the block has no effect on these variables
  • List of refcount directives for a file:

    • assert custom file: marks the file as custom. Can be used in low-level parts of Vere like nock.c to supress checks, especially for tricky protocols like semitransfer.

@dozreg-toplud
dozreg-toplud force-pushed the dozreg/refcount-check branch from e44669e to 8abe4f3 Compare July 21, 2026 10:19
@dozreg-toplud
dozreg-toplud force-pushed the dozreg/refcount-check branch from 43d0e40 to ff5b07d Compare August 6, 2026 14:14
dozreg-toplud added a commit that referenced this pull request Aug 17, 2026
Cherrypicked from #1059.

I added u3_none treatment in the linter per @joemfb's request, found
some omitted checks.

Also extended the linter to check functions that neither take nor return
nouns (common for callbacks for external code), found some leaks as
well. Refcount action in eval in main.c was very confusing, I refactored
it.
sigilante added a commit that referenced this pull request Sep 12, 2026
…rminating +range

Folds in the findings from ~dozreg-toplud's refcount linter (#1059)
on the rebased #1057 and the follow-up review notes.

- `_get_length` and the `mmul` dimension reads used `u3x_atom` products as
  C integers; an indirect atom in a shape would blow up.  Read them through
  `u3r_cat`, so an indirect dimension bails instead of being truncated.
  (Also makes the `Na != Nb` check compare values rather than noun words.)
- `u3wi_la_dot` kept borrowed references into `x_meta` (`x_shape` &c)
  across `u3r_sing(x_meta, y_meta)`.  Unifying equality rewrites equal
  subnouns in place, so the borrowed `x_shape` could be freed before its
  use in the result.  The wrapper now reads every field it returns only
  after the last comparison that could have rewritten it.
- `u3qi_la_*` / `u3wi_la_*` are typed `u3_weak` in the definitions and in
  q.h/w.h, since they return `u3_none` to punt.
- `_set_rounding` keeps develop's `u3_atom` parameter (the mode is a noun,
  and c3_w would truncate it on the 64-bit loom) while yielding c3n on an
  unrecognized mode so callers punt.
- The dot and trace wrappers use manual noun splitting like the rest of
  the file on develop (#1009) instead of `u3r_mean`.
- +range: where the Hoon can never terminate -- a NaN upper bound, or a
  stalled accumulator (zero/underflowing step, NaN, infinite start) --
  the jet bails %fail rather than punting.  An infinite loop is a
  non-deterministic failure, not a deterministic %exit, and punting it
  to the Nock hangs the ship.  The absurd-count cap is still a punt.

Checked with the refcount linter against the lagoon translation unit
(0 findings).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rqqco86RgnvtiC4axw8Px3
sigilante added a commit that referenced this pull request Sep 12, 2026
Rebasing the math.hoon transcendental jets onto post-vere64 develop:

- The four single-argument @r jet helpers (_rd_jet/_rs_jet/_rh_jet/
  _rq_jet) bound the u3_weak product of u3r_at(u3x_sam, cor) to a
  u3_noun before the u3_none/u3ud guard; declare it u3_weak (refcount
  linter, #1059).
- u3r_mean was rewritten on develop (urbit/urbit dozreg/refcount-check
  lineage) to take {axe, &out} pairs via u3r_vmean; convert the five
  call sites from the old flat, 0-terminated form to braced pairs, or
  -Wmissing-braces fails the -Werror build.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rqqco86RgnvtiC4axw8Px3
sigilante added a commit that referenced this pull request Sep 12, 2026
Adds the /lib/twoc scalar two's-complement jets (twoc.c/twoc.h) and the
%int2 element-wise array path in the lagoon jets, registered in the 135/
136/137 trees.  Rebased onto #1057: the %int2 cases slot into
that PR's rewritten +add/+sub/+mul/+div/+mod wrappers, and every jet that
declines with u3_none (all the twid ops, the %int2 helper, and the int2
wrapper results) is typed u3_weak, matching the #1057 convention and the
refcount linter (#1059).

twoc.c is byte-identical to the ship-verified urbit/numerics copy and uses
only fixed-width types (c3_b/s/w/d and __int128), so it is loom-agnostic.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rqqco86RgnvtiC4axw8Px3
sigilante added a commit that referenced this pull request Sep 12, 2026
Adds the %int2 (signed two's-complement) element-wise comparison
(gth/gte/lth/lte) and reduction (cumsum/dot/min/max/argmin/argmax) array
jets, on top of the twoc scalar jets, rebased onto #1057.

- The %int2 cases slot into #1057's rewritten comparison and reduction
  wrappers; the dot case is added to #1057's restructured +dot wrapper
  (its unifying-equality-safe field reads made the auto-merge miss it).
- All the new _la_int2_* helpers and the wrapper int2 results are typed
  u3_weak (they decline with u3_none); _la_int2_box, which transfers its
  r_data into the boxed ray, is annotated `@Refcount: transfers `r_data``
  so the reduction wrappers do not read as leaking it (refcount linter,
  #1059).
- The reduction result shape is the all-ones-of-rank +scalar-to-ray box,
  matching #1057's i754 _ones_shape; #1048's separate i754 reduction-
  shape fix is already carried by #1057, so only the %int2 additions
  land here.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rqqco86RgnvtiC4axw8Px3
sigilante added a commit that referenced this pull request Sep 12, 2026
_sip_rub() writes a freshly-decoded, transferred atom into its `val`
out-parameter whenever the pointer is non-null.  Without an annotation
dozreg's refcount checker (#1059) cannot model the pointee protocol, so
_sip_rub and both its callers (_sip_gaze, u3qi_sip_grab) come back as
[complicated] / [annotation] and are not analyzed.

Annotate it `@Refcount: fills transferred `val``.  Comment-only; no
behavior change.  The checker now reports 0 findings on sip.c (also
under --strict-weak).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rqqco86RgnvtiC4axw8Px3
sigilante added a commit that referenced this pull request Sep 12, 2026
Every u3qi_unum_* / u3wi_unum_* can return u3_none -- the arm punts on
an unsupported bloq (posit64/128, not in SoftUnum), and each wrapper
punts when _unum_bloq() cannot read the door sample -- and the unary
wrappers read the sample with u3r_at (a u3_weak product).  Declaring
them u3_noun made dozreg's refcount checker (#1059) flag 109 [u3_none]
findings: none-literal returns from a u3_noun function, and u3r_at
products stored into u3_noun locals.

Retype the wrapper return types and the u3r_at locals (incl.
_unum_bloq's) to u3_weak, in the .c definitions and the q.h/w.h
declarations.  u3_weak is a typedef of u3_noun (pkg/noun/types.h), so
this is a type-annotation change only, with no runtime effect.  The
checker now reports 0 findings on unum.c (also under --strict-weak).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rqqco86RgnvtiC4axw8Px3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants