From 143d6c545301bd253301393a7acfb2fcbfa1bb45 Mon Sep 17 00:00:00 2001 From: ChrisRackauckas-Claude Date: Thu, 8 Oct 2026 06:47:23 -0400 Subject: [PATCH 1/2] Fix two silent memory-safety bugs in utils.jl recursivecopy!(b::AbstractArray{<:StaticArray}, a) looped over eachindex(a), so a shorter dest silently wrote out of bounds instead of throwing. copyat_or_push! only checked length(a) >= i, so i=0 (or negative) fell into the @inbounds update branch and corrupted memory instead of throwing a BoundsError. recursivecopy! now uses eachindex(b, a) (throws DimensionMismatch on a length mismatch, matching Base's own idiom). copyat_or_push! now rejects i < 1 with a BoundsError before the @inbounds block. Before the fix, both cases segfault the Julia process on 1.10 and 1.12 (see PR description for the exact crash traces). After the fix they throw DimensionMismatch / BoundsError and the existing test suite (Core group, 120/120 Utils Tests) still passes unchanged on both versions. Co-Authored-By: Chris Rackauckas Co-Authored-By: Claude Agent-Harness: Claude Code 2.1.294 Agent-Model: claude-sonnet-5 Agent-Session: https://claude.ai/code/session_01LPHREnnonfLg1VcE1EJovv --- src/utils.jl | 3 ++- test/Core/utils_test.jl | 25 +++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/utils.jl b/src/utils.jl index 0209140a..cfe8045d 100644 --- a/src/utils.jl +++ b/src/utils.jl @@ -86,7 +86,7 @@ function recursivecopy!( T2 <: StaticArraysCore.StaticArray, N, } - return @inbounds for i in eachindex(a) + return @inbounds for i in eachindex(b, a) # TODO: Check for `setindex!`` and use `copy!(b[i],a[i])` or `b[i] = a[i]`, see #19 b[i] = copy(a[i]) end @@ -367,6 +367,7 @@ values == [[1, 2], [3, 4]] ``` """ function copyat_or_push!(a::AbstractVector{T}, i::Int, x, perform_copy = true) where {T} + i >= 1 || throw(BoundsError(a, i)) @inbounds if length(a) >= i if !ArrayInterface.ismutable(T) || !perform_copy # TODO: Check for `setindex!`` if T <: StaticArraysCore.StaticArray and use `copy!(b[i],a[i])` diff --git a/test/Core/utils_test.jl b/test/Core/utils_test.jl index 3ef3bb0c..c2d0374c 100644 --- a/test/Core/utils_test.jl +++ b/test/Core/utils_test.jl @@ -152,6 +152,31 @@ end @test a.u[1][1] == 1.0 end +@testset "recursivecopy! destination shorter than source throws" begin + b = [MVector{2, Float64}(0, 0)] + a = [MVector{2, Float64}(1, 1), MVector{2, Float64}(2, 2)] + @test_throws DimensionMismatch recursivecopy!(b, a) + + # matching lengths still work + b2 = [MVector{2, Float64}(0, 0), MVector{2, Float64}(0, 0)] + recursivecopy!(b2, a) + @test b2[1] == [1.0, 1.0] + @test b2[2] == [2.0, 2.0] +end + +@testset "copyat_or_push! rejects non-positive indices" begin + vals = [[1, 2], [3, 4]] + @test_throws BoundsError copyat_or_push!(vals, 0, [9, 9]) + @test_throws BoundsError copyat_or_push!(vals, -1, [9, 9]) + @test vals == [[1, 2], [3, 4]] # unchanged by the rejected calls + + # normal update and append still work + copyat_or_push!(vals, 1, [9, 9]) + @test vals[1] == [9, 9] + copyat_or_push!(vals, 3, [5, 6]) + @test vals == [[9, 9], [3, 4], [5, 6]] +end + # OrdinaryDiffEq.jl#1365: recursivecopy must not broadcast-assign into an # immutable outer container such as SVector of arrays. @testset "recursivecopy VectorOfArray with SVector outer (ODE#1365)" begin From c7b71d3b41cdbccd27660a09568208aef2882b58 Mon Sep 17 00:00:00 2001 From: ChrisRackauckas-Claude Date: Thu, 8 Oct 2026 15:24:41 -0400 Subject: [PATCH 2/2] Fix copyat_or_push! bounds guard to use firstindex, not literal 1 Round-2 review (Grok 4.7, pr-667/round2): the i >= 1 guard turned a previously correct call into a throw for any AbstractVector whose firstindex isn't 1. copyat_or_push! is exported and its docstring promises to copy into a[i] "when that element exists" -- on an OffsetArray with indices 0:1, index 0 exists and master updates it; the i >= 1 guard threw BoundsError there instead. Use i >= firstindex(a), which is false only when i is below the array's own first valid index, matching master's behavior for every case the original fix needed to cover (the i=0/i=-1 segfault on a plain, 1-based Vector) while leaving a 0-based array's own index 0 untouched. Added a minimal 0-based AbstractVector test type (ZeroVec, avoiding an OffsetArrays test dependency that would conflict with #672's Project.toml changes) and a regression test that copyat_or_push!(z, 0, ...) still updates index 0 while copyat_or_push!(z, -1, ...) still throws. Fails against the i >= 1 code (BoundsError at a correct, in-bounds index 0) and passes after this fix, on both Julia 1.10 and 1.12. Co-Authored-By: Chris Rackauckas Co-Authored-By: Claude Agent-Harness: Claude Code 2.1.294 Agent-Model: claude-sonnet-5 Agent-Session: https://claude.ai/code/session_01LPHREnnonfLg1VcE1EJovv --- src/utils.jl | 2 +- test/Core/utils_test.jl | 22 +++++++++++++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/utils.jl b/src/utils.jl index cfe8045d..7b0ef882 100644 --- a/src/utils.jl +++ b/src/utils.jl @@ -367,7 +367,7 @@ values == [[1, 2], [3, 4]] ``` """ function copyat_or_push!(a::AbstractVector{T}, i::Int, x, perform_copy = true) where {T} - i >= 1 || throw(BoundsError(a, i)) + i >= firstindex(a) || throw(BoundsError(a, i)) @inbounds if length(a) >= i if !ArrayInterface.ismutable(T) || !perform_copy # TODO: Check for `setindex!`` if T <: StaticArraysCore.StaticArray and use `copy!(b[i],a[i])` diff --git a/test/Core/utils_test.jl b/test/Core/utils_test.jl index c2d0374c..da93cfba 100644 --- a/test/Core/utils_test.jl +++ b/test/Core/utils_test.jl @@ -2,6 +2,17 @@ using RecursiveArrayTools, StaticArrays using RecursiveArrayToolsShorthandConstructors using Test +# A vector whose own indices do not start at 1, e.g. `ZeroVec(v)` has axes +# `0:length(v)-1`, like `OffsetArrays.OffsetArray(v, 0:length(v)-1)`. +struct ZeroVec{T} <: AbstractVector{T} + data::Vector{T} +end +Base.axes(z::ZeroVec) = (0:(length(z.data) - 1),) +Base.size(z::ZeroVec) = size(z.data) +Base.getindex(z::ZeroVec, i::Int) = z.data[i + 1] +Base.setindex!(z::ZeroVec, v, i::Int) = (z.data[i + 1] = v) +Base.IndexStyle(::Type{<:ZeroVec}) = IndexLinear() + t = collect(range(0, stop = 10, length = 200)) randomized = VectorOfArray([0.01randn(2) for i in 1:10]) data = convert(Array, randomized) @@ -164,7 +175,7 @@ end @test b2[2] == [2.0, 2.0] end -@testset "copyat_or_push! rejects non-positive indices" begin +@testset "copyat_or_push! rejects indices below firstindex" begin vals = [[1, 2], [3, 4]] @test_throws BoundsError copyat_or_push!(vals, 0, [9, 9]) @test_throws BoundsError copyat_or_push!(vals, -1, [9, 9]) @@ -175,6 +186,15 @@ end @test vals[1] == [9, 9] copyat_or_push!(vals, 3, [5, 6]) @test vals == [[9, 9], [3, 4], [5, 6]] + + # a 0-based AbstractVector's own index 0 is in-bounds and must still + # update, matching master: the bounds check is against `firstindex(a)`, + # not the literal `1`. + z = ZeroVec([[1, 2], [3, 4]]) + copyat_or_push!(z, 0, [9, 9]) + @test z[0] == [9, 9] + @test z[1] == [3, 4] + @test_throws BoundsError copyat_or_push!(z, -1, [9, 9]) end # OrdinaryDiffEq.jl#1365: recursivecopy must not broadcast-assign into an