From 26a4b137c67d5dc5cdfd340a85bac0b5cb89b34e Mon Sep 17 00:00:00 2001 From: ringabout <43030857+ringabout@users.noreply.github.com> Date: Tue, 11 Jun 2024 11:55:08 +0800 Subject: [PATCH] [backport] fixes #23690; SIGSEGV with object variants and RTTI (#23703) fixes #23690 ```nim dest.`:state` = src.`:state` var :tmp_553651276 = dest.e1.a `=wasMoved`(dest.e1.a) dest.e1.a.kind = src.e1.a.kind case dest.e1.a.kind of 0: dest.e1.a.a = src.e1.a.a of 1: `=copy`(dest.e1.a.c, src.e1.a.c) case :tmp_553651276.kind of 0: of 1: `=destroy`(:tmp_553651276.c) ``` `dest.e1.a.kind = src.e1.a.kind` changes the discrimant but it fails to clear the memory of `dest.e1.a`. Before using hooks for copying, we need to clear the dest, e.g. `=wasMoved(dest.e1.a.c)`. ```nim dest.`:state` = src.`:state` var :tmp_553651276 = dest.e1.a `=wasMoved`(dest.e1.a) dest.e1.a.kind = src.e1.a.kind case dest.e1.a.kind of 0: `=wasMoved`(dest.e1.a.a) dest.e1.a.a = src.e1.a.a `=wasMoved`(dest.e1.a.b) of 1: `=wasMoved`(dest.e1.a.c) `=copy`(dest.e1.a.c, src.e1.a.c) case :tmp_553651276.kind of 0: of 1: `=destroy`(:tmp_553651276.c) ``` (cherry picked from commit 262ff648aad7cc2062eb94cb6bf3357f9910c448) --- compiler/liftdestructors.nim | 9 ++++++--- tests/arc/tcaseobj.nim | 26 ++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/compiler/liftdestructors.nim b/compiler/liftdestructors.nim index 749e4ff491..0b5a6f0821 100644 --- a/compiler/liftdestructors.nim +++ b/compiler/liftdestructors.nim @@ -156,7 +156,7 @@ proc genWasMovedCall(c: var TLiftCtx; op: PSym; x: PNode): PNode = result.add(newSymNode(op)) result.add genAddr(c, x) -proc fillBodyObj(c: var TLiftCtx; n, body, x, y: PNode; enforceDefaultOp: bool) = +proc fillBodyObj(c: var TLiftCtx; n, body, x, y: PNode; enforceDefaultOp: bool, enforceWasMoved = false) = case n.kind of nkSym: if c.filterDiscriminator != nil: return @@ -166,6 +166,8 @@ proc fillBodyObj(c: var TLiftCtx; n, body, x, y: PNode; enforceDefaultOp: bool) enforceDefaultOp: defaultOp(c, f.typ, body, x.dotField(f), b) else: + if enforceWasMoved: + body.add genBuiltin(c, mWasMoved, "`=wasMoved`", x.dotField(f)) fillBody(c, f.typ, body, x.dotField(f), b) of nkNilLit: discard of nkRecCase: @@ -204,7 +206,7 @@ proc fillBodyObj(c: var TLiftCtx; n, body, x, y: PNode; enforceDefaultOp: bool) branch[^1] = newNodeI(nkStmtList, c.info) fillBodyObj(c, n[i].lastSon, branch[^1], x, y, - enforceDefaultOp = localEnforceDefaultOp) + enforceDefaultOp = localEnforceDefaultOp, enforceWasMoved = c.kind == attachedAsgn) if branch[^1].len == 0: inc emptyBranches caseStmt.add(branch) if emptyBranches != n.len-1: @@ -215,7 +217,7 @@ proc fillBodyObj(c: var TLiftCtx; n, body, x, y: PNode; enforceDefaultOp: bool) fillBodyObj(c, n[0], body, x, y, enforceDefaultOp = false) c.filterDiscriminator = oldfilterDiscriminator of nkRecList: - for t in items(n): fillBodyObj(c, t, body, x, y, enforceDefaultOp) + for t in items(n): fillBodyObj(c, t, body, x, y, enforceDefaultOp, enforceWasMoved) else: illFormedAstLocal(n, c.g.config) @@ -281,6 +283,7 @@ proc fillBodyObjT(c: var TLiftCtx; t: PType, body, x, y: PNode) = c.kind = attachedDestructor fillBodyObjTImpl(c, t, body, blob, y) c.kind = prevKind + else: fillBodyObjTImpl(c, t, body, x, y) diff --git a/tests/arc/tcaseobj.nim b/tests/arc/tcaseobj.nim index be1d722edc..3499f5c1e3 100644 --- a/tests/arc/tcaseobj.nim +++ b/tests/arc/tcaseobj.nim @@ -338,3 +338,29 @@ block: doAssert ff.s == 12 mainSync() + +import std/sequtils + +# bug #23690 +type + SomeObj* = object of RootObj + + Item* = object + case kind*: 0..1 + of 0: + a*: int + b*: SomeObj + of 1: + c*: string + + ItemExt* = object + a*: Item + b*: string + +proc do1(x: int): seq[(string, Item)] = + result = @[("zero", Item(kind: 1, c: "first"))] + +proc do2(x: int, e: ItemExt): seq[(string, ItemExt)] = + do1(x).map(proc(v: (string, Item)): auto = (v[0], ItemExt(a: v[1], b: e.b))) + +doAssert $do2(0, ItemExt(a: Item(kind: 1, c: "second"), b: "third")) == """@[("zero", (a: (kind: 1, c: "first"), b: "third"))]"""