Do arrays need to be declared volatile?

I don't think the compiler keeps array elements in registers. So would an array that is modified by an interrupt routine and read by the main program still need to be declared volatile?

Yes. Otherwise the optimizer can assume that the variable has the same contents as the las time you read it.

Can you think of any exceptions to that rule?

I found a counter example that seems to work just fine.

There is no penalty to using the word volatile. But there may be a penalty if you don't use it.
Just because it is not used does not mean the program will fail.
But the only way to be certain the program will work is to use it.

...R

The penalty of the keyword volatile is that the compiler won't optimize its memory access.
But that is (often) the exact reason why you want to use it in the first place.

@joboyton

I found a counter example that seems to work just fine.

can you post it?

robtillaart:
The penalty of the keyword volatile is that the compiler won't optimize its memory access.
But that is (often) the exact reason why you want to use it in the first place.

I have also run into an annoying case where it is necessary to cast a volatile.

@joboytoncan you post it?

The Arduino hardware and software serial libraries both do this. They write to a (non-volatile) array in an ISR and read it outside of an ISR.

Imagine a situation in the main code where you did something like this:

byte a = myArray [i];
...  // some other code
myArray [i]++;

The compiler may have put the value for an array item (the one it put into "a") into a register, and realizing that the index had not changed, assume that the the register still contained that value.

I tried to make some test code to prove it, without success. However what did happen was that accessing an array element with a literal offset (eg. myArray [ 0 ]) the compiler just used the actual memory address (ie. it dereferenced at compile-time). Thus you could effectively say that accessing a specific array element is just like accessing any other non-array variable.

I have also run into an annoying case where it is necessary to cast a volatile.

Can you post that line?

[quote author=Nick Gammon link=msg=1982120 date=1417377482]
Imagine a situation in the main code where you did something like this:

byte a = myArray [i];
...  // some other code
myArray [i]++;

The compiler may have put the value for an array item (the one it put into "a") into a register, and realizing that the index had not changed, assume that the the register still contained that value.[/quote]

But in the case of the serial library this never happens. The ISR writes and the main code reads. So is it safe in that circumstance?

volatile char buffer[10];

void setup()
{
  Serial.begin(115200);
  Serial.println((char *)buffer);
}

void loop() {}

Take out the (char *) and see what happens.
The same issue exists with memcpy() and likely numerous other functions.

This is because those functions have no prototype for the datatype.
In concreto Serial class has no println(volatile char *) overload of println.

The error message
sketch_nov30c:9: error: call of overloaded 'println(volatile char [10])' is ambiguous
means it tries to map it on one of the known overloads.

Remember that you are not writing to the array but to AN ELEMENT of the array. So the same rules apply as for writing to a simple var of that type.

Mark

robtillaart:
This is because those functions have no prototype for the datatype.

And that's an annoyance. Especially if the array doesn't need to be volatile in the first place.

you can fix this more “permanently” (until next version) by adding

    size_t println(volatile char str[])
    {
        int n = print(str);
        return n + println();
    } 
    size_t print(volatile char str[])
    {
        return write((const char *)str);
    }

to Print.h

holmes4:
Remember that you are not writing to the array but to AN ELEMENT of the array. So the same rules apply as for writing to a simple var of that type.

The Arduino HardwareSerial and SoftwareSerial libraries both do this.
Are you suggesting they should be modified to make the buffers volatile?

I think the rules for where volatile is necessary are not that clear.

robtillaart:
you can fix this more "permanently" (until next version) by adding...

Thank you, that would work. Unfortunately print isn't the only function like this.

No I am not suggesting that, but if you want to get rid of the "annoying" cast you have to solve it on another level.

I think the rules for where volatile is necessary are not that clear.

They are 100% unambiguous:

You use volatile if you do not want the compiler to optimize a variable by keeping its value in a register or otherwise. Volatile enforces the compiler to read that variable always from memory and write it always to memory. You use this typically if variables can be modified outside "normal code", like interrupt routines and memory mapped IO.

jboyton:
The Arduino HardwareSerial and SoftwareSerial libraries both do this.
Are you suggesting they should be modified to make the buffers volatile?

I'm inclined to think they should be.

The buffers fairly clearly fall into the definition of something that should be volatile. They are modified inside an ISR and accessed outside one.

The fact that it currently "works" could just be that the compiler has chosen not to optimize such access (and it admittedly is hard to see how it might).

This code compiles, and shows that you can "downgrade" a volatile byte to a non-volatile one:

volatile byte myArray [100];

void myISR () 
  {
  myArray [0]++;
  }
  
  
void setup ()
  {
  attachInterrupt (0, myISR, CHANGE);
  byte a = myArray [0];
  }  // end of setup

void loop () { }

However if you change setup() to:

void setup ()
  {
  attachInterrupt (0, myISR, CHANGE);
  byte a = myArray [0];
  byte * p = myArray;
  }  // end of setup

You get the error:

isr_array_test.ino: In function ‘void setup()’:
isr_array_test:14: error: invalid conversion from ‘volatile byte*’ to ‘byte*’’

However this compiles:

void setup ()
  {
  attachInterrupt (0, myISR, CHANGE);
  byte a = myArray [0];
  volatile byte * p = myArray;
  }  // end of setup

I changed the buffer in HardwareSerial.cpp to:

struct ring_buffer
{
  volatile unsigned char buffer[SERIAL_BUFFER_SIZE];
  volatile unsigned int head;
  volatile unsigned int tail;
};

The first few examples compiled OK. Since ring_buffer (both of them) are private members, and thus all accesses are via Serial.read() (etc.), I don't see how this can cause any problems.

I might raise a bug report about this.

Char is a single byte so there should not be a problem if you drop the volatile.

Mark