From 2e79550c7ce9846313da15c4ce319b49f90920c8 Mon Sep 17 00:00:00 2001
From: zerbina <100542850+zerbina@users.noreply.github.com>
Date: Wed, 25 Sep 2024 00:26:58 +0000
Subject: [PATCH] fix(typeinfo): expanding seq doesn't clear locations

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.
---
 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