some cleanup (#526)

* some cleanup * use wrapped pconvert -> convert to avoid invalidation
JuliaIO · Jan 4, 2024 · f8a9dd3 · f8a9dd3
1 parent 196430f
commit f8a9dd3
Show file tree

Hide file tree

Showing 13 changed files with 51 additions and 405 deletions.
diff --git a/src/JLD2.jl b/src/JLD2.jl
@@ -44,6 +44,9 @@ jlwrite(io, x) = Base.write(io, x)
 jlread(io, x) = Base.read(io, x)
 jlread(io::IO, ::Type{T}, n::Integer) where {T} = T[jlread(io, T) for _=1:n]
 
+# Use internal convert function (for pointer conversion) to avoid invalidations
+pconvert(T, x) = Base.convert(T, x)
+
 jlsizeof(x) = Base.sizeof(x)
 jlunsafe_store!(p, x) = Base.unsafe_store!(p, x)
 jlunsafe_load(p) = Base.unsafe_load(p)

diff --git a/src/Lookup3.jl b/src/Lookup3.jl
@@ -1,5 +1,5 @@
 module Lookup3
-import JLD2: jlunsafe_load
+import JLD2: jlunsafe_load, pconvert
 
 # Original source at http://www.burtleburtle.net/bob/c/lookup3.c
 
@@ -147,11 +147,11 @@ function hash(k::Ptr{UInt8}, n::Integer=length(k), initval::UInt32=UInt32(0))
     #     offset += 12
     # end
     @inbounds while n > 12
-        a += jlunsafe_load(convert(Ptr{UInt32}, ptr))
+        a += jlunsafe_load(pconvert(Ptr{UInt32}, ptr))
         ptr += 4
-        b += jlunsafe_load(convert(Ptr{UInt32}, ptr))
+        b += jlunsafe_load(pconvert(Ptr{UInt32}, ptr))
         ptr += 4
-        c += jlunsafe_load(convert(Ptr{UInt32}, ptr))
+        c += jlunsafe_load(pconvert(Ptr{UInt32}, ptr))
         (a, b, c) = mix(a, b, c)
         ptr += 4
         n -= 12
@@ -175,7 +175,7 @@ function hash(k::Ptr{UInt8}, n::Integer=length(k), initval::UInt32=UInt32(0))
     # end
     @inbounds if n > 0
         if n == 12
-            c += jlunsafe_load(convert(Ptr{UInt32}, ptr+8))
+            c += jlunsafe_load(pconvert(Ptr{UInt32}, ptr+8))
             @goto n8
         elseif n == 11
             c += UInt32(jlunsafe_load(Ptr{UInt8}(ptr+10)))<<16
@@ -190,7 +190,7 @@ function hash(k::Ptr{UInt8}, n::Integer=length(k), initval::UInt32=UInt32(0))
             @goto n8
         elseif n == 8
             @label n8
-            b += jlunsafe_load(convert(Ptr{UInt32}, ptr+4))
+            b += jlunsafe_load(pconvert(Ptr{UInt32}, ptr+4))
             @goto n4
         elseif n == 7
             @label n7
@@ -206,7 +206,7 @@ function hash(k::Ptr{UInt8}, n::Integer=length(k), initval::UInt32=UInt32(0))
             @goto n4
         elseif n == 4
             @label n4
-            a += jlunsafe_load(convert(Ptr{UInt32}, ptr))
+            a += jlunsafe_load(pconvert(Ptr{UInt32}, ptr))
         elseif n == 3
             @label n3
             a += UInt32(jlunsafe_load(Ptr{UInt8}(ptr+2)))<<16

diff --git a/src/data/custom_serialization.jl b/src/data/custom_serialization.jl
@@ -7,6 +7,7 @@ writeas(T::Type) = T
 # respectively. These fall back to convert.
 wconvert(T, x) = convert(T, x)
 rconvert(T, x) = convert(T, x)
+rconvert(::Type{Array{T,N}}, x::Array{T2,N}) where {T, T2, N} = T[rconvert(T, y) for y in x] 
 
 # Select an ODR, incorporating custom serialization only if the types do not
 # match

diff --git a/src/data/number_types.jl b/src/data/number_types.jl
@@ -23,7 +23,7 @@ struct BENumber{T}
 end
 
 jlconvert(::ReadRepresentation{T,BENumber{T}}, ::JLDFile, ptr::Ptr, ::RelOffset) where {T} =
-    bswap(jlunsafe_load(convert(Ptr{T}, ptr)))
+    bswap(jlunsafe_load(pconvert(Ptr{T}, ptr)))
 
 function jltype(f::JLDFile, dt::FixedPointDatatype)
     signed = Bool(dt.bitfield1 >> 3 & 0b1)

diff --git a/src/data/reconstructing_datatypes.jl b/src/data/reconstructing_datatypes.jl
@@ -359,7 +359,7 @@ end
 
 function types_from_refs(f::JLDFile, ptr::Ptr)
     # Test for a potential null pointer indicating an empty array
-    isinit = jlunsafe_load(convert(Ptr{UInt32}, ptr)) != 0
+    isinit = jlunsafe_load(pconvert(Ptr{UInt32}, ptr)) != 0
     unknown_params = false
     if isinit
         refs = jlconvert(ReadRepresentation{RelOffset, Vlen{RelOffset}}(), f, ptr, NULL_REFERENCE)
@@ -684,7 +684,7 @@ end
 # This jlconvert method handles compound types with padding or references
 @generated function jlconvert(::ReadRepresentation{T,S}, f::JLDFile, ptr::Ptr,
                               header_offset::RelOffset) where {T,S}
-    isa(S, DataType) && return :(convert(T, jlunsafe_load(convert(Ptr{S}, ptr))))
+    isa(S, DataType) && return :(convert(T, jlunsafe_load(pconvert(Ptr{S}, ptr))))
     @assert isa(S, OnDiskRepresentation)
 
     offsets = typeof(S).parameters[1]

diff --git a/src/data/specialcased_types.jl b/src/data/specialcased_types.jl
@@ -6,7 +6,7 @@ end
 
 function jlconvert(rr::ReadRepresentation{OpaqueData{N}, NTuple{N,UInt8}}, ::JLDFile, ptr::Ptr, ::RelOffset) where N
     data = Vector{UInt8}(undef, N)
-    unsafe_copyto!(pointer(data), convert(Ptr{UInt8}, ptr), N)
+    unsafe_copyto!(pointer(data), pconvert(Ptr{UInt8}, ptr), N)
     OpaqueData(data)
 end
 
@@ -106,7 +106,7 @@ jlconvert(::ReadRepresentation{Vector{T},Vlen{ODR}}, f::JLDFile, ptr::Ptr, ::Rel
 
 function h5convert!(out::Pointers, fls::FixedLengthString, f::JLDFile, x, ::JLDWriteSession)
     fls.length == jlsizeof(x) || throw(InvalidDataException())
-    (unsafe_copyto!(convert(Ptr{UInt8}, out), pointer(x), fls.length); nothing)
+    (unsafe_copyto!(pconvert(Ptr{UInt8}, out), pointer(x), fls.length); nothing)
 end
 h5convert!(out::Pointers, ::Type{Vlen{String}}, f::JLDFile, x, wsession::JLDWriteSession) =
     store_vlen!(out, UInt8, f, unsafe_wrap(Vector{UInt8}, x), wsession)
@@ -115,25 +115,25 @@ jlconvert(::ReadRepresentation{String,Vlen{String}}, f::JLDFile, ptr::Ptr, ::Rel
     String(jlconvert(ReadRepresentation{UInt8,Vlen{UInt8}}(), f, ptr, NULL_REFERENCE))
 function jlconvert(rr::FixedLengthString{String}, ::JLDFile, ptr::Ptr, ::RelOffset)
     data = Vector{UInt8}(undef, rr.length)
-    unsafe_copyto!(pointer(data), convert(Ptr{UInt8}, ptr), rr.length)
+    unsafe_copyto!(pointer(data), pconvert(Ptr{UInt8}, ptr), rr.length)
     String(data)
 end
 
 # Ascii String
 function jlconvert(rr::AsciiString{NullTerminated}, ::JLDFile, ptr::Ptr, ::RelOffset)
     data = Vector{UInt8}(undef, rr.length)
-    unsafe_copyto!(pointer(data), convert(Ptr{UInt8}, ptr), rr.length)
+    unsafe_copyto!(pointer(data), pconvert(Ptr{UInt8}, ptr), rr.length)
     String(data[1:end-1])
 end
 function jlconvert(rr::ReadRepresentation{String, FixedLengthAsciiString{NullTerminated,N}}, ::JLDFile, ptr::Ptr, ::RelOffset) where {N}
     data = Vector{UInt8}(undef, N)
-    unsafe_copyto!(pointer(data), convert(Ptr{UInt8}, ptr), N)
+    unsafe_copyto!(pointer(data), pconvert(Ptr{UInt8}, ptr), N)
     String(data)
 end
 
 function jlconvert(rr::ReadRepresentation{String, FixedLengthAsciiString{SpacePadded,N}}, ::JLDFile, ptr::Ptr, ::RelOffset) where {N}
     data = Vector{UInt8}(undef, N)
-    unsafe_copyto!(pointer(data), convert(Ptr{UInt8}, ptr), N)
+    unsafe_copyto!(pointer(data), pconvert(Ptr{UInt8}, ptr), N)
     rstrip(String(data))
 end
 odr_sizeof(x::AsciiString) = x.length

diff --git a/src/data/type_defs.jl b/src/data/type_defs.jl
@@ -9,7 +9,7 @@ odr_sizeof(::Nothing) = 0
     function datatype_size(dt::DataType)
         Base.@_foldable_meta
         dt.layout == C_NULL && throw(UndefRefError())
-        size = unsafe_load(convert(Ptr{Base.DataTypeLayout}, dt.layout)).size
+        size = unsafe_load(pconvert(Ptr{Base.DataTypeLayout}, dt.layout)).size
         return Int(size)
     end
     @Base.pure odr_sizeof(x::DataType) = datatype_size(x)

diff --git a/src/data/writing_datatypes.jl b/src/data/writing_datatypes.jl
@@ -71,16 +71,16 @@ const MAX_INLINE_SIZE = 2^10
 # fieldodr gives the on-disk representation of a field of a given type,
 # which is either always initialized (initialized=true) or potentially
 # uninitialized (initialized=false)
-@generated function fieldodr(::Type{T}, initialized::Bool) where T
+function fieldodr(::Type{T}, initialized::Bool) where T
     if isconcretetype(T)
         if !hasfielddata(T)
             # A ghost type, so no need to store at all
             return nothing
         elseif isa(T, DataType) && sizeof(T) ≤ MAX_INLINE_SIZE
             if isbitstype(T)
-                return :(odr(T))
+                return odr(T)
             elseif !ismutabletype(T)
-                return :(initialized ? odr(T) : RelOffset)
+                return initialized ? odr(T) : RelOffset
             end
         end
     end
@@ -89,21 +89,19 @@ end
 
 # h5fieldtype is fieldodr's HDF5 companion. It should give the HDF5
 # datatype reflecting the on-disk representation.
-@generated function h5fieldtype(f::JLDFile, writeas::Type{T}, readas::Type,
+function h5fieldtype(f::JLDFile, writeas::Type{T}, readas::Type,
                                 initialized::Initialized) where T
     if isconcretetype(T)
         if !hasfielddata(T)
             return nothing
-        elseif (isbitstype(T) || (isa(initialized, Type{Type{Val{true}}}) && !ismutabletype(T))) && sizeof(T) ≤ MAX_INLINE_SIZE
-            return quote
-                @lookup_committed f T
-                $(if isempty(T.types)
-                    # Opaque datatype
-                    :(return commit(f, OpaqueDatatype(sizeof(T)), T, readas))
-                else
-                    # Compound type
-                    :(return commit_compound(f, fieldnames(T), T, readas))
-                end)
+        elseif (isbitstype(T) || (isa(initialized, Type{Val{true}}) && !ismutabletype(T))) && sizeof(T) ≤ MAX_INLINE_SIZE
+            @lookup_committed f T
+            if isempty(T.types)
+                # Opaque datatype
+                return commit(f, OpaqueDatatype(sizeof(T)), T, readas)
+            else
+                # Compound type
+                return commit_compound(f, fieldnames(T), T, readas)
             end
         end
     end
@@ -232,7 +230,7 @@ end
 # method handles types with no padding or references where this is just a simple
 # store
 h5convert!(out::Pointers, ::Type{T}, ::JLDFile, x, ::JLDWriteSession) where {T} =
-    (jlunsafe_store!(convert(Ptr{T}, out), x); nothing)
+    (jlunsafe_store!(pconvert(Ptr{T}, out), x); nothing)
 
 # We pack types that have padding using a staged h5convert! method
 @generated function h5convert!(out::Pointers,
@@ -276,7 +274,7 @@ jlconvert_canbeuninitialized(::Any) = false
 # handles types where this is just a simple load
 @inline jlconvert(::ReadRepresentation{T,T}, ::JLDFile, ptr::Ptr,
                   ::RelOffset) where {T} =
-    jlunsafe_load(convert(Ptr{T}, ptr))
+    jlunsafe_load(pconvert(Ptr{T}, ptr))
 
 # When fields are undefined in the file but can't be in the workspace, we need
 # to throw exceptions to prevent errors on null pointer loads
@@ -297,37 +295,37 @@ odr(::Type{RelOffset}) = RelOffset
 @inline function h5convert!(out::Pointers, odr::Type{RelOffset}, f::JLDFile, x::Any,
                             wsession::JLDWriteSession)
     ref = write_ref(f, x, wsession)
-    jlunsafe_store!(convert(Ptr{RelOffset}, out), ref)
+    jlunsafe_store!(pconvert(Ptr{RelOffset}, out), ref)
     nothing
 end
 h5convert_uninitialized!(out::Pointers, odr::Type{RelOffset}) =
-    (jlunsafe_store!(convert(Ptr{RelOffset}, out), NULL_REFERENCE); nothing)
+    (jlunsafe_store!(pconvert(Ptr{RelOffset}, out), NULL_REFERENCE); nothing)
 
 # Reading references as references
 jlconvert(::ReadRepresentation{RelOffset,RelOffset}, f::JLDFile, ptr::Ptr,
           ::RelOffset) =
-    jlunsafe_load(convert(Ptr{RelOffset}, ptr))
+    jlunsafe_load(pconvert(Ptr{RelOffset}, ptr))
 jlconvert_canbeuninitialized(::ReadRepresentation{RelOffset,RelOffset}) = false
 
 # Reading references as other types
 @inline function jlconvert(::ReadRepresentation{T,RelOffset}, f::JLDFile, ptr::Ptr,
                            ::RelOffset) where T
-    x = load_dataset(f, jlunsafe_load(convert(Ptr{RelOffset}, ptr)))
+    x = load_dataset(f, jlunsafe_load(pconvert(Ptr{RelOffset}, ptr)))
     (isa(x, T) ? x : rconvert(T, x))::T
 end
 
 jlconvert_canbeuninitialized(::ReadRepresentation{T,RelOffset}) where {T} = true
 jlconvert_isinitialized(::ReadRepresentation{T,RelOffset}, ptr::Ptr) where {T} =
-    jlunsafe_load(convert(Ptr{RelOffset}, ptr)) != NULL_REFERENCE
+    jlunsafe_load(pconvert(Ptr{RelOffset}, ptr)) != NULL_REFERENCE
 
 ## Routines for variable-length datatypes
 
 # Write variable-length data and store the offset and length to out pointer
 @inline function store_vlen!(out::Pointers, odr, f::JLDFile, x::AbstractVector,
                              wsession::JLDWriteSession)
-    jlunsafe_store!(convert(Ptr{UInt32}, out), length(x))
+    jlunsafe_store!(pconvert(Ptr{UInt32}, out), length(x))
     obj = write_heap_object(f, odr, x, wsession)
-    jlunsafe_store!(convert(Ptr{GlobalHeapID}, out)+4, obj)
+    jlunsafe_store!(pconvert(Ptr{GlobalHeapID}, out)+4, obj)
     nothing
 end
 
@@ -336,14 +334,14 @@ h5convert!(out::Pointers, ::Type{Vlen{T}}, f::JLDFile, x, wsession::JLDWriteSess
 
 @assert odr_sizeof(Vlen) == jlsizeof(UInt128)
 h5convert_uninitialized!(out::Pointers, odr::Type{T}) where {T<:Vlen} =
-    (jlunsafe_store!(convert(Ptr{Int128}, out), 0); nothing)
+    (jlunsafe_store!(pconvert(Ptr{Int128}, out), 0); nothing)
 
 # Read variable-length data given offset and length in ptr
 jlconvert(::ReadRepresentation{T,Vlen{S}}, f::JLDFile, ptr::Ptr, ::RelOffset) where {T,S} =
-    read_heap_object(f, jlunsafe_load(convert(Ptr{GlobalHeapID}, ptr+4)), ReadRepresentation{T, S}())
+    read_heap_object(f, jlunsafe_load(pconvert(Ptr{GlobalHeapID}, ptr+4)), ReadRepresentation{T, S}())
 jlconvert_canbeuninitialized(::ReadRepresentation{T,Vlen{S}}) where {T,S} = true
 jlconvert_isinitialized(::ReadRepresentation{T,Vlen{S}}, ptr::Ptr) where {T,S} =
-    jlunsafe_load(convert(Ptr{GlobalHeapID}, ptr+4)) != GlobalHeapID(RelOffset(0), 0)
+    jlunsafe_load(pconvert(Ptr{GlobalHeapID}, ptr+4)) != GlobalHeapID(RelOffset(0), 0)
 
 
 
@@ -635,7 +633,7 @@ datamode(::DataType) = ReferenceFree()
 datamode(::FixedLengthString) = ReferenceFree()
 datamode(::AsciiString) = ReferenceFree()
 datamode(::Nothing) = ReferenceFree()
-@generated function datamode(odr::OnDiskRepresentation{Offsets,JLTypes,H5Types,Size} where {Offsets,JLTypes,Size}) where H5Types
+function datamode(odr::OnDiskRepresentation{Offsets,JLTypes,H5Types,Size} where {Offsets,JLTypes,Size}) where H5Types
     for ty in H5Types.parameters
         datamode(ty) == HasReferences() && return HasReferences()
     end

diff --git a/src/dataio.jl b/src/dataio.jl
@@ -54,7 +54,7 @@ end
         seek(regulario, inptr - io.startptr)
         unsafe_read(regulario, pointer(v), nb)
     else
-        unsafe_copyto!(pointer(v), convert(Ptr{T}, inptr), n)
+        unsafe_copyto!(pointer(v), pconvert(Ptr{T}, inptr), n)
     end
     io.curptr = inptr + odr_sizeof(T) * n
     v

diff --git a/src/misc.jl b/src/misc.jl
@@ -12,11 +12,11 @@ function define_packed(ty::DataType)
     if sz != jlsizeof(ty)
         @eval begin
             function jlunsafe_store!(p::Ptr{$ty}, x::$ty)
-                $([:(jlunsafe_store!(convert(Ptr{$(ty.types[i])}, p+$(packed_offsets[i])), getfield(x, $i)))
+                $([:(jlunsafe_store!(pconvert(Ptr{$(ty.types[i])}, p+$(packed_offsets[i])), getfield(x, $i)))
                    for i = 1:length(packed_offsets)]...)
             end
             function jlunsafe_load(p::Ptr{$ty})
-                $(Expr(:new, ty, [:(jlunsafe_load(convert(Ptr{$(ty.types[i])}, p+$(packed_offsets[i]))))
+                $(Expr(:new, ty, [:(jlunsafe_load(pconvert(Ptr{$(ty.types[i])}, p+$(packed_offsets[i]))))
                                    for i = 1:length(packed_offsets)]...))
             end
             jlsizeof(::Union{$ty,Type{$ty}}) = $(Int(sz))::Int

diff --git a/src/mmapio.jl b/src/mmapio.jl
@@ -236,7 +236,7 @@ jlread(io::MmapIO, ::Type{T}, n::Integer) where {T} = read(io, T, Int(n))
 function read_bytestring(io::MmapIO)
     # TODO do not try to read outside the buffer
     cp = io.curptr
-    str = unsafe_string(convert(Ptr{UInt8}, cp))
+    str = unsafe_string(pconvert(Ptr{UInt8}, cp))
     io.curptr = cp + jlsizeof(str) + 1
     str
 end
@@ -277,7 +277,7 @@ function IndirectPointer(io::MmapIO, offset::Integer=position(io))
     IndirectPointer(pointer_from_objref(io) + fieldoffset(MmapIO, 4), offset)
 end
 Base.:+(x::IndirectPointer, y::Integer) = IndirectPointer(x.ptr, x.offset+y)
-Base.convert(::Type{Ptr{T}}, x::IndirectPointer) where {T} = Ptr{T}(jlunsafe_load(x.ptr) + x.offset)
+pconvert(::Type{Ptr{T}}, x::IndirectPointer) where {T} = Ptr{T}(jlunsafe_load(x.ptr) + x.offset)
 
 # We sometimes need to compute checksums. We do this by first calling begin_checksum when
 # starting to handle whatever needs checksumming, and calling end_checksum afterwards. Note