Skip to content

Commit

Permalink
fix some issues discovered by JET (#48303)
Browse files Browse the repository at this point in the history
  • Loading branch information
KristofferC authored and aviatesk committed Jan 18, 2023
1 parent 5da6d97 commit 9c99454
Show file tree
Hide file tree
Showing 9 changed files with 44 additions and 35 deletions.
2 changes: 1 addition & 1 deletion base/binaryplatforms.jl
Original file line number Diff line number Diff line change
Expand Up @@ -872,7 +872,7 @@ function detect_libstdcxx_version(max_minor_version::Int=30)
end

# Brute-force our way through GLIBCXX_* symbols to discover which version we're linked against
hdl = Libdl.dlopen(first(libstdcxx_paths))
hdl = Libdl.dlopen(first(libstdcxx_paths))::Ptr{Cvoid}
# Try all GLIBCXX versions down to GCC v4.8:
# https://gcc.gnu.org/onlinedocs/libstdc++/manual/abi.html
for minor_version in max_minor_version:-1:18
Expand Down
28 changes: 16 additions & 12 deletions base/loading.jl
Original file line number Diff line number Diff line change
Expand Up @@ -865,7 +865,11 @@ function explicit_manifest_uuid_path(project_file::String, pkg::PkgId)::Union{No
uuid = get(entry, "uuid", nothing)::Union{Nothing, String}
extensions = get(entry, "extensions", nothing)::Union{Nothing, Dict{String, Any}}
if extensions !== nothing && haskey(extensions, pkg.name) && uuid !== nothing && uuid5(UUID(uuid), pkg.name) == pkg.uuid
p = normpath(dirname(locate_package(PkgId(UUID(uuid), name))), "..")
parent_path = locate_package(PkgId(UUID(uuid), name))
if parent_path === nothing
error("failed to find source of parent package: \"$name\"")
end
p = normpath(dirname(parent_path), "..")
extfiledir = joinpath(p, "ext", pkg.name, pkg.name * ".jl")
isfile(extfiledir) && return extfiledir
return joinpath(p, "ext", pkg.name * ".jl")
Expand Down Expand Up @@ -1138,10 +1142,10 @@ function insert_extension_triggers(env::String, pkg::PkgId)::Union{Nothing,Missi
dep_name in weakdeps || continue
entries::Vector{Any}
if length(entries) != 1
error("expected a single entry for $(repr(name)) in $(repr(project_file))")
error("expected a single entry for $(repr(dep_name)) in $(repr(project_file))")
end
entry = first(entries)::Dict{String, Any}
uuid = get(entry, "uuid", nothing)::Union{String, Nothing}
uuid = entry["uuid"]::String
d_weakdeps[dep_name] = uuid
end
@assert length(d_weakdeps) == length(weakdeps)
Expand Down Expand Up @@ -1247,7 +1251,7 @@ function _tryrequire_from_serialized(modkey::PkgId, build_id::UInt128)
loading = get(package_locks, modkey, false)
if loading !== false
# load already in progress for this module
loaded = wait(loading)
loaded = wait(loading::Threads.Condition)
else
package_locks[modkey] = Threads.Condition(require_lock)
try
Expand Down Expand Up @@ -1282,7 +1286,7 @@ function _tryrequire_from_serialized(modkey::PkgId, path::String, ocachepath::Un
loading = get(package_locks, modkey, false)
if loading !== false
# load already in progress for this module
loaded = wait(loading)
loaded = wait(loading::Threads.Condition)
else
for i in 1:length(depmods)
dep = depmods[i]
Expand Down Expand Up @@ -1324,7 +1328,7 @@ function _tryrequire_from_serialized(pkg::PkgId, path::String, ocachepath::Union
pkgimage = !isempty(clone_targets)
if pkgimage
ocachepath !== nothing || return ArgumentError("Expected ocachepath to be provided")
isfile(ocachepath) || return ArgumentError("Ocachepath $ocachpath is not a file.")
isfile(ocachepath) || return ArgumentError("Ocachepath $ocachepath is not a file.")
ocachepath == ocachefile_from_cachefile(path) || return ArgumentError("$ocachepath is not the expected ocachefile")
# TODO: Check for valid clone_targets?
isvalid_pkgimage_crc(io, ocachepath) || return ArgumentError("Invalid checksum in cache file $ocachepath.")
Expand Down Expand Up @@ -1404,13 +1408,13 @@ end
staledeps = true
break
end
staledeps[i] = dep
(staledeps::Vector{Any})[i] = dep
end
if staledeps === true
ocachefile = nothing
continue
end
restored = _include_from_serialized(pkg, path_to_try, ocachefile, staledeps)
restored = _include_from_serialized(pkg, path_to_try, ocachefile::String, staledeps::Vector{Any})
if !isa(restored, Module)
@debug "Deserialization checks failed while attempting to load cache from $path_to_try" exception=restored
else
Expand Down Expand Up @@ -1667,7 +1671,7 @@ function _require(pkg::PkgId, env=nothing)
loading = get(package_locks, pkg, false)
if loading !== false
# load already in progress for this module
return wait(loading)
return wait(loading::Threads.Condition)
end
package_locks[pkg] = Threads.Condition(require_lock)

Expand Down Expand Up @@ -2160,12 +2164,12 @@ function compilecache(pkg::PkgId, path::String, internal_stderr::IO = stderr, in
rename(tmppath_so, ocachefile::String; force=true)
catch e
e isa IOError || rethrow()
isfile(ocachefile) || rethrow()
isfile(ocachefile::String) || rethrow()
# Windows prevents renaming a file that is in use so if there is a Julia session started
# with a package image loaded, we cannot rename that file.
# The code belows append a `_i` to the name of the cache file where `i` is the smallest number such that
# that cache file does not exist.
ocachename, ocacheext = splitext(ocachefile)
ocachename, ocacheext = splitext(ocachefile::String)
old_cachefiles = Set(readdir(cachepath))
num = 1
while true
Expand All @@ -2185,7 +2189,7 @@ function compilecache(pkg::PkgId, path::String, internal_stderr::IO = stderr, in
finally
rm(tmppath, force=true)
if cache_objects
rm(tmppath_o, force=true)
rm(tmppath_o::String, force=true)
rm(tmppath_so, force=true)
end
end
Expand Down
2 changes: 1 addition & 1 deletion stdlib/Artifacts/src/Artifacts.jl
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ function artifacts_dirs(args...)
return String[abspath(depot, "artifacts", args...) for depot in Base.DEPOT_PATH]
else
# If we've been given an override, use _only_ that directory.
return String[abspath(ARTIFACTS_DIR_OVERRIDE[], args...)]
return String[abspath(ARTIFACTS_DIR_OVERRIDE[]::String, args...)]
end
end

Expand Down
2 changes: 1 addition & 1 deletion stdlib/FileWatching/src/FileWatching.jl
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ mutable struct _FDWatcher
t.refcount = (0, 0)
t.active = (false, false)
@static if Sys.isunix()
if FDWatchers[t.fdnum] == t
if FDWatchers[t.fdnum] === t
FDWatchers[t.fdnum] = nothing
end
end
Expand Down
2 changes: 1 addition & 1 deletion stdlib/LibGit2/src/LibGit2.jl
Original file line number Diff line number Diff line change
Expand Up @@ -848,7 +848,7 @@ function rebase!(repo::GitRepo, upstream::AbstractString="", newbase::AbstractSt
end
finally
if !isempty(newbase)
close(onto_ann)
close(onto_ann::GitAnnotated)
end
close(upst_ann)
close(head_ann)
Expand Down
21 changes: 11 additions & 10 deletions stdlib/LibGit2/src/callbacks.jl
Original file line number Diff line number Diff line change
Expand Up @@ -276,18 +276,20 @@ function credentials_callback(libgit2credptr::Ptr{Ptr{Cvoid}}, url_ptr::Cstring,
# information cached inside the payload.
if isempty(p.url)
p.url = unsafe_string(url_ptr)
m = match(URL_REGEX, p.url)
m = match(URL_REGEX, p.url)::RegexMatch

p.scheme = something(m[:scheme], SubString(""))
p.username = something(m[:user], SubString(""))
p.host = m[:host]
p.host = something(m[:host])

# When an explicit credential is supplied we will make sure to use the given
# credential during the first callback by modifying the allowed types. The
# modification only is in effect for the first callback since `allowed_types` cannot
# be mutated.
if p.explicit !== nothing
cred = p.explicit
cache = p.cache
explicit = p.explicit
if explicit !== nothing
cred = explicit

# Copy explicit credentials to avoid mutating approved credentials.
# invalidation fix from cred being non-inferrable
Expand All @@ -300,16 +302,15 @@ function credentials_callback(libgit2credptr::Ptr{Ptr{Cvoid}}, url_ptr::Cstring,
else
allowed_types &= Cuint(0) # Unhandled credential type
end
elseif p.cache !== nothing
elseif cache !== nothing
cred_id = credential_identifier(p.scheme, p.host)

# Perform a deepcopy as we do not want to mutate approved cached credentials
if haskey(p.cache, cred_id)
# invalidation fix from p.cache[cred_id] being non-inferrable
p.credential = Base.invokelatest(deepcopy, p.cache[cred_id])
if haskey(cache, cred_id)
# invalidation fix from cache[cred_id] being non-inferrable
p.credential = Base.invokelatest(deepcopy, cache[cred_id])
end
end

p.first_pass = true
else
p.first_pass = false
Expand Down Expand Up @@ -447,7 +448,7 @@ function ssh_knownhost_check(
)
if (m = match(r"^(.+):(\d+)$", host)) !== nothing
host = m.captures[1]
port = parse(Int, m.captures[2])
port = parse(Int, something(m.captures[2]))
else
port = 22 # default SSH port
end
Expand Down
5 changes: 3 additions & 2 deletions stdlib/LibGit2/src/gitcredential.jl
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ function Base.shred!(cred::GitCredential)
cred.host = nothing
cred.path = nothing
cred.username = nothing
cred.password !== nothing && Base.shred!(cred.password)
pwd = cred.password
pwd !== nothing && Base.shred!(pwd)
cred.password = nothing
return cred
end
Expand Down Expand Up @@ -122,7 +123,7 @@ function Base.read!(io::IO, cred::GitCredential)
if key == "url"
# Any components which are missing from the URL will be set to empty
# https://git-scm.com/docs/git-credential#git-credential-codeurlcode
Base.shred!(parse(GitCredential, value)) do urlcred
Base.shred!(parse(GitCredential, value::AbstractString)) do urlcred
copy!(cred, urlcred)
end
elseif key in GIT_CRED_ATTRIBUTES
Expand Down
13 changes: 8 additions & 5 deletions stdlib/LibGit2/src/types.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1389,7 +1389,8 @@ CredentialPayload(p::CredentialPayload) = p
function Base.shred!(p::CredentialPayload)
# Note: Avoid shredding the `explicit` or `cache` fields as these are just references
# and it is not our responsibility to shred them.
p.credential !== nothing && Base.shred!(p.credential)
credential = p.credential
credential !== nothing && Base.shred!(credential)
p.credential = nothing
end

Expand Down Expand Up @@ -1430,8 +1431,9 @@ function approve(p::CredentialPayload; shred::Bool=true)

# Each `approve` call needs to avoid shredding the passed in credential as we need
# the credential information intact for subsequent approve calls.
if p.cache !== nothing
approve(p.cache, cred, p.url)
cache = p.cache
if cache !== nothing
approve(cache, cred, p.url)
shred = false # Avoid wiping `cred` as this would also wipe the cached copy
end
if p.allow_git_helpers
Expand Down Expand Up @@ -1460,8 +1462,9 @@ function reject(p::CredentialPayload; shred::Bool=true)

# Note: each `reject` call needs to avoid shredding the passed in credential as we need
# the credential information intact for subsequent reject calls.
if p.cache !== nothing
reject(p.cache, cred, p.url)
cache = p.cache
if cache !== nothing
reject(cache, cred, p.url)
end
if p.allow_git_helpers
reject(p.config, cred, p.url)
Expand Down
4 changes: 2 additions & 2 deletions stdlib/LibGit2/src/utils.jl
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ end

function credential_identifier(url::AbstractString)
m = match(URL_REGEX, url)
scheme = something(m[:scheme], "")
host = m[:host]
scheme = something(m[:scheme], SubString(""))
host = something(m[:host])
credential_identifier(scheme, host)
end

0 comments on commit 9c99454

Please sign in to comment.