Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Intermediate pointers in cast must be "const qualified" - why?

In the following code...

#include <stdlib.h>
#include <stdint.h>

extern void get_buffer_from_HW_driver(volatile uint32_t **p);

void getBuffer(volatile uint32_t **pp)
{
    // Write an address into pp, that is obtained from a driver
    // The underlying HW will be DMA-ing into this address,
    // so the data pointed-to by the pointer returned by this
    // call are volatile.
    get_buffer_from_HW_driver(pp);
}

void work()
{
    uint32_t *p = NULL;
    getBuffer((volatile uint32_t **)&p);
}

...the compiler rightfully detects that any potential accesses to the data pointed to by p inside work are dangerous accesses. As-is, the code instructs the compiler that it is safe to emit code that optimizes away repeated read accesses to *p - which is indeed wrong.

But the weird thing is, that the warning emitted by compiling this code...

$ gcc -c -Wall -Wextra -Wcast-qual constqual.c

...doesn't complain about the loss of volatile - it instead recommends using const:

constqual.c: In function ‘work’:
constqual.c:20:15: warning: to be safe all intermediate pointers in cast from 
                   ‘uint32_t ** {aka unsigned int **}’ to ‘volatile uint32_t ** 
                   {aka volatile unsigned int **}’ must be ‘const’ qualified
                   [-Wcast-qual]
 getBuffer((volatile uint32_t **)&p);
           ^

I cannot see how const makes sense here.

P.S. Note that adding volatile in front of the uint32_t *p, as expected, fixes the issue. My question is why GCC recommends const instead of volatile.

like image 722
ttsiodras Avatar asked Feb 01 '18 10:02

ttsiodras


1 Answers

Well, I raised a ticket in GCC's Bugzilla about this... and Joseph Myers has answered with a laconic answer:

No, GCC is not confused. It's saying that it's type-safe to convert uint32_t ** to volatile uint32_t *const *, but not to convert it to volatile uint32_t *.

...and he also added a reference to this part of the C FAQ.

I have to admit that my first reaction to this was a "say what?". I quickly tested the suggestion, changing the code to make it use the proposed declaration (and cast) instead...

#include <stdlib.h>
#include <stdint.h>

extern void get_buffer_from_HW_driver(volatile uint32_t * const *p);
void getBuffer(volatile uint32_t * const *pp)
{
    // Write an address into pp, that is obtained from a driver
    // The underlying HW will be DMA-ing into this address,
    // so the data pointed-to by the pointer returned by this
    // call are volatile.
    get_buffer_from_HW_driver(pp);
}

void work()
{
    uint32_t *p = NULL;
    getBuffer((volatile uint32_t * const *)&p);
}

$ gcc -c -Wall -Wextra -Wcast-qual constqual.c

$ 

...and indeed, no warning anymore.

So I went ahead and read the relevant FAQ - and I think I understand a bit more of what is happening. By adding the const modifier, the parameter we are passing is (reading from right to left, as we're supposed to do in this kind of C syntax):

a pointer to a constant pointer (that will never change) that points to volatile data

This indeed maps very well to what is happening here: I am getting a pointer that points to volatile data, that is a driver-provided buffer - i.e. one that I indeed am not allowed to change, since it comes from pre-allocated lists of buffers that the driver itself allocated. Modifying the pointer that get_buffer_from_HW_driver returned would make no sense; it's not mine to modify, I can only use it as-is.

I confess I am really surprised that C's typesystem (augmented with the really strong static-analysis checks of -Wcast-qual) can actually help in guaranteeing these semantics.

Many thanks to Joseph - and I'll leave the question open for a few weeks, in case someone else wants to elaborate more.

P.S. Adding a mental note: from now on, when anyone claims that C is a simple language, I think I'll point them here.

like image 152
ttsiodras Avatar answered Sep 18 '22 19:09

ttsiodras