map! resulting in undefined values

[goretkin]

julia> VERSION
v"1.5.0-beta1.0"

julia> map!(+, [0,0,0], [1,2], [10,20,30], [100])
3-element Array{Int64,1}:
 111
  31
   0

julia> map!(+, [0,0,0], [1,2], [10,20,30], [100])
3-element Array{Int64,1}:
        111
 4616658790
          0

This also happened on v1.4.1

Pretty sure this commit removed the necessary bounds check: f9645ff

Probably, if @boundscheck LinearIndices(dest) == idxs1 && all(x -> LinearIndices(x) == idxs1, As) evaluates to false, then BoundsError should be thrown.

If I understand the intent correct, map!(+, [0,0,0], [1,2], [10,20,30], [100]) is supposed to error because the trailing arguments don't match the axes of the second argument. Or at the very least because the last argument is smaller than the second argument.

[jakobnissen]

Could we do something like this instead of the existing map_n!?

function map_n!(f, dest::AbstractArray, As)
    idx = eachindex(As...)
    @boundscheck checkbounds(dest, idx)
    @inbounds for i in idx
        I = ith_all(i, As)
        val = f(I...)
        dest[i] = val
    end
    return dest
end

[goretkin]

That seams reasonable. There's bound checking that happens in eachindex, too, which as far as I can tell don't get elided with @inbounds. It would be nice if they did, but that shouldn't block this bug fix.

[goretkin]

I just tried the example on Julia 1.6-rc1, and I was not able to generate undefined values.

[goretkin]

julia> VERSION
v"1.8.2"

julia> map!(+, [0,0,0], [1,2], [10,20,30], [100])
3-element Vector{Int64}:
        111
 4622078430
          0

julia> map!(+, [0,0,0], [1,2], [10,20,30], [100])
3-element Vector{Int64}:
        111
 4622107614
          0

julia> map!(+, [0,0,0], [1,2], [10,20,30], [100])
3-element Vector{Int64}:
        111
 4622120478
          0

[maleadt]

This also happened on 1.9, so shouldn't be on the 1.10 milestone?

[adienes]

@maleadt then add it to the 1.9 milestone? I don't think the right answer for a bug like this is to just forget about it for another 3 years

[maleadt]

Milestones are generally used to keep track of regressions, or of bugs that should hold up a release. This bugs doesn't fit that description, so I don't think it should be on the 1.10 milestone.

As long as there's an open issue, it's not considered forgotten about though.

[adienes]

I think this bug (exposing undefined behavior to the user, with tools as simple as map on Arrays) should hold up the release

[maleadt]

I don't think it needs to be release blocking, as the bug has been open for years without too many people noticing. I agree it's not a good look though; I'll see if this is easy to fix.

[LilithHafner]

map! also does not check that the destination is large enough to accommodate the answer, resulting in segfaults when there are many args:

julia> map!(+, [], [1,2,3,4], rand(1000))
Any[]

julia> map!(+, [], [1,2,3,4], rand(1000), [])

[127897] signal 11 (1): Segmentation fault
[...]