fixes #26094; memory leak on exception unwinding — raising proc's res… (#26100)

…ult is never destroyed

fixes #26094



Conceptually, the new lowering for `destination = raisingCall()` is:

```nim
var tmp: T
try:
  tmp = raisingCall()
  let value = tmp
  wasMoved(tmp)
  destination = value
finally:
  destroy(tmp)
```
So whether `raisingCall` Succeeds or not, `tmp` is destroyed

---------

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
This commit is contained in:
ringabout
2026-09-02 18:33:21 +08:00
committed by GitHub
parent a5afc78638
commit 48bfe01a83
5 changed files with 80 additions and 49 deletions

View File

@@ -91,26 +91,7 @@ proc isHarmlessStore(p: BProc; canRaise: bool; d: TLoc): bool =
else:
result = false
proc cleanupTemp(p: BProc; returnType: PType, tmp: TLoc): bool =
if returnType.kind in {tyVar, tyLent}:
# we don't need to worry about var/lent return types
result = false
elif hasDestructor(returnType) and getAttachedOp(p.module.g.graph, returnType, attachedDestructor) != nil:
let dtor = getAttachedOp(p.module.g.graph, returnType, attachedDestructor)
var op = initLocExpr(p, newSymNode(dtor))
var callee = rdLoc(op)
let destroyArg =
if dtor.typ.firstParamType.kind == tyVar:
cAddr(rdLoc(tmp))
else:
rdLoc(tmp)
let destroy = cCall(callee, destroyArg)
raiseExitCleanup(p, destroy)
result = true
else:
result = false
proc fixupCall(p: BProc, le: PNode, ri: PNode, d: var TLoc,
proc fixupCall(p: BProc, le, ri: PNode, d: var TLoc,
result: var Builder, call: var CallBuilder) =
let canRaise = p.config.exc == excGoto and canRaiseDisp(p, ri.firstSon)
genLineDir(p, ri)
@@ -166,25 +147,18 @@ proc fixupCall(p: BProc, le: PNode, ri: PNode, d: var TLoc,
if canRaise: raiseExit(p)
elif isHarmlessStore(p, canRaise, d):
var useTemp = false
if d.k == locNone:
useTemp = true
d = getTemp(p, typ.returnType)
if d.k == locNone: d = getTemp(p, typ.returnType)
assert(d.t != nil) # generate an assignment to d:
var list = initLoc(locCall, d.lode, OnUnknown)
list.snippet = extract(result)
genAssignment(p, d, list, flags+{needAssignCall}) # no need for deep copying
if canRaise:
if not (useTemp and cleanupTemp(p, typ.returnType, d)):
raiseExit(p)
if canRaise: raiseExit(p)
else:
var tmp: TLoc = getTemp(p, typ.returnType, needsInit=true)
var list = initLoc(locCall, d.lode, OnUnknown)
list.snippet = extract(result)
genAssignment(p, tmp, list, flags+{needAssignCall}) # no need for deep copying
if canRaise:
if not cleanupTemp(p, typ.returnType, tmp):
raiseExit(p)
if canRaise: raiseExit(p)
genAssignment(p, d, tmp, {})
else:
finishCallBuilder(result, call)

View File

@@ -823,20 +823,6 @@ proc raiseExit(p: BProc) =
else:
p.s(cpsStmts).addGoto("LA" & $p.nestedTryStmts[^1].label & "_")
proc raiseExitCleanup(p: BProc, destroy: string) =
assert p.config.exc == excGoto
if nimErrorFlagDisabled notin p.flags:
p.flags.incl nimErrorFlagAccessed
p.s(cpsStmts).addSingleIfStmt(cUnlikely(cDeref("nimErr_"))):
p.s(cpsStmts).addStmt():
p.s(cpsStmts).add(destroy)
if p.nestedTryStmts.len == 0:
p.flags.incl beforeRetNeeded
# easy case, simply goto 'ret':
p.s(cpsStmts).addGoto("BeforeRet_")
else:
p.s(cpsStmts).addGoto("LA" & $p.nestedTryStmts[^1].label & "_")
proc finallyActions(p: BProc) =
if p.config.exc != excGoto:
# Walk past compiler-injected `nkHiddenTryStmt` wrappers (e.g. ARC's

View File

@@ -1074,7 +1074,6 @@ proc putLocIntoDest(p: BProc, d: var TLoc, s: TLoc)
proc genLiteral(p: BProc; n: PNode; result: var Builder)
proc genOtherArg(p: BProc; ri: PNode; i: int; typ: PType; result: var Builder; argBuilder: var CallBuilder)
proc raiseExit(p: BProc)
proc raiseExitCleanup(p: BProc, destroy: string)
proc initLocExpr(p: BProc; e: PNode, flags: TLocFlags = {}): TLoc =
result = initLoc(locNone, e, OnUnknown, flags)

View File

@@ -544,7 +544,8 @@ proc containsConstSeq(n: PNode): bool =
if containsConstSeq(son): return true
else: discard
proc ensureDestruction(arg, orig: PNode; c: var Con; s: var Scope): PNode =
proc ensureDestruction(arg, orig: PNode; c: var Con; s: var Scope;
consume = false): PNode =
# it can happen that we need to destroy expression contructors
# like [], (), closures explicitly in order to not leak them.
if arg.typ != nil and hasDestructor(c, arg.typ):
@@ -553,7 +554,9 @@ proc ensureDestruction(arg, orig: PNode; c: var Con; s: var Scope): PNode =
result = newNodeIT(nkStmtListExpr, arg.info, arg.typ)
let tmp = c.getTemp(s, arg.typ, arg.info, true)
result.add c.genSink(s, tmp, arg, {IsDecl})
result.add tmp
# In consumed mode, the destination takes ownership of the data.
# Clear the temporary after moving from it to prevent double destruction.
result.add if consume: destructiveMoveVar(tmp, c, s) else: tmp
s.final.add c.genDestroy(tmp)
else:
result = arg
@@ -970,11 +973,13 @@ proc p(n: PNode; c: var Con; s: var Scope; mode: ProcessMode; tmpFlags = {sfSing
else:
result[0] = p(n[0], c, s, normal)
if canRaise(n[0]): s.needsTry = true
if mode == normal:
# A raising call needs owned storage even when its value is consumed: the
# callee can partially initialize the result before control unwinds.
if mode == normal or (canRaise(n[0]) and inSpawn == 0):
if result.typ != nil and result.typ.kind notin {tyOpenArray, tyVarargs}:
# Returns of openarray types shouldn't be destroyed
# bug #19435; # bug #23247
result = ensureDestruction(result, n, c, s)
result = ensureDestruction(result, n, c, s, consume = mode != normal)
of nkDiscardStmt: # Small optimization
result = shallowCopy(n)
if n[0].kind != nkEmpty: