add Cstring/Cwstring types for safe passing of NUL-terminated strings to ccall (see JuliaLang#10958, JuliaLang#10991)

Author: stevengj

NEWS.md

@@ -148,16 +148,20 @@ Library improvements
 
     * `Givens` type doesn't have a size anymore and is no longer a subtype of `AbstractMatrix` ([#8660]).
 
-    * Large speedup in sparse ``\`` and splitting of Cholesky and LDLᵀ factorizations into ``cholfact`` and ``ldltfact`` ([#10117]).
+    * Large speedup in sparse `\` and splitting of Cholesky and LDLᵀ factorizations into `cholfact` and `ldltfact` ([#10117]).
 
-    * Add sparse least squares to ``\`` by adding ``qrfact`` for sparse matrices based on the SPQR library. ([#10180])
+    * Add sparse least squares to `\` by adding `qrfact` for sparse matrices based on the SPQR library. ([#10180])
 
     * Split `Triangular` type into `UpperTriangular`, `LowerTriangular`, `UnitUpperTriagular` and `UnitLowerTriangular` ([#9779])
 
     * OpenBLAS 64-bit (ILP64) interface is now compiled with a `64_` suffix ([#8734]) to avoid conflicts with external libraries using a 32-bit BLAS ([#4923]).
 
   * Strings
 
+    * NUL-terminated strings should now be passed to C via the new `Cstring` type, not `Ptr{UInt8}` or `Ptr{Cchar}`,
+      in order to check whether the string is free of NUL characters (which would cause silent truncation in C).
+      The analogous type `Cwstring` should be used for NUL-terminated `wchar_t*` strings ([#10994]).
+
     * `graphemes(s)` returns an iterator over grapheme substrings of `s` ([#9261]).
 
     * Character predicates such as `islower()`, `isspace()`, etc. use
@@ -1380,11 +1384,12 @@ Too numerous to mention.
 [#10659]: https://github.com/JuliaLang/julia/issues/10659
 [#10679]: https://github.com/JuliaLang/julia/issues/10679
 [#10709]: https://github.com/JuliaLang/julia/issues/10709
-[#10714]: https://github.com/JuliaLang/julia/pull/10714
+[#10714]: https://github.com/JuliaLang/julia/issues/10714
 [#10747]: https://github.com/JuliaLang/julia/issues/10747
 [#10844]: https://github.com/JuliaLang/julia/issues/10844
 [#10870]: https://github.com/JuliaLang/julia/issues/10870
 [#10885]: https://github.com/JuliaLang/julia/issues/10885
-[#10888]: https://github.com/JuliaLang/julia/pull/10888
-[#10893]: https://github.com/JuliaLang/julia/pull/10893
+[#10888]: https://github.com/JuliaLang/julia/issues/10888
+[#10893]: https://github.com/JuliaLang/julia/issues/10893
 [#10914]: https://github.com/JuliaLang/julia/issues/10914
+[#10994]: https://github.com/JuliaLang/julia/issues/10994

base/c.jl

@@ -1,6 +1,6 @@
 # definitions related to C interface
 
-import Core.Intrinsics.cglobal
+import Core.Intrinsics: cglobal, box, unbox
 
 cfunction(f::Function, r, a) = ccall(:jl_function_ptr, Ptr{Void}, (Any, Any, Any), f, r, a)
 
@@ -43,6 +43,34 @@ end
 
 typealias Coff_t FileOffset
 
+# C NUL-terminated string pointers; these can be used in ccall
+# instead of Ptr{Cchar} and Ptr{Cwchar_t}, respectively, to enforce
+# a check for embedded NUL chars in the string (to avoid silent truncation).
+if Int === Int64
+    bitstype 64 Cstring
+    bitstype 64 Cwstring
+else
+    bitstype 32 Cstring
+    bitstype 32 Cwstring
+end
+
+convert{T<:Union(Int8,UInt8)}(::Type{Cstring}, p::Ptr{T}) = box(Cstring, unbox(Ptr{T}, p))
+convert(::Type{Cwstring}, p::Ptr{Cwchar_t}) = box(Cwstring, unbox(Ptr{Cwchar_t}, p))
+
+containsnul(p::Ptr, len) = C_NULL != ccall(:memchr, Ptr{Cchar}, (Ptr{Cchar}, Cint, Csize_t), p, 0, len)
+function unsafe_convert(::Type{Cstring}, s::ByteString)
+    p = unsafe_convert(Ptr{Cchar}, s)
+    if containsnul(p, sizeof(s))
+        throw(ArgumentError("embedded NUL chars are not allowed in C strings"))
+    end
+    return Cstring(p)
+end
+
+# symbols are guaranteed not to contain embedded NUL
+convert(::Type{Cstring}, s::Symbol) = Cstring(unsafe_convert(Ptr{Cchar}, s))
+
+# in string.jl: unsafe_convert(::Type{Cwstring}, s::WString)
+
 # deferring (or un-deferring) ctrl-c handler for external C code that
 # is not interrupt safe (see also issue #2622).  The sigatomic_begin/end
 # functions should always be called in matched pairs, ideally via:
@@ -56,11 +84,11 @@ disable_sigint(f::Function) = try sigatomic_begin(); f(); finally sigatomic_end(
 reenable_sigint(f::Function) = try sigatomic_end(); f(); finally sigatomic_begin(); end
 
 function ccallable(f::Function, rt::Type, argt::Type, name::Union(AbstractString,Symbol)=string(f))
-    ccall(:jl_extern_c, Void, (Any, Any, Any, Ptr{UInt8}), f, rt, argt, name)
+    ccall(:jl_extern_c, Void, (Any, Any, Any, Cstring), f, rt, argt, name)
 end
 
 function ccallable(f::Function, argt::Type, name::Union(AbstractString,Symbol)=string(f))
-    ccall(:jl_extern_c, Void, (Any, Ptr{Void}, Any, Ptr{UInt8}), f, C_NULL, argt, name)
+    ccall(:jl_extern_c, Void, (Any, Ptr{Void}, Any, Cstring), f, C_NULL, argt, name)
 end
 
 macro ccallable(def)

base/client.jl

@@ -150,18 +150,18 @@ function syntax_deprecation_warnings(f::Function, warn::Bool)
     end
 end
 
-function parse_input_line(s::AbstractString)
-    # s = bytestring(s)
+function parse_input_line(s::ByteString)
     # (expr, pos) = parse(s, 1)
     # (ex, pos) = ccall(:jl_parse_string, Any,
-    #                   (Ptr{UInt8},Int32,Int32),
-    #                   s, pos-1, 1)
+    #                   (Ptr{UInt8},Csize_t,Int32,Int32),
+    #                   s, sizeof(s), pos-1, 1)
     # if !is(ex,())
     #     throw(ParseError("extra input after end of expression"))
     # end
     # expr
-    ccall(:jl_parse_input_line, Any, (Ptr{UInt8},), s)
+    ccall(:jl_parse_input_line, Any, (Ptr{UInt8}, Csize_t), s, sizeof(s))
 end
+parse_input_line(s::AbstractString) = parse_input_line(bytestring(s))
 
 function parse_input_line(io::IO)
     s = ""

base/datafmt.jl

@@ -334,13 +334,13 @@ function colval{T<:Integer, S<:ByteString}(sbuff::S, startpos::Int, endpos::Int,
     isnull(n) || (cells[row,col] = get(n))
     isnull(n)
 end
-function colval{S<:ByteString}(sbuff::S, startpos::Int, endpos::Int, cells::Array{Float64,2}, row::Int, col::Int)
-    n = ccall(:jl_try_substrtod, Nullable{Float64}, (Ptr{UInt8},Csize_t,Cint), sbuff, startpos-1, endpos-startpos+1)
+function colval(sbuff::ByteString, startpos::Int, endpos::Int, cells::Array{Float64,2}, row::Int, col::Int)
+    n = ccall(:jl_try_substrtod, Nullable{Float64}, (Ptr{UInt8},Csize_t,Csize_t), sbuff, startpos-1, endpos-startpos+1)
     isnull(n) || (cells[row,col] = get(n))
     isnull(n)
 end
-function colval{S<:ByteString}(sbuff::S, startpos::Int, endpos::Int, cells::Array{Float32,2}, row::Int, col::Int)
-    n = ccall(:jl_try_substrtof, Nullable{Float32}, (Ptr{UInt8},Csize_t,Cint), sbuff, startpos-1, endpos-startpos+1)
+function colval(sbuff::ByteString, startpos::Int, endpos::Int, cells::Array{Float32,2}, row::Int, col::Int)
+    n = ccall(:jl_try_substrtof, Nullable{Float32}, (Ptr{UInt8},Csize_t,Csize_t), sbuff, startpos-1, endpos-startpos+1)
     isnull(n) || (cells[row,col] = get(n))
     isnull(n)
 end
@@ -358,7 +358,7 @@ function colval{S<:ByteString}(sbuff::S, startpos::Int, endpos::Int, cells::Arra
         isnull(nb) || (cells[row,col] = get(nb); return false)
 
         # check float64
-        nf64 = ccall(:jl_try_substrtod, Nullable{Float64}, (Ptr{UInt8},Csize_t,Cint), sbuff, startpos-1, endpos-startpos+1)
+        nf64 = ccall(:jl_try_substrtod, Nullable{Float64}, (Ptr{UInt8},Csize_t,Csize_t), sbuff, startpos-1, endpos-startpos+1)
         isnull(nf64) || (cells[row,col] = get(nf64); return false)
     end
     cells[row,col] = SubString(sbuff, startpos, endpos)

base/env.jl

@@ -1,5 +1,5 @@
 @unix_only begin
-    _getenv(var::AbstractString) = ccall(:getenv, Ptr{UInt8}, (Ptr{UInt8},), var)
+    _getenv(var::AbstractString) = ccall(:getenv, Ptr{UInt8}, (Cstring,), var)
     _hasenv(s::AbstractString) = _getenv(s) != C_NULL
 end
 @windows_only begin
@@ -24,12 +24,11 @@ function FormatMessage(e=GetLastError())
     return utf8(UTF16String(buf))
 end
 
-_getenvlen(var::UTF16String) = ccall(:GetEnvironmentVariableW,stdcall,UInt32,(Ptr{UInt16},Ptr{UInt8},UInt32),utf16(var),C_NULL,0)
-_hasenv(s::UTF16String) = _getenvlen(s)!=0 || GetLastError()!=ERROR_ENVVAR_NOT_FOUND
-_hasenv(s::AbstractString) = _hasenv(utf16(s))
+_getenvlen(var::AbstractString) = ccall(:GetEnvironmentVariableW,stdcall,UInt32,(Cwstring,Ptr{UInt8},UInt32),var,C_NULL,0)
+_hasenv(s::AbstractString) = _getenvlen(s)!=0 || GetLastError()!=ERROR_ENVVAR_NOT_FOUND
 function _jl_win_getenv(s::UTF16String,len::UInt32)
     val=zeros(UInt16,len)
-    ret=ccall(:GetEnvironmentVariableW,stdcall,UInt32,(Ptr{UInt16},Ptr{UInt16},UInt32),s,val,len)
+    ret=ccall(:GetEnvironmentVariableW,stdcall,UInt32,(Cwstring,Ptr{UInt16},UInt32),s,val,len)
     if ret==0 || ret != len-1 || val[end] != 0
         error(string("getenv: ", s, ' ', len, "-1 != ", ret, ": ", FormatMessage()))
     end
@@ -62,13 +61,13 @@ end
 
 function _setenv(var::AbstractString, val::AbstractString, overwrite::Bool)
     @unix_only begin
-        ret = ccall(:setenv, Int32, (Ptr{UInt8},Ptr{UInt8},Int32), var, val, overwrite)
+        ret = ccall(:setenv, Int32, (Cstring,Cstring,Int32), var, val, overwrite)
         systemerror(:setenv, ret != 0)
     end
     @windows_only begin
         var = utf16(var)
         if overwrite || !_hasenv(var)
-            ret = ccall(:SetEnvironmentVariableW,stdcall,Int32,(Ptr{UInt16},Ptr{UInt16}),utf16(var),utf16(val))
+            ret = ccall(:SetEnvironmentVariableW,stdcall,Int32,(Cwstring,Cwstring),var,val)
             systemerror(:setenv, ret == 0)
         end
     end
@@ -78,11 +77,11 @@ _setenv(var::AbstractString, val::AbstractString) = _setenv(var, val, true)
 
 function _unsetenv(var::AbstractString)
     @unix_only begin
-        ret = ccall(:unsetenv, Int32, (Ptr{UInt8},), var)
+        ret = ccall(:unsetenv, Int32, (Cstring,), var)
         systemerror(:unsetenv, ret != 0)
     end
     @windows_only begin
-        ret = ccall(:SetEnvironmentVariableW,stdcall,Int32,(Ptr{UInt16},Ptr{UInt16}),utf16(var),C_NULL)
+        ret = ccall(:SetEnvironmentVariableW,stdcall,Int32,(Cwstring,Ptr{UInt16}),var,C_NULL)
         systemerror(:setenv, ret == 0)
     end
 end

base/exports.jl

@@ -144,6 +144,8 @@ export
     Culonglong,
     Cushort,
     Cwchar_t,
+    Cstring,
+    Cwstring,
 
 # Exceptions
     ArgumentError,

base/fftw.jl

@@ -71,7 +71,7 @@ typealias fftwTypeSingle Union(Type{Float32},Type{Complex64})
 # FFTW's api/import-wisdom-from-file.c file].
 
 function export_wisdom(fname::AbstractString)
-    f = ccall(:fopen, Ptr{Void}, (Ptr{UInt8},Ptr{UInt8}), fname, "w")
+    f = ccall(:fopen, Ptr{Void}, (Cstring,Ptr{UInt8}), fname, "w")
     systemerror("could not open wisdom file $fname for writing", f == C_NULL)
     ccall((:fftw_export_wisdom_to_file,libfftw), Void, (Ptr{Void},), f)
     ccall(:fputs, Int32, (Ptr{UInt8},Ptr{Void}), " "^256, f)
@@ -80,7 +80,7 @@ function export_wisdom(fname::AbstractString)
 end
 
 function import_wisdom(fname::AbstractString)
-    f = ccall(:fopen, Ptr{Void}, (Ptr{UInt8},Ptr{UInt8}), fname, "r")
+    f = ccall(:fopen, Ptr{Void}, (Cstring,Ptr{UInt8}), fname, "r")
     systemerror("could not open wisdom file $fname for reading", f == C_NULL)
     if ccall((:fftw_import_wisdom_from_file,libfftw),Int32,(Ptr{Void},),f)==0||
        ccall((:fftwf_import_wisdom_from_file,libfftwf),Int32,(Ptr{Void},),f)==0

base/file.jl

@@ -8,7 +8,7 @@ function pwd()
 end
 
 function cd(dir::AbstractString)
-    uv_error("chdir $dir", ccall(:uv_chdir, Cint, (Ptr{UInt8},), dir))
+    uv_error("chdir $dir", ccall(:uv_chdir, Cint, (Cstring,), dir))
 end
 cd() = cd(homedir())
 
@@ -35,8 +35,8 @@ end
 cd(f::Function) = cd(f, homedir())
 
 function mkdir(path::AbstractString, mode::Unsigned=0o777)
-    @unix_only ret = ccall(:mkdir, Int32, (Ptr{UInt8},UInt32), path, mode)
-    @windows_only ret = ccall(:_wmkdir, Int32, (Ptr{UInt16},), utf16(path))
+    @unix_only ret = ccall(:mkdir, Int32, (Cstring,UInt32), path, mode)
+    @windows_only ret = ccall(:_wmkdir, Int32, (Cwstring,), path)
     systemerror(:mkdir, ret != 0)
 end
 
@@ -61,8 +61,8 @@ function rm(path::AbstractString; recursive::Bool=false)
                 rm(joinpath(path, p), recursive=true)
             end
         end
-        @unix_only ret = ccall(:rmdir, Int32, (Ptr{UInt8},), path)
-        @windows_only ret = ccall(:_wrmdir, Int32, (Ptr{UInt16},), utf16(path))
+        @unix_only ret = ccall(:rmdir, Int32, (Cstring,), path)
+        @windows_only ret = ccall(:_wrmdir, Int32, (Cwstring,), path)
         systemerror(:rmdir, ret != 0)
     end
 end
@@ -168,8 +168,7 @@ end
 tempname(uunique::UInt32=UInt32(0)) = tempname(tempdir(), uunique)
 function tempname(temppath::AbstractString,uunique::UInt32)
     tname = Array(UInt16,32767)
-    uunique = ccall(:GetTempFileNameW,stdcall,UInt32,(Ptr{UInt16},Ptr{UInt16},UInt32,Ptr{UInt16}),
-        utf16(temppath),utf16("jul"),uunique,tname)
+    uunique = ccall(:GetTempFileNameW,stdcall,UInt32,(Cwstring,Ptr{UInt16},UInt32,Ptr{UInt16}), temppath,utf16("jul"),uunique,tname)
     lentname = findfirst(tname,0)-1
     if uunique == 0 || lentname <= 0
         error("GetTempFileName failed: $(FormatMessage())")
@@ -223,7 +222,7 @@ function readdir(path::AbstractString)
     uv_readdir_req = zeros(UInt8, ccall(:jl_sizeof_uv_fs_t, Int32, ()))
 
     # defined in sys.c, to call uv_fs_readdir, which sets errno on error.
-    file_count = ccall(:jl_readdir, Int32, (Ptr{UInt8}, Ptr{UInt8}),
+    file_count = ccall(:jl_readdir, Int32, (Cstring, Ptr{UInt8}),
                         path, uv_readdir_req)
     systemerror("unable to read directory $path", file_count < 0)
 

base/fs.jl

@@ -72,7 +72,7 @@ _uv_fs_result(req) = ccall(:jl_uv_fs_result,Int32,(Ptr{Void},),req)
 
 function open(f::File,flags::Integer,mode::Integer=0)
     req = Libc.malloc(_sizeof_uv_fs)
-    ret = ccall(:uv_fs_open,Int32,(Ptr{Void},Ptr{Void},Ptr{UInt8},Int32,Int32,Ptr{Void}),
+    ret = ccall(:uv_fs_open,Int32,(Ptr{Void},Ptr{Void},Cstring,Int32,Int32,Ptr{Void}),
                 eventloop(), req, f.path, flags,mode, C_NULL)
     f.handle = _uv_fs_result(req)
     ccall(:uv_fs_req_cleanup,Void,(Ptr{Void},),req)
@@ -96,7 +96,7 @@ function close(f::File)
 end
 
 function unlink(p::AbstractString)
-    err = ccall(:jl_fs_unlink, Int32, (Ptr{UInt8},), p)
+    err = ccall(:jl_fs_unlink, Int32, (Cstring,), p)
     uv_error("unlink",err)
 end
 function unlink(f::File)
@@ -112,7 +112,7 @@ end
 
 # For move command
 function rename(src::AbstractString, dst::AbstractString)
-    err = ccall(:jl_fs_rename, Int32, (Ptr{UInt8}, Ptr{UInt8}), src, dst)
+    err = ccall(:jl_fs_rename, Int32, (Cstring, Cstring), src, dst)
 
     # on error, default to cp && rm
     if err < 0
@@ -159,7 +159,7 @@ end
 @non_windowsxp_only function symlink(p::AbstractString, np::AbstractString)
     flags = 0
     @windows_only if isdir(p); flags |= UV_FS_SYMLINK_JUNCTION; p = abspath(p); end
-    err = ccall(:jl_fs_symlink, Int32, (Ptr{UInt8}, Ptr{UInt8}, Cint), p, np, flags)
+    err = ccall(:jl_fs_symlink, Int32, (Cstring, Cstring, Cint), p, np, flags)
     @windows_only if err < 0
         Base.warn_once("Note: on Windows, creating file symlinks requires Administrator privileges.")
     end
@@ -171,7 +171,7 @@ end
 function readlink(path::AbstractString)
     req = Libc.malloc(_sizeof_uv_fs)
     ret = ccall(:uv_fs_readlink, Int32,
-        (Ptr{Void}, Ptr{Void}, Ptr{UInt8}, Ptr{Void}),
+        (Ptr{Void}, Ptr{Void}, Cstring, Ptr{Void}),
         eventloop(), req, path, C_NULL)
     uv_error("readlink", ret)
     tgt = bytestring(ccall(:jl_uv_fs_t_ptr, Ptr{Cchar}, (Ptr{Void}, ), req))
@@ -181,7 +181,7 @@ function readlink(path::AbstractString)
 end
 
 function chmod(p::AbstractString, mode::Integer)
-    err = ccall(:jl_fs_chmod, Int32, (Ptr{UInt8}, Cint), p, mode)
+    err = ccall(:jl_fs_chmod, Int32, (Cstring, Cint), p, mode)
     uv_error("chmod",err)
 end
 

base/gmp.jl

@@ -77,17 +77,25 @@ BigInt(x::BigInt) = x
 
 function tryparse_internal(::Type{BigInt}, s::AbstractString, startpos::Int, endpos::Int, base::Int, raise::Bool)
     _n = Nullable{BigInt}()
-    sgn, base, i = Base.parseint_preamble(true,base,s,startpos,endpos)
+
+    # don't make a copy in the common case where we are parsing a whole bytestring
+    bstr = startpos == start(s) && endpos == endof(s) ? bytestring(s) : bytestring(SubString(s,i,endpos))
+
+    sgn, base, i = Base.parseint_preamble(true,base,bstr,start(bstr),endof(bstr))
     if i == 0
-        raise && throw(ArgumentError("premature end of integer: $(repr(s))"))
+        raise && throw(ArgumentError("premature end of integer: $(repr(bstr))"))
         return _n
     end
     z = BigInt()
-    err = ccall((:__gmpz_set_str, :libgmp),
-               Int32, (Ptr{BigInt}, Ptr{UInt8}, Int32),
-               &z, SubString(s,i,endpos), base)
+    if Base.containsnul(bstr)
+        err = -1 # embedded NUL char (not handled correctly by GMP)
+    else
+        err = ccall((:__gmpz_set_str, :libgmp),
+                    Int32, (Ptr{BigInt}, Ptr{UInt8}, Int32),
+                    &z, pointer(bstr)+(i-start(bstr)), base)
+    end
     if err != 0
-        raise && throw(ArgumentError("invalid BigInt: $(repr(s))"))
+        raise && throw(ArgumentError("invalid BigInt: $(repr(bstr))"))
         return _n
     end
     Nullable(sgn < 0 ? -z : z)

base/interactiveutil.jl

@@ -107,6 +107,9 @@ end
 
 @windows_only begin # TODO: these functions leak memory and memory locks if they throw an error
     function clipboard(x::AbstractString)
+        if containsnul(x)
+            throw(ArgumentError("Windows clipboard strings cannot contain NUL character"))
+        end
         systemerror(:OpenClipboard, 0==ccall((:OpenClipboard, "user32"), stdcall, Cint, (Ptr{Void},), C_NULL))
         systemerror(:EmptyClipboard, 0==ccall((:EmptyClipboard, "user32"), stdcall, Cint, ()))
         x_u16 = utf16(x)
@@ -355,7 +358,7 @@ end
 
 @windows_only function download(url::AbstractString, filename::AbstractString)
     res = ccall((:URLDownloadToFileW,:urlmon),stdcall,Cuint,
-                (Ptr{Void},Ptr{UInt16},Ptr{UInt16},Cint,Ptr{Void}),0,utf16(url),utf16(filename),0,0)
+                (Ptr{Void},Cwstring,Cwstring,Cint,Ptr{Void}),C_NULL,url,filename,0,0)
     if res != 0
         error("automatic download failed (error: $res): $url")
     end

base/iostream.jl

@@ -86,7 +86,7 @@ function open(fname::AbstractString, rd::Bool, wr::Bool, cr::Bool, tr::Bool, ff:
     s = IOStream(string("<file ",fname,">"))
     systemerror("opening file $fname",
                 ccall(:ios_file, Ptr{Void},
-                      (Ptr{UInt8}, Ptr{UInt8}, Int32, Int32, Int32, Int32),
+                      (Ptr{UInt8}, Cstring, Int32, Int32, Int32, Int32),
                       s.ios, fname, rd, wr, cr, tr) == C_NULL)
     if ff
         systemerror("seeking to end of file $fname", ccall(:ios_seek_end, FileOffset, (Ptr{Void},), s.ios) != 0)