From ea0c4d3ca9e26f622a7db06087c94704daa58450 Mon Sep 17 00:00:00 2001 From: Jakob Nybo Nissen Date: Mon, 6 Oct 2025 10:04:09 +0200 Subject: [PATCH] Audit use of GC.@preserve Fix one use of GC.@preserve and add TODOs where an invalid pointer escapes GC.@preserve. --- src/lazy.jl | 17 ++++++++++++++++- src/parse.jl | 2 ++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/lazy.jl b/src/lazy.jl index 5f3e16c..e675cfb 100644 --- a/src/lazy.jl +++ b/src/lazy.jl @@ -270,6 +270,7 @@ function applyobject(keyvalfunc, x::LazyValues) b == UInt8('}') && return pos + 1 while true # parsestring returns key as a PtrString + # TODO: Key contains an invalid pointer here key, pos = @inline parsestring(LazyValue(buf, pos, JSONTypes.STRING, opts, false)) @nextbyte if b != UInt8(':') @@ -440,7 +441,18 @@ function Base.convert(::Type{T}, x::PtrString) where {T <: Enum} throw(ArgumentError("invalid `$T` string value: \"$sym\"")) end -Base.:(==)(x::PtrString, y::AbstractString) = x.len == sizeof(y) && ccall(:memcmp, Cint, (Ptr{UInt8}, Ptr{UInt8}, Csize_t), x.ptr, pointer(y), x.len) == 0 +function Base.:(==)(x::PtrString, y::AbstractString) + # If the string is backed by UInt8 codeunits, the `sizeof` computation below is correct + return if codeunit(y) == UInt8 + x.len == sizeof(y) || return false + ref = Base.cconvert(Ptr{UInt8}, y) + cmp = GC.@preserve ref ccall(:memcmp, Cint, (Ptr{UInt8}, Ptr{UInt8}, Csize_t), x.ptr, Base.unsafe_convert(Ptr{UInt8}, ref), x.len) + iszero(cmp) + else + x == String(y)::String + end +end + Base.:(==)(x::PtrString, y::PtrString) = x.len == y.len && ccall(:memcmp, Cint, (Ptr{UInt8}, Ptr{UInt8}, Csize_t), x.ptr, y.ptr, x.len) == 0 Base.isequal(x::PtrString, y::AbstractString) = x == y Base.isequal(x::PtrString, y::PtrString) = x == y @@ -482,6 +494,8 @@ function parsestring(x::LazyValue) end @nextbyte(false) end + # TODO: The pointer escapes here, but callers of this function do not use GC.@preserve. + # Hence, the pointer is invalid by the time it's returned. str = PtrString(pointer(buf, spos), pos - spos, escaped) return str, pos + 1 @@ -732,6 +746,7 @@ function Base.show(io::IO, x::LazyValue) show(io, MIME"text/plain"(), la) end elseif T == JSONTypes.STRING + # TODO: str contains an invalid pointer here str, _ = parsestring(x) Base.print(io, "JSON.LazyValue(", repr(convert(String, str)), ")") elseif T == JSONTypes.NULL diff --git a/src/parse.jl b/src/parse.jl index 81f71f6..3d0baaf 100644 --- a/src/parse.jl +++ b/src/parse.jl @@ -277,6 +277,7 @@ function applyvalue(f, x::LazyValues, null) f(arr) return pos elseif type == JSONTypes.STRING + # TODO: str contains an invalid pointer here str, pos = parsestring(x) f(convert(String, str)) return pos @@ -344,6 +345,7 @@ end function StructUtils.lift(style::StructStyle, ::Type{T}, x::LazyValues, tags=(;)) where {T} type = gettype(x) if type == JSONTypes.STRING + # TODO: ptrstr contains an invalid pointer here ptrstr, pos = parsestring(x) str, _ = StructUtils.lift(style, T, ptrstr, tags) return str, pos