Destructors: case objects have issues

[cooldome]

Test case:

type
  TMyObj = object
    p: pointer
    len: int

proc `=destroy`(o: var TMyObj) =
  if o.p != nil:
    dealloc o.p
    o.p = nil
    echo "myobj destroyed"

proc `=`(dst: var TMyObj, src: TMyObj) =
  `=destroy`(dst)
  dst.p = alloc(src.len)
  dst.len = src.len


proc `=sink`(dst: var TMyObj, src: TMyObj) =
  `=destroy`(dst)
  dst.p = src.p
  dst.len = src.len
 
type
  TObjKind = enum Z, A, B
  TCaseObj = object
    case kind: TObjKind
    of Z: discard
    of A:
      x1: int # this int plays important role 
      x2: TMyObj
    of B:
      y: TMyObj


proc test: TCaseObj =
  result = TCaseObj(kind: A, x1: 5000, x2: TMyObj(len: 5, p: alloc(5)))
  result = TCaseObj(kind: B, y: TMyObj(len: 3, p: alloc(3)))

let x1 = test()

The test crashes at the runtime with the following stack trace:

myobj destroyed
Traceback (most recent call last)
C:\Nim\zcase.nim(39)     zcase
C:\Nim\zcase.nim(36)     test
C:\Nim\zcase.nim(19)     =sink
C:\Nim\zcase.nim(8)      =destroy
C:\Nim\lib\system\alloc.nim(945) dealloc
C:\Nim\lib\system\alloc.nim(819) rawDealloc
C:\Nim\lib\system\alloc.nim(370) isSmallChunk
SIGSEGV: Illegal storage access. (Attempt to read from nil?)

I have investigated the reason of the crash. Sink and copy operators generated in the following way:

proc `=`(dst: var TCaseObj, src: TCaseObj) =
  `=destroy`(fieldsUnderCase)  # all good
  dst.kind = src.kind # all good
  `=`(dst.fieldsUnderCase, src.fieldsUnderCase)  # problem here: dst.fieldsUnderCase contains garbage when union is switched and it will try to destroy a garbage

Solution:
We need to reset memory before invoking copy/sink or use weakAsgn/memMove proposed by Clybber

[cooldome]

@Araq, what would be your preferred solution? do you think weakAsgn is good idea?

[disruptek]

Try his fix for #13102 and let's see if we can narrow the scope even further.

[Araq]

See #13151

[cooldome]

Cool, so you are on the case already