From e29fd4a04a3eae0c2965531471549cab9e43ff99 Mon Sep 17 00:00:00 2001 From: Jeff Bezanson Date: Mon, 16 Sep 2019 15:53:47 -0400 Subject: [PATCH] Array fixes for non-power-of-2 sized elements, fixes #26026 --- base/array.jl | 12 ++++-------- base/reflection.jl | 17 +++++++++++++++++ src/array.c | 9 ++++++--- src/julia.h | 2 +- test/core.jl | 25 +++++++++++++++++++++++++ 5 files changed, 53 insertions(+), 12 deletions(-) diff --git a/base/array.jl b/base/array.jl index 88c235fd559dc4..3449d7bc49a2db 100644 --- a/base/array.jl +++ b/base/array.jl @@ -212,7 +212,7 @@ function bitsunionsize(u::Union) end length(a::Array) = arraylen(a) -elsize(::Type{<:Array{T}}) where {T} = isbitsunion(T) ? bitsunionsize(T) : (allocatedinline(T) ? sizeof(T) : sizeof(Ptr)) +elsize(::Type{<:Array{T}}) where {T} = aligned_sizeof(T) sizeof(a::Array) = Core.sizeof(a) function isassigned(a::Array, i::Int...) @@ -238,7 +238,7 @@ function unsafe_copyto!(dest::Ptr{T}, src::Ptr{T}, n) where T # Do not use this to copy data between pointer arrays. # It can't be made safe no matter how carefully you checked. ccall(:memmove, Ptr{Cvoid}, (Ptr{Cvoid}, Ptr{Cvoid}, UInt), - dest, src, n*sizeof(T)) + dest, src, n * aligned_sizeof(T)) return dest end @@ -257,7 +257,7 @@ function unsafe_copyto!(dest::Array{T}, doffs, src::Array{T}, soffs, n) where T t2 = @_gc_preserve_begin src if isbitsunion(T) ccall(:memmove, Ptr{Cvoid}, (Ptr{Cvoid}, Ptr{Cvoid}, UInt), - pointer(dest, doffs), pointer(src, soffs), n * Base.bitsunionsize(T)) + pointer(dest, doffs), pointer(src, soffs), n * aligned_sizeof(T)) # copy selector bytes ccall(:memmove, Ptr{Cvoid}, (Ptr{Cvoid}, Ptr{Cvoid}, UInt), ccall(:jl_array_typetagdata, Ptr{UInt8}, (Any,), dest) + doffs - 1, @@ -1565,13 +1565,9 @@ function vcat(arrays::Vector{T}...) where T arr = Vector{T}(undef, n) ptr = pointer(arr) if isbitsunion(T) - elsz = bitsunionsize(T) selptr = ccall(:jl_array_typetagdata, Ptr{UInt8}, (Any,), arr) - elseif allocatedinline(T) - elsz = Core.sizeof(T) - else - elsz = Core.sizeof(Ptr{Cvoid}) end + elsz = aligned_sizeof(T) t = @_gc_preserve_begin arr for a in arrays na = length(a) diff --git a/base/reflection.jl b/base/reflection.jl index a8b7dbe5be4a02..f7004b3247dbbf 100644 --- a/base/reflection.jl +++ b/base/reflection.jl @@ -337,6 +337,23 @@ function datatype_alignment(dt::DataType) return Int(alignment & 0x1FF) end +# amount of total space taken by T when stored in a container +function aligned_sizeof(T) + @_pure_meta + if isbitsunion(T) + sz = Ref{Csize_t}(0) + algn = Ref{Csize_t}(0) + ccall(:jl_islayout_inline, Cint, (Any, Ptr{Csize_t}, Ptr{Csize_t}), u, sz, algn) + al = algn[] + return (sz[] + al - 1) & -al + elseif allocatedinline(T) + al = datatype_alignment(T) + return (Core.sizeof(T) + al - 1) & -al + else + return Core.sizeof(Ptr{Cvoid}) + end +end + gc_alignment(sz::Integer) = Int(ccall(:jl_alignment, Cint, (Csize_t,), sz)) gc_alignment(T::Type) = gc_alignment(Core.sizeof(T)) diff --git a/src/array.c b/src/array.c index c5af42736451c2..a0627ff24ab8e5 100644 --- a/src/array.c +++ b/src/array.c @@ -156,6 +156,9 @@ static inline jl_array_t *_new_array(jl_value_t *atype, uint32_t ndims, size_t * elsz = sizeof(void*); al = elsz; } + else { + elsz = LLT_ALIGN(elsz, al); + } return _new_array_(atype, ndims, dims, isunboxed, isunion, elsz); } @@ -207,7 +210,7 @@ JL_DLLEXPORT jl_array_t *jl_reshape_array(jl_value_t *atype, jl_array_t *data, int isboxed = !jl_islayout_inline(eltype, &elsz, &align); assert(isboxed == data->flags.ptrarray); if (!isboxed) { - a->elsize = elsz; + a->elsize = LLT_ALIGN(elsz, align); jl_value_t *ownerty = jl_typeof(owner); size_t oldelsz = 0, oldalign = 0; if (ownerty == (jl_value_t*)jl_string_type) { @@ -323,7 +326,7 @@ JL_DLLEXPORT jl_array_t *jl_ptr_to_array_1d(jl_value_t *atype, void *data, #ifdef STORE_ARRAY_LEN a->length = nel; #endif - a->elsize = elsz; + a->elsize = LLT_ALIGN(elsz, align); a->flags.ptrarray = !isunboxed; a->flags.ndims = 1; a->flags.isshared = 1; @@ -389,7 +392,7 @@ JL_DLLEXPORT jl_array_t *jl_ptr_to_array(jl_value_t *atype, void *data, #ifdef STORE_ARRAY_LEN a->length = nel; #endif - a->elsize = elsz; + a->elsize = LLT_ALIGN(elsz, align); a->flags.ptrarray = !isunboxed; a->flags.ndims = ndims; a->offset = 0; diff --git a/src/julia.h b/src/julia.h index 26d7fc83d13ef7..86b7f66aa8b1e7 100644 --- a/src/julia.h +++ b/src/julia.h @@ -173,7 +173,7 @@ JL_EXTENSION typedef struct { size_t length; #endif jl_array_flags_t flags; - uint16_t elsize; + uint16_t elsize; // element size including alignment (dim 1 memory stride) uint32_t offset; // for 1-d only. does not need to get big. size_t nrows; union { diff --git a/test/core.jl b/test/core.jl index 5115f327df1d39..0d6b99c73ee6e1 100644 --- a/test/core.jl +++ b/test/core.jl @@ -6461,6 +6461,31 @@ let A=[0, missing], B=[missing, 0], C=Vector{Union{Int, Missing}}(undef, 6) @test isequal(C, [0, missing, missing, missing, 0, missing]) end +# non-power-of-2 element sizes, issue #26026 +primitive type TypeWith24Bits 24 end +TypeWith24Bits(x::UInt32) = Core.Intrinsics.trunc_int(TypeWith24Bits, x) +let x = TypeWith24Bits(0x112233), y = TypeWith24Bits(0x445566), z = TypeWith24Bits(0x778899) + a = [x, x] + Core.arrayset(true, a, y, 2) + @test a == [x, y] + a[2] = z + @test a == [x, z] + @test pointer(a, 2) - pointer(a, 1) == 4 + + b = [(x, x), (x, x)] + Core.arrayset(true, b, (x, y), 2) + @test b == [(x, x), (x, y)] + b[2] = (y, z) + @test b == [(x, x), (y, z)] + + V = Vector{TypeWith24Bits}(undef, 1000) + p = Ptr{UInt8}(pointer(V)) + for i = 1:sizeof(V) + unsafe_store!(p, i % UInt8, i) + end + @test V[1:4] == [TypeWith24Bits(0x030201), TypeWith24Bits(0x070605), TypeWith24Bits(0x0b0a09), TypeWith24Bits(0x0f0e0d)] +end + # issue #29718 function f29718() nt = NamedTuple{(:a, :b, :c, :d, :e, :f,),