16

I have a global volatile unsigned char array volatile unsigned char buffer[10] to which data is written in an interupt. I have a function that takes an unsigned char * and stores that value to hardware (EEPROM) void storeArray(unsigned char *array), in this example the first three values. Is it safe to cast the volatile array to a non-volatile array like so?

store_array((unsigned char *) buffer);

I read the following, which I do not quite understand, but which concerns me:

6.7.3:5 If an attempt is made to refer to an object defined with a volatile-qualified type through use of an lvalue with non-volatile-qualified type, the behavior is undefined.

Does this affect my code?

Then I have this followup question: The buffer array only has a section of data I want to store (can't change that), for this example beginning with the third value. Is it legit to do the following?

store_array((unsigned char *) buffer + 3);

If it is, how is the cast affected, if 3 is added to the array? BR and thank you!

EDIT: @Cacahuete Frito linked a very similar question: Is `memcpy((void *)dest, src, n)` with a `volatile` array safe?

earthling
  • 620
  • 6
  • 20
  • 2
    IMO, since this is platform-specific code anyway, if you can determine that `volatile` does what you want on your platform, then it's okay for you to use it. You're relying on non-portable aspects of `volatile` anyway, and ... in for a penny, in for a pound. If your compiler's treatment of `volatile` changes, your code can break anyway. So what do you have to lose? – David Schwartz Jun 25 '19 at 08:20
  • 2
    Possible duplicate of [Is \`memcpy((void \*)dest, src, n)\` with a \`volatile\` array safe?](https://stackoverflow.com/questions/54964154/is-memcpyvoid-dest-src-n-with-a-volatile-array-safe) – alx - recommends codidact Jun 25 '19 at 12:27
  • Regarding `storeArray((unsigned char *) buffer + 3);`: How does the function know where the array ends? If it has the size hardcoded, the pointer arithmetics may force the function to read 3 bytes beyond the buffer limits, and therefore UB. – alx - recommends codidact Jun 25 '19 at 12:42
  • @DavidSchwartz the focus of this question was aimend to give a better understanding of this issue. However I like your point! I think I can determine the behaviour, as the code is quite stringent. This might be the "slimest" way to go. TY – earthling Jun 25 '19 at 12:52
  • @CacahueteFrito your linked question is very similar to mine! I do not quite think, that it is a duplicate, but i will edit it into my answer. I think the keywords are different, but I will accept the duplicate if said otherwise. Regarding your question: I stated in the question, that the function will handle the first three elements. This is purely an example and the focus does not lie here. TY – earthling Jun 25 '19 at 12:55
  • @earthling I also had doubts about the duplicate because of that: the main point, which is the discarding of `volatile` is the same, but the keywords are a little different; we'll see what others think. And sorry, I didn't read that of the 3 elements :) – alx - recommends codidact Jun 25 '19 at 12:58
  • 1
    For the implications of `volatile` see also https://electronics.stackexchange.com/q/409545/6383 – JimmyB Jun 25 '19 at 13:17

4 Answers4

14

Yes, the standard quote you've posted covers precisely what you're trying to do. By doing the cast, you're pretending the objects in the array are unsigned char when they're actually volatile unsigned char, so inside the function, you're referring to volatile object through an lvalue without a volatile qualifier. Undefined Behaviour.

If you cannot change the function storeArray, you will have to copy the data from the volatile array to a non-volatile one before passing it to the function.

Regarding the second question: the pointer arithmetic is fine, it will simply convert buffer to an unsigned char* and then add 3 to the resulting pointer, pointing to buffer[3] (but with the wrong qualification).

Angew is no longer proud of SO
  • 167,307
  • 17
  • 350
  • 455
  • thanks for yout input. I wrote a helper function to copy the contents of the volatile array to a non-volatile array. I used `*memcpy` as a template, i just changed the parameter types. The question linked in my question does something similar. – earthling Jun 25 '19 at 13:04
  • Wait what? What you actually said by that cast is "I guarantee this value won't change out from under the code I'm passing this pointer to." He probably has a terrible memory model problem waiting for him but this isn't immediately undefined. – Joshua Jun 25 '19 at 18:30
  • @Joshua The *cast* itself is not undefined. However, dereferencing the pointer obtained by the cast *is.* That's precisely what the standard quoted in the question says. If an object is `volatile`, all access to it must be through `volatile`-qualified lvalues. – Angew is no longer proud of SO Jun 25 '19 at 18:44
6

You have found the correct section of the standard, this code leads to undefined behavior.

A function writing something "to hardware" should probably have a volatile-qualifier parameter, depending on what "hardware" is. If it is a memory-mapped register, a DMA buffer or non-volatile memory, then the parameter should definitely have been volatile unsigned char* (or optionally, volatile uint8_t* which also is to be regarded as a character type).

Details: C allows us to iterate through any chunk of data using a character pointer, C17 6.3.2.3/7:

When a pointer to an object is converted to a pointer to a character type, the result points to the lowest addressed byte of the object. Successive increments of the result, up to the size of the object, yield pointers to the remaining bytes of the object.

The part you quote about accessing a "lvalue" refers to accesing data through a different pointer type than what's actually stored in that location. Plainly: no matter how much you cast various pointers pointing at it, the actual data retains its original type.

Accessing the data through the wrong pointer type is normally not even allowed, but again character access is a special exception to the "strict aliasing rule", C17 6.5/7:

An object shall have its stored value accessed only by an lvalue expression that has one of the following types:
...
- a character type.

So you can access any kind of data through a character pointer, but if that pointer is not volatile-qualified, you invoke undefined behavior as per the part you quoted, C17 6.7.3/5.

In practice, using a non-volatile pointer type could cause the compiler to optimize the access in unexpected ways. So this isn't just theoretical "language-lawyering", you could in practice get very strange code generated with optimizations enabled. Lots of very hard to find bugs in embedded systems originate from such a missing volatile.

Regarding your follow-up question, the cast and the buffer + 3 changes nothing: you are still dealing with a character pointer without volatile qualifier - same type. The actual data remains of type volatile unsigned char, so you can't access it from the function through a unsigned char*.

Lundin
  • 195,001
  • 40
  • 254
  • 396
  • with "hardware" I meant something like an EEPROM, i added that to my question. Thank you for your elaborate explanation! This was an intersting read. – earthling Jun 25 '19 at 13:20
  • 1
    @earthling The answer then might very well be to copy the `volatile` buffer into a non-volatile one first. Because you really don't want data changing on the fly in the middle of EEPROM programming. And you might have interrupts disabled, which would block the data from getting updated anyway. Etc. Inside the EEPROM driver, the pointer to the memory-mapped area to be written has to be declared `volatile` though, but that's another story. – Lundin Jun 25 '19 at 13:28
  • This is what I did, see my comment on Angew's answer. The EEPROM has some logic itself, it is controlled with I2C, I think I do not have to worry about pointers :) – earthling Jun 25 '19 at 13:38
  • 1
    @earthling Ok then it isn't memory-mapped and less sensitive to deal with. (But I2C registers in your MCU will be `volatile` qualified) – Lundin Jun 25 '19 at 14:12
3

  1. If the array is changes in interrupt you need to provide a mechanism to acces and modify it atomic way. If you don't any RW or RMW operation may be unsuccessful and the data inconsistent.

  2. You access volatile data make the f=unction parameters volatile as well. storeArray(volatile unsigned char *) and no cast will be needed. The cast only removes the warning. Even you pass non-volatile data to it, it will work as well.

0___________
  • 60,014
  • 4
  • 34
  • 74
1

As you found, you're relying on "undefined behavior". However, depending among other things on the separation of compilation units (and things like "whole-program-optimization" (WPO)) it will probably work. In most cases, the compiler (at least gcc) is not "smart enough" to optimize array accesses across functions in different compilation units. That said, the clean, safe and portable way would be to copy the array, making the dependency of the non-volatile array's values on the volatile ones visible to the compiler.

JimmyB
  • 12,101
  • 2
  • 28
  • 44
  • One example of how it can go wrong: suppose the first item in the array is some status flag. `while(array[0]) { do_stuff(array[i]); }`. Without `volatile`, this will likely get translated into a single read, after which the compiler can decide to leave the function or hang in an eternal loop. – Lundin Jun 25 '19 at 09:27
  • @Lundin Yes, of course. However, as the function parameter is declared non-voltaile, why should the function implementation be relying on it behaving as a volatile? – JimmyB Jun 25 '19 at 09:34
  • @JimmyB I think your answer is very similar to DavidSchwartz's comment. I now made a helper function, as suggested by Angew, also see my comment on his answer. – earthling Jun 25 '19 at 13:07