1

I have a global base pointer to a memory space void *pAddr. In some methods, I cast that pointer to few structures to have a better way to access the memory.

class MyClass {

  private:

    struct MapMem {
      uint8_t dummy1;
      uint64_t dummy12;
    };
    void* pAddr;
    MapMem* representationPointer;
    void myMethod();
}

voidmyMethod() {
   representationPointer = static_cast<volatile MapMem*>(pAddr);
   /* Doing something with representationPointer */
}

Since the pAddr is the result of a mmap() call to my driver doing an IO memory mapping.

Because I need to access the registers by 8/16/32/64bits exclusively, I need to use volatile. I need to avoid optimization to all the memory access using pAddr or all the other pointers pointing to that address.

  • Where should I set the volatile keyword?

On all the structures/data types that point to the pAddr or only to the pAddr?

Alexis
  • 2,136
  • 2
  • 19
  • 47
  • I suspect that you are still new to C++. A structure would not point to `pAddr`. Something that points to `pAddr` would be a `volatile void**` (double indirection), and that's quite rare. – MSalters Oct 13 '20 at 10:01
  • I gave a quite simple code example, I didn't explain it correctly, I'll delete my question because actually it doesn't make sense. The `volatile` is on the type not a value of a pointer. Therefore, I need to add volatile on every struct/datatype pointer I give the `pAddr`'s value (=the address of my IO memory). – Alexis Oct 13 '20 at 10:24
  • Aside: "In some methods" plural? I'd define the struct once, as a member type of `MyClass`, and have `pAddr` be of that type – Caleth Oct 13 '20 at 11:21
  • Definitely, I just wanted to give a very simple example just to know if the compiler takes the type qualifier volatile automatically or if I need to be explicit every time. Several structures will depends on that `pAddr` and offset that's why I keep a `void* pAddr`. If you have any links that could give me ideas of best practices, feel free to comment! Thanks @Caleth! – Alexis Oct 13 '20 at 11:24
  • For reference, that could have been a solution: https://stackoverflow.com/questions/38243501/does-accessing-a-declared-non-volatile-object-through-a-volatile-reference-point?rq=1 – Alexis Oct 13 '20 at 11:34

2 Answers2

1

volatile tells the compiler something else than the generated code might have updated the pointed content. This is why volatile is needed whenever a pointer's address point on some HW registers (whose content might be updated by the HW independently of the code execution).

volatile struct MapMem *representationPointer = static_cast<struct MapMem *>(pAddr);

should be enough.

Jean-Marc Volle
  • 3,113
  • 1
  • 16
  • 20
  • `volatile` on `pAddr` isn't enough? Isn't the compiler smart enough to not optimize all the types that has the same ptr value or derived with pointer arithmetic? – Alexis Oct 13 '20 at 10:40
  • 1
    @Alexis it is enough, provided you don't use `const_cast` or its equivalents. To this end, you'll have to get `volatile` derived pointers after casting. – Ruslan Oct 13 '20 at 10:46
  • Note that semantically it should be a `volatile const` pointer - the contents change but the pointer address would remain constant. I'd also use a `typedef struct` to generate the address layout, and maybe even use a pack pragma to be sure that no gaps are added between the fields - see https://en.wikipedia.org/wiki/Data_structure_alignment – Den-Jason Oct 13 '20 at 10:54
  • In terms of combining `volatile` and `const`, there are multiple options - see https://embeddedgurus.com/barr-code/2012/01/combining-cs-volatile-and-const-keywords/ – Den-Jason Oct 13 '20 at 11:03
  • @Den-Jason it should be `volatile Type*const`, not `volatile const Type*`. Otherwise, with the latter declaration you promise to not write into the pointee, but can read it, and can change the pointer. – Ruslan Oct 13 '20 at 11:04
  • @Den-Jason you don't need `typedef struct` in C++.`volatile MapMem * representationPointer` is sufficient. – Caleth Oct 13 '20 at 11:05
  • @Caleth - see https://stackoverflow.com/questions/252780/why-should-we-typedef-a-struct-so-often-in-c – Den-Jason Oct 13 '20 at 11:06
  • @Den-Jason It can't be `const` because I get the pointer's value after a `mmap()` thanks for the link I'll read it! – Alexis Oct 13 '20 at 11:07
  • @Den-Jason C is not C++ – Caleth Oct 13 '20 at 11:07
  • @Alexis I'm pretty sure you can cast a non-const to a const. It's all a question of expressing intent and narrowing restrictions. – Den-Jason Oct 13 '20 at 11:09
  • @Den-Jason The pointer can't be `const` because it is set after construction. The pointee can't be `const` because we want to write to it. – Caleth Oct 13 '20 at 11:11
  • @Caleth I suggest you read this: https://embeddedgurus.com/barr-code/2012/01/combining-cs-volatile-and-const-keywords/ – Den-Jason Oct 13 '20 at 11:11
  • @Den-Jason I read it. Just because you *can* declare something `const`, doesn't mean you *should*. The `MapMem` is going to be mutated by these methods. – Caleth Oct 13 '20 at 14:56
  • @Caleth in C++ circles, the philosophy is that if you can declare something const, then you should. Some like to put `constexpr` everywhere they possibly can..... – Den-Jason Oct 13 '20 at 15:01
  • @Den-Jason you seem to be totally ignoring **The MapMem is going to be mutated by these methods.** It *can* be declared `const` and then `const_cast` away, but it shouldn't – Caleth Oct 13 '20 at 15:03
  • The mapped memory will be mutated, but the pointer will not. Hence it is a `volatile Mapmem* const`. This prevents accidental mutation of the pointer. I think wer'e going round in circles now. – Den-Jason Oct 13 '20 at 15:05
  • 1
    @Alexis see this WRT asserting the register layout is as you expect: https://youtu.be/3VtGCPIoBfs?t=3243 – Den-Jason Oct 14 '20 at 02:15
  • 1
    @Den-Jason Thanks, I don't have time to change all my workflow. I have `structs` in FPGA and I want the same structures in my app to avoid having too many addresses defined in my code and using actual addresses and ptr arithmetic. I just want to get the base address and then the compiler does the job for me by abstracting all the addresses by reading/writing `packed struct`'s members. – Alexis Oct 14 '20 at 02:37
1

Since your mmapped address points to MMIO, it should be stored in a pointer-to-volatile variable. I.e.,

volatile void* pAddr;

Then, when you need to interpret this address as a pointer to MapMem, you should do the appropriate cast. If you try doing static_cast<MapMem*>(pAddr), you'll get compilation error: e.g. GCC will tell you that the cast casts away qualifiers. And rightly so: your structure is still a structure in MMIO space, so it should be volatile. So your cast should look like

auto representationPointer = static_cast<volatile MapMem*>(pAddr);

Now you can use your representationPointer to work with the structure fields as HW registers.

Ruslan
  • 18,162
  • 8
  • 67
  • 136
  • Isn't the same solution as the other answer? Using `auto` instead of explicit. – Alexis Oct 13 '20 at 11:18
  • @Alexis no, the other answer ignores `pAddr`. This is more "volatile-correct" – Caleth Oct 13 '20 at 11:19
  • That could have been interesting if I could use `auto representationPointer = static_cast(pAddr);` so that `auto` could keep the `volatile` type qualifier of `pAddr` but it seems we still need to be explicit. – Alexis Oct 13 '20 at 11:19
  • 1
    @Alexis you could make your own `qualifier_preserving_static_cast` template if you really wanted, but I think being explicit about constness and volatility is better. Also, you don't need to repeat `struct` before `MapMem` in C++. – Ruslan Oct 13 '20 at 12:42
  • @Caleth. What matters is that `representationPointer` is `volatile`. Whether it "derives" from a volatile or not `pAddr` is irrelevant. – Jean-Marc Volle Oct 13 '20 at 13:02
  • @Jean-MarcVolle if `pAddr` points to `volatile` data, it should be `volatile void *`, just the same as if it pointed to `const` data it should be `const void *` – Caleth Oct 13 '20 at 14:55