memory corruption for seq.add(seq) with gc:arc and d:useMalloc

[ghost]

Example

nim c -d:release -d:useMalloc --gc:arc file.nim

var s = @[1,2,3]
for _ in 1..6:
  s.add(s)
echo s

Current Output

@[1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1769808, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3, 1, 2, 3]

Expected Output

only with numbers 1,2,3

Possible Solution

• not using gc:arc and d:useMalloc together
• passing s2 which is a shallowCopy of s as second parameter for add

Additional Information

Nim Compiler Version 1.3.5 [Windows: amd64]
Compiled at 2020-07-02
Copyright (c) 2006-2020 by Andreas Rumpf

git hash: 366b9a7e4a067cece3b1215de7fb4dffc703e88a
active boot switches: -d:release

[ghost]

Maybe this is the same as here #14472, my nim version is from 2020-07-02.

[Yardanico]

Can't reproduce on latest devel, I get same results with refc/arc/arc + useMalloc
Can see valgrind errors

[ghost]

It also happens with

Nim Compiler Version 1.3.5 [Windows: amd64]
Compiled at 2020-07-13
Copyright (c) 2006-2020 by Andreas Rumpf

active boot switches: -d:release

and

gcc (Rev5, Built by MSYS2 project) 5.3.0

[cooldome]

Retested.
Works for me as of 04-Sep-2020

[ghost]

I can't confirm that it works with

Nim Compiler Version 1.3.5 [Windows: amd64]
Compiled at 2020-09-04
Copyright (c) 2006-2020 by Andreas Rumpf

active boot switches: -d:release

and

gcc (Rev5, Built by MSYS2 project) 5.3.0

Update: I now compiled the latest nim version from source and the result is the same as before.

Compiling with nim cpp -d:release -d:useMalloc --gc:arc file.nim instead of c gives the correct output, but maybe there are errors with valgrind too.

[ghost]

Here is a similar code showing the problem more obviously:

Compile with nim c -d:release -d:useMalloc --gc:arc file.nim

file.nim

proc add2*[T](x: var seq[T], y: openArray[T]) =
  var y2 = @y # works without this line
  let xl = x.len
  setLen(x, xl + y.len)
  for i in 0..high(y):
    x[xl+i] = y[i]


var s = @[1,2,3]
add2(s, s)
echo s

[cooldome]

So, it is the parameter aliasing problem. First parameter is var and second is not. If you pass the same s as both arguments, you get undefined behavior.

IMO, we are leaning towards that the compiler should not allow it, detecting it is the aliasing problem. In future it would not compile and users will have to write:

var s = @[1,2,3]
add2(s, copy(s))
echo s

[Araq]

I'm leaning to the compiler producing a copy instead when the aliasing can cause trouble. No need to break code.

[ghost]

@Araq should I open a new issue for the generic problem like shown in #14983 (comment) ?
I think your fix #15320 adresses only the specific case for system.add.

[Araq]

The more general problem is covered in https://nim-lang.org/docs/manual_experimental.html#aliasing-restrictions-in-parameter-passing with an expected implementation in 2021.