Open workingjubilee opened 1 month ago
uh what?
I have no idea what happened here. I'm going to reuse this for something I found, one sec.
cc @cuviper I believe you were the one to add the NEED_ALTSTACK
var. it was quite a while ago, but do you happen to remember anything useful to know about the reasons underlying that PR and why the global variable approach? I ask mostly because Handler::new()
is only called from the thread impl, and it could be an entirely different function, for instance.
Perhaps it couldn't be at the time, I think new_main
is relatively recent, but
I guess you must have found commit 676b9bc477dfe58971b7df9df4e3a053bb187dee to "blame" me, but I don't remember any more about that offhand. I think it's global because the decision is made once when registering the signals, but needs to affect all threads.
(I won't be able to dig any more until next week at the earliest.)
dw, I figured 4 years ago might've been too long ago to remember!
@cuviper for reference this has mostly led to this question, because I started thinking about the ordering of when we should set NEED_ALTSTACK
: https://github.com/rust-lang/rust/pull/127843/files#r1680398732
There are two parts to our SIGSEGV
handler:
sigaction
. This is shared per-process.sigaltstack
. This is per-thread.In the Rust runtime initialization code, we attempt to install our signal handler. If there is no handler previously installed, we need to create a signal stack for every thread we spawn, so that the handler has a stack to run on (the main thread obviously can't be used, it already overflowed). If there is already a handler installed, we do not overwrite it. Thus, when we create another thread, we do and should not allocate a signal stack, as that might interfere with the code that registered the action. NEED_ALTSTACK
is used to propagate this check from the main initialization code to the initialization code for threads. It also ensures that the PAGE_SIZE
variable used to cache the platform page size has the current value (we could use a OnceLock
for this, too, but this is more efficient).
Part of the reason this code is hard to follow is that there are several unsafe fn
with no obvious safety conditions on them. Thus it feels like everything is overly exciting, when in reality the unsafe operations are usually completely harmless inspections.
I've already started cleaning this up as part of https://github.com/rust-lang/rust/issues/127747
Relevant history:
It's not clear, to me, why the update of the
NEEDS_ALTSTACK
variable is occurring. I don't know everything there is to know about signal handlers, but I do knowWe have two fairly distinct codepaths for
make_handler
. In code we control, we only call one or the other (i.e. a constant input). They're really quite different functions. So it's not clear where the logical dependencies emerge between the two.