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).
TODO:
- [ ] test writing of .nif files
- [x] implement loading of fields in PType/PSym that might not have been
loaded
- [ ] implement interface logic
- [ ] implement pragma "replays"
- [ ] implement special logic for `converter`
- [ ] implement special logic for `method`
- [ ] test the logic holds up for `export`
- [ ] implement logic to free the memory of PSym/PType if memory
pressure is high
- [ ] implement logic to close memory mapped files if too many are open.
---------
Co-authored-by: demotomohiro <gpuppur@gmail.com>
Co-authored-by: ringabout <43030857+ringabout@users.noreply.github.com>
Co-authored-by: Jacek Sieka <arnetheduck@gmail.com>
* micro implementation of rfc 149
refs https://github.com/nim-lang/RFCs/issues/149
* number/array/seq literals, more statements
* try fix number literal alias issue
* renew expectedType with if/case/try branch types
* fix (nerf) index type handling and float typed int
* use typeAllowed
* tweaks + const test (tested locally) [skip ci]
* fill out more of the checklist
* more literals, change @ order, type conversions
Not copying the full call tree before the typedesc call check
in `semIndirectOp` is also a small performance improvement.
* disable self-conversion warning
* revert type conversions (maybe separate op later)
* deal with CI for now (seems unrelated), try enums
* workaround CI different way
* proper fix
* again
* see sizes
* lol
* overload selection, simplify int literal -> float
* range, new @ solution, try use fitNode for nil
* use new magic
* try fix ranges, new magic, deal with #20193
* add documentation, support templates
Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
* make strict funcs analysis smarter: varParam[i] = v is different from varParam[i][] = v
* added a test case
* Update compiler/varpartitions.nim
Co-authored-by: Clyybber <darkmine956@gmail.com>
* IC: dead code elimination pass
* preparations for a different codegen strategy
* added documentation to the newly written code
* IC: backend code
* IC: backend adjustments
* optimized the compiler a bit
* IC: yet another massive refactoring
* fixes regressions
* cleanups
* Nil checking
* Enable current older not nil checking again, run new checking only under flag, skip our test
* Enable tests, work on try/except and bugs, fix notnil tests
* Enable strictNotNil tests (currently with lowercase category) and add some expected output
* Work on try/except/finally: still some things unclear and a lot of code can raise out of try
* Fix the notnil build by going back to the old version of a test which I shouldn't have changed
* Fix test : use action compile
* Work on mutation and aliasing: not finished
* Render var parititions graph, try to understand it, fix a nilcheck if bug
* Rebase, progress on working with partitions
* Improve time logic
* Fix some bugs, use graph indices instead of symbol in nil map
* Fix bugs, test simpler ident aliasing for now, support two mutation levels
* Support ContentMutation and ReAssignment: for now just detect possible re assignment for var parameters of calls
* Enable several simple passing tests
* Cleanup a bit, fix condition/branch infix-related bug
* Remove some files, address some comments by Araq
* Use internalError and no quit for now
* Separate tests with expected warnings and with expected ok, fix a bug with if with a single branch related to copyMap
* Fix new data structures, bugs: make tests pass, disable some for now
* Work on fixing errors with non-sym nodes, aliasing: tests fail
* Work on alias support: simple set-based logic, todo more tests and ref sets?
* Use ref sets: TODO can we think of handle seq-s similar to varpartitions' Araq ones
* Handle defers in one place, stop raising in reverse to make an async test compile with strictNotNil, add a commented out test
* Dot expressions: call/reassignment. Other refactorings and distinct, SeqOfDistinct support. Checkout an older varpartitions
* Work on field tracking
* Backup : trying to fix bugs when running some stdlib stuff for running an async test
* Start a section about strict not nil checking in experimental manual
* Fix experimental strict not nil manual section and move it to another file based on Araq feedback
* Fix unstructured flow and double warning problems, fix manual, cleanup
* Fix if/elif/else : take in account structure according to Araq feedback
* Refactor a bit
* Work on bracket expr support, re-enable tests, clarify in manual/tests/implementation static index support for now
* Work on compiling stdlib and compiler with strictNotNil
* Small fixes to the manual for strictNotNil
* Fix idgen for strict check nil rebase
* Enable some simple tests, remove old stuff, comment out code/print
* Copy the original varpartitions source instead of my changes
* Remove some files
* implements https://github.com/nim-lang/RFCs/issues/258
* don't be too strict with custom pragma blocks
* cast pragmas: documentation
* added most missing inference query procs to effecttraits.nim
* spec for view types
* spec additions
* refactoring; there are two different kinds of views
* refactorings and spec additions
* enforce that view types are initialized
* enforce borrowing from the first formal parameter
* enforce lifetimes for borrowing of locals
* typo in the manual
* clarify in the implementation what a borrow operation really is
