CODEWITHSUNDEEP
c++embeddedconstantsvolatile
Timo
Timo

Reputation: 881

Reading a struct from a read only memory

I'm working on a embedded system, where some calibration data is stored in the flash memory. The calibration data is stored in a struct which is placed in a special section that the linker knows to place in the flash:

struct data_block {
    calibration_data mData;
    uint16_t mCheckSum;
};

//Define to compile the fixed flash location for image data
const data_block __attribute__((section (".caldata"))) gCalibrationData{};

where calibration_data is another POD struct which contains the actual values.

The problem is that if I now simply write the following:

const data_block data{gCalibrationData};

if (CheckSum(&(data.mData)) == data.mCheckSum) {
    //do stuff
} else {
    //Error
}

this always goes to the error branch, even though the actual checksum in flash is absolutely correct (writing this a bit differently makes it work, see below).

This is of course understandable: the compiler sees a const global object, which is default-initialized, so it knows all the values, so I guess it actually optimizes out the whole if (if I debug-printf data via a uint16_t *, I actually get the correct values).

The way I think would be correct is to define 

const volatile data_block __attribute__((section (".caldata"))) gCalibrationData{};

However, now I have the problem that I can't assign a volatile struct to non-volatile, i.e. const data{gCalibrationData}; does not compile. The same problem also appears if I try to access through a const volatile data_block *.

There's at least two or three ways I can make this work, and I don't like any of them:

  1. remove the const (and volatile) qualifier from gCalibrationData. However, this is a bit of a hack based on the compiler not being clever enough to guarantee that gCalibrationData is never touched in my program, and on the other hand, I'd like to keep to const qualifier, since trying to write to gCalibrationData by assigning is a hard fault.
  2. access gCalibrationData via const gCalibrationData * volatile pData (yes, the volatile is exactly where I mean it). Accessing through a pointer which is volatile forces the compiler to actually load the data. Again, this seems like a hack, since the pointer itself certainly isn't volatile.
  3. give data_block and calibration_data an assignment operator taking const volatile &, and assign field by field in them. This seems to be correct from the language point of view, but then whenever calibration_data changes I need to edit the assignment operator by hand. Failing to do so will produce hard-to-detect bugs.

My question: what would be the correct way to read the calibration data? My ideal criteria would be:

Upvotes: 6

Views: 848

Answers (2)

Oliv
Oliv

Reputation: 18081

One solution could be to declare a buffer in a separate source file, to inform the linker of size of data_block and then define gCalibrationData to be a symbol whose value is the begining of this buffer:

data_block.cpp:

//no initialization performed here, just used to
//transmit to the linker the information of the size
//and alignment of data_block
extern "C"{//simpler name mangling
[[gnu::section(".caldata")]] volatile
aligned_storage<sizeof(data_block),alignof(data_block)> datablock_buffer;
}

//then we specify that gCalibrationData refers to this buffer
extern const volatile data_block
gCalibrationData [[gnu::alias("datablock_buffer")]];

Alternatively the definition of gCalibrationData symbol can be done via a linker script:

SECTIONS{
  .caldata : {
    gCalibrationData = . ;
    data_block.o(.caldata)
    }
  }

gCalibrationData is an alias to an data_block_buffer. This will not cause undefined behavior because such aliasing is permitted by the language: data_block_buffer provides storage for gCalibrationData.

Semanticaly, the extern specifier is used to say that this declaration is not a definition of the value of gCalibrationData. Nevertheless the alias attribute is a definition of the symbol for the linker.

data_block.hpp

extern const volatile data_block gCalibrationData;

//and copy must be enabled for volatile:
struct data_block{
  /*...*/
  data_block(const data_block&) =default; 

  data_block& operator=(const data_block&) =default;

  data_block(const volatile data_block& other){
    //the const cast means: you are responsible not to 
    //perform this operation while performing a rom update.
    memcpy(this,const_cast<const data_block*>(&other);
    }

  data_block& operator=(const volatile data_block& other){
    memmove(this,const_cast<const data_block*>(&other);
    //or memcpy if you are sure a self assignment will never happen.
    return *this;
    }
  };

Upvotes: 2

Sneftel
Sneftel

Reputation: 41503

The most practical approach would be to lose the const. By a strict reading of the standard, gCalibrationData shouldn't be allowed to be const, since writing to a const object -- regardless of who does it -- leads to undefined behavior.

Failing that, though, just define it as extern const (and, if necessary to placate the linker, put the non-extern definition in its own translation unit. That will get you your const-correctness checking, allow the compiler to, e.g., do hoisting optimizations based on the initial values of the calibration data, while still preventing it from making any specific assumptions about those values at compile time.

Upvotes: 2

Related Questions