Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix some issues discovered by JET #48303

Merged
merged 1 commit into from
Jan 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion base/binaryplatforms.jl
Original file line number Diff line number Diff line change
Expand Up @@ -873,7 +873,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}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this should be inside dlopen?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dlopen optionally can take throw_error=false as a kwarg and return nothing

# 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