fixes #25595; cursor inference: a recorded mutation extends the variable's liveness (#25864)

fixes #25595

## Bug

A `let` bound to a field of a value-type **case object** with a `ref`
field is inferred as a non-owning cursor, but the cursor's source can be
mutated through the cursor's own ref during a call, freeing the ref
while the borrow still reads it. Use-after-free under arc/orc (refc is
unaffected, it has no cursor inference):

```nim
var destroyed = false
type
  O = ref object
    value: int
    home: H
  W = object
    case k: bool
    of true: r: O
    of false: discard
  H = ref object
    w: W
proc `=destroy`(o: var typeof(O()[])) =
  destroyed = true
proc clear(o: O): int =
  o.home.w = W()             # overwrites h.w via the back-reference -> frees the ref
  doAssert not destroyed     # fails: the element was destroyed during the call
  result = o.value
proc go(h: H): int =
  let c = h.w                # inferred cursor (borrow of h.w)
  result = clear(c.r)
proc main =
  let h = H()
  let o = O(value: 42)
  o.home = h
  h.w = W(k: true, r: o)
  doAssert go(h) == 42
main()
```

The `not destroyed` assert fails: the element is destroyed during the
call, so the following `o.value` read is a use-after-free. The same code
with the `=destroy` guard removed (so the freed `o.value` is actually
read) is reported as `heap-use-after-free` by ASan under `-d:useMalloc
-fsanitize=address`. Longstanding (reproduces back to 2.2.0).
`--cursorInference:off` is a workaround.

## Root cause

Cursor inference (`varpartitions.computeCursors`) cursors `let c = h.w`
unless `dangerousMutation` finds a mutation of `c`'s graph within `c`'s
alive range `aliveStart..aliveEnd`. Here the mutation (the `clear(c.r)`
call) *is* connected to `c`'s graph and *is* recorded with `isMutated`,
but it is recorded at an `abstractTime` just past `c.aliveEnd`, so the
range check misses it.

The gap is timing. `aliveEnd` is set from the last `nkSym` use of `c`. A
call records its argument's mutation *after* traversing the whole
argument subtree (`potentialMutationViaArg`). When the argument is `c.r`
on a case object it is an `nkCheckedFieldExpr` (the discriminant check),
whose extra nodes advance `abstractTime` past `c`'s last `nkSym`. A
plain `nkDotExpr` has no such gap, so the bug needs a case object.

## Fix

In `potentialMutation`, extend the mutated variable's liveness to the
mutation time:

```nim
v.s[id].aliveEnd = max(v.s[id].aliveEnd, v.abstractTime)
```

A variable mutated at time T is provably alive at T, so this only
completes the liveness computation that `dangerousMutation` relies on.
The worst case is an extra copy, never an unsound cursor.

## Note on the locus

The fix is conservative by mechanism (it runs at every recorded
mutation) but perf-neutral in practice: it only suppresses a cursor
where the corrected liveness proves the borrow unsafe (cursor counts are
unchanged on the suites). I can scope it to call arguments if you'd
prefer it narrower.

## Test

`tests/arc/t25595.nim`, matrix `--mm:orc; --mm:arc; --mm:refc`: the
repro above as a `doAssert`. Fails (UAF) on arc/orc before the fix and
passes after. refc passes throughout.

## Checks

- repro passes on orc/arc after the fix. The guard-removed variant
(which reads the freed value) is ASan-clean after the fix and was
heap-use-after-free before. refc unaffected.
- testament `destructor` 90/90, `arc` 120/120. `views` 5/6, same as
stock (the one failure is environmental and pre-exists this change).
- perf-neutral: inferred-cursor count is identical stock vs fix across
the `arc` and `destructor` test files under `--mm:orc` (322 vs 322).
This commit is contained in:
Corey Leavitt
2026-06-02 23:25:33 -06:00
committed by GitHub
parent 73986c03a1
commit c8e805a2fa
2 changed files with 46 additions and 0 deletions

View File

@@ -185,6 +185,9 @@ proc root(v: var Partitions; start: int): int =
proc potentialMutation(v: var Partitions; s: PSym; level: int; info: TLineInfo) =
let id = variableId(v, s)
if id >= 0:
# mutated here => alive here: keep aliveEnd in sync so dangerousMutation catches
# mutations recorded after the var's last use (e.g. via a call arg). See #25595.
v.s[id].aliveEnd = max(v.s[id].aliveEnd, v.abstractTime)
let r = root(v, id)
let flags = if s.kind == skParam:
if isConstParam(s):

43
tests/arc/t25595.nim Normal file
View File

@@ -0,0 +1,43 @@
discard """
matrix: "--mm:orc; --mm:arc; --mm:refc"
"""
# bug #25595: cursor inference must not borrow a case object whose source can be
# mutated through the cursor's own ref across a call. `let c = h.w` was inferred as a
# non-owning cursor; `clear(c.r)` overwrites `h.w` via the cursor's back-reference,
# freeing the ref while the borrow still uses it -> use-after-free. Detected here
# deterministically: the element's destructor must not run during the call.
var destroyed = false
type
O = ref object
value: int
home: H
W = object
case k: bool
of true: r: O
of false: discard
H = ref object
w: W
proc `=destroy`(o: var typeof(O()[])) =
destroyed = true
proc clear(o: O): int =
o.home.w = W()
doAssert not destroyed, "use-after-free: element destroyed during the call"
result = o.value
proc go(h: H): int =
let c = h.w
result = clear(c.r)
proc main =
let h = H()
let o = O(value: 42)
o.home = h
h.w = W(k: true, r: o)
doAssert go(h) == 42
main()