Add a new RunWithFaultHandler utility

[mmdriley]

This avoids the need for a global afterspeculation label.

Use to rewrite the Meltdown example.

[mmdriley]

This is based on #126. Will update the base branch to master once #126 lands.

[ssbr]

Well, I learned a lot today. The memories of call/cc just come flooding back, don't they? :)

[ssbr]

This file is tricky, is it a reasonable request to ask for unit tests? (Thinking of like three tests, one which sends no signal, one which sends SIGSEGV, and one which sends some other signal.)

[mmdriley]

Definitely reasonable. Tests added in next revision.

[ssbr]

This doesn't seem to be used yet except with an empty callback. Will we need it, or can callers always check the return value instead?

Signal handlers are super scary / hard to do right thing (signal safety is high impossible), so I'd prefer if we didn't expose it at all, if at all possible.

And here's a case in point: if we do want to support it, we must not use std::function, because std::function::operator() does not document itself as signal safe, which makes it UB to call per the rules of [support.signal] (3.1).

[mmdriley]

That's true, this seems vestigial at this point. Initially I was concerned that sigsetjmp would disrupt the cache enough we wouldn't be able to read the sidechannel, but testing doesn't seem to show that's the case. I'll remove this knob for now; we can add it back if need be.

[ssbr]

Unfortunately, making them thread_local makes them UB to access within a signal handler, per [support.signal] (3.2). Since it was only ever aspirational anyway, just delete.

[mmdriley]

So, a few things about this and other comments on signal safety.

First, yes, getting async signal safety right, to the standards of the sort of systems where it usually comes up, is very hard. That puts us in tension because the more we work to make this "right", the harder it is to understand.

For our application, here, I think the right tradeoff is to be very easy to understand, and at least mostly right. Obviously we'll fix bugs or unreliability we hit on the platforms we support.

There are some other mitigating factors for us, aside from the obvious ones like "our entire repository is predicated on the existence of undefined but predictable behavior". The signals we care about, faults like SIGSEGV, are synchronous -- they are caused by instructions in the program, rather than external events.

These signals are also usually fatal, so we can say with some confidence that they are not normally raised by code not under our control, like the standard library. They also will not be raised during execution of the signal handler itself unless we have a bug, in which case we get the failure we wanted.

All this to say: for now, however distasteful it might seem, I suggest we use "works as expected in concrete implementations" as our standard rather than "works on the abstract machine". I would be fine adding comments to the code to that effect.

Fun trivia - Rust also accesses thread-local data in a SIGSEGV handler. rust-lang/rust#43146

[mmdriley]

I still removed thread_local.

[ssbr]

FWIW I think atomics would be likely to work with our use case, without any undefined, unspecified, or implementation-defined behavior.

There's one other issue that made me cautious that I should've mentioned first: different platforms send signals to different threads, so the moment we have thread-local signal handlers, even if there's no signals raised during assignment to the threadlocal we're in Very Difficult territory, which makes it harder, rather than easier, to understand.

[ssbr]

Ah, I think this should be extern "C" per https://en.cppreference.com/w/cpp/utility/program/signal "Signal handlers are expected to have C linkage and, in general, only use the features from the common subset of C and C++. It is implementation-defined if a function with C++ linkage can be used as a signal handler."

[mmdriley]

Changed.

[mmdriley]

Added one last tweak to run the new unit test in CI.