From 94b4ba904b1476c95bcd15ab9cae2bfa0746d728 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Thu, 2 Oct 2025 17:49:43 +0000 Subject: [PATCH 1/3] iobuffer: fix overflow OOB read/write bugs maxsize is usually typemax, so need to be careful to not do comparisons after adding to it. Substracting from it should normally be perfectly fine. At worst we should compute a negative amount of space remaining. --- base/iobuffer.jl | 10 ++++++---- base/stream.jl | 3 ++- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/base/iobuffer.jl b/base/iobuffer.jl index 1a793ec2eef71..6645ffeeb777a 100644 --- a/base/iobuffer.jl +++ b/base/iobuffer.jl @@ -604,7 +604,8 @@ end end # The fast path here usually checks there is already room, then does nothing. # When append is true, new data is added after io.size, not io.ptr - existing_space = min(lastindex(io.data), io.maxsize + get_offset(io)) - (io.append ? io.size : io.ptr - 1) + start_offset = io.append ? io.size : io.ptr - 1 + existing_space = min(lastindex(io.data) - start_offset, io.maxsize - (start_offset - get_offset(io))) if existing_space < nshort % Int # Outline this function to make it more likely that ensureroom inlines itself return ensureroom_slowpath(io, nshort, existing_space) @@ -898,12 +899,13 @@ function unsafe_write(to::GenericIOBuffer, p::Ptr{UInt8}, nb::UInt) append = to.append ptr = append ? size+1 : to.ptr data = to.data - to_write = min(nb, (min(Int(length(data))::Int, to.maxsize + get_offset(to)) - ptr + 1) % UInt) % Int + start_offset = ptr - 1 + to_write = max(0, min(nb, (min(Int(length(data))::Int - start_offset, to.maxsize - (start_offset - get_offset(to)))) % UInt) % Int) # Dispatch based on the type of data, to possibly allow using memcpy _unsafe_write(data, p, ptr, to_write % UInt) # Update to.size only if the ptr has advanced to higher than # the previous size. Otherwise, we just overwrote existing data - to.size = max(size, ptr + to_write - 1) + to.size = max(size, start_offset + to_write) # If to.append, we only update size, not ptr. if !append to.ptr = ptr + to_write @@ -941,7 +943,7 @@ end ptr = (to.append ? to.size+1 : to.ptr) # We have just ensured there is room for 1 byte, EXCEPT if we were to exceed # maxsize. So, we just need to check that here. - if ptr > to.maxsize + get_offset(to) + if ptr - get_offset(to) > to.maxsize return 0 else to.data[ptr] = a diff --git a/base/stream.jl b/base/stream.jl index 40106436ebd05..0ccf314f184aa 100644 --- a/base/stream.jl +++ b/base/stream.jl @@ -612,7 +612,8 @@ end function alloc_request(buffer::IOBuffer, recommended_size::UInt) ensureroom(buffer, recommended_size) ptr = buffer.append ? buffer.size + 1 : buffer.ptr - nb = min(length(buffer.data), buffer.maxsize + get_offset(buffer)) - ptr + 1 + start_offset = ptr - 1 + nb = min(length(buffer.data) - start_offset, buffer.maxsize - (start_offset - get_offset(buffer))) return (Ptr{Cvoid}(pointer(buffer.data, ptr)), nb) end From 5a41455d442bc74fcba4fa6384a4dc5985e1150d Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Thu, 2 Oct 2025 14:19:20 -0400 Subject: [PATCH 2/3] Update stream.jl --- base/stream.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/stream.jl b/base/stream.jl index 0ccf314f184aa..ead65d9b59706 100644 --- a/base/stream.jl +++ b/base/stream.jl @@ -613,7 +613,7 @@ function alloc_request(buffer::IOBuffer, recommended_size::UInt) ensureroom(buffer, recommended_size) ptr = buffer.append ? buffer.size + 1 : buffer.ptr start_offset = ptr - 1 - nb = min(length(buffer.data) - start_offset, buffer.maxsize - (start_offset - get_offset(buffer))) + nb = max(0, min(length(buffer.data) - start_offset, buffer.maxsize - (start_offset - get_offset(buffer)))) return (Ptr{Cvoid}(pointer(buffer.data, ptr)), nb) end From 97ff2317723f1e4ef1255465f3f36fe0543257c2 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Wed, 1 Oct 2025 13:22:15 +0000 Subject: [PATCH 3/3] array: reduce overallocation when inserting into small arrays Fixes a minor nuisance where length 0 would grow to 8 and length 7 would also grow to 8. Instead growing by a flat 3x even initially. Fixes #58640 --- base/array.jl | 6 +++--- test/arrayops.jl | 6 ++++++ test/core.jl | 3 --- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/base/array.jl b/base/array.jl index 308f31c40371c..0a24dc4fd0785 100644 --- a/base/array.jl +++ b/base/array.jl @@ -1087,14 +1087,14 @@ end # Specifically we are wasting ~10% of memory for small arrays # by not picking memory sizes that max out a GC pool function overallocation(maxsize) - maxsize < 8 && return 8; - # compute maxsize = maxsize + 4*maxsize^(7/8) + maxsize/8 + # compute maxsize = maxsize + 3*maxsize^(7/8) + maxsize/8 # for small n, we grow faster than O(n) # for large n, we grow at O(n/8) # and as we reach O(memory) for memory>>1MB, # this means we end by adding about 10% of memory each time + # most commonly, this will take steps of 0-3-9-34 or 1-4-16-66 or 2-8-33 exp2 = sizeof(maxsize) * 8 - Core.Intrinsics.ctlz_int(maxsize) - maxsize += (1 << div(exp2 * 7, 8)) * 4 + div(maxsize, 8) + maxsize += (1 << div(exp2 * 7, 8)) * 3 + div(maxsize, 8) return maxsize end diff --git a/test/arrayops.jl b/test/arrayops.jl index d4b401152d7ef..95314c3c85fb2 100644 --- a/test/arrayops.jl +++ b/test/arrayops.jl @@ -525,6 +525,12 @@ end v = empty!(collect(1:100)) pushfirst!(v, 1) @test length(v.ref.mem) == 100 + + # test that insert! at position 1 doesn't allocate for empty arrays with capacity (issue #58640) + v = empty!(Vector{Int}(undef, 5)) + insert!(v, 1, 10) + @test v == [10] + @test length(v.ref.mem) == 5 end @testset "popat!(::Vector, i, [default])" begin diff --git a/test/core.jl b/test/core.jl index 5e8acf5740f34..618189fea6bbe 100644 --- a/test/core.jl +++ b/test/core.jl @@ -6852,9 +6852,6 @@ Base._growat!(A, 4, 1) Base._growat!(A, 2, 3) @test getindex(A, 1) === 0x01 @test getindex(A, 2) === missing -@test getindex(A, 3) === missing -@test getindex(A, 4) === missing -@test getindex(A, 5) === missing @test getindex(A, 6) === 0x03 @test getindex(A, 7) === missing @test getindex(A, 8) === missing