Comment by sethev

8 hours ago

Yes, there is a data race there. The value of a volatile can be changed by something outside the current thread. That’s what volatile means and why it exists.

Edit: thread=thread of execution. I’m not making a point about thread safety within a program.

Not from the standard’s point of view. The traditional (in some circles) use of volatile for atomic variables was not sanctioned by the C11/C++11 thread model; if you want an atomic, write atomic, not volatile, or be aware of your dependency on a compiler (like MSVC) that explicitly amends the language definition so as to allow cross-thread access to volatile variables.

  • Thread was a poor choice of word. Outside the control of the program is a better way to put it. Like memory mapped io.

    • It's almost universally better to use inline assembly via a macro to read/write mmio rather than use volatile.

Can also represent a register that has an effect reading it. Reading a memory mapped register can have side effects. Like memory mapped io on a UART will fetch the next byte to be read.

Was going to say the same thing until I saw this comment. volatile is defined the way I'd expect, plus it's a strange code example.

Not sure why you're being downvoted. That's completely right. The example is silly. The code is obviously bad, doesn't matter if it's UB or not.

I'm also not convinced (yet) that the example really is UB: I agree reading a volatile is "a side effect" in some sense, and GP cited a paragraph that says just that. But GP doesn't clearly quote that it's a side effect on the object (or how a side effect on an object is defined). Reading an object doesn't mutate it after all.

But whatever language lawyer things, the code is obviously broken, with an obvious fix, so I'm not so interested in what its semantics should be. Here is the fix:

    volatile int x;
    // ...
    int val = x;  // volatile read
    printf("%x %d\n", val, val);

  • The problem is that the function call as a whole is UB. Having the original example compile to the equivalent of

      volatile int x;
      int a = x;
      int b = x;
      printf("%x %d\n", a, b);
    

    is equally valid as

      volatile int x;
      int a = x;
      int b = x;
      printf("%x %d\n", b, a);
    

    , and neither needs to have the same output as your proposed fix.

    C could've specified something like "arguments are evaluated left-to-right" or "if two arguments have the same expression, the expression is [only evaluated once]/[always evaluated twice]". But it didn't, so the developer is left gingerly navigating a minefield every time they use volatile.

    • Not only is "arguments are evaluated left-to-right" less easy to formalize than you think, it would also make all C code run slower, because the compiler would no longer be able to interleave computations for more efficient pipelining. The same goes for "expression is [only evaluated once]/[always evaluated twice]".

      Of course the developer is navigating a minefield every time they use volatile, that's why it's called "volatile" - an English word otherwise only commonly used in chemistry, where it means "stuff that wants to go boom".

      2 replies →

    • I understand, that's why I said the code is obviously broken. The problem is not about order of evaluation. It's not about an UB arising from unsequenced volatile reads or whatever.

      The problem is simply that the there are two volatile reads where only one was intended. It doesn't matter if there is UB or not. The code doesn't express the intention either way. All you need to know to understand that is that volatile might be modified concurrently (a little bit similar but not the same semantics as atomics).