Segfault when mixing seqs, orc, variants and futures

[jmgomez]

Description

The code below makes a segfault

import std/[asyncdispatch]

type
  Result2*[T, E] = object
    case o: bool
      of false:
        e: E
      of true:
        v: T

  UnpackDefect* = object of Defect
template ok*[T, E](R: type Result2[T, E], x: untyped): R =
  R(o: true, v: x)

type
  MGErrorKind* = enum
    mgeUnexpected, mgeNotFound

type Foo = object
  case kind* : MGErrorKind
   of mgeUnexpected:
    ex : Exception
   else: discard 

type Boo = object
  a* : seq[int]

proc startSession*() : Future[Result2[Boo, Foo]] {.async.} =
  let fut = newFuture[string]("")
  fut.complete("")
  discard await fut

  return Result2[Boo, Foo].ok(Boo(a: @[2, 5]))

discard waitFor startSession()

Nim Version

1.6.10 and devel

Current Output

virtualFree failing!Error: execution of an external program failed

Expected Output

Ok

Possible Solution

No response

Additional Information

A extended version of the problem was tested on MacOs and it failed too.

[ringabout]

Works with default allocator on Windows with 1.6.8 and devel version. Segfault with -d:useMalloc.

[Araq]

But what does valgrind say?

[ringabout]

I didn't see leaka with valgrind.

On Linux, it provides more detailed traceback.

default allocator

/home/wind/.choosenim/toolchains/nim-#devel/lib/pure/asyncfutures.nim(38) test
/home/wind/.choosenim/toolchains/nim-#devel/lib/system/alloc.nim(1065) dealloc
/home/wind/.choosenim/toolchains/nim-#devel/lib/system/alloc.nim(964) rawDealloc
/home/wind/.choosenim/toolchains/nim-#devel/lib/system/alloc.nim(764) addToSharedFreeListBigChunks
SIGSEGV: Illegal storage access. (Attempt to read from nil?)
Segmentation fault

with `-d:useMalloc

free(): invalid pointer
Traceback (most recent call last)
/home/wind/.choosenim/toolchains/nim-#devel/lib/pure/asyncfutures.nim(38) test
SIGABRT: Abnormal termination.

[Araq]

So ... a double free?

[ringabout]

I have no idea, I'm trying to reduce it anyway.

[ringabout]

Yeah, it seems to be a double free still not sure

import std/[asyncdispatch]

type
  Result2*[T, E] = object
    case o: bool
      of false:
        e: E
      of true:
        v: T

type
  MGErrorKind* = enum
    mgeUnexpected, mgeNotFound

type Foo = object
  case kind*: MGErrorKind
  of mgeUnexpected:
    ex : Exception
  else: discard

type Boo = object
  a* : seq[int]

proc startSession*(): Future[Result2[Boo, Foo]] {.async.} =
  return Result2[Boo, Foo](o: true)

proc main =
  discard waitFor startSession()

main()

The example above is a reduced case, adding nodestroy to main makes it work.

var
  :tmpD
  :tmpD_1
try:
  discard
    :tmpD_1 = waitFor do:
      :tmpD = startSession()
      :tmpD
    :tmpD_1
finally:
  `=destroy`(:tmpD_1)
  `=destroy_1`(:tmpD)

[ringabout]

Where is the copy hook generation for case objects?

nimZeroMem((void *)(&(*dest)), sizeof(tyObject_Result2__vrqN9a5Tqls15C0YVhViY3A));
(*dest) = TM__GABJwbeb9a3OPxl9cisa0Vyg_12;
(*dest).o = src.o;
switch ((*dest).o)

The code above looks suspicious to me.

nimZeroMem((void *)(&(*dest)), sizeof(tyObject_Result2__vrqN9a5Tqls15C0YVhViY3A));
(*dest) = TM__GABJwbeb9a3OPxl9cisa0Vyg_12;
(*dest).o = src.o;
...probably emit zero memory and initialization again...
switch ((*dest).o)

[jmgomez]

If it's a double free, wouldnt manually call GC_ref work as a workaround if it were possible to do it over seq[T]? @Araq dont you think it worth to consider to have the option?

Just got another issue with refc now and Im setting --mm to none. It's not a big deal because it isnt going to prod any time soon (it's a startup prototype idea) but IMO it would be great to have the option to get around it without a refactoring the app by just doing GC_ref

[Araq]

Where is the copy hook generation for case objects?

compiler/liftdestructors.nim

[ringabout]

On Linux, the original code works with -d:useMalloc --mm:orc but not with arc.

[Araq]

Why does it matter, we have already established that the generated =copy hook is completely wrong.

[planetis-m]

...probably emit zero memory and initialization again...

Maybe my commit broke it https://github.com/nim-lang/Nim/pull/20300 ?

[ringabout]

Maybe my commit broke it https://github.com/nim-lang/Nim/pull/20300 ?

It should not be related since I can produce the issue with 1.6.6 which predates the mentioned PR.

[Araq]

The =copy is not wrong. The example uses ex: Exception and not ex: ref Exception which is totally weird but it does compile so it should run as well.

[Araq]

Further reduced (still wrong according to Valgrind):

type
  MGErrorKind* = enum
    mgeUnexpected, mgeNotFound

type Foo = object
  kind*: MGErrorKind
  ex: Exception

type Boo = object
  a*: seq[int]

type
  Result2* = object
    case o: bool
    of false:
      e: Foo
    of true:
      v: Boo

proc startSessionSync*(): Result2 =
  return Result2(o: true)

proc mainSync =
  let ff = startSessionSync()

mainSync()

[Araq]

And it starts working with the line return Result2(o: true, v: Boo()). Object construction that uses default values must use default values for the corresponding case branch.

[ringabout]

On it