Skip to content

Commit

Permalink
Merge pull request #45765 from JuliaLang/vc/safe_atexit
Browse files Browse the repository at this point in the history 
Don't segfault when running atexit before jl_threads_init
  • Loading branch information
@staticfloat
staticfloat authored Jun 29, 2022
2 parents 84bf42a + 21ab24e commit b11ccae
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 36 deletions.
4 changes: 3 additions & 1 deletion src/gc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -481,9 +481,11 @@ static void schedule_all_finalizers(arraylist_t *flist) JL_NOTSAFEPOINT
void jl_gc_run_all_finalizers(jl_task_t *ct)
{
schedule_all_finalizers(&finalizer_list_marked);
// This could be run before we had a chance to setup all threads
for (int i = 0;i < jl_n_threads;i++) {
jl_ptls_t ptls2 = jl_all_tls_states[i];
schedule_all_finalizers(&ptls2->finalizers);
if (ptls2)
schedule_all_finalizers(&ptls2->finalizers);
}
run_finalizers(ct);
}
Expand Down
15 changes: 11 additions & 4 deletions src/jitlayers.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -814,13 +814,20 @@ namespace {
SmallVector<std::string, 10> targetFeatures(target.second.begin(), target.second.end());
std::string errorstr;
const Target *TheTarget = TargetRegistry::lookupTarget("", TheTriple, errorstr);
if (!TheTarget)
jl_errorf("%s", errorstr.c_str());
if (!TheTarget) {
// Note we are explicitly not using `jl_errorf()` here, as it will attempt to
// collect a backtrace, but we're too early in LLVM initialization for that.
jl_printf(JL_STDERR, "ERROR: %s", errorstr.c_str());
exit(1);
}
if (jl_processor_print_help || (target_flags & JL_TARGET_UNKNOWN_NAME)) {
std::unique_ptr<MCSubtargetInfo> MSTI(
TheTarget->createMCSubtargetInfo(TheTriple.str(), "", ""));
if (!MSTI->isCPUStringValid(TheCPU))
jl_errorf("Invalid CPU name \"%s\".", TheCPU.c_str());
if (!MSTI->isCPUStringValid(TheCPU)) {
// Same as above, we are too early to use `jl_errorf()` here.
jl_printf(JL_STDERR, "ERROR: Invalid CPU name \"%s\".", TheCPU.c_str());
exit(1);
}
if (jl_processor_print_help) {
// This is the only way I can find to print the help message once.
// It'll be nice if we can iterate through the features and print our own help
Expand Down
74 changes: 43 additions & 31 deletions test/cmdlineargs.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,16 @@ function format_filename(s)
return r
end

# Returns true if the given command errors, but doesn't signal
function errors_not_signals(cmd::Cmd)
p = run(pipeline(ignorestatus(cmd); stdout=devnull, stderr=devnull))
return errors_not_signals(p)
end
function errors_not_signals(p::Base.Process)
wait(p)
return process_exited(p) && !Base.process_signaled(p) && !success(p)
end

let
fn = format_filename("a%d %p %i %L %l %u z")
hd = withenv("HOME" => nothing) do
Expand Down Expand Up @@ -161,22 +171,22 @@ let exename = `$(Base.julia_cmd()) --startup-file=no --color=no`

# --eval
@test success(`$exename -e "exit(0)"`)
@test !success(`$exename -e "exit(1)"`)
@test errors_not_signals(`$exename -e "exit(1)"`)
@test success(`$exename --eval="exit(0)"`)
@test !success(`$exename --eval="exit(1)"`)
@test !success(`$exename -e`)
@test !success(`$exename --eval`)
@test errors_not_signals(`$exename --eval="exit(1)"`)
@test errors_not_signals(`$exename -e`)
@test errors_not_signals(`$exename --eval`)
# --eval --interactive (replaced --post-boot)
@test success(`$exename -i -e "exit(0)"`)
@test !success(`$exename -i -e "exit(1)"`)
@test errors_not_signals(`$exename -i -e "exit(1)"`)
# issue #34924
@test success(`$exename -e 'const LOAD_PATH=1'`)

# --print
@test read(`$exename -E "1+1"`, String) == "2\n"
@test read(`$exename --print="1+1"`, String) == "2\n"
@test !success(`$exename -E`)
@test !success(`$exename --print`)
@test errors_not_signals(`$exename -E`)
@test errors_not_signals(`$exename --print`)

# --load
let testfile = tempname()
Expand Down Expand Up @@ -209,12 +219,13 @@ let exename = `$(Base.julia_cmd()) --startup-file=no --color=no`
end
end
# -L, --load requires an argument
@test !success(`$exename -L`)
@test !success(`$exename --load`)
@test errors_not_signals(`$exename -L`)
@test errors_not_signals(`$exename --load`)

# --cpu-target (requires LLVM enabled)
@test !success(`$exename -C invalidtarget`)
@test !success(`$exename --cpu-target=invalidtarget`)
# Strictly test for failed error, not a segfault, since we had a false positive with just `success()` before.
@test errors_not_signals(`$exename -C invalidtarget`)
@test errors_not_signals(`$exename --cpu-target=invalidtarget`)

# -t, --threads
code = "print(Threads.nthreads())"
Expand All @@ -240,18 +251,21 @@ let exename = `$(Base.julia_cmd()) --startup-file=no --color=no`
withenv("JULIA_NUM_THREADS" => string(cpu_threads)) do
@test read(`$exename -e $code`, String) == string(cpu_threads)
end
@test !success(`$exename -t 0`)
@test !success(`$exename -t -1`)
@test errors_not_signals(`$exename -t 0`)
@test errors_not_signals(`$exename -t -1`)

# Combining --threads and --procs: --threads does propagate
withenv("JULIA_NUM_THREADS" => nothing) do
code = "print(sum(remotecall_fetch(Threads.nthreads, x) for x in procs()))"
@test read(`$exename -p2 -t2 -e $code`, String) == "6"
end

# Combining --threads and invalid -C should yield a decent error
@test errors_not_signals(`$exename -t 2 -C invalidtarget`)

# --procs
@test readchomp(`$exename -q -p 2 -e "println(nworkers())"`) == "2"
@test !success(`$exename -p 0`)
@test errors_not_signals(`$exename -p 0`)
let p = run(`$exename --procs=1.0`, wait=false)
wait(p)
@test p.exitcode == 1 && p.termsignal == 0
Expand All @@ -278,14 +292,14 @@ let exename = `$(Base.julia_cmd()) --startup-file=no --color=no`
# --color
@test readchomp(`$exename --color=yes -E "Base.have_color"`) == "true"
@test readchomp(`$exename --color=no -E "Base.have_color"`) == "false"
@test !success(`$exename --color=false`)
@test errors_not_signals(`$exename --color=false`)

# --history-file
@test readchomp(`$exename -E "Bool(Base.JLOptions().historyfile)"
--history-file=yes`) == "true"
@test readchomp(`$exename -E "Bool(Base.JLOptions().historyfile)"
--history-file=no`) == "false"
@test !success(`$exename --history-file=false`)
@test errors_not_signals(`$exename --history-file=false`)

# --code-coverage
mktempdir() do dir
Expand Down Expand Up @@ -449,16 +463,16 @@ let exename = `$(Base.julia_cmd()) --startup-file=no --color=no`
--check-bounds=no`)) == JL_OPTIONS_CHECK_BOUNDS_OFF
end
# check-bounds takes yes/no as argument
@test !success(`$exename -E "exit(0)" --check-bounds=false`)
@test errors_not_signals(`$exename -E "exit(0)" --check-bounds=false`)

# --depwarn
@test readchomp(`$exename --depwarn=no -E "Base.JLOptions().depwarn"`) == "0"
@test readchomp(`$exename --depwarn=yes -E "Base.JLOptions().depwarn"`) == "1"
@test !success(`$exename --depwarn=false`)
@test errors_not_signals(`$exename --depwarn=false`)
# test deprecated syntax
@test !success(`$exename -e "foo (x::Int) = x * x" --depwarn=error`)
@test errors_not_signals(`$exename -e "foo (x::Int) = x * x" --depwarn=error`)
# test deprecated method
@test !success(`$exename -e "
@test errors_not_signals(`$exename -e "
foo() = :foo; bar() = :bar
@deprecate foo() bar()
foo()
Expand All @@ -476,7 +490,7 @@ let exename = `$(Base.julia_cmd()) --startup-file=no --color=no`
Foo.Deprecated
"""

@test !success(`$exename -E "$code" --depwarn=error`)
@test errors_not_signals(`$exename -E "$code" --depwarn=error`)

@test readchomperrors(`$exename -E "$code" --depwarn=yes`) ==
(true, "true", "WARNING: Foo.Deprecated is deprecated, use NotDeprecated instead.\n likely near none:8")
Expand All @@ -490,14 +504,14 @@ let exename = `$(Base.julia_cmd()) --startup-file=no --color=no`
@test readchomp(`$exename --inline=yes -E "Bool(Base.JLOptions().can_inline)"`) == "true"
@test readchomp(`$exename --inline=no -E "Bool(Base.JLOptions().can_inline)"`) == "false"
# --inline takes yes/no as argument
@test !success(`$exename --inline=false`)
@test errors_not_signals(`$exename --inline=false`)

# --polly
@test readchomp(`$exename -E "Bool(Base.JLOptions().polly)"`) == "true"
@test readchomp(`$exename --polly=yes -E "Bool(Base.JLOptions().polly)"`) == "true"
@test readchomp(`$exename --polly=no -E "Bool(Base.JLOptions().polly)"`) == "false"
# --polly takes yes/no as argument
@test !success(`$exename --polly=false`)
@test errors_not_signals(`$exename --polly=false`)

# --fast-math
let JL_OPTIONS_FAST_MATH_DEFAULT = 0,
Expand All @@ -515,7 +529,7 @@ let exename = `$(Base.julia_cmd()) --startup-file=no --color=no`

# --worker takes default / custom as argument (default/custom arguments
# tested in test/parallel.jl)
@test !success(`$exename --worker=true`)
@test errors_not_signals(`$exename --worker=true`)

# test passing arguments
mktempdir() do dir
Expand Down Expand Up @@ -551,7 +565,7 @@ let exename = `$(Base.julia_cmd()) --startup-file=no --color=no`
@test readchomp(`$exename -L $testfile $testfile`) == output
@test readchomp(`$exename --startup-file=yes $testfile`) == output

@test !success(`$exename --foo $testfile`)
@test errors_not_signals(`$exename --foo $testfile`)
end
end

Expand Down Expand Up @@ -620,7 +634,7 @@ let exename = `$(Base.julia_cmd()) --startup-file=no --color=no`
"Bool(Base.JLOptions().use_compiled_modules)"`) == "true"
@test readchomp(`$exename --compiled-modules=no -E
"Bool(Base.JLOptions().use_compiled_modules)"`) == "false"
@test !success(`$exename --compiled-modules=foo -e "exit(0)"`)
@test errors_not_signals(`$exename --compiled-modules=foo -e "exit(0)"`)

# issue #12671, starting from a non-directory
# rm(dir) fails on windows with Permission denied
Expand Down Expand Up @@ -666,8 +680,7 @@ let exename = `$(Base.julia_cmd().exec[1]) -t 1`
@test !occursin("Segmentation fault", s)
@test !occursin("EXCEPTION_ACCESS_VIOLATION", s)
end
@test !success(p)
@test !Base.process_signaled(p)
@test errors_not_signals(p)
@test p.exitcode == 1
end
end
Expand All @@ -677,8 +690,7 @@ let exename = `$(Base.julia_cmd().exec[1]) -t 1`
let s = read(err, String)
@test s == "ERROR: System image file failed consistency check: maybe opened the wrong version?\n"
end
@test !success(p)
@test !Base.process_signaled(p)
@test errors_not_signals(p)
@test p.exitcode == 1
end
end
Expand All @@ -696,7 +708,7 @@ let exename = Base.julia_cmd()
@test parse(Int,readchomp(`$exename -E "Base.JLOptions().startupfile"
--startup-file=no`)) == JL_OPTIONS_STARTUPFILE_OFF
end
@test !success(`$exename --startup-file=false`)
@test errors_not_signals(`$exename --startup-file=false`)
end

# Make sure `julia --lisp` doesn't break
Expand Down

2 comments on commit b11ccae

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your package evaluation job has completed - possible new issues were detected. A full report can be found here.

Please sign in to comment.