memory corruption in tmarshall.nim

[timotheecour]

comment either and it'll work;

import tests/stdlib/tmarshal
import tests/stdlib/tparsesql

nim c -r bug/run_all.nim

{"age": 12, "bio": "Я Cletus", "blob": [65, 66, 67, 128], "name": "Cletus"}
true
true
alpha 100
omega 200
Traceback (most recent call last)
/Users/timothee/git_clone/nim/Nim/tests/stdlib/tparsesql.nim(196) tparsesql
/Users/timothee/git_clone/nim/Nim/lib/pure/parsesql.nim(1514) parseSQL
/Users/timothee/git_clone/nim/Nim/lib/pure/parsesql.nim(1506) parseSQL
/Users/timothee/git_clone/nim/Nim/lib/pure/parsesql.nim(1181) parse
/Users/timothee/git_clone/nim/Nim/lib/pure/parsesql.nim(1170) parseStmt
/Users/timothee/git_clone/nim/Nim/lib/pure/parsesql.nim(1082) parseSelect
/Users/timothee/git_clone/nim/Nim/lib/pure/parsesql.nim(950) parseWhere
/Users/timothee/git_clone/nim/Nim/lib/pure/parsesql.nim(786) parseExpr
/Users/timothee/git_clone/nim/Nim/lib/pure/parsesql.nim(778) lowestExprAux
/Users/timothee/git_clone/nim/Nim/lib/pure/parsesql.nim(771) lowestExprAux
/Users/timothee/git_clone/nim/Nim/lib/pure/parsesql.nim(737) primary
/Users/timothee/git_clone/nim/Nim/lib/pure/parsesql.nim(697) identOrLiteral
/Users/timothee/git_clone/nim/Nim/lib/pure/parsesql.nim(576) newNode
/Users/timothee/git_clone/nim/Nim/lib/system/gc.nim(496) newObjRC1
/Users/timothee/git_clone/nim/Nim/lib/system/alloc.nim(781) rawAlloc
SIGSEGV: Illegal storage access. (Attempt to read from nil?)
Error: execution of an external program failed: '/Users/timothee/git_clone/nim/Nim/bug/run_all '

note

I ran into this while working on #9581
this seems like a different bug from #9752

[timotheecour]

marking high priority as it affects PR's like #10355 and subsequent PR's that aim at making CI run faster by joining more tests in megatest

[timotheecour]

EDIT: removed high priority label because I found a workaround, see #10355

[LemonBoy]

No wonder it's crashing, the test7 in tmarshal.nim serializes a bunch of pointers that become invalid before the object is reconstructed.

Notabug @Araq ?

[Araq]

Seems like a valid bug to me, marshal is supposed to work with ref.

[LemonBoy]

I don't get how it's supposed to work safely. Here you marshal the ref pointer as, well, an integer... and then the object it points to gets collected by the GC before the un-marshaling is done.

[Araq]

The marshalling is supposed to produce (ID, value) pairs for the first occurence of the data and then only ID for backrefs and this whole mechanism will produce a copy of the linked list, the GC can collect the old list. Marshal is supposed to be memory safe and the test doesn't use cast.

[LemonBoy]

Alright, I had a deeper look at this. If you turn on -d:nimBurnFree you can easily see that we got a double free here and the root of the problem seems to be the definition of testit: if you replace it with the following definition that uses a temporary variable the test case runs just fine with -d:nimBurnFree -d:useGcAssert -d:useSysAssert

template testit(x) =
  let x1 = to[type(x)]($$(x))
  discard $$x1

It seems that the node we end up freeing twice is somehow stuck in the ZCT but I'm not 100% this is the case since I'm not a GC expert.

[Araq]

Note: it works with --gc:markAndSweep.

[krux02]

This is not a compiler crash.