tests/fuzz/fuzz-open_channel: fix bad local variable access after longjmp

[whitslack]

You can't access a local variable from a point before it was initialized and expect it to have the initialized value. Move the setjmp() call to after run_ctx is initialized so that the tal_free() call at cleanup will see the correct address and not crash.

Fixes: #9131

Checklist

Before submitting the PR, ensure the following tasks are completed. If an item is not applicable to your PR, please mark it as checked:

• The changelog has been updated in the relevant commit(s) according to the guidelines.
• Tests have been added or modified to reflect the changes. N/A
• Documentation has been reviewed and updated as needed. N/A
• Related issues have been listed and linked, including any that this PR closes.
• Important All PRs must consider how to reverse any persistent changes for tools/lightning-downgrade N/A

[Andezion]

The fix correctly identifies and resolves a real UB bug per the C standards setjmp/longjmp rules!

[Andezion]

What do you think, did tests/fuzz/fuzz-handle_onion_message.c (also using jmp_buf fuzz_env and setjmp(fuzz_env) at line 78) has the same ordering issue?

[whitslack]

The use of setjmp/longjmp in tests/fuzz/fuzz-handle_onion_message.c shouldn't cause a crash since the only local variable (daemon) accessed after the longjmp is initialized (to NULL) before the setjmp. However, the call to tal_free(daemon->master) may or may not happen as intended, depending on whether the compiler emits instructions to reload the register holding the value of daemon after the longjmp. If the register is reloaded (from the stack), then it may hold the non-null address to which daemon was set from the return value of new_daemon() (depending on whether the compiler emitted instructions after the call to new_daemon() to flush the new value of daemon back onto the stack), and tal_free(daemon->master) will be called if this has occurred. On the other hand, if the register is not reloaded, then it will still hold the value NULL (the value to which daemon was initialized before the setjmp call), and tal_free(daemon->master) will not be called. I would argue that it's not a good idea to have control flow vary depending upon compiler optimizations. You can prevent the compiler from caching the daemon local variable in a register by declaring it (i.e., the pointer itself, not the pointed-to object) volatile, but I generally wouldn't recommend that, as volatile is detrimental to compiler optimizations. A nicer fix would be to insert a second call to setjmp after daemon is set to the return value from new_daemon(). That would ensure that the code at the cleanup label will always see the latest value of daemon, even in the case that cleanup is reached via a longjmp.