Possible unsoundness on architectures without atomics

ROMemories commented

2025-04-08 08:18:04 -05:00

(Migrated from github.com)

My understanding is that log relies on atomics to implement some of its functionality, especially set_logger() and set_max_level().

It seems there is an internal "fake" AtomicUsize on platforms that do not provide atomics, with the assumption that no such architectures are multicore:

github.com/rust-lang/log@5359483974/src/lib.rs (L451-L454)

However this assumption does not hold on the RP2040 MCU, which features two Cortex-M0+ (ARMv6-M) cores.

I am not sure this actually currently results in unsoundness because it is not yet clear to me whether the instances of AtomicUsize are actually used in ways that could result in race conditions on these architectures.

I suppose leveraging portable-atomic on architectures that do not provide atomics could help fix this, if this is an acceptable dependency for log, which would only be required on such architectures.

For context, we at Ariel OS are building an embedded OS which, among many other things, abstracts over log and makes it easily usable on many processor architectures, making this easy to test.

My understanding is that `log` relies on atomics to implement some of its functionality, especially [`set_logger()`](https://docs.rs/log/latest/log/fn.set_logger.html) and [`set_max_level()`](https://docs.rs/log/latest/log/fn.set_max_level.html). It seems there is an internal "fake" `AtomicUsize` on platforms that do not provide atomics, with the assumption that no such architectures are multicore: https://github.com/rust-lang/log/blob/53594839745cb2bc8ba09d6ba9a3758f45c5da47/src/lib.rs#L451-L454 **However this assumption does not hold on the [RP2040](https://www.raspberrypi.com/products/rp2040/) MCU, which features two Cortex-M0+ (ARMv6-M) cores.** I am not sure this actually currently results in unsoundness because it is not yet clear to me whether the instances of `AtomicUsize` are actually used in ways that could result in race conditions on these architectures. I suppose leveraging [portable-atomic](https://github.com/taiki-e/portable-atomic) on architectures that do not provide atomics could help fix this, if this is an acceptable dependency for `log`, which would only be required on such architectures. --- For context, we at [Ariel OS](https://github.com/ariel-os/ariel-os) are building an embedded OS which, among many other things, [abstracts over `log` and makes it easily usable on many processor architectures](https://ariel-os.github.io/ariel-os/dev/docs/book/debug-console.html#logging), making this easy to test.

Thomasdezeeuw commented

2025-04-08 09:05:06 -05:00

(Migrated from github.com)

Note that set_logger has the cfg(target_has_atomic = "ptr") attribute, i.e. it shouldn't be available if the atomic are not present
github.com/rust-lang/log@5359483974/src/lib.rs (L1402-L1405)

In that case the unsafe set_logger_racy has to be used:
github.com/rust-lang/log@5359483974/src/lib.rs (L1454-L1467)

That doesn't make things any safer, but it means that log isn't unsound.

Note that `set_logger` has the `cfg(target_has_atomic = "ptr")` attribute, i.e. it shouldn't be available if the atomic are not present https://github.com/rust-lang/log/blob/53594839745cb2bc8ba09d6ba9a3758f45c5da47/src/lib.rs#L1402-L1405 In that case the `unsafe` `set_logger_racy` has to be used: https://github.com/rust-lang/log/blob/53594839745cb2bc8ba09d6ba9a3758f45c5da47/src/lib.rs#L1454-L1467 That doesn't make things any safer, but it means that `log` isn't unsound.

ROMemories commented

2025-04-08 12:43:14 -05:00

(Migrated from github.com)

Pondering on this some more, it seems to be me that the platform being multicore or not is actually irrelevant, it just has to support multi-threading for this to be an issue.

In that case the unsafe set_logger_racy has to be used

I am aware of this function, but then the logger() getter function actually relies on the happens-before relationship between LOGGER and STATE:

github.com/rust-lang/log@5359483974/src/lib.rs (L1506-L1521)

However, on architectures without atomics, STATE really is not an atomic, and so this happens-before relationship is not guaranteed.

Pondering on this some more, it seems to be me that the platform being multicore or not is actually irrelevant, it just has to support multi-threading for this to be an issue. > In that case the `unsafe` `set_logger_racy` has to be used I am aware of this function, but then the `logger()` getter function actually relies on the happens-before relationship between `LOGGER` and `STATE`: https://github.com/rust-lang/log/blob/53594839745cb2bc8ba09d6ba9a3758f45c5da47/src/lib.rs#L1506-L1521 However, on architectures without atomics, `STATE` really is not an atomic, and so this happens-before relationship is not guaranteed.

Thomasdezeeuw commented

2025-04-09 04:37:02 -05:00

(Migrated from github.com)

However, on architectures without atomics, STATE really is not an atomic, and so this happens-before relationship is not guaranteed.

Agreed. But then you'll most likely also not have threads. Personally I don't see how threads can do any cross-thread communication safely without atomics.

> However, on architectures without atomics, `STATE` really is not an atomic, and so this happens-before relationship is not guaranteed. Agreed. But then you'll most likely also not have threads. Personally I don't see how threads can do any cross-thread communication safely without atomics.

ROMemories commented

2025-04-09 09:20:38 -05:00

(Migrated from github.com)

But then you'll most likely also not have threads.

These MCUs—the RP2040 and the ESP32-C3 (RISC-V without the (A) extension) for instance—do support threading however, and we do support it in Ariel OS (even multicore).

Personally I don't see how threads can do any cross-thread communication safely without atomics.

This is done through critical sections on bare metal, which involve temporarily disabling interrupts (see the critical-section crate and the implementation in the cortex-m crate for instance). These can be used to emulate atomics on architectures that do not support them, which is what portable-atomic does (when needed).

> But then you'll most likely also not have threads. These MCUs—the RP2040 and the ESP32-C3 (RISC-V without the (A) extension) for instance—*do* support threading however, and we do support it in [Ariel OS](https://github.com/ariel-os/ariel-os) (even multicore). > Personally I don't see how threads can do any cross-thread communication safely without atomics. This is done through critical sections on bare metal, which involve temporarily disabling interrupts (see the [`critical-section` crate](https://github.com/rust-embedded/critical-section) and [the implementation in the `cortex-m` crate](https://github.com/rust-embedded/cortex-m/blob/898b11c7783246fe0137537d20898b91e58ff6aa/cortex-m/src/critical_section.rs) for instance). These can be used to emulate atomics on architectures that do not support them, which is what [portable-atomic](https://github.com/taiki-e/portable-atomic) does (when needed).

Thomasdezeeuw commented

2025-04-09 09:24:33 -05:00

(Migrated from github.com)

This is done through critical sections on bare metal, which involve temporarily disabling interrupts (see the critical-section crate and the implementation in the cortex-m crate for instance). These can be used to emulate atomics on architectures that do not support them, which is what portable-atomic does (when needed).

Interesting, I didn't know about that -- I don't do a lot of embedded work myself.

Would calling set_logger_racy in such a critical section be sufficient? (it's something to docs already call out: https://docs.rs/log/latest/log/fn.set_logger_racy.html#safety)

> This is done through critical sections on bare metal, which involve temporarily disabling interrupts (see the [`critical-section` crate](https://github.com/rust-embedded/critical-section) and [the implementation in the `cortex-m` crate](https://github.com/rust-embedded/cortex-m/blob/898b11c7783246fe0137537d20898b91e58ff6aa/cortex-m/src/critical_section.rs) for instance). These can be used to emulate atomics on architectures that do not support them, which is what [portable-atomic](https://github.com/taiki-e/portable-atomic) does (when needed). Interesting, I didn't know about that -- I don't do a lot of embedded work myself. Would calling `set_logger_racy` in such a critical section be sufficient? (it's something to docs already call out: https://docs.rs/log/latest/log/fn.set_logger_racy.html#safety)

ROMemories commented

2025-04-10 03:06:54 -05:00

(Migrated from github.com)

(it's something to docs already call out: https://docs.rs/log/latest/log/fn.set_logger_racy.html#safety)

The docs mention that:

It is safe to use other logging functions while this function runs (including all logging macros).

So I think this is not exactly complete, as it would not be safe to call logger() while set_logger_racy() runs.

Would calling set_logger_racy in such a critical section be sufficient?

Wrapping these calls in a critical section should indeed enforce that.

> (it's something to docs already call out: https://docs.rs/log/latest/log/fn.set_logger_racy.html#safety) The docs mention that: > It is safe to use other logging functions while this function runs (including all logging macros). So I think this is not exactly complete, as it would not be safe to call `logger()` while `set_logger_racy()` runs. > Would calling `set_logger_racy` in such a critical section be sufficient? Wrapping these calls in a critical section should indeed enforce that.

Rows
Columns

Possible unsoundness on architectures without atomics #678