Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #14655 setLen(seq) now zeros memory #14656

Merged
merged 2 commits into from
Jun 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 15 additions & 14 deletions lib/system/sysstr.nim
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@
# we don't use refcounts because that's a behaviour
# the programmer may not want


proc dataPointer(a: PGenericSeq, elemAlign: int): pointer =
cast[pointer](cast[ByteAddress](a) +% align(GenericSeqSize, elemAlign))

proc dataPointer(a: PGenericSeq, elemAlign, elemSize, index: int): pointer =
cast[pointer](cast[ByteAddress](a) +% align(GenericSeqSize, elemAlign) +% (index*%elemSize))

proc resize(old: int): int {.inline.} =
if old <= 0: result = 4
elif old < 65536: result = old * 2
Expand Down Expand Up @@ -267,9 +274,6 @@ proc incrSeqV2(seq: PGenericSeq, elemSize, elemAlign: int): PGenericSeq {.compil
result = cast[PGenericSeq](growObj(result, align(GenericSeqSize, elemAlign) + elemSize * r))
result.reserved = r

template `+!`(p: pointer, s: int): pointer =
cast[pointer](cast[int](p) +% s)

proc incrSeqV3(s: PGenericSeq, typ: PNimType): PGenericSeq {.compilerproc.} =
if s == nil:
result = cast[PGenericSeq](newSeq(typ, 1))
Expand All @@ -281,7 +285,7 @@ proc incrSeqV3(s: PGenericSeq, typ: PNimType): PGenericSeq {.compilerproc.} =
when defined(nimIncrSeqV3):
result = cast[PGenericSeq](newSeq(typ, r))
result.len = s.len
copyMem(result +! align(GenericSeqSize, typ.base.align), s +! align(GenericSeqSize, typ.base.align), s.len * typ.base.size)
copyMem(dataPointer(result, typ.base.align), dataPointer(s, typ.base.align), s.len * typ.base.size)
# since we steal the content from 's', it's crucial to set s's len to 0.
s.len = 0
else:
Expand All @@ -303,7 +307,7 @@ proc setLengthSeq(seq: PGenericSeq, elemSize, elemAlign, newLen: int): PGenericS
when false: # compileOption("gc", "v2"):
for i in newLen..result.len-1:
let len0 = gch.tempStack.len
forAllChildrenAux(cast[pointer](cast[ByteAddress](result) +% align(GenericSeqSize, elemAlign) +% (i*%elemSize)),
forAllChildrenAux(dataPointer(result, elemAlign, elemSize, i),
extGetCellType(result).base, waPush)
let len1 = gch.tempStack.len
for i in len0 ..< len1:
Expand All @@ -312,8 +316,7 @@ proc setLengthSeq(seq: PGenericSeq, elemSize, elemAlign, newLen: int): PGenericS
else:
if ntfNoRefs notin extGetCellType(result).base.flags:
for i in newLen..result.len-1:
forAllChildrenAux(cast[pointer](cast[ByteAddress](result) +%
align(GenericSeqSize, elemAlign) +% (i*%elemSize)),
forAllChildrenAux(dataPointer(result, elemAlign, elemSize, i),
extGetCellType(result).base, waZctDecRef)

# XXX: zeroing out the memory can still result in crashes if a wiped-out
Expand All @@ -322,8 +325,7 @@ proc setLengthSeq(seq: PGenericSeq, elemSize, elemAlign, newLen: int): PGenericS
# presence of user defined destructors, the user will expect the cell to be
# "destroyed" thus creating the same problem. We can destroy the cell in the
# finalizer of the sequence, but this makes destruction non-deterministic.
zeroMem(cast[pointer](cast[ByteAddress](result) +% align(GenericSeqSize, elemAlign) +%
(newLen*%elemSize)), (result.len-%newLen) *% elemSize)
zeroMem(dataPointer(result, elemAlign, elemSize, newLen), (result.len-%newLen) *% elemSize)
result.len = newLen

proc setLengthSeqV2(s: PGenericSeq, typ: PNimType, newLen: int): PGenericSeq {.
Expand All @@ -338,7 +340,7 @@ proc setLengthSeqV2(s: PGenericSeq, typ: PNimType, newLen: int): PGenericSeq {.
if s.space < newLen:
let r = max(resize(s.space), newLen)
result = cast[PGenericSeq](newSeq(typ, r))
copyMem(result +! align(GenericSeqSize, elemAlign), s +! align(GenericSeqSize, elemAlign), s.len * elemSize)
copyMem(dataPointer(result, elemAlign), dataPointer(s, elemAlign), s.len * elemSize)
# since we steal the content from 's', it's crucial to set s's len to 0.
s.len = 0
elif newLen < s.len:
Expand All @@ -349,8 +351,7 @@ proc setLengthSeqV2(s: PGenericSeq, typ: PNimType, newLen: int): PGenericSeq {.
not defined(gcRegions):
if ntfNoRefs notin typ.base.flags:
for i in newLen..result.len-1:
forAllChildrenAux(cast[pointer](cast[ByteAddress](result) +%
align(GenericSeqSize, elemAlign) +% (i*%elemSize)),
forAllChildrenAux(dataPointer(result, elemAlign, elemSize, i),
extGetCellType(result).base, waZctDecRef)

# XXX: zeroing out the memory can still result in crashes if a wiped-out
Expand All @@ -359,10 +360,10 @@ proc setLengthSeqV2(s: PGenericSeq, typ: PNimType, newLen: int): PGenericSeq {.
# presence of user defined destructors, the user will expect the cell to be
# "destroyed" thus creating the same problem. We can destroy the cell in the
# finalizer of the sequence, but this makes destruction non-deterministic.
zeroMem(cast[pointer](cast[ByteAddress](result) +% align(GenericSeqSize, elemAlign) +%
(newLen*%elemSize)), (result.len-%newLen) *% elemSize)
zeroMem(dataPointer(result, elemAlign, elemSize, newLen), (result.len-%newLen) *% elemSize)
else:
result = s
zeroMem(dataPointer(result, elemAlign, elemSize, result.len), (newLen-%result.len) *% elemSize)
result.len = newLen
else:
result = setLengthSeq(s, typ.base.size, newLen)
9 changes: 8 additions & 1 deletion tests/collections/tseq.nim
Original file line number Diff line number Diff line change
Expand Up @@ -206,4 +206,11 @@ block ttoseq:
stdout.write("\n")

block tseqmapitchain:
doAssert @[101, 102] == [1, 2].mapIt(func (x: int): int = it + x).mapIt(it(100))
doAssert @[101, 102] == [1, 2].mapIt(func (x: int): int = it + x).mapIt(it(100))


for i in 0..100:
# fix #14655
var test = newSeqOfCap[uint32](1)
test.setLen(1)
doAssert test[0] == 0, $(test[0], i)