Merge pull request #10994 from stevengj/nullsafe2

add Cstring/Cwstring types for safe passing of NUL-terminated strings to ccall

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

@@ -145,6 +145,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

@@ -70,9 +70,9 @@ uvtype(::File) = Base.UV_RAW_FD
 
 _uv_fs_result(req) = ccall(:jl_uv_fs_result,Int32,(Ptr{Void},),req)
 
-function open(f::File,flags::Integer,mode::Integer)
+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
@@ -127,35 +127,39 @@ end
 
 # For copy command
 function sendfile(src::AbstractString, dst::AbstractString)
-    src_file = open(src, JL_O_RDONLY)
-    if !src_file.open
-        throw(ArgumentError("source file \"$(src.path)\" is not open"))
-    end
-
-    dst_file = open(dst, JL_O_CREAT | JL_O_TRUNC | JL_O_WRONLY,
-                    S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP| S_IROTH | S_IWOTH)
-    if !dst_file.open
-        throw(ArgumentError("destination file \"$(dst.path)\" is not open"))
-    end
-
-    src_stat = stat(src_file)
-    err = ccall(:jl_fs_sendfile, Int32, (Int32, Int32, Int64, Csize_t),
-                fd(src_file), fd(dst_file), 0, src_stat.size)
-    uv_error("sendfile", err)
-
-    if src_file.open
-        close(src_file)
-    end
-    if dst_file.open
-        close(dst_file)
+    src_file = File(src)
+    dst_file = File(dst)
+    try
+        open(src_file, JL_O_RDONLY)
+        if !src_file.open
+            throw(ArgumentError("source file \"$(src.path)\" is not open"))
+        end
+
+        open(dst_file, JL_O_CREAT | JL_O_TRUNC | JL_O_WRONLY,
+             S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP| S_IROTH | S_IWOTH)
+        if !dst_file.open
+            throw(ArgumentError("destination file \"$(dst.path)\" is not open"))
+        end
+
+        src_stat = stat(src_file)
+        err = ccall(:jl_fs_sendfile, Int32, (Int32, Int32, Int64, Csize_t),
+                    fd(src_file), fd(dst_file), 0, src_stat.size)
+        uv_error("sendfile", err)
+    finally
+        if src_file.open
+            close(src_file)
+        end
+        if dst_file.open
+            close(dst_file)
+        end
     end
 end
 
 @windows_only const UV_FS_SYMLINK_JUNCTION = 0x0002
 @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
@@ -167,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))
@@ -177,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