Use const operand for ARM syscall implementation

[Ioan-Cristian]

asm_const feature has been stabilized in Rust 1.82. Using const operands for inline assembly eliminates a lot of the duplicated code in the ARM syscall implementation.

This commit has been tested by running a simple application that periodically blinks and writes a message on Raspberry Pi Pico.

[alevy]

Note that this requires an update of the Rust version. @jrvanwhy (and others) are any significant implications for downstream users)?

[jrvanwhy]

Note that this requires an update of the Rust version. @jrvanwhy (and others) are any significant implications for downstream users)?

I discussed this with Lawrence, it sounds like there would be minimal impact (i.e. they would add #![feature(asm_const)] to libtock-rs when built on their toolchain).

Longer term, we may want to have a stricter MSRV policy, but this PR isn't an issue.

[lschuermann]

Are the CI failures related to this change?

[Ioan-Cristian]

Yes, they seem to fail on this branch, but not on master. I am not familiar with the test environment, so I don't know why they fail.

[jrvanwhy]

The error message makes it looks like the toolchain is catching something that would be UB -- a panic unwinding across the syscall boundary. However, subscribe catches unwinding and calls the Exit syscall. As far as I can tell, that Exit call should not unwind, so I don't know why we're getting the error message.

Regardless, the CI failure is almost certainly due to the toolchain update.

I don't have time to debug further for at least a few more days, sadly.

[jrvanwhy]

It seems the mechanism we use to catch unwinding upcalls is unreliable: rust-lang/rust#123231. I'm still reading through the thread; I don't have time to finish reading it (much less understand it) tonight. I suspect that this is defined behavior, but an abort is definitely not how we want this to be handled.

If so, then the unit test failure is correct: our code is not handling an unwinding upcall in the manner it is supposed to.

On another note, libtock-rs's current intended behavior in the event of an unwinding upcall is to exit with a successful completion code, which doesn't seem right to me. We should have a mechanism for libtock-rs to communicate that an upcall unwound, such as a dedicated completion code for it.

[jrvanwhy]

I've done some more reading. The good news: this is not and never was UB. Further, catching the unwind wasn't necessary for safety anyway -- code that tries to unwind across an extern "C" ABI will reliably abort. Also, due to rust-lang/rust#129582, this will start working again as of Rust 1.84.

The bad news: That behavior is not spec'd yet, so we can't rely on it. In that respect, the unit test failure is correct: our code does not reliably work. The only proper fix I see is to require the caller to inject a catch_unwind implementation, which might require some pretty major API changes to libtock-rs. If we do add new Subscribe APIs to libtock-rs (e.g. in #494), we should keep this in mind; perhaps we can provide better behaviors then.

For this PR, lets just delete the unwinding_upcall test.

[Ioan-Cristian]

I commented out the test explaining why it was removed and that it should be readded when Rust is updated to 1.84.

[jrvanwhy]

Oh, CI is still failing. This time it appears to just be due to new lints.

[Ioan-Cristian]

Do you think I should create a separate PR to fix the lint checks?

[hudson-ayers]

IMO same PR in a standalone commit would be fine. Alternatively you could do a standalone PR for the Rust update which would include removing the test and the lint fixes, and this PR could just be the asm_const change and nothing else.