Re: warning: dereferencing type-punned pointer

On 24.07.24 19:48, Tom Lane wrote:
> Peter Eisentraut <peter@eisentraut.org> writes:
>> This is basically the textbook example of aliasing violation, isn't it?
>> Wouldn't it be just as simple to do
> 
>> memcpy(&file_crc, &disk_state, sizeof(file_crc));
> 
> +1.  Also, it seems thoroughly bizarre to me that this case is handled
> before checking for read failure.  I'd move the stanza to after the
> "if (readBytes < 0)" one.

(older thread)

I committed this fix, since it was simple and obvious.

I also researched again the other warnings mentioned in this thread. 
There are also a few new similar ones from commit 44eba8f06e55.

My conclusion is that casting Node * to a different pointer-to-struct is 
always an aliasing violation.  The reason why the compiler only warns in 
a few cases is probably that those are fully within the same translation 
unit, while most other ones are across file boundaries that the compiler 
cannot analyze reliably.  If you dial up the warning verbosity with 
-Wstrict-aliasing=1/2 (lower = more warnings but less reliable), you get 
more warnings about this.

Unlike what PostgreSQL code appears to assume, there is no rule in C (or 
C++) that overlaying similar structs is a valid aliasing.  So this was 
never correct, but compilers have only gotten more aggressive about this 
over time.

One correct way would be to embed the Node struct, like

typedef struct ErrorSaveContext
{
     Node        node;
     bool        error_occurred;
     bool        details_wanted;
     ErrorData  *error_data;
} ErrorSaveContext;

because you can cast between a struct and one of its field types.

We already do this node embedding for inheritance for other nodes, so 
this approach would probably be somewhat natural, but it would obviously 
require a bit of adjustment here and there (but maybe not that much).

Another approach is to annotate some fundamental types like Node and 
Expr with __attribute__((may_alias)).

Similarly with struct varlena and all its variants and related types.

Anyway, this is a battle for another day.  We're definitely going to 
have to stick with -fno-strict-aliasing for the foreseeable time.