7 ms·

Ways to break your systems code using volatile (2010)

124 points by luu 3y ago

lelanthran 3y ago
This is a very good post; too many people (even myself, sometimes) forget that volatile doesn't mean that the statement containing it cannot be reordered. [EDIT: the one thing he missed, which I would have liked to know, is about using volatile with int (or sig_atomic_t) as an "eventually consistent" value, for example one global `sig_atomic_t end_flag = 0;`, a single writer (a SIGINT handler to set it to 1), and many threads with `while (end_flag == 0) { ... }` loops. I've been using this pattern for a while with no obvious problems - access to `end_flag` can be rearranged by the compiler, barriers are irrelevant, the value can be corrupted by a race on every read and it won't matter - the thread will get the eventual value of end_flag on the next loop and end.]
- kelnos 3y ago
  I suspect sig_atomic_t does work fine when we're talking about POSIX signals, but OP was probably thinking more from an embedded programming and hardware interrupt handlers, which don't conform to POSIX signal semantics.
  - lelanthran 3y ago
    > I suspect sig_atomic_t does work fine when we're talking about POSIX signals, but OP was probably thinking more from an embedded programming and hardware interrupt handlers, which don't conform to POSIX signal semantics. It's not the sig_atomic_t that I think is wrong (could be plain int), it's the "Is it safe to have one writer to a zero-initialised volatile value, and many readers checking for non-zero of that value?" Now I wouldn't use this and expect correctness in the value, but even when the value that is read is corrupted because that single write did not finish (it's zero, one or something else), it will be non-zero eventually, and so the thread will end.
    - comex 3y ago
      Technically, using volatile between threads is a data race and therefore UB [1]; the guarantees made around sig_atomic_t only apply between a thread and a signal handler on the same thread. Though, I'd argue that the no-optimization guarantee of volatile actually does justify reasoning of the form "it's not undefined behavior because the hardware guarantees it", which is a mistake anywhere else in C. On essentially all architectures, loads and stores of volatile integers act the same way as loads and stores of atomics using memory_order_relaxed (or stronger, depending on the architecture). So it may be legal to rely on volatile being atomic, as long as you don't expect the code to be compiled on some hypothetical architecture that doesn't have this feature. [1] https://stackoverflow.com/questions/71895449/is-it-a-data-race https://stackoverflow.com/questions/71895449/is-it-a-data-ra...
      - manwe150 3y ago
        At the hardware level, there is no distinction between a regular load, a volatile load, and a relaxed atomic load (assuming small sizes and optimal alignments). But the compiler can still can do things that break your code or that miss optimizations when given incorrect ordering annotations. It seems to me that volatile ends up falling almost between acquire and relaxed ordering, in terms of the behavior of most CPUs and compilers, for small aligned values: it doesn’t synchronize any other operation but does prevent folding of consecutive operations.
      - gpderetta 3y ago
        On a very practical note, it will trigger tsan potentially hiding actual bugs in the noise. There is no reason not to mark the flag as atomic.
- bonzini 3y ago
  > one global `sig_atomic_t end_flag = 0;`, a single writer (a SIGINT handler to set it to 1), and many threads with `while (end_flag == 0) { ... }` loops. While unlikely to cause problems, this is a data race (a set of at least two concurrent accesses, of which at least one is not an atomic access and at least one is a write) and therefore constitutes undefined behavior. sig_atomic_t is only safe to use from one thread, where concurrency is given by a signal handler.
- gpderetta 3y ago
  That use of volatile for signalling thread exit, although it might work ok in practice, is still a data race and so UB under C11/C++11 MM rules.
comex 3y ago
(2010) The post is still accurate, but in 2011 C and C++ added atomics, which are a more portable alternative to uses of volatile for atomicity. They can be more efficient in some cases than the locks suggested by the post, especially in CPUs with higher core counts. (Note that dual-core consumer CPUs were around by 2010 but had only existed for a few years. Linux only finished removing the Big Kernel Lock in 2011.)
- lelanthran 3y ago
  > The post is still accurate, but in 2011 C and C++ added atomics, which are a more portable alternative to uses of volatile for atomicity. Atomics and volatile solve different problems, though. Atomics ensure a read or write completes uninterrupted. Volatile ensures that a read or write is never optimised away. I think C11 atomics can be optimised away (for example, reading a value twice in a row might result in only a single actual read). Happy to be corrected, though.
  - deleted 3y ago
    [deleted]
  - comex 3y ago
    > Atomics and volatile solve different problems, though. Yep, that's why atomics are only an alternative to uses of volatile _for atomicity_. For the original use case of accessing hardware registers, volatile is still the correct choice. It is indeed possible for C11 atomics to be optimized, although interestingly, the three major compilers do very little such optimization. This paper [1] lists some optimizations that are implemented in LLVM and some that aren't; it's from 2015 but from some quick testing it seems like not much has changed since. [1] https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4455.html https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n44...
    - binarycoffee 3y ago
      I have always wondered about that. If optimizing repeated atomic loads is indeed allowed, waiting for a signal by spinning on an atomic load could loop forever. Yet I have the feeling most people consider such code to be valid. Are they wrong?
david2ndaccount 3y ago
In a discussion like this you have to mention that Microsoft’s compiler extended volatile to mean atomic[1], although its default behavior depends on target ISA apparently. Regardless, just use c11/c++11 atomics at this point. 1. https://learn.microsoft.com/en-us/cpp/cpp/volatile-cpp?view=msvc-170 https://learn.microsoft.com/en-us/cpp/cpp/volatile-cpp?view=...
- gpderetta 3y ago
  Yes but they consider the additional semantics a mistake: "we strongly recommend that you specify /volatile:iso, and use explicit synchronization primitives and compiler intrinsics when you are dealing with memory that is shared across threads." /volatile:iso is the default for ARM as the extended semantics would be extremely penalising.
2rsf 3y ago
As a former embedded engineer on older Motorola and ARM processors I have seen reordering happening, and more than once had to check the generated assembly code, the other items more or less makes sense if you don't expect too much from your compiler, for example using volatile to get atomicity. Using volatile on multi-threaded code is ok as long as you know what you are doing, for example kicking a watchdog at a defined physical address could be fine from different threads.
kazinator 3y ago
There are gaping omissions in this article. There is a need for volatile in portable C programming, in two circumstances: 1. When an asynchronous signal handler modifies a variable that is inspected by the interrupted mainline code (e.g. to set a flag indicating that the signal went off), that variable must be of type "volatile sig_atomic_t". (The article points to some outside discussion by Hans Boehm about this in relation to Unix, but it's not just for Unix; it's in ISO C.) 2. When a function, after saving a context with setjmp(), modifies automatic local variables, and then the context is restored with longjmp(), those variables must be declared volatile, or else they will not reliably have the correct values. (E.g. the longjmp may or may not restore the values which they had at setjmp time, or do it for some of those variables but not others.) No matter how a C compiler treats volatile, in order to be ISO C conforming, if the program correctly uses volatile in the above situations, it must somehow work. Even if it is useless for anything else: threads, hardware, ... { volatile int i = 0; if (setjmp(jmp_buf) == 0) { i++; longjmp(jmp_buf, 1); } else { printf("i == %d\n", i); } } Here, the printf should produce i == 1, which is not required if the volatile is removed. For instance, if i is located in a machine register, and setjmp/longjmp work by saving and restoring registers (or just some registers, including that one), the longjmp operation will restore the value that existed in the register at the time setjmp was called, which is zero. If that's a problem in a given compiler, even if it has a garbage implementation of volatile, it has to pay attention to the fact that volatile is being used in setjmp code.
- jstimpfle 3y ago
  For 2. that seems like a defect in the specification of setjmp()? The equivalent "just works" for functions like pthread_mutex_lock() etc. Those calls implicitly add barriers to force reloading.
  - gpderetta 3y ago
    Think how the compiler would compile it. The compiler must assume pthread_mutex_lock (as any opaque non-otherwise decorated functions) clobbers memory. So if the address of 'i' escapes the containing function, then the compiler must make sure it is correctly written to memory before the call. If it doesn't escape, the compiler can potentially leave it in a (callee-save) register and there still wouldn't be any multithreading issues. longjump is special: it can return twice and the effect is observable even for non-escaping variables: so values that must be preserved across function calls need to be forced to memory by declaring them volatile. GCC specifically marks longjmp as returns_twice, which as far as I can tell prevents the surrounding function from being inlinable and additionally treats all local variables as volatile at the point of the longjmp call (forcing them to memory even if not escaping), but that's a GCC extension.
bjackman 3y ago
In the Linux Kernel instead of marking variables/types as volatile you mark _accesses_ as volatile. there's a pair of macros READ_ONCE/WRITE_ONCE that temporarily cast the pointer for you. I think this is a better way to use volatile. Even then I think it's rarely useful outside of x86-specific code (where the CPU gives you quite a lot of memory ordering guarantees). Would be interesting to check how often it gets used elsewhere.
unwind 3y ago
(Meta: As already pointed out by @comex, this post is from 2010 and it would be helpful with a tag in the title to make that clearer.) That said, it's an awesome post (not surprising considering the source). I found the initial set-up explaining the concept of C's abstract machine very succinct and nice, it's something I would wish more people discussing the language to be (well) aware of. Great post, thanks!
- rpaddock 3y ago
  I can add a bit of historical context here. John wrote that during the development of WINAVR, the first GCC compiler for AVR. The discussions can be found in the AVR-GCC mailing list archives. When the 'Small' optimization is used in AVR GCC it is aggressive and leads often to broken code if 'volatile' is not properly used. AVR GCC, using Small optimization, would remove any variable that has no side effects from the compiler's view. Setting a value in an interrupt handler would be outside of the view of the compiler's abstract machine, so 'flag' variables were often removed. Changes in hardware registers fall into the same category. These removals result in broken code. Volatile is never a replacement for atomics or proper locks/mutexes/semaphores. If hardware such as an interrupt handler or hardware register is not involved, then using volatile is a creating a race condition, which may or may not ever become apparent. Erich Styger wrote more recently about volatile here. https://mcuoneclipse.com/2021/10/12/spilling-the-beans-volatile-qualifier/ https://mcuoneclipse.com/2021/10/12/spilling-the-beans-volat... John and I both make an appearance in the linked parent particle to that one. Any earlier one of Erich's: https://mcuoneclipse.com/2013/08/14/volatile-can-be-harmful/ https://mcuoneclipse.com/2013/08/14/volatile-can-be-harmful/ If changing the optimization level breaks something, it is probably a missing volatile when doing bare hardware.
google234123 3y ago
Reminded me of this article : http://www.ddj.com/cpp/184403766 http://www.ddj.com/cpp/184403766 - volatile: The Multithreaded Programmer's Best Friend By Andrei Alexandrescu, February 01, 2001
deleted 3y ago
[deleted]
dfox 3y ago
I'm not entirely convinced that the ninth case is necessarily an miscompilation. My understanding of the standard is that C99 6.7.3 p6 and C11 6.7.3 p7 allows for exactly this behavior. One can argue (as the author does in the linked paper) that the last sentence is about what it means hardware-wise, but completely optimizing away statements that have no effect on the abstract machine level ('x;', 'x = x;', ...) is something that seems not only permitted but even reasonable. But it does not make much sense to actually want to force a read for side-effect in portable code. For a MMIO register of some MCU/SoC the code is not going to be portable in any meaningful sense anyway, and for things like portable OS drivers for weird hardware (like VGA...) you have to use some kind of OS-provided macro that does the right thing wrt. caching and barriers anyway.
deleted 3y ago
[deleted]
qooiii2 3y ago
I've been bitten by reordering. In my case, the toolchain developers implemented the reordering step in the assembler as an extra optimization step (on by default of course), so I had to disassemble the binary to even find the problem. They had redefined the assembly language semantics to require "volatile" keywords wherever you needed ordering maintained. I turned that particular optimization off.
vvern 3y ago
A place that volatile shows up in C today that is stunningly handy is when writing eBPF programs. In eBPF you have to appease a verifier which is trying to prove safety and liveness properties of the compiled program. To prove safety properties you often need to ensure that some offset into a buffer (bpf map) will be in bounds. Even if you judiciously sprinkle such bound checks into your code, the compiler may eliminate them entirely or perform them on some different register or stack value it knows to be semantically sufficient. Unfortunately the verifier is not as smart as the compiler. Using volatile to reload some offset just before bounds checking it and using it to index the map is a very reliable approach to getting code to verify.

124 points by luu 3y ago

This is a very good post; too many people (even myself, sometimes) forget that volatile doesn't mean that the statement containing it cannot be reordered. [EDIT: the one thing he missed, which I would have liked to know, is about using volatile with int (or sig_atomic_t) as an "eventually consistent" value, for example one global `sig_atomic_t end_flag = 0;`, a single writer (a SIGINT handler to set it to 1), and many threads with `while (end_flag == 0) { ... }` loops. I've been using this pattern for a while with no obvious problems - access to `end_flag` can be rearranged by the compiler, barriers are irrelevant, the value can be corrupted by a race on every read and it won't matter - the thread will get the eventual value of end_flag on the next loop and end.]

kelnos 3y ago
I suspect sig_atomic_t does work fine when we're talking about POSIX signals, but OP was probably thinking more from an embedded programming and hardware interrupt handlers, which don't conform to POSIX signal semantics.
- lelanthran 3y ago
  > I suspect sig_atomic_t does work fine when we're talking about POSIX signals, but OP was probably thinking more from an embedded programming and hardware interrupt handlers, which don't conform to POSIX signal semantics. It's not the sig_atomic_t that I think is wrong (could be plain int), it's the "Is it safe to have one writer to a zero-initialised volatile value, and many readers checking for non-zero of that value?" Now I wouldn't use this and expect correctness in the value, but even when the value that is read is corrupted because that single write did not finish (it's zero, one or something else), it will be non-zero eventually, and so the thread will end.
  - comex 3y ago
    Technically, using volatile between threads is a data race and therefore UB [1]; the guarantees made around sig_atomic_t only apply between a thread and a signal handler on the same thread. Though, I'd argue that the no-optimization guarantee of volatile actually does justify reasoning of the form "it's not undefined behavior because the hardware guarantees it", which is a mistake anywhere else in C. On essentially all architectures, loads and stores of volatile integers act the same way as loads and stores of atomics using memory_order_relaxed (or stronger, depending on the architecture). So it may be legal to rely on volatile being atomic, as long as you don't expect the code to be compiled on some hypothetical architecture that doesn't have this feature. [1] https://stackoverflow.com/questions/71895449/is-it-a-data-race https://stackoverflow.com/questions/71895449/is-it-a-data-ra...
    - manwe150 3y ago
      At the hardware level, there is no distinction between a regular load, a volatile load, and a relaxed atomic load (assuming small sizes and optimal alignments). But the compiler can still can do things that break your code or that miss optimizations when given incorrect ordering annotations. It seems to me that volatile ends up falling almost between acquire and relaxed ordering, in terms of the behavior of most CPUs and compilers, for small aligned values: it doesn’t synchronize any other operation but does prevent folding of consecutive operations.
    - gpderetta 3y ago
      On a very practical note, it will trigger tsan potentially hiding actual bugs in the noise. There is no reason not to mark the flag as atomic.
bonzini 3y ago
> one global `sig_atomic_t end_flag = 0;`, a single writer (a SIGINT handler to set it to 1), and many threads with `while (end_flag == 0) { ... }` loops. While unlikely to cause problems, this is a data race (a set of at least two concurrent accesses, of which at least one is not an atomic access and at least one is a write) and therefore constitutes undefined behavior. sig_atomic_t is only safe to use from one thread, where concurrency is given by a signal handler.
gpderetta 3y ago
That use of volatile for signalling thread exit, although it might work ok in practice, is still a data race and so UB under C11/C++11 MM rules.

comex 3y ago

(2010) The post is still accurate, but in 2011 C and C++ added atomics, which are a more portable alternative to uses of volatile for atomicity. They can be more efficient in some cases than the locks suggested by the post, especially in CPUs with higher core counts. (Note that dual-core consumer CPUs were around by 2010 but had only existed for a few years. Linux only finished removing the Big Kernel Lock in 2011.)

lelanthran 3y ago
> The post is still accurate, but in 2011 C and C++ added atomics, which are a more portable alternative to uses of volatile for atomicity. Atomics and volatile solve different problems, though. Atomics ensure a read or write completes uninterrupted. Volatile ensures that a read or write is never optimised away. I think C11 atomics can be optimised away (for example, reading a value twice in a row might result in only a single actual read). Happy to be corrected, though.
- deleted 3y ago
  [deleted]
- comex 3y ago
  > Atomics and volatile solve different problems, though. Yep, that's why atomics are only an alternative to uses of volatile _for atomicity_. For the original use case of accessing hardware registers, volatile is still the correct choice. It is indeed possible for C11 atomics to be optimized, although interestingly, the three major compilers do very little such optimization. This paper [1] lists some optimizations that are implemented in LLVM and some that aren't; it's from 2015 but from some quick testing it seems like not much has changed since. [1] https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4455.html https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n44...
  - binarycoffee 3y ago
    I have always wondered about that. If optimizing repeated atomic loads is indeed allowed, waiting for a signal by spinning on an atomic load could loop forever. Yet I have the feeling most people consider such code to be valid. Are they wrong?

david2ndaccount 3y ago

In a discussion like this you have to mention that Microsoft’s compiler extended volatile to mean atomic[1], although its default behavior depends on target ISA apparently. Regardless, just use c11/c++11 atomics at this point. 1. https://learn.microsoft.com/en-us/cpp/cpp/volatile-cpp?view=msvc-170 https://learn.microsoft.com/en-us/cpp/cpp/volatile-cpp?view=...

gpderetta 3y ago
Yes but they consider the additional semantics a mistake: "we strongly recommend that you specify /volatile:iso, and use explicit synchronization primitives and compiler intrinsics when you are dealing with memory that is shared across threads." /volatile:iso is the default for ARM as the extended semantics would be extremely penalising.

2rsf 3y ago

As a former embedded engineer on older Motorola and ARM processors I have seen reordering happening, and more than once had to check the generated assembly code, the other items more or less makes sense if you don't expect too much from your compiler, for example using volatile to get atomicity. Using volatile on multi-threaded code is ok as long as you know what you are doing, for example kicking a watchdog at a defined physical address could be fine from different threads.

kazinator 3y ago

There are gaping omissions in this article. There is a need for volatile in portable C programming, in two circumstances: 1. When an asynchronous signal handler modifies a variable that is inspected by the interrupted mainline code (e.g. to set a flag indicating that the signal went off), that variable must be of type "volatile sig_atomic_t". (The article points to some outside discussion by Hans Boehm about this in relation to Unix, but it's not just for Unix; it's in ISO C.) 2. When a function, after saving a context with setjmp(), modifies automatic local variables, and then the context is restored with longjmp(), those variables must be declared volatile, or else they will not reliably have the correct values. (E.g. the longjmp may or may not restore the values which they had at setjmp time, or do it for some of those variables but not others.) No matter how a C compiler treats volatile, in order to be ISO C conforming, if the program correctly uses volatile in the above situations, it must somehow work. Even if it is useless for anything else: threads, hardware, ... { volatile int i = 0; if (setjmp(jmp_buf) == 0) { i++; longjmp(jmp_buf, 1); } else { printf("i == %d\n", i); } } Here, the printf should produce i == 1, which is not required if the volatile is removed. For instance, if i is located in a machine register, and setjmp/longjmp work by saving and restoring registers (or just some registers, including that one), the longjmp operation will restore the value that existed in the register at the time setjmp was called, which is zero. If that's a problem in a given compiler, even if it has a garbage implementation of volatile, it has to pay attention to the fact that volatile is being used in setjmp code.

jstimpfle 3y ago
For 2. that seems like a defect in the specification of setjmp()? The equivalent "just works" for functions like pthread_mutex_lock() etc. Those calls implicitly add barriers to force reloading.
- gpderetta 3y ago
  Think how the compiler would compile it. The compiler must assume pthread_mutex_lock (as any opaque non-otherwise decorated functions) clobbers memory. So if the address of 'i' escapes the containing function, then the compiler must make sure it is correctly written to memory before the call. If it doesn't escape, the compiler can potentially leave it in a (callee-save) register and there still wouldn't be any multithreading issues. longjump is special: it can return twice and the effect is observable even for non-escaping variables: so values that must be preserved across function calls need to be forced to memory by declaring them volatile. GCC specifically marks longjmp as returns_twice, which as far as I can tell prevents the surrounding function from being inlinable and additionally treats all local variables as volatile at the point of the longjmp call (forcing them to memory even if not escaping), but that's a GCC extension.

bjackman 3y ago

In the Linux Kernel instead of marking variables/types as volatile you mark _accesses_ as volatile. there's a pair of macros READ_ONCE/WRITE_ONCE that temporarily cast the pointer for you. I think this is a better way to use volatile. Even then I think it's rarely useful outside of x86-specific code (where the CPU gives you quite a lot of memory ordering guarantees). Would be interesting to check how often it gets used elsewhere.

unwind 3y ago

(Meta: As already pointed out by @comex, this post is from 2010 and it would be helpful with a tag in the title to make that clearer.) That said, it's an awesome post (not surprising considering the source). I found the initial set-up explaining the concept of C's abstract machine very succinct and nice, it's something I would wish more people discussing the language to be (well) aware of. Great post, thanks!

rpaddock 3y ago
I can add a bit of historical context here. John wrote that during the development of WINAVR, the first GCC compiler for AVR. The discussions can be found in the AVR-GCC mailing list archives. When the 'Small' optimization is used in AVR GCC it is aggressive and leads often to broken code if 'volatile' is not properly used. AVR GCC, using Small optimization, would remove any variable that has no side effects from the compiler's view. Setting a value in an interrupt handler would be outside of the view of the compiler's abstract machine, so 'flag' variables were often removed. Changes in hardware registers fall into the same category. These removals result in broken code. Volatile is never a replacement for atomics or proper locks/mutexes/semaphores. If hardware such as an interrupt handler or hardware register is not involved, then using volatile is a creating a race condition, which may or may not ever become apparent. Erich Styger wrote more recently about volatile here. https://mcuoneclipse.com/2021/10/12/spilling-the-beans-volatile-qualifier/ https://mcuoneclipse.com/2021/10/12/spilling-the-beans-volat... John and I both make an appearance in the linked parent particle to that one. Any earlier one of Erich's: https://mcuoneclipse.com/2013/08/14/volatile-can-be-harmful/ https://mcuoneclipse.com/2013/08/14/volatile-can-be-harmful/ If changing the optimization level breaks something, it is probably a missing volatile when doing bare hardware.

google234123 3y ago

Reminded me of this article : http://www.ddj.com/cpp/184403766 http://www.ddj.com/cpp/184403766 - volatile: The Multithreaded Programmer's Best Friend By Andrei Alexandrescu, February 01, 2001

deleted 3y ago

[deleted]

dfox 3y ago

I'm not entirely convinced that the ninth case is necessarily an miscompilation. My understanding of the standard is that C99 6.7.3 p6 and C11 6.7.3 p7 allows for exactly this behavior. One can argue (as the author does in the linked paper) that the last sentence is about what it means hardware-wise, but completely optimizing away statements that have no effect on the abstract machine level ('x;', 'x = x;', ...) is something that seems not only permitted but even reasonable. But it does not make much sense to actually want to force a read for side-effect in portable code. For a MMIO register of some MCU/SoC the code is not going to be portable in any meaningful sense anyway, and for things like portable OS drivers for weird hardware (like VGA...) you have to use some kind of OS-provided macro that does the right thing wrt. caching and barriers anyway.

deleted 3y ago

[deleted]

qooiii2 3y ago

I've been bitten by reordering. In my case, the toolchain developers implemented the reordering step in the assembler as an extra optimization step (on by default of course), so I had to disassemble the binary to even find the problem. They had redefined the assembly language semantics to require "volatile" keywords wherever you needed ordering maintained. I turned that particular optimization off.

vvern 3y ago

A place that volatile shows up in C today that is stunningly handy is when writing eBPF programs. In eBPF you have to appease a verifier which is trying to prove safety and liveness properties of the compiled program. To prove safety properties you often need to ensure that some offset into a buffer (bpf map) will be in bounds. Even if you judiciously sprinkle such bound checks into your code, the compiler may eliminate them entirely or perform them on some different register or stack value it knows to be semantically sufficient. Unfortunately the verifier is not as smart as the compiler. Using volatile to reload some offset just before bounds checking it and using it to index the map is a very reliable approach to getting code to verify.