Use asm-based shim for sigsetjmp

[thomcc]

See rust-lang/unsafe-code-guidelines#404 (comment) for a proof of concept. This should make our setjmp handling less UB (without requiring unstable features), at the cost of needing some inline asm.

I'd have to write one for aarch64 too, and we'd use the current approach as a fallback for targets without a shim.

[thomcc]

Thinking more about it, we should just call it in our C shim.

It's not just the overhead of maintaining x86_64 vs aarch64 (and macos/linux/potentially windows), it's also tricky to get the relocations right -- e.g. call setjmp vs mov _setjmp@GOTPCREL(%rip), %r14; call *%r14;.

[eeeebbbbrrrr]

fun fact: plrust purposely doesn't include the cshim.

[thomcc]

Ah, right, dang. Okay, it's probably not as bad as I made it out to be above (although both snippets are valid). Ugh.

[eeeebbbbrrrr]

Something I was thinking about last night but didn't know how to express in words...

We currently stash the "jumpbuf" (the first argument to sigsetjmp) in Postgres's global exception stack. My assembly is at a 3rd grade level, but I'm not seeing how we can do that with this particular combination of instructions.

[thomcc]

Right, that's another part of why I'd rather do this in C if possible, since our implementation would be quite a bit more involved.

[eeeebbbbrrrr]

Can you explain what's wrong with our current approach?

I've always thought what we're doing is fine since we're only calling an extern "C" function while the jump is set.

[thomcc]

Basically setjmp/sigsetjmp is a function that returns twice, and the compiler doesn't know that.

As an example of how this might be an issue, consider stack slot coloring, which would normally allow a stack slot used for a variable at the time of (sig)setjmp to be reused for another variable later in the function if, based on the function's control flow, the original variable is dead. This is a problem if we longjmp back and try to access said variable.

That's just one example, there are a number of similar issues for functions like this.

[eeeebbbbrrrr]

Right, I generally understand why they're UB. I guess my question is... is how we're using them really UB and we've been getting lucky, or are we actually okay because we only do 1 thing after calling sigsetjmp?

[thomcc]

I'm not sure. There are a few possibilities, but my belief is: Error cases are rare, and this is only really UB in the case where the error occurs (otherwise setjmp does not return twice) meaning that the case where this causes issues probably doesn't get hit a lot, and may never occur in situations where the UB manifests.

The other possibility is that LLVM internally "knows" that sigsetjmp/setjmp are symbols that refer to functions that return twice. This is also possible but I'd want to verify that before really relying on it.

[thomcc]

We also could use ffi_returns_twice as an unstable feature, only for plrust. I'd rather not, but given that it's already using a custom rustc driver, it wouldn't be out of the question.

I'm going to noodle on this a bit, the more I think about it the more likely it seems that this is actually a real problem we're getting away with only because hitting errors is uncommon.

[thomcc]

It also occurs to me that we could push the jmp_buf on the postgres error stack prior to calling into that asm shim. There would be a period of time where the postgres error stack has an invalid jmp_buf on it, but that's fine so long as we do it right before the asm shim. (We don't have to worry about concurrency and just have to be sure not to panic between the two operations)

[workingjubilee]

note: https://docs.rs/cee-scape/latest/cee_scape/