From c61e95be906102cc57f9b826179fbebb33c51be5 Mon Sep 17 00:00:00 2001
From: zerbina <100542850+zerbina@users.noreply.github.com>
Date: Wed, 25 Sep 2024 03:57:05 +0200
Subject: [PATCH] fix(typeinfo): expanding seq doesn't clear locations (#1463)

## Summary

New seq slots are now zeroed when expanding the seq via `invokeNewSeq`
or `extendSeq`, making the behaviour consistent with `setLen` and
`newSeq`, and also fixing crashes with `marshal` caused by the
uninitialized memory.

## Details

Zeroing the memory is not correct for types that don't have a zero-
default, but those cannot be detected with just RTTI. Zeroing the
memory is usually still better then leaving it as is.

For the JavaScript and VM backends, the `zeroMem` call is excluded
from compilation. Using `invokeNewSeq` and `extendSeq` is already
not possible on these backends.

Fixes https://github.com/nim-works/nimskull/issues/1462
---
 lib/core/typeinfo.nim                      | 18 +++++++++++++++---
 tests/stdlib/metaprogramming/ttypeinfo.nim | 14 ++++++++++++++
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/lib/core/typeinfo.nim b/lib/core/typeinfo.nim
index 69dfc6b6fa6..3a637dd49e4 100644
--- a/lib/core/typeinfo.nim
+++ b/lib/core/typeinfo.nim
@@ -108,6 +108,9 @@ proc prepareSeqAdd(len: int; p: pointer; addlen, elemSize, elemAlign: int): poin
 
 template `+!!`(a, b): untyped = cast[pointer](cast[ByteAddress](a) + b)
 
+proc align(address, alignment: int): int =
+  result = (address + (alignment - 1)) and not (alignment - 1)
+
 proc getDiscriminant(aa: pointer, n: ptr TNimNode): int =
   assert(n.kind == nkCase)
   var d: int
@@ -180,6 +183,11 @@ proc invokeNewSeq*(x: Any, len: int) =
     s.len = len
     let elem = x.rawType.base
     s.p = cast[ptr NimSeqPayloadReimpl](newSeqPayload(len, elem.size, elem.align))
+    if len > 0:
+      # zeroing the memory might not be legal depending on the type, but there's
+      # curently no way to check that here
+      when declared(zeroMem): # not the case for JS and the VM
+        zeroMem(s.p +!! align(sizeof(int), elem.align), elem.size * len)
 
 proc extendSeq*(x: Any) =
   ## Performs `setLen(x, x.len+1)`. `x` needs to represent a `seq`.
@@ -189,6 +197,13 @@ proc extendSeq*(x: Any) =
     let elem = x.rawType.base
     if s.p == nil or s.p.cap < s.len+1:
       s.p = cast[ptr NimSeqPayloadReimpl](prepareSeqAdd(s.len, s.p, 1, elem.size, elem.align))
+
+    # zeroing the memory might not be legal depending on the type, but there's
+    # curently no way to check that here
+    when declared(zeroMem): # not the case for JS and the VM
+      let headerSize = align(sizeof(int), elem.align)
+      zeroMem(s.p +!! (headerSize + s.len * elem.size), elem.size)
+
     inc s.len
 
 proc setObjectRuntimeType*(x: Any) =
@@ -203,9 +218,6 @@ proc skipRange(x: PNimType): PNimType {.inline.} =
   result = x
   if result.kind == tyRange: result = result.base
 
-proc align(address, alignment: int): int =
-  result = (address + (alignment - 1)) and not (alignment - 1)
-
 proc `[]`*(x: Any, i: int): Any =
   ## Accessor for an any `x` that represents an array or a sequence.
   case x.rawType.kind
diff --git a/tests/stdlib/metaprogramming/ttypeinfo.nim b/tests/stdlib/metaprogramming/ttypeinfo.nim
index 61c661a58e2..a8a83b9ad3a 100644
--- a/tests/stdlib/metaprogramming/ttypeinfo.nim
+++ b/tests/stdlib/metaprogramming/ttypeinfo.nim
@@ -69,3 +69,17 @@ block:
 
   doAssert getEnumOrdinal(y, "Hello") == 0
   doAssert getEnumOrdinal(y, "hello") == 1
+
+block seq_extension_zeroes:
+  # make sure ``invokeNewSeq`` and ``extendSeq`` zero the new locations
+  var s: seq[int]
+  toAny(s).invokeNewSeq(100)
+
+  # check that the values are all zero and change them to something else
+  for it in s.mitems:
+    doAssert it == 0
+    it = 1
+
+  s.setLen(0) # clear the seq, which should leave the memory untouched
+  toAny(s).extendSeq() # grow by one
+  doAssert s[0] == 0