Skip to content

Commit

Permalink
fix some more invalid unsafe code
Browse files Browse the repository at this point in the history
More cases similar to those identified in #23914
fix #33899
  • Loading branch information
vtjnash committed Nov 25, 2019
1 parent 4002ccd commit e1086fe
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 28 deletions.
2 changes: 1 addition & 1 deletion base/stream.jl
Original file line number Diff line number Diff line change
Expand Up @@ -917,7 +917,7 @@ function readuntil(x::LibuvStream, c::UInt8; keep::Bool=false)
return bytes
end

uv_write(s::LibuvStream, p::Vector{UInt8}) = uv_write(s, pointer(p), UInt(sizeof(p)))
uv_write(s::LibuvStream, p::Vector{UInt8}) = GC.@preserve p uv_write(s, pointer(p), UInt(sizeof(p)))

# caller must have acquired the iolock
function uv_write(s::LibuvStream, p::Ptr{UInt8}, n::UInt)
Expand Down
8 changes: 4 additions & 4 deletions base/strings/search.jl
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ function _search(a::Union{String,ByteArray}, b::Union{Int8,UInt8}, i::Integer =
return i == n+1 ? 0 : throw(BoundsError(a, i))
end
p = pointer(a)
q = ccall(:memchr, Ptr{UInt8}, (Ptr{UInt8}, Int32, Csize_t), p+i-1, b, n-i+1)
q == C_NULL ? 0 : Int(q-p+1)
q = GC.@preserve a ccall(:memchr, Ptr{UInt8}, (Ptr{UInt8}, Int32, Csize_t), p+i-1, b, n-i+1)
return q == C_NULL ? 0 : Int(q-p+1)
end

function _search(a::ByteArray, b::AbstractChar, i::Integer = 1)
Expand Down Expand Up @@ -74,8 +74,8 @@ function _rsearch(a::Union{String,ByteArray}, b::Union{Int8,UInt8}, i::Integer =
return i == n+1 ? 0 : throw(BoundsError(a, i))
end
p = pointer(a)
q = ccall(:memrchr, Ptr{UInt8}, (Ptr{UInt8}, Int32, Csize_t), p, b, i)
q == C_NULL ? 0 : Int(q-p+1)
q = GC.@preserve a ccall(:memrchr, Ptr{UInt8}, (Ptr{UInt8}, Int32, Csize_t), p, b, i)
return q == C_NULL ? 0 : Int(q-p+1)
end

function _rsearch(a::ByteArray, b::AbstractChar, i::Integer = length(a))
Expand Down
17 changes: 9 additions & 8 deletions base/strings/string.jl
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ codeunit(s::String) = UInt8

@inline function codeunit(s::String, i::Integer)
@boundscheck checkbounds(s, i)
GC.@preserve s unsafe_load(pointer(s, i))
b = GC.@preserve s unsafe_load(pointer(s, i))
return b
end

## comparison ##
Expand All @@ -112,7 +113,7 @@ typemin(::String) = typemin(String)

## thisind, nextind ##

Base.@propagate_inbounds thisind(s::String, i::Int) = _thisind_str(s, i)
@propagate_inbounds thisind(s::String, i::Int) = _thisind_str(s, i)

# s should be String or SubString{String}
@inline function _thisind_str(s, i::Int)
Expand All @@ -133,7 +134,7 @@ Base.@propagate_inbounds thisind(s::String, i::Int) = _thisind_str(s, i)
return i
end

Base.@propagate_inbounds nextind(s::String, i::Int) = _nextind_str(s, i)
@propagate_inbounds nextind(s::String, i::Int) = _nextind_str(s, i)

# s should be String or SubString{String}
@inline function _nextind_str(s, i::Int)
Expand Down Expand Up @@ -251,7 +252,7 @@ getindex(s::String, r::UnitRange{<:Integer}) = s[Int(first(r)):Int(last(r))]
j = nextind(s, j) - 1
n = j - i + 1
ss = _string_n(n)
unsafe_copyto!(pointer(ss), pointer(s, i), n)
GC.@preserve s ss unsafe_copyto!(pointer(ss), pointer(s, i), n)
return ss
end

Expand Down Expand Up @@ -323,7 +324,7 @@ function repeat(c::Char, r::Integer)
n = 4 - (leading_zeros(u | 0xff) >> 3)
s = _string_n(n*r)
p = pointer(s)
if n == 1
GC.@preserve s if n == 1
ccall(:memset, Ptr{Cvoid}, (Ptr{UInt8}, Cint, Csize_t), p, u % UInt8, r)
elseif n == 2
p16 = reinterpret(Ptr{UInt16}, p)
Expand All @@ -340,7 +341,7 @@ function repeat(c::Char, r::Integer)
unsafe_store!(p, b3, 3i + 3)
end
elseif n == 4
p32 = reinterpret(Ptr{UInt32}, pointer(s))
p32 = reinterpret(Ptr{UInt32}, p)
for i = 1:r
unsafe_store!(p32, u, i)
end
Expand All @@ -349,11 +350,11 @@ function repeat(c::Char, r::Integer)
end

function filter(f, s::String)
out = Base.StringVector(sizeof(s))
out = StringVector(sizeof(s))
offset = 1
for c in s
if f(c)
offset += Base.__unsafe_string!(out, c, offset)
offset += __unsafe_string!(out, c, offset)
end
end
resize!(out, offset-1)
Expand Down
34 changes: 20 additions & 14 deletions base/strings/substring.jl
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,11 @@ convert(::Type{SubString{S}}, s::AbstractString) where {S<:AbstractString} =
SubString(convert(S, s))
convert(::Type{T}, s::T) where {T<:SubString} = s

String(s::SubString{String}) = unsafe_string(pointer(s.string, s.offset+1), s.ncodeunits)
function String(s::SubString{String})
parent = s.string
copy = GC.@preserve parent unsafe_string(pointer(parent, s.offset+1), s.ncodeunits)
return copy
end

ncodeunits(s::SubString) = s.ncodeunits
codeunit(s::SubString) = codeunit(s.string)
Expand Down Expand Up @@ -151,25 +155,27 @@ end
string(a::String) = String(a)
string(a::SubString{String}) = String(a)

@inline function __unsafe_string!(out, c::Char, offs::Integer)
@inline function __unsafe_string!(out, c::Char, offs::Integer) # out is a (new) String (or StringVector)
x = bswap(reinterpret(UInt32, c))
n = ncodeunits(c)
unsafe_store!(pointer(out, offs), x % UInt8)
n == 1 && return n
x >>= 8
unsafe_store!(pointer(out, offs+1), x % UInt8)
n == 2 && return n
x >>= 8
unsafe_store!(pointer(out, offs+2), x % UInt8)
n == 3 && return n
x >>= 8
unsafe_store!(pointer(out, offs+3), x % UInt8)
GC.@preserve out begin
unsafe_store!(pointer(out, offs), x % UInt8)
n == 1 && return n
x >>= 8
unsafe_store!(pointer(out, offs+1), x % UInt8)
n == 2 && return n
x >>= 8
unsafe_store!(pointer(out, offs+2), x % UInt8)
n == 3 && return n
x >>= 8
unsafe_store!(pointer(out, offs+3), x % UInt8)
end
return n
end

@inline function __unsafe_string!(out, s::Union{String, SubString{String}}, offs::Integer)
n = sizeof(s)
unsafe_copyto!(pointer(out, offs), pointer(s), n)
GC.@preserve s out unsafe_copyto!(pointer(out, offs), pointer(s), n)
return n
end

Expand Down Expand Up @@ -200,7 +206,7 @@ function repeat(s::Union{String, SubString{String}}, r::Integer)
ccall(:memset, Ptr{Cvoid}, (Ptr{UInt8}, Cint, Csize_t), out, b, r)
else
for i = 0:r-1
unsafe_copyto!(pointer(out, i*n+1), pointer(s), n)
GC.@preserve s out unsafe_copyto!(pointer(out, i*n+1), pointer(s), n)
end
end
return out
Expand Down
2 changes: 1 addition & 1 deletion base/strings/util.jl
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ function replace(str::String, pat_repl::Pair; count::Integer=typemax(Int))
out = IOBuffer(sizehint=floor(Int, 1.2sizeof(str)))
while j != 0
if i == a || i <= k
unsafe_write(out, pointer(str, i), UInt(j-i))
GC.@preserve str unsafe_write(out, pointer(str, i), UInt(j-i))
_replace(out, repl, str, r, pattern)
end
if k < j
Expand Down

0 comments on commit e1086fe

Please sign in to comment.